From 49eb1d388ae618b61058d14b811684a89bdf84b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 20 Dec 2022 13:39:48 +0100 Subject: [PATCH 1/9] submodule absorbgitdirs tests: add missing "Migrating git..." tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a blind spots in the tests surrounding "submodule absorbgitdirs" and test what output we emit, and how emitted the message and behavior interacts with a "git worktree" where the repository isn't at the base of the working directory. The "$(pwd)" instead of "$PWD" here is needed due to Windows, where the latter will be a path like "/d/a/git/[...]", whereas we need "D:/a/git/[...]". Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- t/t7412-submodule-absorbgitdirs.sh | 64 ++++++++++++++++++++++++++---- 1 file changed, 57 insertions(+), 7 deletions(-) diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh index 2859695c6d..f778321857 100755 --- a/t/t7412-submodule-absorbgitdirs.sh +++ b/t/t7412-submodule-absorbgitdirs.sh @@ -10,6 +10,7 @@ TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success 'setup a real submodule' ' + cwd="$(pwd)" && git init sub1 && test_commit -C sub1 first && git submodule add ./sub1 && @@ -18,13 +19,21 @@ test_expect_success 'setup a real submodule' ' ' test_expect_success 'absorb the git dir' ' + >expect && + >actual && >expect.1 && >expect.2 && >actual.1 && >actual.2 && git status >expect.1 && git -C sub1 rev-parse HEAD >expect.2 && - git submodule absorbgitdirs && + cat >expect <<-EOF && + Migrating git directory of '\''sub1'\'' from + '\''$cwd/sub1/.git'\'' to + '\''$cwd/.git/modules/sub1'\'' + EOF + git submodule absorbgitdirs 2>actual && + test_cmp expect actual && git fsck && test -f sub1/.git && test -d .git/modules/sub1 && @@ -37,7 +46,8 @@ test_expect_success 'absorb the git dir' ' test_expect_success 'absorbing does not fail for deinitialized submodules' ' test_when_finished "git submodule update --init" && git submodule deinit --all && - git submodule absorbgitdirs && + git submodule absorbgitdirs 2>err && + test_must_be_empty err && test -d .git/modules/sub1 && test -d sub1 && ! test -e sub1/.git @@ -56,7 +66,13 @@ test_expect_success 'setup nested submodule' ' test_expect_success 'absorb the git dir in a nested submodule' ' git status >expect.1 && git -C sub1/nested rev-parse HEAD >expect.2 && - git submodule absorbgitdirs && + cat >expect <<-EOF && + Migrating git directory of '\''sub1/nested'\'' from + '\''$cwd/sub1/nested/.git'\'' to + '\''$cwd/.git/modules/sub1/modules/nested'\'' + EOF + git submodule absorbgitdirs 2>actual && + test_cmp expect actual && test -f sub1/nested/.git && test -d .git/modules/sub1/modules/nested && git status >actual.1 && @@ -87,7 +103,13 @@ test_expect_success 're-setup nested submodule' ' test_expect_success 'absorb the git dir in a nested submodule' ' git status >expect.1 && git -C sub1/nested rev-parse HEAD >expect.2 && - git submodule absorbgitdirs && + cat >expect <<-EOF && + Migrating git directory of '\''sub1'\'' from + '\''$cwd/sub1/.git'\'' to + '\''$cwd/.git/modules/sub1'\'' + EOF + git submodule absorbgitdirs 2>actual && + test_cmp expect actual && test -f sub1/.git && test -f sub1/nested/.git && test -d .git/modules/sub1/modules/nested && @@ -97,6 +119,27 @@ test_expect_success 'absorb the git dir in a nested submodule' ' test_cmp expect.2 actual.2 ' +test_expect_success 'absorb the git dir outside of primary worktree' ' + test_when_finished "rm -rf repo-bare.git" && + git clone --bare . repo-bare.git && + test_when_finished "rm -rf repo-wt" && + git -C repo-bare.git worktree add ../repo-wt && + + test_when_finished "rm -f .gitconfig" && + test_config_global protocol.file.allow always && + git -C repo-wt submodule update --init && + git init repo-wt/sub2 && + test_commit -C repo-wt/sub2 A && + git -C repo-wt submodule add ./sub2 sub2 && + cat >expect <<-EOF && + Migrating git directory of '\''sub2'\'' from + '\''$cwd/repo-wt/sub2/.git'\'' to + '\''$cwd/repo-bare.git/worktrees/repo-wt/modules/sub2'\'' + EOF + git -C repo-wt submodule absorbgitdirs 2>actual && + test_cmp expect actual +' + test_expect_success 'setup a gitlink with missing .gitmodules entry' ' git init sub2 && test_commit -C sub2 first && @@ -107,7 +150,11 @@ test_expect_success 'setup a gitlink with missing .gitmodules entry' ' test_expect_success 'absorbing the git dir fails for incomplete submodules' ' git status >expect.1 && git -C sub2 rev-parse HEAD >expect.2 && - test_must_fail git submodule absorbgitdirs && + cat >expect <<-\EOF && + fatal: could not lookup name for submodule '\''sub2'\'' + EOF + test_must_fail git submodule absorbgitdirs 2>actual && + test_cmp expect actual && git -C sub2 fsck && test -d sub2/.git && git status >actual && @@ -127,8 +174,11 @@ test_expect_success 'setup a submodule with multiple worktrees' ' ' test_expect_success 'absorbing fails for a submodule with multiple worktrees' ' - test_must_fail git submodule absorbgitdirs sub3 2>error && - test_i18ngrep "not supported" error + cat >expect <<-\EOF && + fatal: could not lookup name for submodule '\''sub2'\'' + EOF + test_must_fail git submodule absorbgitdirs 2>actual && + test_cmp expect actual ' test_done From 0d1806e53de6ac26021d8aab918d76381287f339 Mon Sep 17 00:00:00 2001 From: Glen Choo Date: Tue, 20 Dec 2022 13:39:49 +0100 Subject: [PATCH 2/9] read-tree + fetch tests: test failing "--super-prefix" interaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ever since "git fetch --refetch" was introduced in 0f5e8851737 (Merge branch 'rc/fetch-refetch', 2022-04-04) the test being added here would fail. This is because "restore" will "read-tree .. --reset ", which will in turn invoke "fetch". The "fetch" will then die with: fatal: fetch doesn't support --super-prefix This edge case and other "--super-prefix" bugs will be fixed in subsequent commits, but let's first add a "test_expect_failure" test for it. It passes until the very last command in the test. Signed-off-by: Glen Choo Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- t/t5616-partial-clone.sh | 43 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh index 037941b95d..2846ec6629 100755 --- a/t/t5616-partial-clone.sh +++ b/t/t5616-partial-clone.sh @@ -644,6 +644,49 @@ test_expect_success 'repack does not loosen promisor objects' ' grep "loosen_unused_packed_objects/loosened:0" trace ' +test_expect_failure 'lazy-fetch in submodule succeeds' ' + # setup + test_config_global protocol.file.allow always && + + test_when_finished "rm -rf src-sub" && + git init src-sub && + git -C src-sub config uploadpack.allowfilter 1 && + git -C src-sub config uploadpack.allowanysha1inwant 1 && + + # This blob must be missing in the subsequent commit. + echo foo >src-sub/file && + git -C src-sub add file && + git -C src-sub commit -m "submodule one" && + SUB_ONE=$(git -C src-sub rev-parse HEAD) && + + echo bar >src-sub/file && + git -C src-sub add file && + git -C src-sub commit -m "submodule two" && + SUB_TWO=$(git -C src-sub rev-parse HEAD) && + + test_when_finished "rm -rf src-super" && + git init src-super && + git -C src-super config uploadpack.allowfilter 1 && + git -C src-super config uploadpack.allowanysha1inwant 1 && + git -C src-super submodule add ../src-sub src-sub && + + git -C src-super/src-sub checkout $SUB_ONE && + git -C src-super add src-sub && + git -C src-super commit -m "superproject one" && + + git -C src-super/src-sub checkout $SUB_TWO && + git -C src-super add src-sub && + git -C src-super commit -m "superproject two" && + + # the fetch + test_when_finished "rm -rf client" && + git clone --filter=blob:none --also-filter-submodules \ + --recurse-submodules "file://$(pwd)/src-super" client && + + # Trigger lazy-fetch from the superproject + git -C client restore --recurse-submodules --source=HEAD^ :/ +' + . "$TEST_DIRECTORY"/lib-httpd.sh start_httpd From f0a5e5ad57ae729d9971bdb6bdaa82c1d85bd062 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 20 Dec 2022 13:39:50 +0100 Subject: [PATCH 3/9] submodule.c & submodule--helper: pass along "super_prefix" param MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Start passing the "super_prefix" along as a parameter to get_submodule_displaypath() and absorb_git_dir_into_superproject(), rather than get the value directly as a global. This is in preparation for subsequent commits, where we'll gradually phase out get_super_prefix() for an alternative way of getting the "super_prefix". Most of the users of this get a get_super_prefix() value, either directly or by indirection. The exceptions are: - builtin/rm.c: Doesn't declare SUPPORT_SUPER_PREFIX, so we'd have died if this was provided, so it's safe to pass "NULL". - deinit_submodule(): The "deinit_submodule()" function has never been able to use the "git -super-prefix". It will call "absorb_git_dir_into_superproject()", but it will only do so from the top-level project. If "absorbgitdirs" recurses will use the "path" passed to "absorb_git_dir_into_superproject()" in "deinit_submodule()" as its starting "--super-prefix". So we can safely remove the get_super_prefix() call here, and pass NULL instead. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/rm.c | 2 +- builtin/submodule--helper.c | 35 ++++++++++++++++++++++------------- submodule.c | 15 +++++++++------ submodule.h | 3 ++- 4 files changed, 34 insertions(+), 21 deletions(-) diff --git a/builtin/rm.c b/builtin/rm.c index d4989d4d86..4a4aec0d00 100644 --- a/builtin/rm.c +++ b/builtin/rm.c @@ -86,7 +86,7 @@ static void submodules_absorb_gitdir_if_needed(void) continue; if (!submodule_uses_gitfile(name)) - absorb_git_dir_into_superproject(name); + absorb_git_dir_into_superproject(name, NULL); } } diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 05f2c9bc98..da844cad5c 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -113,10 +113,9 @@ static char *resolve_relative_url(const char *rel_url, const char *up_path, int } /* the result should be freed by the caller. */ -static char *get_submodule_displaypath(const char *path, const char *prefix) +static char *get_submodule_displaypath(const char *path, const char *prefix, + const char *super_prefix) { - const char *super_prefix = get_super_prefix(); - if (prefix && super_prefix) { BUG("cannot have prefix '%s' and superprefix '%s'", prefix, super_prefix); @@ -294,7 +293,8 @@ static void runcommand_in_submodule_cb(const struct cache_entry *list_item, struct child_process cp = CHILD_PROCESS_INIT; char *displaypath; - displaypath = get_submodule_displaypath(path, info->prefix); + displaypath = get_submodule_displaypath(path, info->prefix, + get_super_prefix()); sub = submodule_from_path(the_repository, null_oid(), path); @@ -447,7 +447,8 @@ static void init_submodule(const char *path, const char *prefix, const char *upd; char *url = NULL, *displaypath; - displaypath = get_submodule_displaypath(path, prefix); + displaypath = get_submodule_displaypath(path, prefix, + get_super_prefix()); sub = submodule_from_path(the_repository, null_oid(), path); @@ -624,7 +625,8 @@ static void status_submodule(const char *path, const struct object_id *ce_oid, die(_("no submodule mapping found in .gitmodules for path '%s'"), path); - displaypath = get_submodule_displaypath(path, prefix); + displaypath = get_submodule_displaypath(path, prefix, + get_super_prefix()); if ((CE_STAGEMASK & ce_flags) >> CE_STAGESHIFT) { print_status(flags, 'U', path, null_oid(), displaypath); @@ -948,7 +950,8 @@ static void generate_submodule_summary(struct summary_cb *info, dst_abbrev = xstrndup(oid_to_hex(&p->oid_dst), 7); } - displaypath = get_submodule_displaypath(p->sm_path, info->prefix); + displaypath = get_submodule_displaypath(p->sm_path, info->prefix, + get_super_prefix()); if (!missing_src && !missing_dst) { struct child_process cp_rev_list = CHILD_PROCESS_INIT; @@ -1239,7 +1242,8 @@ static void sync_submodule(const char *path, const char *prefix, super_config_url = xstrdup(""); } - displaypath = get_submodule_displaypath(path, prefix); + displaypath = get_submodule_displaypath(path, prefix, + get_super_prefix()); if (!(flags & OPT_QUIET)) printf(_("Synchronizing submodule url for '%s'\n"), @@ -1365,7 +1369,7 @@ static void deinit_submodule(const char *path, const char *prefix, if (!sub || !sub->name) goto cleanup; - displaypath = get_submodule_displaypath(path, prefix); + displaypath = get_submodule_displaypath(path, prefix, NULL); /* remove the submodule work tree (unless the user already did it) */ if (is_directory(path)) { @@ -1379,7 +1383,7 @@ static void deinit_submodule(const char *path, const char *prefix, ".git file by using absorbgitdirs."), displaypath); - absorb_git_dir_into_superproject(path); + absorb_git_dir_into_superproject(path, NULL); } @@ -1958,7 +1962,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce, enum submodule_update_type update_type; char *key; const struct update_data *ud = suc->update_data; - char *displaypath = get_submodule_displaypath(ce->name, ud->prefix); + char *displaypath = get_submodule_displaypath(ce->name, ud->prefix, + get_super_prefix()); struct strbuf sb = STRBUF_INIT; int needs_cloning = 0; int need_free_url = 0; @@ -2608,7 +2613,8 @@ static int update_submodules(struct update_data *update_data) goto fail; update_data->displaypath = get_submodule_displaypath( - update_data->sm_path, update_data->prefix); + update_data->sm_path, update_data->prefix, + get_super_prefix()); code = update_submodule(update_data); FREE_AND_NULL(update_data->displaypath); fail: @@ -2828,6 +2834,7 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix) int i; struct pathspec pathspec = { 0 }; struct module_list list = MODULE_LIST_INIT; + const char *super_prefix; struct option embed_gitdir_options[] = { OPT_END() }; @@ -2843,8 +2850,10 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix) if (module_list_compute(argv, prefix, &pathspec, &list) < 0) goto cleanup; + super_prefix = get_super_prefix(); for (i = 0; i < list.nr; i++) - absorb_git_dir_into_superproject(list.entries[i]->name); + absorb_git_dir_into_superproject(list.entries[i]->name, + super_prefix); ret = 0; cleanup: diff --git a/submodule.c b/submodule.c index 8ac2fca855..bf19598b8c 100644 --- a/submodule.c +++ b/submodule.c @@ -2145,7 +2145,8 @@ int submodule_move_head(const char *path, if (!(flags & SUBMODULE_MOVE_HEAD_DRY_RUN)) { if (old_head) { if (!submodule_uses_gitfile(path)) - absorb_git_dir_into_superproject(path); + absorb_git_dir_into_superproject(path, + get_super_prefix()); } else { struct strbuf gitdir = STRBUF_INIT; submodule_name_to_gitdir(&gitdir, the_repository, @@ -2315,7 +2316,8 @@ static void relocate_single_git_dir_into_superproject(const char *path) strbuf_release(&new_gitdir); } -static void absorb_git_dir_into_superproject_recurse(const char *path) +static void absorb_git_dir_into_superproject_recurse(const char *path, + const char *super_prefix) { struct child_process cp = CHILD_PROCESS_INIT; @@ -2323,8 +2325,8 @@ static void absorb_git_dir_into_superproject_recurse(const char *path) cp.dir = path; cp.git_cmd = 1; cp.no_stdin = 1; - strvec_pushf(&cp.args, "--super-prefix=%s%s/", - get_super_prefix_or_empty(), path); + strvec_pushf(&cp.args, "--super-prefix=%s%s/", super_prefix ? + super_prefix : "", path); strvec_pushl(&cp.args, "submodule--helper", "absorbgitdirs", NULL); prepare_submodule_repo_env(&cp.env); @@ -2337,7 +2339,8 @@ static void absorb_git_dir_into_superproject_recurse(const char *path) * having its git directory within the working tree to the git dir nested * in its superprojects git dir under modules/. */ -void absorb_git_dir_into_superproject(const char *path) +void absorb_git_dir_into_superproject(const char *path, + const char *super_prefix) { int err_code; const char *sub_git_dir; @@ -2386,7 +2389,7 @@ void absorb_git_dir_into_superproject(const char *path) } strbuf_release(&gitdir); - absorb_git_dir_into_superproject_recurse(path); + absorb_git_dir_into_superproject_recurse(path, super_prefix); } int get_superproject_working_tree(struct strbuf *buf) diff --git a/submodule.h b/submodule.h index b52a4ff1e7..f90ee547d0 100644 --- a/submodule.h +++ b/submodule.h @@ -164,7 +164,8 @@ void submodule_unset_core_worktree(const struct submodule *sub); */ void prepare_submodule_repo_env(struct strvec *env); -void absorb_git_dir_into_superproject(const char *path); +void absorb_git_dir_into_superproject(const char *path, + const char *super_prefix); /* * Return the absolute path of the working tree of the superproject, which this From bb61a962d2e759754fca35e4b31f73122eed49fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 20 Dec 2022 13:39:51 +0100 Subject: [PATCH 4/9] submodule--helper: don't use global --super-prefix in "absorbgitdirs" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "--super-prefix" facility was introduced in [1] has always been a transitory hack, which is why we've made it an error to supply it as an option to "git" to commands that don't know about it. That's been a good goal, as it has a global effect we haven't wanted calls to get_super_prefix() from built-ins we didn't expect. But it has meant that when we've had chains of different built-ins using it all of the processes in that "chain" have needed to support it, and worse processes that don't need it have needed to ask for "SUPPORT_SUPER_PREFIX" because their parent process needs it. That's how "fsmonitor--daemon" ended up with it, per [2] it's called from (among other things) "submodule--helper absorbgitdirs", but as we declared "submodule--helper" as "SUPPORT_SUPER_PREFIX" we needed to declare "fsmonitor--daemon" as accepting it too, even though it doesn't care about it. But in the case of "absorbgitdirs" it only needed "--super-prefix" to invoke itself recursively, and we'd never have another "in-between" process in the chain. So we didn't need the bigger hammer of "git --super-prefix", and the "setenv(GIT_SUPER_PREFIX_ENVIRONMENT, ...)" that it entails. Let's instead accept a hidden "--super-prefix" option to "submodule--helper absorbgitdirs" itself. Eventually (as with all other "--super-prefix" users) we'll want to clean this code up so that this all happens in-process. I.e. needing any variant of "--super-prefix" is itself a hack around our various global state, and implicit reliance on "the_repository". This stepping stone makes such an eventual change easier, as we'll need to deal with less global state at that point. The "fsmonitor--daemon" test adjusted here was added in [3]. To assert that it didn't run into the "--super-prefix" message it was asserting the output it didn't have. Let's instead assert the full output that we *do* have, using the same pattern as a preceding change to "t/t7412-submodule-absorbgitdirs.sh" used. We could also remove the test entirely (as [4] did), but even though the initial reason for having it is gone we're still getting some marginal benefit from testing the "fsmonitor" and "submodule absorbgitdirs" interaction, so let's keep it. The change here to have either a NULL or non-"" string as a "super_prefix" instead of the previous arrangement of "" or non-"" is somewhat arbitrary. We could also decide to never have to check for NULL. As we'll be changing the rest of the "git --super-prefix" users to the same pattern, leaving them all consistent makes sense. Why not pick "" over NULL? Because that's how the "prefix" works[5], and having "prefix" and "super_prefix" work the same way will be less confusing. That "prefix" picked NULL instead of "" is itself arbitrary, but as it's easy to make this small bit of our overall API consistent, let's go with that. 1. 74866d75793 (git: make super-prefix option, 2016-10-07) 2. 53fcfbc84f6 (fsmonitor--daemon: allow --super-prefix argument, 2022-05-26) 3. 53fcfbc84f6 (fsmonitor--daemon: allow --super-prefix argument, 2022-05-26) 4. https://lore.kernel.org/git/20221109004708.97668-5-chooglen@google.com/ 5. 9725c8dda20 (built-ins: trust the "prefix" from run_builtin(), 2022-02-16) Signed-off-by: Glen Choo Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 7 +++---- git.c | 2 +- parse-options.h | 4 ++++ submodule.c | 12 +++++++----- t/t7527-builtin-fsmonitor.sh | 36 ++++++++++++++---------------------- 5 files changed, 29 insertions(+), 32 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index da844cad5c..b8b2bc776d 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2834,8 +2834,9 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix) int i; struct pathspec pathspec = { 0 }; struct module_list list = MODULE_LIST_INIT; - const char *super_prefix; + const char *super_prefix = NULL; struct option embed_gitdir_options[] = { + OPT__SUPER_PREFIX(&super_prefix), OPT_END() }; const char *const git_submodule_helper_usage[] = { @@ -2850,7 +2851,6 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix) if (module_list_compute(argv, prefix, &pathspec, &list) < 0) goto cleanup; - super_prefix = get_super_prefix(); for (i = 0; i < list.nr; i++) absorb_git_dir_into_superproject(list.entries[i]->name, super_prefix); @@ -3391,8 +3391,7 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix) if (strcmp(subcmd, "clone") && strcmp(subcmd, "update") && strcmp(subcmd, "foreach") && strcmp(subcmd, "status") && - strcmp(subcmd, "sync") && strcmp(subcmd, "absorbgitdirs") && - get_super_prefix()) + strcmp(subcmd, "sync") && get_super_prefix()) /* * xstrfmt() rather than "%s %s" to keep the translated * string identical to git.c's. diff --git a/git.c b/git.c index 277a8cce84..e473b4b7d7 100644 --- a/git.c +++ b/git.c @@ -539,7 +539,7 @@ static struct cmd_struct commands[] = { { "format-patch", cmd_format_patch, RUN_SETUP }, { "fsck", cmd_fsck, RUN_SETUP }, { "fsck-objects", cmd_fsck, RUN_SETUP }, - { "fsmonitor--daemon", cmd_fsmonitor__daemon, SUPPORT_SUPER_PREFIX | RUN_SETUP }, + { "fsmonitor--daemon", cmd_fsmonitor__daemon, RUN_SETUP }, { "gc", cmd_gc, RUN_SETUP }, { "get-tar-commit-id", cmd_get_tar_commit_id, NO_PARSEOPT }, { "grep", cmd_grep, RUN_SETUP_GENTLY }, diff --git a/parse-options.h b/parse-options.h index b6ef86e0d1..50d852f299 100644 --- a/parse-options.h +++ b/parse-options.h @@ -369,6 +369,10 @@ int parse_opt_tracking_mode(const struct option *, const char *, int); { OPTION_CALLBACK, 0, "abbrev", (var), N_("n"), \ N_("use digits to display object names"), \ PARSE_OPT_OPTARG, &parse_opt_abbrev_cb, 0 } +#define OPT__SUPER_PREFIX(var) \ + OPT_STRING_F(0, "super-prefix", (var), N_("prefix"), \ + N_("prefixed path to initial superproject"), PARSE_OPT_HIDDEN) + #define OPT__COLOR(var, h) \ OPT_COLOR_FLAG(0, "color", (var), (h)) #define OPT_COLUMN(s, l, v, h) \ diff --git a/submodule.c b/submodule.c index bf19598b8c..46a0347319 100644 --- a/submodule.c +++ b/submodule.c @@ -2275,7 +2275,8 @@ int validate_submodule_git_dir(char *git_dir, const char *submodule_name) * Embeds a single submodules git directory into the superprojects git dir, * non recursively. */ -static void relocate_single_git_dir_into_superproject(const char *path) +static void relocate_single_git_dir_into_superproject(const char *path, + const char *super_prefix) { char *old_git_dir = NULL, *real_old_git_dir = NULL, *real_new_git_dir = NULL; struct strbuf new_gitdir = STRBUF_INIT; @@ -2305,7 +2306,7 @@ static void relocate_single_git_dir_into_superproject(const char *path) real_new_git_dir = real_pathdup(new_gitdir.buf, 1); fprintf(stderr, _("Migrating git directory of '%s%s' from\n'%s' to\n'%s'\n"), - get_super_prefix_or_empty(), path, + super_prefix ? super_prefix : "", path, real_old_git_dir, real_new_git_dir); relocate_gitdir(path, real_old_git_dir, real_new_git_dir); @@ -2325,10 +2326,11 @@ static void absorb_git_dir_into_superproject_recurse(const char *path, cp.dir = path; cp.git_cmd = 1; cp.no_stdin = 1; - strvec_pushf(&cp.args, "--super-prefix=%s%s/", super_prefix ? - super_prefix : "", path); strvec_pushl(&cp.args, "submodule--helper", "absorbgitdirs", NULL); + strvec_pushf(&cp.args, "--super-prefix=%s%s/", super_prefix ? + super_prefix : "", path); + prepare_submodule_repo_env(&cp.env); if (run_command(&cp)) die(_("could not recurse into submodule '%s'"), path); @@ -2382,7 +2384,7 @@ void absorb_git_dir_into_superproject(const char *path, char *real_common_git_dir = real_pathdup(get_git_common_dir(), 1); if (!starts_with(real_sub_git_dir, real_common_git_dir)) - relocate_single_git_dir_into_superproject(path); + relocate_single_git_dir_into_superproject(path, super_prefix); free(real_sub_git_dir); free(real_common_git_dir); diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh index 4abc74db2b..76d0220daa 100755 --- a/t/t7527-builtin-fsmonitor.sh +++ b/t/t7527-builtin-fsmonitor.sh @@ -866,27 +866,9 @@ test_expect_success 'submodule always visited' ' # the submodule, and someone does a `git submodule absorbgitdirs` # in the super, Git will recursively invoke `git submodule--helper` # to do the work and this may try to read the index. This will -# try to start the daemon in the submodule *and* pass (either -# directly or via inheritance) the `--super-prefix` arg to the -# `git fsmonitor--daemon start` command inside the submodule. -# This causes a warning because fsmonitor--daemon does take that -# global arg (see the table in git.c) -# -# This causes a warning when trying to start the daemon that is -# somewhat confusing. It does not seem to hurt anything because -# the fsmonitor code maps the query failure into a trivial response -# and does the work anyway. -# -# It would be nice to silence the warning, however. +# try to start the daemon in the submodule. -have_t2_error_event () { - log=$1 - msg="fsmonitor--daemon doesnQt support --super-prefix" && - - tr '\047' Q <$1 | grep -e "$msg" -} - -test_expect_success "stray submodule super-prefix warning" ' +test_expect_success "submodule absorbgitdirs implicitly starts daemon" ' test_when_finished "rm -rf super; \ rm -rf sub; \ rm super-sub.trace" && @@ -904,10 +886,20 @@ test_expect_success "stray submodule super-prefix warning" ' test_path_is_dir super/dir_1/dir_2/sub/.git && + cwd="$(cd super && pwd)" && + cat >expect <<-EOF && + Migrating git directory of '\''dir_1/dir_2/sub'\'' from + '\''$cwd/dir_1/dir_2/sub/.git'\'' to + '\''$cwd/.git/modules/dir_1/dir_2/sub'\'' + EOF GIT_TRACE2_EVENT="$PWD/super-sub.trace" \ - git -C super submodule absorbgitdirs && + git -C super submodule absorbgitdirs >out 2>actual && + test_cmp expect actual && + test_must_be_empty out && - ! have_t2_error_event super-sub.trace + # Confirm that the trace2 log contains a record of the + # daemon starting. + test_subcommand git fsmonitor--daemon start Date: Tue, 20 Dec 2022 13:39:52 +0100 Subject: [PATCH 5/9] submodule--helper: convert "foreach" to its own "--super-prefix" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As with a preceding commit to convert "absorbgitdirs", we can convert "submodule--helper foreach" to use its own "--super-prefix", instead of relying on the global "--super-prefix" argument to "git" itself. See that earlier commit for the rationale and background. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index b8b2bc776d..2a6ced13f6 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -278,6 +278,7 @@ struct foreach_cb { int argc; const char **argv; const char *prefix; + const char *super_prefix; int quiet; int recursive; }; @@ -294,7 +295,7 @@ static void runcommand_in_submodule_cb(const struct cache_entry *list_item, char *displaypath; displaypath = get_submodule_displaypath(path, info->prefix, - get_super_prefix()); + info->super_prefix); sub = submodule_from_path(the_repository, null_oid(), path); @@ -364,10 +365,10 @@ static void runcommand_in_submodule_cb(const struct cache_entry *list_item, cpr.dir = path; prepare_submodule_repo_env(&cpr.env); - strvec_pushl(&cpr.args, "--super-prefix", NULL); - strvec_pushf(&cpr.args, "%s/", displaypath); strvec_pushl(&cpr.args, "submodule--helper", "foreach", "--recursive", NULL); + strvec_pushl(&cpr.args, "--super-prefix", NULL); + strvec_pushf(&cpr.args, "%s/", displaypath); if (info->quiet) strvec_push(&cpr.args, "--quiet"); @@ -391,6 +392,7 @@ static int module_foreach(int argc, const char **argv, const char *prefix) struct pathspec pathspec = { 0 }; struct module_list list = MODULE_LIST_INIT; struct option module_foreach_options[] = { + OPT__SUPER_PREFIX(&info.super_prefix), OPT__QUIET(&info.quiet, N_("suppress output of entering each submodule command")), OPT_BOOL(0, "recursive", &info.recursive, N_("recurse into nested submodules")), @@ -3390,8 +3392,8 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix) subcmd = argv[0]; if (strcmp(subcmd, "clone") && strcmp(subcmd, "update") && - strcmp(subcmd, "foreach") && strcmp(subcmd, "status") && - strcmp(subcmd, "sync") && get_super_prefix()) + strcmp(subcmd, "status") && strcmp(subcmd, "sync") && + get_super_prefix()) /* * xstrfmt() rather than "%s %s" to keep the translated * string identical to git.c's. From 99a32d87f82683a3d3ff4a35dd551060c113a82c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 20 Dec 2022 13:39:53 +0100 Subject: [PATCH 6/9] submodule--helper: convert "sync" to its own "--super-prefix" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As with a preceding commit to convert "absorbgitdirs", we can convert "submodule--helper sync" to use its own "--super-prefix", instead of relying on the global "--super-prefix" argument to "git" itself. See that earlier commit for the rationale and background. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 2a6ced13f6..7262ce72ed 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -1208,12 +1208,13 @@ static int module_summary(int argc, const char **argv, const char *prefix) struct sync_cb { const char *prefix; + const char *super_prefix; unsigned int flags; }; #define SYNC_CB_INIT { 0 } static void sync_submodule(const char *path, const char *prefix, - unsigned int flags) + const char *super_prefix, unsigned int flags) { const struct submodule *sub; char *remote_key = NULL; @@ -1244,8 +1245,7 @@ static void sync_submodule(const char *path, const char *prefix, super_config_url = xstrdup(""); } - displaypath = get_submodule_displaypath(path, prefix, - get_super_prefix()); + displaypath = get_submodule_displaypath(path, prefix, super_prefix); if (!(flags & OPT_QUIET)) printf(_("Synchronizing submodule url for '%s'\n"), @@ -1282,10 +1282,11 @@ static void sync_submodule(const char *path, const char *prefix, cpr.dir = path; prepare_submodule_repo_env(&cpr.env); - strvec_push(&cpr.args, "--super-prefix"); - strvec_pushf(&cpr.args, "%s/", displaypath); strvec_pushl(&cpr.args, "submodule--helper", "sync", "--recursive", NULL); + strvec_push(&cpr.args, "--super-prefix"); + strvec_pushf(&cpr.args, "%s/", displaypath); + if (flags & OPT_QUIET) strvec_push(&cpr.args, "--quiet"); @@ -1308,7 +1309,8 @@ static void sync_submodule_cb(const struct cache_entry *list_item, void *cb_data { struct sync_cb *info = cb_data; - sync_submodule(list_item->name, info->prefix, info->flags); + sync_submodule(list_item->name, info->prefix, info->super_prefix, + info->flags); } static int module_sync(int argc, const char **argv, const char *prefix) @@ -1319,6 +1321,7 @@ static int module_sync(int argc, const char **argv, const char *prefix) int quiet = 0; int recursive = 0; struct option module_sync_options[] = { + OPT__SUPER_PREFIX(&info.super_prefix), OPT__QUIET(&quiet, N_("suppress output of synchronizing submodule url")), OPT_BOOL(0, "recursive", &recursive, N_("recurse into nested submodules")), @@ -2887,7 +2890,7 @@ static int module_set_url(int argc, const char **argv, const char *prefix) config_name = xstrfmt("submodule.%s.url", path); config_set_in_gitmodules_file_gently(config_name, newurl); - sync_submodule(path, prefix, quiet ? OPT_QUIET : 0); + sync_submodule(path, prefix, NULL, quiet ? OPT_QUIET : 0); free(config_name); @@ -3392,8 +3395,7 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix) subcmd = argv[0]; if (strcmp(subcmd, "clone") && strcmp(subcmd, "update") && - strcmp(subcmd, "status") && strcmp(subcmd, "sync") && - get_super_prefix()) + strcmp(subcmd, "status") && get_super_prefix()) /* * xstrfmt() rather than "%s %s" to keep the translated * string identical to git.c's. From 04f1fab4a13ad9eb2757809765720508abeb38da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 20 Dec 2022 13:39:54 +0100 Subject: [PATCH 7/9] submodule--helper: convert "status" to its own "--super-prefix" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As with a preceding commit to convert "absorbgitdirs", we can convert "submodule--helper status" to use its own "--super-prefix", instead of relying on the global "--super-prefix" argument to "git" itself. See that earlier commit for the rationale and background. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 7262ce72ed..448896f1b1 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -573,6 +573,7 @@ cleanup: struct status_cb { const char *prefix; + const char *super_prefix; unsigned int flags; }; #define STATUS_CB_INIT { 0 } @@ -611,7 +612,7 @@ static int handle_submodule_head_ref(const char *refname UNUSED, static void status_submodule(const char *path, const struct object_id *ce_oid, unsigned int ce_flags, const char *prefix, - unsigned int flags) + const char *super_prefix, unsigned int flags) { char *displaypath; struct strvec diff_files_args = STRVEC_INIT; @@ -627,8 +628,7 @@ static void status_submodule(const char *path, const struct object_id *ce_oid, die(_("no submodule mapping found in .gitmodules for path '%s'"), path); - displaypath = get_submodule_displaypath(path, prefix, - get_super_prefix()); + displaypath = get_submodule_displaypath(path, prefix, super_prefix); if ((CE_STAGEMASK & ce_flags) >> CE_STAGESHIFT) { print_status(flags, 'U', path, null_oid(), displaypath); @@ -686,10 +686,10 @@ static void status_submodule(const char *path, const struct object_id *ce_oid, cpr.dir = path; prepare_submodule_repo_env(&cpr.env); - strvec_push(&cpr.args, "--super-prefix"); - strvec_pushf(&cpr.args, "%s/", displaypath); strvec_pushl(&cpr.args, "submodule--helper", "status", "--recursive", NULL); + strvec_push(&cpr.args, "--super-prefix"); + strvec_pushf(&cpr.args, "%s/", displaypath); if (flags & OPT_CACHED) strvec_push(&cpr.args, "--cached"); @@ -713,7 +713,7 @@ static void status_submodule_cb(const struct cache_entry *list_item, struct status_cb *info = cb_data; status_submodule(list_item->name, &list_item->oid, list_item->ce_flags, - info->prefix, info->flags); + info->prefix, info->super_prefix, info->flags); } static int module_status(int argc, const char **argv, const char *prefix) @@ -723,6 +723,7 @@ static int module_status(int argc, const char **argv, const char *prefix) struct module_list list = MODULE_LIST_INIT; int quiet = 0; struct option module_status_options[] = { + OPT__SUPER_PREFIX(&info.super_prefix), OPT__QUIET(&quiet, N_("suppress submodule status output")), OPT_BIT(0, "cached", &info.flags, N_("use commit stored in the index instead of the one stored in the submodule HEAD"), OPT_CACHED), OPT_BIT(0, "recursive", &info.flags, N_("recurse into nested submodules"), OPT_RECURSIVE), @@ -3395,7 +3396,7 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix) subcmd = argv[0]; if (strcmp(subcmd, "clone") && strcmp(subcmd, "update") && - strcmp(subcmd, "status") && get_super_prefix()) + get_super_prefix()) /* * xstrfmt() rather than "%s %s" to keep the translated * string identical to git.c's. From f5a6be9d547700b7bd3018f1c0daf3d257cb2762 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 20 Dec 2022 13:39:55 +0100 Subject: [PATCH 8/9] submodule--helper: convert "{update,clone}" to their own "--super-prefix" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As with a preceding commit to convert "absorbgitdirs", we can convert "submodule--helper status" to use its own "--super-prefix", instead of relying on the global "--super-prefix" argument to "git". We need to convert both of these away from the global "--super-prefix" at the same time, because "update" will call "clone", but "clone" itself didn't make use of the global "--super-prefix" for displaying paths. It was only on the list of sub-commands that accepted it because "update"'s use of it would set it in its environment. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 32 +++++++++++++------------------- git.c | 2 +- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 448896f1b1..a0aa4635a0 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -437,11 +437,13 @@ static int starts_with_dot_dot_slash(const char *const path) struct init_cb { const char *prefix; + const char *super_prefix; unsigned int flags; }; #define INIT_CB_INIT { 0 } static void init_submodule(const char *path, const char *prefix, + const char *super_prefix, unsigned int flags) { const struct submodule *sub; @@ -449,8 +451,7 @@ static void init_submodule(const char *path, const char *prefix, const char *upd; char *url = NULL, *displaypath; - displaypath = get_submodule_displaypath(path, prefix, - get_super_prefix()); + displaypath = get_submodule_displaypath(path, prefix, super_prefix); sub = submodule_from_path(the_repository, null_oid(), path); @@ -526,7 +527,8 @@ static void init_submodule_cb(const struct cache_entry *list_item, void *cb_data { struct init_cb *info = cb_data; - init_submodule(list_item->name, info->prefix, info->flags); + init_submodule(list_item->name, info->prefix, info->super_prefix, + info->flags); } static int module_init(int argc, const char **argv, const char *prefix) @@ -792,6 +794,7 @@ struct summary_cb { int argc; const char **argv; const char *prefix; + const char *super_prefix; unsigned int cached: 1; unsigned int for_status: 1; unsigned int files: 1; @@ -954,7 +957,7 @@ static void generate_submodule_summary(struct summary_cb *info, } displaypath = get_submodule_displaypath(p->sm_path, info->prefix, - get_super_prefix()); + info->super_prefix); if (!missing_src && !missing_dst) { struct child_process cp_rev_list = CHILD_PROCESS_INIT; @@ -1893,6 +1896,7 @@ static void submodule_update_clone_release(struct submodule_update_clone *suc) struct update_data { const char *prefix; + const char *super_prefix; char *displaypath; enum submodule_update_type update_default; struct object_id suboid; @@ -1969,7 +1973,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce, char *key; const struct update_data *ud = suc->update_data; char *displaypath = get_submodule_displaypath(ce->name, ud->prefix, - get_super_prefix()); + ud->super_prefix); struct strbuf sb = STRBUF_INIT; int needs_cloning = 0; int need_free_url = 0; @@ -2449,11 +2453,11 @@ static void update_data_to_args(const struct update_data *update_data, { enum submodule_update_type update_type = update_data->update_default; + strvec_pushl(args, "submodule--helper", "update", "--recursive", NULL); if (update_data->displaypath) { strvec_push(args, "--super-prefix"); strvec_pushf(args, "%s/", update_data->displaypath); } - strvec_pushl(args, "submodule--helper", "update", "--recursive", NULL); strvec_pushf(args, "--jobs=%d", update_data->max_jobs); if (update_data->quiet) strvec_push(args, "--quiet"); @@ -2620,7 +2624,7 @@ static int update_submodules(struct update_data *update_data) update_data->displaypath = get_submodule_displaypath( update_data->sm_path, update_data->prefix, - get_super_prefix()); + update_data->super_prefix); code = update_submodule(update_data); FREE_AND_NULL(update_data->displaypath); fail: @@ -2646,6 +2650,7 @@ static int module_update(int argc, const char **argv, const char *prefix) LIST_OBJECTS_FILTER_INIT; int ret; struct option module_update_options[] = { + OPT__SUPER_PREFIX(&opt.super_prefix), OPT__FORCE(&opt.force, N_("force checkout updates"), 0), OPT_BOOL(0, "init", &opt.init, N_("initialize uninitialized submodules before update")), @@ -2742,6 +2747,7 @@ static int module_update(int argc, const char **argv, const char *prefix) module_list_active(&list); info.prefix = opt.prefix; + info.super_prefix = opt.super_prefix; if (opt.quiet) info.flags |= OPT_QUIET; @@ -3368,8 +3374,6 @@ cleanup: int cmd_submodule__helper(int argc, const char **argv, const char *prefix) { - const char *cmd = argv[0]; - const char *subcmd; parse_opt_subcommand_fn *fn = NULL; const char *const usage[] = { N_("git submodule--helper "), @@ -3393,16 +3397,6 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix) OPT_END() }; argc = parse_options(argc, argv, prefix, options, usage, 0); - subcmd = argv[0]; - - if (strcmp(subcmd, "clone") && strcmp(subcmd, "update") && - get_super_prefix()) - /* - * xstrfmt() rather than "%s %s" to keep the translated - * string identical to git.c's. - */ - die(_("%s doesn't support --super-prefix"), - xstrfmt("'%s %s'", cmd, subcmd)); return fn(argc, argv, prefix); } diff --git a/git.c b/git.c index e473b4b7d7..2c31ba2704 100644 --- a/git.c +++ b/git.c @@ -610,7 +610,7 @@ static struct cmd_struct commands[] = { { "stash", cmd_stash, RUN_SETUP | NEED_WORK_TREE }, { "status", cmd_status, RUN_SETUP | NEED_WORK_TREE }, { "stripspace", cmd_stripspace }, - { "submodule--helper", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX }, + { "submodule--helper", cmd_submodule__helper, RUN_SETUP }, { "switch", cmd_switch, RUN_SETUP | NEED_WORK_TREE }, { "symbolic-ref", cmd_symbolic_ref, RUN_SETUP }, { "tag", cmd_tag, RUN_SETUP | DELAY_PAGER_CONFIG }, From 4002ec3dcf0f89db46fbdf56549218c573a9c0f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 20 Dec 2022 13:39:56 +0100 Subject: [PATCH 9/9] read-tree: add "--super-prefix" option, eliminate global MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "--super-prefix" option to "git" was initially added in [1] for use with "ls-files"[2], and shortly thereafter "submodule--helper"[3] and "grep"[4]. It wasn't until [5] that "read-tree" made use of it. At the time [5] made sense, but since then we've made "ls-files" recurse in-process in [6], "grep" in [7], and finally "submodule--helper" in the preceding commits. Let's also remove it from "read-tree", which allows us to remove the option to "git" itself. We can do this because the only remaining user of it is the submodule API, which will now invoke "read-tree" with its new "--super-prefix" option. It will only do so when the "submodule_move_head()" function is called. That "submodule_move_head()" function was then only invoked by "read-tree" itself, but now rather than setting an environment variable to pass "--super-prefix" between cmd_read_tree() we: - Set a new "super_prefix" in "struct unpack_trees_options". The "super_prefixed()" function in "unpack-trees.c" added in [5] will now use this, rather than get_super_prefix() looking up the environment variable we set earlier in the same process. - Add the same field to the "struct checkout", which is only needed to ferry the "super_prefix" in the "struct unpack_trees_options" all the way down to the "entry.c" callers of "submodule_move_head()". Those calls which used the super prefix all originated in "cmd_read_tree()". The only other caller is the "unlink_entry()" caller in "builtin/checkout.c", which now passes a "NULL". 1. 74866d75793 (git: make super-prefix option, 2016-10-07) 2. e77aa336f11 (ls-files: optionally recurse into submodules, 2016-10-07) 3. 89c86265576 (submodule helper: support super prefix, 2016-12-08) 4. 0281e487fd9 (grep: optionally recurse into submodules, 2016-12-16) 5. 3d415425c7b (unpack-trees: support super-prefix option, 2017-01-17) 6. 188dce131fa (ls-files: use repository object, 2017-06-22) 7. f9ee2fcdfa0 (grep: recurse in-process using 'struct repository', 2017-08-02) Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- Documentation/git.txt | 8 +------- builtin.h | 4 ---- builtin/checkout.c | 2 +- builtin/read-tree.c | 1 + cache.h | 2 -- entry.c | 12 ++++++------ entry.h | 6 +++++- environment.c | 13 ------------- git.c | 37 +++++-------------------------------- submodule.c | 29 ++++++++++------------------- submodule.h | 5 ++--- t/t1001-read-tree-m-2way.sh | 2 +- t/t5616-partial-clone.sh | 2 +- unpack-trees.c | 24 ++++++++++++++---------- unpack-trees.h | 1 + 15 files changed, 48 insertions(+), 100 deletions(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index 1d33e083ab..f9a7a4554c 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -13,8 +13,7 @@ SYNOPSIS [--exec-path[=]] [--html-path] [--man-path] [--info-path] [-p|--paginate|-P|--no-pager] [--no-replace-objects] [--bare] [--git-dir=] [--work-tree=] [--namespace=] - [--super-prefix=] [--config-env==] - [] + [--config-env==] [] DESCRIPTION ----------- @@ -169,11 +168,6 @@ If you just want to run git as if it was started in `` then use details. Equivalent to setting the `GIT_NAMESPACE` environment variable. ---super-prefix=:: - Currently for internal use only. Set a prefix which gives a path from - above a repository down to its root. One use is to give submodules - context about the superproject that invoked it. - --bare:: Treat the repository as a bare repository. If GIT_DIR environment is not set, it is set to the current working diff --git a/builtin.h b/builtin.h index aa955466b4..46cc789789 100644 --- a/builtin.h +++ b/builtin.h @@ -51,10 +51,6 @@ * on bare repositories. * This only makes sense when `RUN_SETUP` is also set. * - * `SUPPORT_SUPER_PREFIX`: - * - * The built-in supports `--super-prefix`. - * * `DELAY_PAGER_CONFIG`: * * If RUN_SETUP or RUN_SETUP_GENTLY is set, git.c normally handles diff --git a/builtin/checkout.c b/builtin/checkout.c index a0142c815f..cd04f0bf57 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -232,7 +232,7 @@ static int checkout_stage(int stage, const struct cache_entry *ce, int pos, pos++; } if (!overlay_mode) { - unlink_entry(ce); + unlink_entry(ce, NULL); return 0; } if (stage == 2) diff --git a/builtin/read-tree.c b/builtin/read-tree.c index f702f9d47b..3ce7541783 100644 --- a/builtin/read-tree.c +++ b/builtin/read-tree.c @@ -114,6 +114,7 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix) int prefix_set = 0; struct lock_file lock_file = LOCK_INIT; const struct option read_tree_options[] = { + OPT__SUPER_PREFIX(&opts.super_prefix), OPT_CALLBACK_F(0, "index-output", NULL, N_("file"), N_("write resulting index to "), PARSE_OPT_NONEG, index_output_cb), diff --git a/cache.h b/cache.h index 07d40b0964..7f7e2b5a38 100644 --- a/cache.h +++ b/cache.h @@ -480,7 +480,6 @@ static inline enum object_type object_type(unsigned int mode) #define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE" #define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE" #define GIT_PREFIX_ENVIRONMENT "GIT_PREFIX" -#define GIT_SUPER_PREFIX_ENVIRONMENT "GIT_INTERNAL_SUPER_PREFIX" #define DEFAULT_GIT_DIR_ENVIRONMENT ".git" #define DB_ENVIRONMENT "GIT_OBJECT_DIRECTORY" #define INDEX_ENVIRONMENT "GIT_INDEX_FILE" @@ -566,7 +565,6 @@ int get_common_dir_noenv(struct strbuf *sb, const char *gitdir); int get_common_dir(struct strbuf *sb, const char *gitdir); const char *get_git_namespace(void); const char *strip_namespace(const char *namespaced_ref); -const char *get_super_prefix(void); const char *get_git_work_tree(void); /* diff --git a/entry.c b/entry.c index 616e4f073c..971ab26871 100644 --- a/entry.c +++ b/entry.c @@ -383,7 +383,7 @@ static int write_entry(struct cache_entry *ce, char *path, struct conv_attrs *ca return error("cannot create submodule directory %s", path); sub = submodule_from_ce(ce); if (sub) - return submodule_move_head(ce->name, + return submodule_move_head(ce->name, state->super_prefix, NULL, oid_to_hex(&ce->oid), state->force ? SUBMODULE_MOVE_HEAD_FORCE : 0); break; @@ -476,7 +476,7 @@ int checkout_entry_ca(struct cache_entry *ce, struct conv_attrs *ca, * no pathname to return. */ BUG("Can't remove entry to a path"); - unlink_entry(ce); + unlink_entry(ce, state->super_prefix); return 0; } @@ -510,10 +510,10 @@ int checkout_entry_ca(struct cache_entry *ce, struct conv_attrs *ca, if (!(st.st_mode & S_IFDIR)) unlink_or_warn(ce->name); - return submodule_move_head(ce->name, + return submodule_move_head(ce->name, state->super_prefix, NULL, oid_to_hex(&ce->oid), 0); } else - return submodule_move_head(ce->name, + return submodule_move_head(ce->name, state->super_prefix, "HEAD", oid_to_hex(&ce->oid), state->force ? SUBMODULE_MOVE_HEAD_FORCE : 0); } @@ -560,12 +560,12 @@ int checkout_entry_ca(struct cache_entry *ce, struct conv_attrs *ca, return write_entry(ce, path.buf, ca, state, 0, nr_checkouts); } -void unlink_entry(const struct cache_entry *ce) +void unlink_entry(const struct cache_entry *ce, const char *super_prefix) { const struct submodule *sub = submodule_from_ce(ce); if (sub) { /* state.force is set at the caller. */ - submodule_move_head(ce->name, "HEAD", NULL, + submodule_move_head(ce->name, super_prefix, "HEAD", NULL, SUBMODULE_MOVE_HEAD_FORCE); } if (check_leading_path(ce->name, ce_namelen(ce), 1) >= 0) diff --git a/entry.h b/entry.h index 9be4659881..2d4fbb88c8 100644 --- a/entry.h +++ b/entry.h @@ -8,6 +8,7 @@ struct checkout { struct index_state *istate; const char *base_dir; int base_dir_len; + const char *super_prefix; struct delayed_checkout *delayed_checkout; struct checkout_metadata meta; unsigned force:1, @@ -48,8 +49,11 @@ int finish_delayed_checkout(struct checkout *state, int show_progress); /* * Unlink the last component and schedule the leading directories for * removal, such that empty directories get removed. + * + * The "super_prefix" is either NULL, or the "--super-prefix" passed + * down from "read-tree" et al. */ -void unlink_entry(const struct cache_entry *ce); +void unlink_entry(const struct cache_entry *ce, const char *super_prefix); void *read_blob_entry(const struct cache_entry *ce, size_t *size); int fstat_checkout_output(int fd, const struct checkout *state, struct stat *st); diff --git a/environment.c b/environment.c index 18d042b467..1ee3686fd8 100644 --- a/environment.c +++ b/environment.c @@ -102,8 +102,6 @@ char *git_work_tree_cfg; static char *git_namespace; -static char *super_prefix; - /* * Repository-local GIT_* environment variables; see cache.h for details. */ @@ -121,7 +119,6 @@ const char * const local_repo_env[] = { NO_REPLACE_OBJECTS_ENVIRONMENT, GIT_REPLACE_REF_BASE_ENVIRONMENT, GIT_PREFIX_ENVIRONMENT, - GIT_SUPER_PREFIX_ENVIRONMENT, GIT_SHALLOW_FILE_ENVIRONMENT, GIT_COMMON_DIR_ENVIRONMENT, NULL @@ -234,16 +231,6 @@ const char *strip_namespace(const char *namespaced_ref) return NULL; } -const char *get_super_prefix(void) -{ - static int initialized; - if (!initialized) { - super_prefix = xstrdup_or_null(getenv(GIT_SUPER_PREFIX_ENVIRONMENT)); - initialized = 1; - } - return super_prefix; -} - static int git_work_tree_initialized; /* diff --git a/git.c b/git.c index 2c31ba2704..32a5be68a1 100644 --- a/git.c +++ b/git.c @@ -14,9 +14,8 @@ * RUN_SETUP for reading from the configuration file. */ #define NEED_WORK_TREE (1<<3) -#define SUPPORT_SUPER_PREFIX (1<<4) -#define DELAY_PAGER_CONFIG (1<<5) -#define NO_PARSEOPT (1<<6) /* parse-options is not used */ +#define DELAY_PAGER_CONFIG (1<<4) +#define NO_PARSEOPT (1<<5) /* parse-options is not used */ struct cmd_struct { const char *cmd; @@ -29,8 +28,7 @@ const char git_usage_string[] = " [--exec-path[=]] [--html-path] [--man-path] [--info-path]\n" " [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--bare]\n" " [--git-dir=] [--work-tree=] [--namespace=]\n" - " [--super-prefix=] [--config-env==]\n" - " []"); + " [--config-env==] []"); const char git_more_info_string[] = N_("'git help -a' and 'git help -g' list available subcommands and some\n" @@ -226,20 +224,6 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) setenv(GIT_WORK_TREE_ENVIRONMENT, cmd, 1); if (envchanged) *envchanged = 1; - } else if (!strcmp(cmd, "--super-prefix")) { - if (*argc < 2) { - fprintf(stderr, _("no prefix given for --super-prefix\n" )); - usage(git_usage_string); - } - setenv(GIT_SUPER_PREFIX_ENVIRONMENT, (*argv)[1], 1); - if (envchanged) - *envchanged = 1; - (*argv)++; - (*argc)--; - } else if (skip_prefix(cmd, "--super-prefix=", &cmd)) { - setenv(GIT_SUPER_PREFIX_ENVIRONMENT, cmd, 1); - if (envchanged) - *envchanged = 1; } else if (!strcmp(cmd, "--bare")) { char *cwd = xgetcwd(); is_bare_repository_cfg = 1; @@ -449,11 +433,6 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) trace_repo_setup(prefix); commit_pager_choice(); - if (!help && get_super_prefix()) { - if (!(p->option & SUPPORT_SUPER_PREFIX)) - die(_("%s doesn't support --super-prefix"), p->cmd); - } - if (!help && p->option & NEED_WORK_TREE) setup_work_tree(); @@ -504,7 +483,7 @@ static struct cmd_struct commands[] = { { "check-ref-format", cmd_check_ref_format, NO_PARSEOPT }, { "checkout", cmd_checkout, RUN_SETUP | NEED_WORK_TREE }, { "checkout--worker", cmd_checkout__worker, - RUN_SETUP | NEED_WORK_TREE | SUPPORT_SUPER_PREFIX }, + RUN_SETUP | NEED_WORK_TREE }, { "checkout-index", cmd_checkout_index, RUN_SETUP | NEED_WORK_TREE}, { "cherry", cmd_cherry, RUN_SETUP }, @@ -583,7 +562,7 @@ static struct cmd_struct commands[] = { { "pull", cmd_pull, RUN_SETUP | NEED_WORK_TREE }, { "push", cmd_push, RUN_SETUP }, { "range-diff", cmd_range_diff, RUN_SETUP | USE_PAGER }, - { "read-tree", cmd_read_tree, RUN_SETUP | SUPPORT_SUPER_PREFIX}, + { "read-tree", cmd_read_tree, RUN_SETUP }, { "rebase", cmd_rebase, RUN_SETUP | NEED_WORK_TREE }, { "receive-pack", cmd_receive_pack }, { "reflog", cmd_reflog, RUN_SETUP }, @@ -727,9 +706,6 @@ static void execv_dashed_external(const char **argv) struct child_process cmd = CHILD_PROCESS_INIT; int status; - if (get_super_prefix()) - die(_("%s doesn't support --super-prefix"), argv[0]); - if (use_pager == -1 && !is_builtin(argv[0])) use_pager = check_pager_config(argv[0]); commit_pager_choice(); @@ -799,9 +775,6 @@ static int run_argv(int *argcp, const char ***argv) */ trace2_cmd_name("_run_git_alias_"); - if (get_super_prefix()) - die("%s doesn't support --super-prefix", **argv); - commit_pager_choice(); strvec_push(&cmd.args, "git"); diff --git a/submodule.c b/submodule.c index 46a0347319..d730b64b07 100644 --- a/submodule.c +++ b/submodule.c @@ -2054,14 +2054,6 @@ void submodule_unset_core_worktree(const struct submodule *sub) strbuf_release(&config_path); } -static const char *get_super_prefix_or_empty(void) -{ - const char *s = get_super_prefix(); - if (!s) - s = ""; - return s; -} - static int submodule_has_dirty_index(const struct submodule *sub) { struct child_process cp = CHILD_PROCESS_INIT; @@ -2080,7 +2072,7 @@ static int submodule_has_dirty_index(const struct submodule *sub) return finish_command(&cp); } -static void submodule_reset_index(const char *path) +static void submodule_reset_index(const char *path, const char *super_prefix) { struct child_process cp = CHILD_PROCESS_INIT; prepare_submodule_repo_env(&cp.env); @@ -2089,10 +2081,10 @@ static void submodule_reset_index(const char *path) cp.no_stdin = 1; cp.dir = path; - strvec_pushf(&cp.args, "--super-prefix=%s%s/", - get_super_prefix_or_empty(), path); /* TODO: determine if this might overwright untracked files */ strvec_pushl(&cp.args, "read-tree", "-u", "--reset", NULL); + strvec_pushf(&cp.args, "--super-prefix=%s%s/", + (super_prefix ? super_prefix : ""), path); strvec_push(&cp.args, empty_tree_oid_hex()); @@ -2105,10 +2097,9 @@ static void submodule_reset_index(const char *path) * For edge cases (a submodule coming into existence or removing a submodule) * pass NULL for old or new respectively. */ -int submodule_move_head(const char *path, - const char *old_head, - const char *new_head, - unsigned flags) +int submodule_move_head(const char *path, const char *super_prefix, + const char *old_head, const char *new_head, + unsigned flags) { int ret = 0; struct child_process cp = CHILD_PROCESS_INIT; @@ -2146,7 +2137,7 @@ int submodule_move_head(const char *path, if (old_head) { if (!submodule_uses_gitfile(path)) absorb_git_dir_into_superproject(path, - get_super_prefix()); + super_prefix); } else { struct strbuf gitdir = STRBUF_INIT; submodule_name_to_gitdir(&gitdir, the_repository, @@ -2155,7 +2146,7 @@ int submodule_move_head(const char *path, strbuf_release(&gitdir); /* make sure the index is clean as well */ - submodule_reset_index(path); + submodule_reset_index(path, super_prefix); } if (old_head && (flags & SUBMODULE_MOVE_HEAD_FORCE)) { @@ -2173,9 +2164,9 @@ int submodule_move_head(const char *path, cp.no_stdin = 1; cp.dir = path; - strvec_pushf(&cp.args, "--super-prefix=%s%s/", - get_super_prefix_or_empty(), path); strvec_pushl(&cp.args, "read-tree", "--recurse-submodules", NULL); + strvec_pushf(&cp.args, "--super-prefix=%s%s/", + (super_prefix ? super_prefix : ""), path); if (flags & SUBMODULE_MOVE_HEAD_DRY_RUN) strvec_push(&cp.args, "-n"); diff --git a/submodule.h b/submodule.h index f90ee547d0..c55a25ca37 100644 --- a/submodule.h +++ b/submodule.h @@ -150,9 +150,8 @@ int validate_submodule_git_dir(char *git_dir, const char *submodule_name); #define SUBMODULE_MOVE_HEAD_DRY_RUN (1<<0) #define SUBMODULE_MOVE_HEAD_FORCE (1<<1) -int submodule_move_head(const char *path, - const char *old, - const char *new_head, +int submodule_move_head(const char *path, const char *super_prefix, + const char *old_head, const char *new_head, unsigned flags); void submodule_unset_core_worktree(const struct submodule *sub); diff --git a/t/t1001-read-tree-m-2way.sh b/t/t1001-read-tree-m-2way.sh index 516a6112fd..3fb1b0c162 100755 --- a/t/t1001-read-tree-m-2way.sh +++ b/t/t1001-read-tree-m-2way.sh @@ -370,7 +370,7 @@ test_expect_success 'read-tree supports the super-prefix' ' cat <<-EOF >expect && error: Updating '\''fictional/a'\'' would lose untracked files in it EOF - test_must_fail git --super-prefix fictional/ read-tree -u -m "$treeH" "$treeM" 2>actual && + test_must_fail git read-tree --super-prefix fictional/ -u -m "$treeH" "$treeM" 2>actual && test_cmp expect actual ' diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh index 2846ec6629..f519d2a87a 100755 --- a/t/t5616-partial-clone.sh +++ b/t/t5616-partial-clone.sh @@ -644,7 +644,7 @@ test_expect_success 'repack does not loosen promisor objects' ' grep "loosen_unused_packed_objects/loosened:0" trace ' -test_expect_failure 'lazy-fetch in submodule succeeds' ' +test_expect_success 'lazy-fetch in submodule succeeds' ' # setup test_config_global protocol.file.allow always && diff --git a/unpack-trees.c b/unpack-trees.c index 8a762aa077..2b8e5da4c2 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -71,7 +71,7 @@ static const char *unpack_plumbing_errors[NB_UNPACK_TREES_WARNING_TYPES] = { ? ((o)->msgs[(type)]) \ : (unpack_plumbing_errors[(type)]) ) -static const char *super_prefixed(const char *path) +static const char *super_prefixed(const char *path, const char *super_prefix) { /* * It is necessary and sufficient to have two static buffers @@ -83,7 +83,6 @@ static const char *super_prefixed(const char *path) static unsigned idx = ARRAY_SIZE(buf) - 1; if (super_prefix_len < 0) { - const char *super_prefix = get_super_prefix(); if (!super_prefix) { super_prefix_len = 0; } else { @@ -236,7 +235,8 @@ static int add_rejected_path(struct unpack_trees_options *o, return -1; if (!o->show_all_errors) - return error(ERRORMSG(o, e), super_prefixed(path)); + return error(ERRORMSG(o, e), super_prefixed(path, + o->super_prefix)); /* * Otherwise, insert in a list for future display by @@ -263,7 +263,8 @@ static void display_error_msgs(struct unpack_trees_options *o) error_displayed = 1; for (i = 0; i < rejects->nr; i++) strbuf_addf(&path, "\t%s\n", rejects->items[i].string); - error(ERRORMSG(o, e), super_prefixed(path.buf)); + error(ERRORMSG(o, e), super_prefixed(path.buf, + o->super_prefix)); strbuf_release(&path); } string_list_clear(rejects, 0); @@ -290,7 +291,8 @@ static void display_warning_msgs(struct unpack_trees_options *o) warning_displayed = 1; for (i = 0; i < rejects->nr; i++) strbuf_addf(&path, "\t%s\n", rejects->items[i].string); - warning(ERRORMSG(o, e), super_prefixed(path.buf)); + warning(ERRORMSG(o, e), super_prefixed(path.buf, + o->super_prefix)); strbuf_release(&path); } string_list_clear(rejects, 0); @@ -312,7 +314,8 @@ static int check_submodule_move_head(const struct cache_entry *ce, if (o->reset) flags |= SUBMODULE_MOVE_HEAD_FORCE; - if (submodule_move_head(ce->name, old_id, new_id, flags)) + if (submodule_move_head(ce->name, o->super_prefix, old_id, new_id, + flags)) return add_rejected_path(o, ERROR_WOULD_LOSE_SUBMODULE, ce->name); return 0; } @@ -415,6 +418,7 @@ static int check_updates(struct unpack_trees_options *o, int i, pc_workers, pc_threshold; trace_performance_enter(); + state.super_prefix = o->super_prefix; state.force = 1; state.quiet = 1; state.refresh_cache = 1; @@ -445,7 +449,7 @@ static int check_updates(struct unpack_trees_options *o, if (ce->ce_flags & CE_WT_REMOVE) { display_progress(progress, ++cnt); - unlink_entry(ce); + unlink_entry(ce, o->super_prefix); } } @@ -2959,8 +2963,8 @@ int bind_merge(const struct cache_entry * const *src, if (a && old) return o->quiet ? -1 : error(ERRORMSG(o, ERROR_BIND_OVERLAP), - super_prefixed(a->name), - super_prefixed(old->name)); + super_prefixed(a->name, o->super_prefix), + super_prefixed(old->name, o->super_prefix)); if (!a) return keep_entry(old, o); else @@ -3021,7 +3025,7 @@ int stash_worktree_untracked_merge(const struct cache_entry * const *src, if (worktree && untracked) return error(_("worktree and untracked commit have duplicate entries: %s"), - super_prefixed(worktree->name)); + super_prefixed(worktree->name, o->super_prefix)); return merged_entry(worktree ? worktree : untracked, NULL, o); } diff --git a/unpack-trees.h b/unpack-trees.h index 6ab0d74c84..3a7b3e5f00 100644 --- a/unpack-trees.h +++ b/unpack-trees.h @@ -75,6 +75,7 @@ struct unpack_trees_options { skip_cache_tree_update; enum unpack_trees_reset_type reset; const char *prefix; + const char *super_prefix; int cache_bottom; struct pathspec *pathspec; merge_fn_t fn;