From 3b5fb32da836f5aead1cef319bc3e0a9b975ea35 Mon Sep 17 00:00:00 2001 From: Nasser Grainawi Date: Tue, 3 Mar 2026 15:40:44 -0800 Subject: [PATCH 01/11] submodule: fetch missing objects from default remote When be76c21282 (fetch: ensure submodule objects fetched, 2018-12-06) added support for fetching a missing submodule object by id, it hardcoded the remote name as "origin" and deferred anything more complicated for a later patch. Implement the NEEDSWORK item to remove the hardcoded assumption by adding and using a submodule helper subcmd 'get-default-remote'. Fixing this lets 'git fetch --recurse-submodules' succeed when the fetched commit(s) in the superproject trigger a submodule fetch, and that submodule's default remote name is not "origin". Add non-"origin" remote tests to t5526-fetch-submodules.sh and t5572-pull-submodule.sh demonstrating this works as expected and add dedicated tests for get-default-remote. Signed-off-by: Nasser Grainawi Reviewed-by: Jacob Keller Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 38 +++++ submodule.c | 17 ++- t/meson.build | 1 + t/t5526-fetch-submodules.sh | 71 ++++++++- t/t5572-pull-submodule.sh | 21 ++- t/t7426-submodule-get-default-remote.sh | 186 ++++++++++++++++++++++++ 6 files changed, 330 insertions(+), 4 deletions(-) create mode 100755 t/t7426-submodule-get-default-remote.sh diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index d537ab087a02e9..b180a240910024 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -112,6 +112,43 @@ static int get_default_remote_submodule(const char *module_path, char **default_ return 0; } +static int module_get_default_remote(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) +{ + const char *path; + char *resolved_path = NULL; + char *default_remote = NULL; + int code; + struct option options[] = { + OPT_END() + }; + const char *const usage[] = { + N_("git submodule--helper get-default-remote "), + NULL + }; + + argc = parse_options(argc, argv, prefix, options, usage, 0); + if (argc != 1) + usage_with_options(usage, options); + + path = argv[0]; + if (prefix && *prefix && !is_absolute_path(path)) { + resolved_path = xstrfmt("%s%s", prefix, path); + path = resolved_path; + } + + code = get_default_remote_submodule(path, &default_remote); + if (code) { + free(resolved_path); + return code; + } + + printf("%s\n", default_remote); + free(default_remote); + free(resolved_path); + return 0; +} + /* the result should be freed by the caller. */ static char *get_submodule_displaypath(const char *path, const char *prefix, const char *super_prefix) @@ -3608,6 +3645,7 @@ int cmd_submodule__helper(int argc, OPT_SUBCOMMAND("set-url", &fn, module_set_url), OPT_SUBCOMMAND("set-branch", &fn, module_set_branch), OPT_SUBCOMMAND("create-branch", &fn, module_create_branch), + OPT_SUBCOMMAND("get-default-remote", &fn, module_get_default_remote), OPT_END() }; argc = parse_options(argc, argv, prefix, options, usage, 0); diff --git a/submodule.c b/submodule.c index 40a5c6fb9d1545..6599657f341e10 100644 --- a/submodule.c +++ b/submodule.c @@ -1706,6 +1706,8 @@ static int get_next_submodule(struct child_process *cp, struct strbuf *err, if (spf->oid_fetch_tasks_nr) { struct fetch_task *task = spf->oid_fetch_tasks[spf->oid_fetch_tasks_nr - 1]; + struct child_process cp_remote = CHILD_PROCESS_INIT; + struct strbuf remote_name = STRBUF_INIT; spf->oid_fetch_tasks_nr--; child_process_init(cp); @@ -1719,8 +1721,19 @@ static int get_next_submodule(struct child_process *cp, struct strbuf *err, strvec_pushf(&cp->args, "--submodule-prefix=%s%s/", spf->prefix, task->sub->path); - /* NEEDSWORK: have get_default_remote from submodule--helper */ - strvec_push(&cp->args, "origin"); + cp_remote.git_cmd = 1; + strvec_pushl(&cp_remote.args, "submodule--helper", + "get-default-remote", task->sub->path, NULL); + + if (!capture_command(&cp_remote, &remote_name, 0)) { + strbuf_trim_trailing_newline(&remote_name); + strvec_push(&cp->args, remote_name.buf); + } else { + /* Fallback to "origin" if the helper fails */ + strvec_push(&cp->args, "origin"); + } + strbuf_release(&remote_name); + oid_array_for_each_unique(task->commits, append_oid_to_argv, &cp->args); diff --git a/t/meson.build b/t/meson.build index 459c52a48972e4..6ff54cf2769038 100644 --- a/t/meson.build +++ b/t/meson.build @@ -887,6 +887,7 @@ integration_tests = [ 't7422-submodule-output.sh', 't7423-submodule-symlinks.sh', 't7424-submodule-mixed-ref-formats.sh', + 't7426-submodule-get-default-remote.sh', 't7450-bad-git-dotfiles.sh', 't7500-commit-template-squash-signoff.sh', 't7501-commit-basic-functionality.sh', diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index 5e566205ba4b95..1242ee918526ea 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -834,11 +834,18 @@ test_expect_success "fetch new submodule commits on-demand outside standard refs git commit -m "updated submodules outside of refs/heads" && E=$(git rev-parse HEAD) && git update-ref refs/changes/3 $E && + FETCH_TRACE="$(pwd)/trace.out" && + test_when_finished "rm -f \"$FETCH_TRACE\"" && ( cd downstream && - git fetch --recurse-submodules origin refs/changes/3:refs/heads/my_branch && + GIT_TRACE="$FETCH_TRACE" git fetch --recurse-submodules origin \ + refs/changes/3:refs/heads/my_branch && git -C submodule cat-file -t $C && git -C sub1 cat-file -t $D && + test_grep "trace: built-in: git submodule--helper get-default-remote sub1" \ + "$FETCH_TRACE" && + test_grep "trace: built-in: git fetch .* --submodule-prefix=sub1/ origin" \ + "$FETCH_TRACE" && git checkout --recurse-submodules FETCH_HEAD ) ' @@ -929,6 +936,68 @@ test_expect_success 'fetch new submodule commit intermittently referenced by sup ) ' +test_expect_success 'fetch new submodule commits on-demand outside standard refspec with custom remote name' ' + # depends on the previous test for setup + + # Rename the remote in sub1 from "origin" to "custom_remote" + git -C downstream/sub1 remote rename origin custom_remote && + + # Create new commits in the original submodules + C=$(git -C submodule commit-tree \ + -m "change outside refs/heads for custom remote" HEAD^{tree}) && + git -C submodule update-ref refs/changes/custom1 $C && + git update-index --cacheinfo 160000 $C submodule && + test_tick && + + D=$(git -C sub1 commit-tree \ + -m "change outside refs/heads for custom remote" HEAD^{tree}) && + git -C sub1 update-ref refs/changes/custom2 $D && + git update-index --cacheinfo 160000 $D sub1 && + + git commit \ + -m "updated submodules outside of refs/heads for custom remote" && + E=$(git rev-parse HEAD) && + git update-ref refs/changes/custom3 $E && + FETCH_TRACE="$(pwd)/trace.out" && + test_when_finished "rm -f \"$FETCH_TRACE\"" && + ( + cd downstream && + GIT_TRACE="$FETCH_TRACE" git fetch --recurse-submodules origin \ + refs/changes/custom3:refs/heads/my_other_branch && + git -C submodule cat-file -t $C && + git -C sub1 cat-file -t $D && + test_grep "trace: built-in: git submodule--helper get-default-remote sub1" \ + "$FETCH_TRACE" && + test_grep "trace: built-in: git fetch .* --submodule-prefix=sub1/ custom_remote $D" \ + "$FETCH_TRACE" && + git checkout --recurse-submodules FETCH_HEAD + ) +' + +test_expect_success 'fetch new submodule commit on-demand in FETCH_HEAD from custom remote' ' + # depends on the previous test for setup + + C=$(git -C submodule commit-tree -m "another change outside refs/heads for custom remote" HEAD^{tree}) && + git -C submodule update-ref refs/changes/custom4 $C && + git update-index --cacheinfo 160000 $C submodule && + test_tick && + + D=$(git -C sub1 commit-tree -m "another change outside refs/heads for custom remote" HEAD^{tree}) && + git -C sub1 update-ref refs/changes/custom5 $D && + git update-index --cacheinfo 160000 $D sub1 && + + git commit -m "updated submodules outside of refs/heads" && + E=$(git rev-parse HEAD) && + git update-ref refs/changes/custom6 $E && + ( + cd downstream && + git fetch --recurse-submodules origin refs/changes/custom6 && + git -C submodule cat-file -t $C && + git -C sub1 cat-file -t $D && + git checkout --recurse-submodules FETCH_HEAD + ) +' + add_commit_push () { dir="$1" && msg="$2" && diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh index 45f384dd328054..42d14328b6b42a 100755 --- a/t/t5572-pull-submodule.sh +++ b/t/t5572-pull-submodule.sh @@ -257,7 +257,26 @@ test_expect_success 'fetch submodule remote of different name from superproject' git -C a-submodule reset --hard HEAD^^ && git -C child pull --no-recurse-submodules && - git -C child submodule update + git -C child submodule update && + test_path_is_file child/a-submodule/moreecho.t +' + +test_expect_success 'fetch non-origin submodule remote named different from superproject' ' + git -C child/a-submodule remote rename origin o2 && + + # Create commit that is unreachable from current master branch + # newmain is already reset in the previous test + test_commit -C a-submodule echo_o2 && + test_commit -C a-submodule moreecho_o2 && + subc=$(git -C a-submodule rev-parse --short HEAD) && + + git -C parent/a-submodule fetch && + git -C parent/a-submodule checkout "$subc" && + git -C parent commit -m "update submodule o2" a-submodule && + git -C a-submodule reset --hard HEAD^^ && + + git -C child pull --recurse-submodules && + test_path_is_file child/a-submodule/moreecho_o2.t ' test_done diff --git a/t/t7426-submodule-get-default-remote.sh b/t/t7426-submodule-get-default-remote.sh new file mode 100755 index 00000000000000..b842af9a2d26ff --- /dev/null +++ b/t/t7426-submodule-get-default-remote.sh @@ -0,0 +1,186 @@ +#!/bin/sh + +test_description='git submodule--helper get-default-remote' + +TEST_NO_CREATE_REPO=1 +. ./test-lib.sh + +test_expect_success 'setup' ' + git config --global protocol.file.allow always +' + +test_expect_success 'setup repositories' ' + # Create a repository to be used as submodule + git init sub && + test_commit --no-tag -C sub "initial commit in sub" file.txt "sub content" && + + # Create main repository + git init super && + ( + cd super && + mkdir subdir && + test_commit --no-tag -C subdir "initial commit in super" main.txt "super content" && + git submodule add ../sub subpath && + git commit -m "add submodule 'sub' at subpath" + ) +' + +test_expect_success 'get-default-remote returns origin for initialized submodule' ' + ( + cd super && + git submodule update --init && + echo "origin" >expect && + git submodule--helper get-default-remote subpath >actual && + test_cmp expect actual + ) +' + +test_expect_success 'get-default-remote works from subdirectory' ' + ( + cd super/subdir && + echo "origin" >expect && + git submodule--helper get-default-remote ../subpath >actual && + test_cmp expect actual + ) +' + +test_expect_success 'get-default-remote fails with non-existent path' ' + ( + cd super && + test_must_fail git submodule--helper get-default-remote nonexistent 2>err && + test_grep "could not get a repository handle" err + ) +' + +test_expect_success 'get-default-remote fails with non-submodule path' ' + ( + cd super && + test_must_fail git submodule--helper get-default-remote subdir 2>err && + test_grep "could not get a repository handle" err + ) +' + +test_expect_success 'get-default-remote fails without path argument' ' + ( + cd super && + test_must_fail git submodule--helper get-default-remote 2>err && + test_grep "usage:" err + ) +' + +test_expect_success 'get-default-remote fails with too many arguments' ' + ( + cd super && + test_must_fail git submodule--helper get-default-remote subpath subdir 2>err && + test_grep "usage:" err + ) +' + +test_expect_success 'setup submodule with non-origin default remote name' ' + # Create another submodule path with a different remote name + ( + cd super && + git submodule add ../sub upstream-subpath && + git commit -m "add second submodule in upstream-subpath" && + git submodule update --init upstream-subpath && + + # Change the remote name in the submodule + cd upstream-subpath && + git remote rename origin upstream + ) +' + +test_expect_success 'get-default-remote returns non-origin remote name' ' + ( + cd super && + echo "upstream" >expect && + git submodule--helper get-default-remote upstream-subpath >actual && + test_cmp expect actual + ) +' + +test_expect_success 'get-default-remote handles submodule with multiple remotes' ' + ( + cd super/subpath && + git remote add other-upstream ../../sub && + git remote add myfork ../../sub + ) && + + ( + cd super && + echo "origin" >expect && + git submodule--helper get-default-remote subpath >actual && + test_cmp expect actual + ) +' + +test_expect_success 'get-default-remote handles submodule with multiple remotes and none are origin' ' + ( + cd super/upstream-subpath && + git remote add yet-another-upstream ../../sub && + git remote add yourfork ../../sub + ) && + + ( + cd super && + echo "upstream" >expect && + git submodule--helper get-default-remote upstream-subpath >actual && + test_cmp expect actual + ) +' + +test_expect_success 'setup nested submodule with non-origin remote' ' + git init innersub && + test_commit --no-tag -C innersub "initial commit in innersub" inner.txt "innersub content" && + + ( + cd sub && + git submodule add ../innersub innersubpath && + git commit -m "add nested submodule at innersubpath" + ) && + + ( + cd super/upstream-subpath && + git pull upstream && + git submodule update --init --recursive . && + ( + cd innersubpath && + git remote rename origin another_upstream + ) + ) +' + +test_expect_success 'get-default-remote works with nested submodule' ' + ( + cd super && + echo "another_upstream" >expect && + git submodule--helper get-default-remote upstream-subpath/innersubpath >actual && + test_cmp expect actual + ) +' + +test_expect_success 'get-default-remote works with submodule that has no remotes' ' + # Create a submodule directory manually without remotes + ( + cd super && + git init no-remote-sub && + test_commit --no-tag -C no-remote-sub "local commit" local.txt "local content" + ) && + + # Add it as a submodule + ( + cd super && + git submodule add ./no-remote-sub && + git commit -m "add local submodule 'no-remote-sub'" + ) && + + ( + cd super && + # Should fall back to "origin" remote name when no remotes exist + echo "origin" >expect && + git submodule--helper get-default-remote no-remote-sub >actual && + test_cmp expect actual + ) +' + +test_done From 1ac1d4e761c5f394526873b364ba23cf5b9b0da5 Mon Sep 17 00:00:00 2001 From: Collin Funk Date: Sun, 8 Mar 2026 19:55:11 -0700 Subject: [PATCH 02/11] bloom: remove a misleading const qualifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When building with glibc-2.43 there is the following warning: bloom.c: In function ‘get_or_compute_bloom_filter’: bloom.c:515:52: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] 515 | char *last_slash = strrchr(path, '/'); | ^~~~~~~ In this case, we always write through "path" through the "last_slash" pointer. Therefore, the const qualifier on "path" is misleading and we can just remove it. Signed-off-by: Collin Funk Signed-off-by: Junio C Hamano --- bloom.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bloom.c b/bloom.c index 2d7b951e5bf245..d604e8f07ab8d8 100644 --- a/bloom.c +++ b/bloom.c @@ -501,7 +501,7 @@ struct bloom_filter *get_or_compute_bloom_filter(struct repository *r, struct hashmap_iter iter; for (i = 0; i < diff_queued_diff.nr; i++) { - const char *path = diff_queued_diff.queue[i]->two->path; + char *path = diff_queued_diff.queue[i]->two->path; /* * Add each leading directory of the changed file, i.e. for @@ -523,7 +523,7 @@ struct bloom_filter *get_or_compute_bloom_filter(struct repository *r, free(e); if (!last_slash) - last_slash = (char*)path; + last_slash = path; *last_slash = '\0'; } while (*path); From 02cbae61df7b3493e7f76f8385fc9257de60755f Mon Sep 17 00:00:00 2001 From: Collin Funk Date: Sun, 8 Mar 2026 20:23:06 -0700 Subject: [PATCH 03/11] dir: avoid -Wdiscarded-qualifiers in remove_path() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When building with glibc-2.43 there is the following warning: dir.c:3526:15: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] 3526 | slash = strrchr(name, '/'); | ^ In this case we use a non-const pointer to get the last slash of the unwritable file name, and then use it again to write in the strdup'd file name. We can avoid this warning and make the code a bit more clear by using a separate variable to access the original argument and its strdup'd copy. Signed-off-by: Collin Funk Signed-off-by: Junio C Hamano --- dir.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dir.c b/dir.c index b00821f294fea2..046a8bbf325ac5 100644 --- a/dir.c +++ b/dir.c @@ -3516,15 +3516,15 @@ int get_sparse_checkout_patterns(struct pattern_list *pl) int remove_path(const char *name) { - char *slash; + const char *last; if (unlink(name) && !is_missing_file_error(errno)) return -1; - slash = strrchr(name, '/'); - if (slash) { + last = strrchr(name, '/'); + if (last) { char *dirs = xstrdup(name); - slash = dirs + (slash - name); + char *slash = dirs + (last - name); do { *slash = '\0'; if (startup_info->original_cwd && From 69efd53c8154a26cbb98c9c16ceff26060b288a9 Mon Sep 17 00:00:00 2001 From: Mansi Singh Date: Tue, 10 Mar 2026 22:50:22 +0000 Subject: [PATCH 04/11] t7605: use test_path_is_file instead of test -f Replace old-style 'test -f' path checks with the modern test_path_is_file helper in the merge_c1_to_c2_cmds block. The helper provides clearer failure messages and is the established convention in Git's test suite. Signed-off-by: Mansi Singh Signed-off-by: Junio C Hamano --- t/t7605-merge-resolve.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t7605-merge-resolve.sh b/t/t7605-merge-resolve.sh index 5d56c3854647b2..44de97a480f785 100755 --- a/t/t7605-merge-resolve.sh +++ b/t/t7605-merge-resolve.sh @@ -34,9 +34,9 @@ merge_c1_to_c2_cmds=' test "$(git rev-parse c1)" = "$(git rev-parse HEAD^1)" && test "$(git rev-parse c2)" = "$(git rev-parse HEAD^2)" && git diff --exit-code && - test -f c0.c && - test -f c1.c && - test -f c2.c && + test_path_is_file c0.c && + test_path_is_file c1.c && + test_path_is_file c2.c && test 3 = $(git ls-tree -r HEAD | wc -l) && test 3 = $(git ls-files | wc -l) ' From 088e994bf62a8135b87615c3e786958b397a9fd7 Mon Sep 17 00:00:00 2001 From: Amisha Chhajed Date: Thu, 12 Mar 2026 00:54:53 +0530 Subject: [PATCH 05/11] help: cleanup the contruction of keys_uniq construction of keys_uniq depends on sort operation executed on keys before processing, which does not gurantee that keys_uniq will be sorted. refactor the code to shift the sort operation after the processing to remove dependency on key's sort operation and strictly maintain the sorted order of keys_uniq. move strbuf init and release out of loop to reuse same buffer. dedent sort -u and sed in tests and replace grep with sed, to avoid piping grep's output to sed. Suggested-by: Siddharth Shrimali Signed-off-by: Amisha Chhajed Signed-off-by: Junio C Hamano --- builtin/help.c | 91 ++++++++++++++++++++++++++++++------------------- t/t0012-help.sh | 39 ++++++++++++--------- 2 files changed, 78 insertions(+), 52 deletions(-) diff --git a/builtin/help.c b/builtin/help.c index c09cbc8912deb6..daadc61c70ae78 100644 --- a/builtin/help.c +++ b/builtin/help.c @@ -111,6 +111,49 @@ struct slot_expansion { int found; }; +static void set_config_vars(struct string_list *keys_uniq, struct string_list_item *var) +{ + struct strbuf sb = STRBUF_INIT; + const char *str = var->string; + const char *wildcard = strchr(str, '*'); + const char *tag = strchr(str, '<'); + const char *cut; + + if (wildcard && tag) + cut = wildcard < tag ? wildcard : tag; + else if (wildcard) + cut = wildcard; + else if (tag) + cut = tag; + else { + string_list_append(keys_uniq, str); + return; + } + + strbuf_add(&sb, str, cut - str); + string_list_append(keys_uniq, sb.buf); + strbuf_release(&sb); +} + +static void set_config_sections(struct string_list *keys_uniq, struct string_list_item *var) +{ + struct strbuf sb = STRBUF_INIT; + const char *str = var->string; + const char *dot = strchr(str, '.'); + const char *cut; + + if (dot) + cut = dot; + else { + set_config_vars(keys_uniq, var); + return; + } + + strbuf_add(&sb, str, cut - str); + string_list_append(keys_uniq, sb.buf); + strbuf_release(&sb); +} + static void list_config_help(enum show_config_type type) { struct slot_expansion slot_expansions[] = { @@ -131,13 +174,12 @@ static void list_config_help(enum show_config_type type) struct string_list keys = STRING_LIST_INIT_DUP; struct string_list keys_uniq = STRING_LIST_INIT_DUP; struct string_list_item *item; + struct strbuf sb = STRBUF_INIT; for (p = config_name_list; *p; p++) { const char *var = *p; - struct strbuf sb = STRBUF_INIT; for (e = slot_expansions; e->prefix; e++) { - strbuf_reset(&sb); strbuf_addf(&sb, "%s.%s", e->prefix, e->placeholder); if (!strcasecmp(var, sb.buf)) { @@ -146,60 +188,39 @@ static void list_config_help(enum show_config_type type) break; } } - strbuf_release(&sb); + if (!e->prefix) string_list_append(&keys, var); } + strbuf_release(&sb); + for (e = slot_expansions; e->prefix; e++) if (!e->found) BUG("slot_expansion %s.%s is not used", e->prefix, e->placeholder); - string_list_sort(&keys); for (size_t i = 0; i < keys.nr; i++) { - const char *var = keys.items[i].string; - const char *wildcard, *tag, *cut; - const char *dot = NULL; - struct strbuf sb = STRBUF_INIT; - switch (type) { case SHOW_CONFIG_HUMAN: - puts(var); - continue; + string_list_append(&keys_uniq, keys.items[i].string); + break; case SHOW_CONFIG_SECTIONS: - dot = strchr(var, '.'); + set_config_sections(&keys_uniq, &keys.items[i]); break; case SHOW_CONFIG_VARS: + set_config_vars(&keys_uniq, &keys.items[i]); break; + default: + BUG("%d: unexpected type", type); } - wildcard = strchr(var, '*'); - tag = strchr(var, '<'); - - if (!dot && !wildcard && !tag) { - string_list_append(&keys_uniq, var); - continue; - } - - if (dot) - cut = dot; - else if (wildcard && !tag) - cut = wildcard; - else if (!wildcard && tag) - cut = tag; - else - cut = wildcard < tag ? wildcard : tag; - - strbuf_add(&sb, var, cut - var); - string_list_append(&keys_uniq, sb.buf); - strbuf_release(&sb); - } - string_list_clear(&keys, 0); - string_list_remove_duplicates(&keys_uniq, 0); + + string_list_sort_u(&keys_uniq, 0); for_each_string_list_item(item, &keys_uniq) puts(item->string); string_list_clear(&keys_uniq, 0); + string_list_clear(&keys, 0); } static enum help_format parse_help_format(const char *format) diff --git a/t/t0012-help.sh b/t/t0012-help.sh index d3a0967e9d0752..c33501bdcd2b9b 100755 --- a/t/t0012-help.sh +++ b/t/t0012-help.sh @@ -141,20 +141,23 @@ test_expect_success 'git help -c' ' '\''git help config'\'' for more information EOF - grep -v -E \ - -e "^[^.]+\.[^.]+$" \ - -e "^[^.]+\.[^.]+\.[^.]+$" \ - help.output >actual && + sed -E -e " + /^[^.]+\.[^.]+$/d + /^[^.]+\.[^.]+\.[^.]+$/d + " help.output >actual && test_cmp expect actual ' test_expect_success 'git help --config-for-completion' ' git help -c >human && - grep -E \ - -e "^[^.]+\.[^.]+$" \ - -e "^[^.]+\.[^.]+\.[^.]+$" human | - sed -e "s/\*.*//" -e "s/<.*//" | - sort -u >human.munged && + sed -E -e " + /^[^.]+\.[^.]+$/b out + /^[^.]+\.[^.]+\.[^.]+$/b out + d + : out + s/\*.*// + s/<.*// + " human | sort -u >human.munged && git help --config-for-completion >vars && test_cmp human.munged vars @@ -162,14 +165,16 @@ test_expect_success 'git help --config-for-completion' ' test_expect_success 'git help --config-sections-for-completion' ' git help -c >human && - grep -E \ - -e "^[^.]+\.[^.]+$" \ - -e "^[^.]+\.[^.]+\.[^.]+$" human | - sed -e "s/\..*//" | - sort -u >human.munged && - - git help --config-sections-for-completion >sections && - test_cmp human.munged sections + sed -E -e " + /^[^.]+\.[^.]+$/b out + /^[^.]+\.[^.]+\.[^.]+$/b out + d + : out + s/\..*// + " human | sort -u >expect && + + git help --config-sections-for-completion >actual && + test_cmp expect actual ' test_section_spacing () { From be430f4eaa9fa32420778f322a6a1c0d6162fbce Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 11 Mar 2026 14:35:41 -0700 Subject: [PATCH 06/11] t: allow use of "sed -E" Since early 2019 with e62e225f (test-lint: only use only sed [-n] [-e command] [-f command_file], 2019-01-20), we have been trying to limit the options of "sed" we use in our tests to "-e ", "-n", and "-f ". Before the commit, we were trying to reject only "-i" (which is one of the really-not-portable options), but the commit explicitly wanted to reject use of "-E" (use ERE instead of BRE). The commit cites the then-current POSIX.1 (Issue 7, 2018 edition) to show that "even recent POSIX does not have it!", but the latest edition (Issue 8) documents "-E" as an option to use ERE. But that was 7 years ago, and that is a long time for many things to happen. Besides, we have been using "sed -E" without the check in question triggering in one of the scripts since 2022, with 461fec41 (bisect run: keep some of the post-v2.30.0 output, 2022-11-10). It was hidden because the 'E' was squished with another single letter option. t/t6030-bisect-porcelain.sh: sed -En 's/.*(bisect... This escaped the rather simple pattern used in the checker /\bsed\s+-[^efn]\s+/ and err 'sed option not portable...'; because -E did not appear as a singleton. Let's change the rule to allow the "-E" option, which nobody has complained against for the past 3 years. We rewrite our first use of the "-E" option so that it is caught by the old rule, primarily because we do not want to teach our mischievous developers how to smuggle in an unwanted option undetected by the test lint. And at the same time, loosen the pattern to allow "-E" the same way we allow "-n" and friends. Signed-off-by: Junio C Hamano --- t/check-non-portable-shell.pl | 2 +- t/t6030-bisect-porcelain.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl index 6ee7700eb48013..18d944b8107dd5 100755 --- a/t/check-non-portable-shell.pl +++ b/t/check-non-portable-shell.pl @@ -36,7 +36,7 @@ sub err { $_ = $line; /\bcp\s+-a/ and err 'cp -a is not portable'; - /\bsed\s+-[^efn]\s+/ and err 'sed option not portable (use only -n, -e, -f)'; + /\bsed\s+-[^Eefn]\s+/ and err 'sed option not portable (use only -E, -n, -e, -f)'; /\becho\s+-[neE]/ and err 'echo with option is not portable (use printf)'; /^\s*declare\s+/ and err 'arrays/declare not portable'; /^\s*[^#]\s*which\s/ and err 'which is not portable (use type)'; diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index cdc02706404b34..1ba9ca219e5da9 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -402,7 +402,7 @@ test_expect_success 'git bisect run: negative exit code' " git bisect good $HASH1 && git bisect bad $HASH4 && ! git bisect run ./fail.sh 2>err && - sed -En 's/.*(bisect.*code) (-?[0-9]+) (from.*)/\1 -1 \3/p' err >actual && + sed -E -n 's/.*(bisect.*code) (-?[0-9]+) (from.*)/\1 -1 \3/p' err >actual && test_cmp expect actual " From dfcdd0b960fc1efd2fe19e97b973b435727b4c42 Mon Sep 17 00:00:00 2001 From: Beat Bolli Date: Wed, 11 Mar 2026 23:10:25 +0100 Subject: [PATCH 07/11] imap-send: use the OpenSSL API to access the subject alternative names The OpenSSL 4.0 master branch has made the ASN1_STRING structure opaque, forbidding access to its internal fields. Use the official accessor functions instead. They have existed since OpenSSL v1.1.0. Signed-off-by: Beat Bolli Signed-off-by: Junio C Hamano --- imap-send.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/imap-send.c b/imap-send.c index 26dda7f3287127..1c934c24877e3f 100644 --- a/imap-send.c +++ b/imap-send.c @@ -244,10 +244,14 @@ static int verify_hostname(X509 *cert, const char *hostname) if ((subj_alt_names = X509_get_ext_d2i(cert, NID_subject_alt_name, NULL, NULL))) { int num_subj_alt_names = sk_GENERAL_NAME_num(subj_alt_names); for (i = 0; !found && i < num_subj_alt_names; i++) { + int ntype; GENERAL_NAME *subj_alt_name = sk_GENERAL_NAME_value(subj_alt_names, i); - if (subj_alt_name->type == GEN_DNS && - strlen((const char *)subj_alt_name->d.ia5->data) == (size_t)subj_alt_name->d.ia5->length && - host_matches(hostname, (const char *)(subj_alt_name->d.ia5->data))) + ASN1_STRING *subj_alt_str = GENERAL_NAME_get0_value(subj_alt_name, &ntype); + + if (ntype == GEN_DNS && + strlen((const char *)ASN1_STRING_get0_data(subj_alt_str)) == + ASN1_STRING_length(subj_alt_str) && + host_matches(hostname, (const char *)ASN1_STRING_get0_data(subj_alt_str))) found = 1; } sk_GENERAL_NAME_pop_free(subj_alt_names, GENERAL_NAME_free); From 08fd302fc4b8eaf0bb32856231a5fb46430e3c7e Mon Sep 17 00:00:00 2001 From: Beat Bolli Date: Wed, 11 Mar 2026 23:10:26 +0100 Subject: [PATCH 08/11] imap-send: use the OpenSSL API to access the subject common name The OpenSSL 4.0 master branch has deprecated the X509_NAME_get_text_by_NID function. Use the recommended replacement APIs instead. They have existed since OpenSSL v1.1.0. Take care to get the constness right for pre-4.0 versions. Signed-off-by: Beat Bolli Signed-off-by: Junio C Hamano --- imap-send.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/imap-send.c b/imap-send.c index 1c934c24877e3f..2a904314dd4bfb 100644 --- a/imap-send.c +++ b/imap-send.c @@ -233,9 +233,13 @@ static int host_matches(const char *host, const char *pattern) static int verify_hostname(X509 *cert, const char *hostname) { - int len; +#if (OPENSSL_VERSION_NUMBER >= 0x40000000L) + const X509_NAME *subj; +#else X509_NAME *subj; - char cname[1000]; +#endif + const X509_NAME_ENTRY *cname_entry; + const ASN1_STRING *cname; int i, found; STACK_OF(GENERAL_NAME) *subj_alt_names; @@ -262,12 +266,15 @@ static int verify_hostname(X509 *cert, const char *hostname) /* try the common name */ if (!(subj = X509_get_subject_name(cert))) return error("cannot get certificate subject"); - if ((len = X509_NAME_get_text_by_NID(subj, NID_commonName, cname, sizeof(cname))) < 0) + if ((i = X509_NAME_get_index_by_NID(subj, NID_commonName, -1)) < 0 || + (cname_entry = X509_NAME_get_entry(subj, i)) == NULL || + (cname = X509_NAME_ENTRY_get_data(cname_entry)) == NULL) return error("cannot get certificate common name"); - if (strlen(cname) == (size_t)len && host_matches(hostname, cname)) + if (strlen((const char *)ASN1_STRING_get0_data(cname)) == ASN1_STRING_length(cname) && + host_matches(hostname, (const char *)ASN1_STRING_get0_data(cname))) return 0; return error("certificate owner '%s' does not match hostname '%s'", - cname, hostname); + ASN1_STRING_get0_data(cname), hostname); } static int ssl_socket_connect(struct imap_socket *sock, From 6392a0b75d979ba8e23c85d57b85779aace25370 Mon Sep 17 00:00:00 2001 From: Beat Bolli Date: Wed, 11 Mar 2026 23:10:27 +0100 Subject: [PATCH 09/11] imap-send: move common code into function host_matches() Move the ASN1_STRING access, the associated cast and the check for embedded NUL bytes into host_matches() to simplify both callers. Reformulate the NUL check using memchr() and add a comment to make it more obvious what it is about. Signed-off-by: Beat Bolli Signed-off-by: Junio C Hamano --- imap-send.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/imap-send.c b/imap-send.c index 2a904314dd4bfb..af02c6a689495e 100644 --- a/imap-send.c +++ b/imap-send.c @@ -219,8 +219,14 @@ static int ssl_socket_connect(struct imap_socket *sock UNUSED, #else -static int host_matches(const char *host, const char *pattern) +static int host_matches(const char *host, const ASN1_STRING *asn1_str) { + const char *pattern = (const char *)ASN1_STRING_get0_data(asn1_str); + + /* embedded NUL characters may open a security hole */ + if (memchr(pattern, '\0', ASN1_STRING_length(asn1_str))) + return 0; + if (pattern[0] == '*' && pattern[1] == '.') { pattern += 2; if (!(host = strchr(host, '.'))) @@ -252,10 +258,7 @@ static int verify_hostname(X509 *cert, const char *hostname) GENERAL_NAME *subj_alt_name = sk_GENERAL_NAME_value(subj_alt_names, i); ASN1_STRING *subj_alt_str = GENERAL_NAME_get0_value(subj_alt_name, &ntype); - if (ntype == GEN_DNS && - strlen((const char *)ASN1_STRING_get0_data(subj_alt_str)) == - ASN1_STRING_length(subj_alt_str) && - host_matches(hostname, (const char *)ASN1_STRING_get0_data(subj_alt_str))) + if (ntype == GEN_DNS && host_matches(hostname, subj_alt_str)) found = 1; } sk_GENERAL_NAME_pop_free(subj_alt_names, GENERAL_NAME_free); @@ -270,8 +273,7 @@ static int verify_hostname(X509 *cert, const char *hostname) (cname_entry = X509_NAME_get_entry(subj, i)) == NULL || (cname = X509_NAME_ENTRY_get_data(cname_entry)) == NULL) return error("cannot get certificate common name"); - if (strlen((const char *)ASN1_STRING_get0_data(cname)) == ASN1_STRING_length(cname) && - host_matches(hostname, (const char *)ASN1_STRING_get0_data(cname))) + if (host_matches(hostname, cname)) return 0; return error("certificate owner '%s' does not match hostname '%s'", ASN1_STRING_get0_data(cname), hostname); From 78827970ecf1cb853fc2c9059c180f91fa154c97 Mon Sep 17 00:00:00 2001 From: Tian Yuchen Date: Fri, 13 Mar 2026 00:42:03 +0800 Subject: [PATCH 10/11] builtin/mktree: remove USE_THE_REPOSITORY_VARIABLE The 'cmd_mktree()' function already receives a 'struct repository *repo' pointer, but it was previously marked as UNUSED. Pass the 'repo' pointer down to 'mktree_line()' and 'write_tree()'. Consequently, remove the 'USE_THE_REPOSITORY_VARIABLE' macro, replace usages of 'the_repository', and swap 'parse_oid_hex()' with its context-aware version 'parse_oid_hex_algop()'. This refactoring is safe because 'cmd_mktree()' is registered with the 'RUN_SETUP' flag in 'git.c', which guarantees that the command is executed within a initialized repository, ensuring that the passed 'repo' pointer is never 'NULL'. Signed-off-by: Tian Yuchen Signed-off-by: Junio C Hamano --- builtin/mktree.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/builtin/mktree.c b/builtin/mktree.c index 12772303f504f3..4084e324768b94 100644 --- a/builtin/mktree.c +++ b/builtin/mktree.c @@ -3,7 +3,6 @@ * * Copyright (c) Junio C Hamano, 2006, 2009 */ -#define USE_THE_REPOSITORY_VARIABLE #include "builtin.h" #include "gettext.h" #include "hex.h" @@ -46,7 +45,7 @@ static int ent_compare(const void *a_, const void *b_) b->name, b->len, b->mode); } -static void write_tree(struct object_id *oid) +static void write_tree(struct repository *repo, struct object_id *oid) { struct strbuf buf; size_t size; @@ -60,10 +59,10 @@ static void write_tree(struct object_id *oid) for (i = 0; i < used; i++) { struct treeent *ent = entries[i]; strbuf_addf(&buf, "%o %s%c", ent->mode, ent->name, '\0'); - strbuf_add(&buf, ent->oid.hash, the_hash_algo->rawsz); + strbuf_add(&buf, ent->oid.hash, repo->hash_algo->rawsz); } - odb_write_object(the_repository->objects, buf.buf, buf.len, OBJ_TREE, oid); + odb_write_object(repo->objects, buf.buf, buf.len, OBJ_TREE, oid); strbuf_release(&buf); } @@ -72,7 +71,7 @@ static const char *const mktree_usage[] = { NULL }; -static void mktree_line(char *buf, int nul_term_line, int allow_missing) +static void mktree_line(struct repository *repo, char *buf, int nul_term_line, int allow_missing) { char *ptr, *ntr; const char *p; @@ -93,7 +92,7 @@ static void mktree_line(char *buf, int nul_term_line, int allow_missing) die("input format error: %s", buf); ptr = ntr + 1; /* type */ ntr = strchr(ptr, ' '); - if (!ntr || parse_oid_hex(ntr + 1, &oid, &p) || + if (!ntr || parse_oid_hex_algop(ntr + 1, &oid, &p, repo->hash_algo) || *p != '\t') die("input format error: %s", buf); @@ -124,7 +123,7 @@ static void mktree_line(char *buf, int nul_term_line, int allow_missing) /* Check the type of object identified by oid without fetching objects */ oi.typep = &obj_type; - if (odb_read_object_info_extended(the_repository->objects, &oid, &oi, + if (odb_read_object_info_extended(repo->objects, &oid, &oi, OBJECT_INFO_LOOKUP_REPLACE | OBJECT_INFO_QUICK | OBJECT_INFO_SKIP_FETCH_OBJECT) < 0) @@ -155,7 +154,7 @@ static void mktree_line(char *buf, int nul_term_line, int allow_missing) int cmd_mktree(int ac, const char **av, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { struct strbuf sb = STRBUF_INIT; struct object_id oid; @@ -187,7 +186,7 @@ int cmd_mktree(int ac, break; die("input format error: (blank line only valid in batch mode)"); } - mktree_line(sb.buf, nul_term_line, allow_missing); + mktree_line(repo, sb.buf, nul_term_line, allow_missing); } if (is_batch_mode && got_eof && used < 1) { /* @@ -197,7 +196,7 @@ int cmd_mktree(int ac, */ ; /* skip creating an empty tree */ } else { - write_tree(&oid); + write_tree(repo, &oid); puts(oid_to_hex(&oid)); fflush(stdout); } From 1080981ddb03a072f1e6b55c6325c2af731e733c Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 23 Mar 2026 09:20:08 -0700 Subject: [PATCH 11/11] The 19th batch Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.54.0.adoc | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Documentation/RelNotes/2.54.0.adoc b/Documentation/RelNotes/2.54.0.adoc index d5e833512ee66a..064827b6420a5d 100644 --- a/Documentation/RelNotes/2.54.0.adoc +++ b/Documentation/RelNotes/2.54.0.adoc @@ -210,6 +210,8 @@ Performance, Internal Implementation, Development Support etc. mnemonic notation for certain control characters like "\t", instead of octal notation like "\011". + * Adjust test-lint to allow "sed -E" to use ERE in the patterns. + Fixes since v2.53 ----------------- @@ -344,6 +346,18 @@ Fixes since v2.53 work better with SHA-256 as well as SHA-1. (merge 30310f3cc4 ss/t3200-test-zero-oid later to maint). + * Instead of hardcoded 'origin', use the configured default remote + when fetching from submodules. + (merge 3b5fb32da8 ng/submodule-default-remote later to maint). + + * The code in "git help" that shows configuration items in sorted + order was awkwardly organized and prone to bugs. + + * "imap-send" used to use functions whose use is going to be removed + with OpenSSL 4.0; rewrite them using public API that has been + available since OpenSSL 1.1 since 2016 or so. + (merge 6392a0b75d bb/imap-send-openssl-4.0-prep later to maint). + * Other code cleanup, docfix, build fix, etc. (merge d79fff4a11 jk/remote-tracking-ref-leakfix later to maint). (merge 7a747f972d dd/t5403-modernise later to maint). @@ -385,3 +399,5 @@ Fixes since v2.53 (merge 4c223571be ty/patch-ids-document-lazy-eval later to maint). (merge 476365ac85 jc/doc-wholesale-replace-before-next later to maint). (merge 35f220b639 ss/submodule--helper-use-xmalloc later to maint). + (merge 02cbae61df cf/constness-fixes later to maint). + (merge 69efd53c81 ms/t7605-test-path-is-helpers later to maint).