| Message ID | 20220310004423.2627181-4-emilyshaffer@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | teach submodules to know they're submodules | expand |
Emily Shaffer <emilyshaffer@google.com> writes: > + /* > + * Because get_superproject_working_tree() is older than > + * submodule.hasSuperproject, don't rely on the default "unset = false" > + * - instead, only rely on if submodule.hasSuperproject was explicitly > + * set to false. > + */ That's a round-about way to say that submodule.hassuperproject defaults to true, isn't it ;-)? > + if (! git_config_get_bool("submodule.hassuperproject", &has_superproject_cfg) > + && !has_superproject_cfg) { > + /* > + * If we don't have a superproject, then we're probably not a > + * submodule. If this is failing and shouldn't be, investigate > + * why the config was set to false. > + */ > + error(_("Asked to find a superproject, but submodule.hasSuperproject == false")); s/Asked to/asked to/, probably. > + return 0; > + } > + > if (!strbuf_realpath(&one_up, "../", 0)) > return 0; > > diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh > index 1c2df08333..dd35036bd6 100755 > --- a/t/t1500-rev-parse.sh > +++ b/t/t1500-rev-parse.sh > @@ -244,7 +244,15 @@ test_expect_success 'showing the superproject correctly' ' > test_must_fail git -C super merge branch1 && > > git -C super/dir/sub rev-parse --show-superproject-working-tree >out && > - test_cmp expect out > + test_cmp expect out && > + > + # When submodule.hasSuperproject=false, --show-superproject-working-tree > + # should fail instead of checking the filesystem. > + test_config -C super/dir/sub submodule.hasSuperproject false && > + git -C super/dir/sub rev-parse --show-superproject-working-tree >out && > + # --show-superproject-working-tree should print an error about the > + # broken config > + ! grep "error:.*hasSuperproject" out > ' > > # at least one external project depends on this behavior:
On Wed, Mar 9, 2022 at 10:57 PM Junio C Hamano <gitster@pobox.com> wrote: > Emily Shaffer <emilyshaffer@google.com> writes: > > + error(_("Asked to find a superproject, but submodule.hasSuperproject == false")); > > s/Asked to/asked to/, probably. This is a user-facing error message, not just a programmer-facing message, correct? If so, then perhaps: s/==/is/
diff --git a/submodule.c b/submodule.c index aafbd628ad..64760f1e3a 100644 --- a/submodule.c +++ b/submodule.c @@ -2231,6 +2231,7 @@ int get_superproject_working_tree(struct strbuf *buf) struct strbuf sb = STRBUF_INIT; struct strbuf one_up = STRBUF_INIT; const char *cwd = xgetcwd(); + int has_superproject_cfg = 0; int ret = 0; const char *subpath; int code; @@ -2244,6 +2245,23 @@ int get_superproject_working_tree(struct strbuf *buf) */ return 0; + /* + * Because get_superproject_working_tree() is older than + * submodule.hasSuperproject, don't rely on the default "unset = false" + * - instead, only rely on if submodule.hasSuperproject was explicitly + * set to false. + */ + if (! git_config_get_bool("submodule.hassuperproject", &has_superproject_cfg) + && !has_superproject_cfg) { + /* + * If we don't have a superproject, then we're probably not a + * submodule. If this is failing and shouldn't be, investigate + * why the config was set to false. + */ + error(_("Asked to find a superproject, but submodule.hasSuperproject == false")); + return 0; + } + if (!strbuf_realpath(&one_up, "../", 0)) return 0; diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh index 1c2df08333..dd35036bd6 100755 --- a/t/t1500-rev-parse.sh +++ b/t/t1500-rev-parse.sh @@ -244,7 +244,15 @@ test_expect_success 'showing the superproject correctly' ' test_must_fail git -C super merge branch1 && git -C super/dir/sub rev-parse --show-superproject-working-tree >out && - test_cmp expect out + test_cmp expect out && + + # When submodule.hasSuperproject=false, --show-superproject-working-tree + # should fail instead of checking the filesystem. + test_config -C super/dir/sub submodule.hasSuperproject false && + git -C super/dir/sub rev-parse --show-superproject-working-tree >out && + # --show-superproject-working-tree should print an error about the + # broken config + ! grep "error:.*hasSuperproject" out ' # at least one external project depends on this behavior:
In the previous commit, submodules learned a config 'submodule.hasSuperproject' to indicate whether or not we should attempt to traverse the filesystem to find their superproject. To help test that this config was added everywhere it should have been, begin using it to decide whether to exit early from 'git rev-parse --show-superproject-working-dir'. Because that command is fairly old, only short-circuit if the new config was explicitly set to false. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> --- submodule.c | 18 ++++++++++++++++++ t/t1500-rev-parse.sh | 10 +++++++++- 2 files changed, 27 insertions(+), 1 deletion(-)