[PATCH] [Outreachy PATCH] t4041: use submodule repository for commit abbreviations

[PATCH] [Outreachy PATCH] t4041: use submodule repository for commit abbreviations

[Srujith A Poojary via GitGitGadget]

From: Srujith A Poojary <poojarysrujith@gmail.com>

Use the submodule repository when abbreviating commit IDs in
submodule diff headers, so uniqueness is checked against the
correct object database.

Signed-off-by: Srujith A Poojary <poojarysrujith@gmail.com>
---
    [Outreachy PATCH] t4041: use submodule repository for commit
    abbreviations

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2288%2Fpoojarysrujith%2Fmicroproject-git-libification-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2288/poojarysrujith/microproject-git-libification-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2288

 submodule.c                      | 10 ++++++++--
 t/t4041-diff-submodule-option.sh | 18 ++++++++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/submodule.c b/submodule.c
index 6fcb606f7e..2215a50d9c 100644
--- a/submodule.c
+++ b/submodule.c
@@ -599,9 +599,15 @@ static void show_submodule_header(struct diff_options *o,
 
 output_header:
 	strbuf_addf(&sb, "Submodule %s ", path);
-	strbuf_add_unique_abbrev(&sb, one, DEFAULT_ABBREV);
+	if (sub)
+		strbuf_repo_add_unique_abbrev(&sb, sub, one, DEFAULT_ABBREV);
+	else
+		strbuf_add_unique_abbrev(&sb, one, DEFAULT_ABBREV);
 	strbuf_addstr(&sb, (fast_backward || fast_forward) ? ".." : "...");
-	strbuf_add_unique_abbrev(&sb, two, DEFAULT_ABBREV);
+	if (sub)
+		strbuf_repo_add_unique_abbrev(&sb, sub, two, DEFAULT_ABBREV);
+	else
+		strbuf_add_unique_abbrev(&sb, two, DEFAULT_ABBREV);
 	if (message)
 		strbuf_addf(&sb, " %s\n", message);
 	else
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index 4dd4954260..e80472af32 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -14,6 +14,24 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
+test_expect_success SHA1 'submodule commit abbreviation uses submodule object database' '
+	test_when_finished "rm -rf collision-super" &&
+	git init -q collision-super &&
+	git init -q collision-super/sm1 &&
+	printf "0\n1\n2\n3\n4\n5\n6\n7\n8\n9\n\nb1rwzyc3\n" >collision-super/sm1/a0blgqsjc &&
+	git -C collision-super/sm1 add a0blgqsjc &&
+	tree=$(git -C collision-super/sm1 write-tree) &&
+	commit1=$(GIT_AUTHOR_DATE="1112911993 -0700" GIT_COMMITTER_DATE="1112911993 -0700" git -C collision-super/sm1 commit-tree "$tree" -m a2onsxbvj) &&
+	commit2=$(printf "child\n" | GIT_AUTHOR_DATE="1112912053 -0700" GIT_COMMITTER_DATE="1112912053 -0700" git -C collision-super/sm1 commit-tree "$tree" -p "$commit1") &&
+	git -C collision-super/sm1 checkout -q "$commit2" &&
+	git -C collision-super update-index --add --cacheinfo 160000,"$commit1",sm1 &&
+	git -C collision-super commit -q -m "Add submodule" &&
+	git -C collision-super diff --submodule=log >actual &&
+	printf "Submodule sm1 0000000000e..%s:\n  > child\n" \
+		"$(git -C collision-super/sm1 rev-parse --short "$commit2")" >expected &&
+	test_cmp expected actual
+'
+
 # Test non-UTF-8 encoding in case iconv is available.
 if test_have_prereq ICONV
 then

base-commit: 50fe5cc63f9cd6e89b87a2c9ccf96a444f4cfdb7
-- 
gitgitgadget

Re: [PATCH] [Outreachy PATCH] t4041: use submodule repository for commit abbreviations

[Okhuomon Ajayi]

Hi Srujith,

I applied this on top of master and ran t4041. The new test fails
without the submodule.c change and passes with it, so it does catch
the problem.

A few small things:

On Sat, Oct 31, 2026 at 9:26 AM Srujith A Poojary via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> Use the submodule repository when abbreviating commit IDs in
> submodule diff headers, so uniqueness is checked against the
> correct object database.

It would help to first describe what happens today: the commit IDs
in the "Submodule ..." header are abbreviated against the
superproject's objects, even though the commits live in the
submodule, so the abbreviation may not be unique there.

> [Outreachy PATCH] t4041: use submodule repository for commit
> abbreviations

The change itself is in submodule.c, so the title might read better
with that area, e.g. "submodule: use the submodule repository to
abbreviate commits".

> - strbuf_add_unique_abbrev(&sb, one, DEFAULT_ABBREV);
> + if (sub)
> + strbuf_repo_add_unique_abbrev(&sb, sub, one, DEFAULT_ABBREV);
> + else
> + strbuf_add_unique_abbrev(&sb, one, DEFAULT_ABBREV);

The same if/else appears twice. Maybe choose the repository once,
at the top of the function, and call strbuf_repo_add_unique_abbrev()
with it in both places. I also wonder whether the fallback could be
o->repo instead of the_repository, since the diff options already
carry a repository.

Thanks,
Okhuomon

[PATCH v2] Submodule: use the submodule repository to abbreviate commits

[Srujith A Poojary via GitGitGadget]

From: Srujith A Poojary <poojarysrujith@gmail.com>

The commit IDs shown in a submodule diff header belong to the
submodule's object database. However, strbuf_add_unique_abbrev()
uses the_repository, so it checks the superproject's object
database when abbreviating those IDs.

Use the submodule repository when it is available, and fall back
to the repository from the diff options when it is not.

Signed-off-by: Srujith A Poojary <poojarysrujith@gmail.com>
---
    [Outreachy PATCH] t4041: use submodule repository for commit
    abbreviations

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2288%2Fpoojarysrujith%2Fmicroproject-git-libification-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2288/poojarysrujith/microproject-git-libification-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2288

Range-diff vs v1:

 1:  7b2af1b46a ! 1:  3133cd3017 [Outreachy PATCH] t4041: use submodule repository for commit abbreviations
     @@ Metadata
      Author: Srujith A Poojary <poojarysrujith@gmail.com>
      
       ## Commit message ##
     -    [Outreachy PATCH] t4041: use submodule repository for commit abbreviations
     +    Submodule: use the submodule repository to abbreviate commits
      
     -    Use the submodule repository when abbreviating commit IDs in
     -    submodule diff headers, so uniqueness is checked against the
     -    correct object database.
     +    The commit IDs shown in a submodule diff header belong to the
     +    submodule's object database. However, strbuf_add_unique_abbrev()
     +    uses the_repository, so it checks the superproject's object
     +    database when abbreviating those IDs.
     +
     +    Use the submodule repository when it is available, and fall back
     +    to the repository from the diff options when it is not.
      
          Signed-off-by: Srujith A Poojary <poojarysrujith@gmail.com>
      
       ## submodule.c ##
      @@ submodule.c: static void show_submodule_header(struct diff_options *o,
     + 		struct commit **left, struct commit **right,
     + 		struct commit_list **merge_bases)
     + {
     ++	struct repository *repo = sub ? sub : o->repo;
     + 	const char *message = NULL;
     + 	struct strbuf sb = STRBUF_INIT;
     + 	int fast_forward = 0, fast_backward = 0;
     +@@ submodule.c: static void show_submodule_header(struct diff_options *o,
       
       output_header:
       	strbuf_addf(&sb, "Submodule %s ", path);
      -	strbuf_add_unique_abbrev(&sb, one, DEFAULT_ABBREV);
     -+	if (sub)
     -+		strbuf_repo_add_unique_abbrev(&sb, sub, one, DEFAULT_ABBREV);
     -+	else
     -+		strbuf_add_unique_abbrev(&sb, one, DEFAULT_ABBREV);
     ++	strbuf_repo_add_unique_abbrev(&sb, repo, one, DEFAULT_ABBREV);
       	strbuf_addstr(&sb, (fast_backward || fast_forward) ? ".." : "...");
      -	strbuf_add_unique_abbrev(&sb, two, DEFAULT_ABBREV);
     -+	if (sub)
     -+		strbuf_repo_add_unique_abbrev(&sb, sub, two, DEFAULT_ABBREV);
     -+	else
     -+		strbuf_add_unique_abbrev(&sb, two, DEFAULT_ABBREV);
     ++	strbuf_repo_add_unique_abbrev(&sb, repo, two, DEFAULT_ABBREV);
       	if (message)
       		strbuf_addf(&sb, " %s\n", message);
       	else


 submodule.c                      |  5 +++--
 t/t4041-diff-submodule-option.sh | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/submodule.c b/submodule.c
index 6fcb606f7e..1302c11ddf 100644
--- a/submodule.c
+++ b/submodule.c
@@ -543,6 +543,7 @@ static void show_submodule_header(struct diff_options *o,
 		struct commit **left, struct commit **right,
 		struct commit_list **merge_bases)
 {
+	struct repository *repo = sub ? sub : o->repo;
 	const char *message = NULL;
 	struct strbuf sb = STRBUF_INIT;
 	int fast_forward = 0, fast_backward = 0;
@@ -599,9 +600,9 @@ static void show_submodule_header(struct diff_options *o,
 
 output_header:
 	strbuf_addf(&sb, "Submodule %s ", path);
-	strbuf_add_unique_abbrev(&sb, one, DEFAULT_ABBREV);
+	strbuf_repo_add_unique_abbrev(&sb, repo, one, DEFAULT_ABBREV);
 	strbuf_addstr(&sb, (fast_backward || fast_forward) ? ".." : "...");
-	strbuf_add_unique_abbrev(&sb, two, DEFAULT_ABBREV);
+	strbuf_repo_add_unique_abbrev(&sb, repo, two, DEFAULT_ABBREV);
 	if (message)
 		strbuf_addf(&sb, " %s\n", message);
 	else
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index 4dd4954260..e80472af32 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -14,6 +14,24 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
+test_expect_success SHA1 'submodule commit abbreviation uses submodule object database' '
+	test_when_finished "rm -rf collision-super" &&
+	git init -q collision-super &&
+	git init -q collision-super/sm1 &&
+	printf "0\n1\n2\n3\n4\n5\n6\n7\n8\n9\n\nb1rwzyc3\n" >collision-super/sm1/a0blgqsjc &&
+	git -C collision-super/sm1 add a0blgqsjc &&
+	tree=$(git -C collision-super/sm1 write-tree) &&
+	commit1=$(GIT_AUTHOR_DATE="1112911993 -0700" GIT_COMMITTER_DATE="1112911993 -0700" git -C collision-super/sm1 commit-tree "$tree" -m a2onsxbvj) &&
+	commit2=$(printf "child\n" | GIT_AUTHOR_DATE="1112912053 -0700" GIT_COMMITTER_DATE="1112912053 -0700" git -C collision-super/sm1 commit-tree "$tree" -p "$commit1") &&
+	git -C collision-super/sm1 checkout -q "$commit2" &&
+	git -C collision-super update-index --add --cacheinfo 160000,"$commit1",sm1 &&
+	git -C collision-super commit -q -m "Add submodule" &&
+	git -C collision-super diff --submodule=log >actual &&
+	printf "Submodule sm1 0000000000e..%s:\n  > child\n" \
+		"$(git -C collision-super/sm1 rev-parse --short "$commit2")" >expected &&
+	test_cmp expected actual
+'
+
 # Test non-UTF-8 encoding in case iconv is available.
 if test_have_prereq ICONV
 then

base-commit: 50fe5cc63f9cd6e89b87a2c9ccf96a444f4cfdb7
-- 
gitgitgadget

Re: [PATCH] [Outreachy PATCH] t4041: use submodule repository for commit abbreviations

[Srujith Poojary]

On Sat, Oct 31, 2026 at 10:16 PM Okhuomon Ajayi
<okhuomonajayi54@gmail.com> wrote:
>
> Hi Srujith,
>
> I applied this on top of master and ran t4041. The new test fails
> without the submodule.c change and passes with it, so it does catch
> the problem.
>
> A few small things:
>
> On Sat, Oct 31, 2026 at 9:26 AM Srujith A Poojary via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> > Use the submodule repository when abbreviating commit IDs in
> > submodule diff headers, so uniqueness is checked against the
> > correct object database.
>
> It would help to first describe what happens today: the commit IDs
> in the "Submodule ..." header are abbreviated against the
> superproject's objects, even though the commits live in the
> submodule, so the abbreviation may not be unique there.
>
> > [Outreachy PATCH] t4041: use submodule repository for commit
> > abbreviations
>
> The change itself is in submodule.c, so the title might read better
> with that area, e.g. "submodule: use the submodule repository to
> abbreviate commits".
>
> > - strbuf_add_unique_abbrev(&sb, one, DEFAULT_ABBREV);
> > + if (sub)
> > + strbuf_repo_add_unique_abbrev(&sb, sub, one, DEFAULT_ABBREV);
> > + else
> > + strbuf_add_unique_abbrev(&sb, one, DEFAULT_ABBREV);
>
> The same if/else appears twice. Maybe choose the repository once,
> at the top of the function, and call strbuf_repo_add_unique_abbrev()
> with it in both places. I also wonder whether the fallback could be
> o->repo instead of the_repository, since the diff options already
> carry a repository.
>
> Thanks,
> Okhuomon

Hi Okhuomon,

Thanks for reviewing the patch and testing the new test case.

I've made the suggested changes and submitted v2. I now use `sub` when
available and fall back to `o->repo` when it isn't. I also simplified
the code and updated the commit message.

Here is the updated patch:
https://lore.kernel.org/git/pull.2288.v2.git.1793473062113.gitgitgadget@gmail.com

Thanks again!

Regards,
Srujith

Re: [PATCH v2] Submodule: use the submodule repository to abbreviate commits

[Okhuomon Ajayi]

Hi Srujith,

On Sat, Oct 31, 2026 at 7:57 PM Srujith A Poojary via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> + struct repository *repo = sub ? sub : o->repo;

Thanks, v2 looks good to me.

> Submodule: use the submodule repository to abbreviate commits

One small nit: "git log --no-merges submodule.c" shows the area is
always written in lowercase ("submodule:"), so "submodule: use the
submodule repository to abbreviate commits" would match.

Thanks,
Okhuomon

On Sat, Oct 31, 2026 at 7:57 PM Srujith A Poojary via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Srujith A Poojary <poojarysrujith@gmail.com>
>
> The commit IDs shown in a submodule diff header belong to the
> submodule's object database. However, strbuf_add_unique_abbrev()
> uses the_repository, so it checks the superproject's object
> database when abbreviating those IDs.
>
> Use the submodule repository when it is available, and fall back
> to the repository from the diff options when it is not.
>
> Signed-off-by: Srujith A Poojary <poojarysrujith@gmail.com>
> ---
>     [Outreachy PATCH] t4041: use submodule repository for commit
>     abbreviations
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2288%2Fpoojarysrujith%2Fmicroproject-git-libification-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2288/poojarysrujith/microproject-git-libification-v2
> Pull-Request: https://github.com/gitgitgadget/git/pull/2288
>
> Range-diff vs v1:
>
>  1:  7b2af1b46a ! 1:  3133cd3017 [Outreachy PATCH] t4041: use submodule repository for commit abbreviations
>      @@ Metadata
>       Author: Srujith A Poojary <poojarysrujith@gmail.com>
>
>        ## Commit message ##
>      -    [Outreachy PATCH] t4041: use submodule repository for commit abbreviations
>      +    Submodule: use the submodule repository to abbreviate commits
>
>      -    Use the submodule repository when abbreviating commit IDs in
>      -    submodule diff headers, so uniqueness is checked against the
>      -    correct object database.
>      +    The commit IDs shown in a submodule diff header belong to the
>      +    submodule's object database. However, strbuf_add_unique_abbrev()
>      +    uses the_repository, so it checks the superproject's object
>      +    database when abbreviating those IDs.
>      +
>      +    Use the submodule repository when it is available, and fall back
>      +    to the repository from the diff options when it is not.
>
>           Signed-off-by: Srujith A Poojary <poojarysrujith@gmail.com>
>
>        ## submodule.c ##
>       @@ submodule.c: static void show_submodule_header(struct diff_options *o,
>      +          struct commit **left, struct commit **right,
>      +          struct commit_list **merge_bases)
>      + {
>      ++ struct repository *repo = sub ? sub : o->repo;
>      +  const char *message = NULL;
>      +  struct strbuf sb = STRBUF_INIT;
>      +  int fast_forward = 0, fast_backward = 0;
>      +@@ submodule.c: static void show_submodule_header(struct diff_options *o,
>
>        output_header:
>         strbuf_addf(&sb, "Submodule %s ", path);
>       - strbuf_add_unique_abbrev(&sb, one, DEFAULT_ABBREV);
>      -+ if (sub)
>      -+         strbuf_repo_add_unique_abbrev(&sb, sub, one, DEFAULT_ABBREV);
>      -+ else
>      -+         strbuf_add_unique_abbrev(&sb, one, DEFAULT_ABBREV);
>      ++ strbuf_repo_add_unique_abbrev(&sb, repo, one, DEFAULT_ABBREV);
>         strbuf_addstr(&sb, (fast_backward || fast_forward) ? ".." : "...");
>       - strbuf_add_unique_abbrev(&sb, two, DEFAULT_ABBREV);
>      -+ if (sub)
>      -+         strbuf_repo_add_unique_abbrev(&sb, sub, two, DEFAULT_ABBREV);
>      -+ else
>      -+         strbuf_add_unique_abbrev(&sb, two, DEFAULT_ABBREV);
>      ++ strbuf_repo_add_unique_abbrev(&sb, repo, two, DEFAULT_ABBREV);
>         if (message)
>                 strbuf_addf(&sb, " %s\n", message);
>         else
>
>
>  submodule.c                      |  5 +++--
>  t/t4041-diff-submodule-option.sh | 18 ++++++++++++++++++
>  2 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index 6fcb606f7e..1302c11ddf 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -543,6 +543,7 @@ static void show_submodule_header(struct diff_options *o,
>                 struct commit **left, struct commit **right,
>                 struct commit_list **merge_bases)
>  {
> +       struct repository *repo = sub ? sub : o->repo;
>         const char *message = NULL;
>         struct strbuf sb = STRBUF_INIT;
>         int fast_forward = 0, fast_backward = 0;
> @@ -599,9 +600,9 @@ static void show_submodule_header(struct diff_options *o,
>
>  output_header:
>         strbuf_addf(&sb, "Submodule %s ", path);
> -       strbuf_add_unique_abbrev(&sb, one, DEFAULT_ABBREV);
> +       strbuf_repo_add_unique_abbrev(&sb, repo, one, DEFAULT_ABBREV);
>         strbuf_addstr(&sb, (fast_backward || fast_forward) ? ".." : "...");
> -       strbuf_add_unique_abbrev(&sb, two, DEFAULT_ABBREV);
> +       strbuf_repo_add_unique_abbrev(&sb, repo, two, DEFAULT_ABBREV);
>         if (message)
>                 strbuf_addf(&sb, " %s\n", message);
>         else
> diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
> index 4dd4954260..e80472af32 100755
> --- a/t/t4041-diff-submodule-option.sh
> +++ b/t/t4041-diff-submodule-option.sh
> @@ -14,6 +14,24 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
>  . ./test-lib.sh
>
> +test_expect_success SHA1 'submodule commit abbreviation uses submodule object database' '
> +       test_when_finished "rm -rf collision-super" &&
> +       git init -q collision-super &&
> +       git init -q collision-super/sm1 &&
> +       printf "0\n1\n2\n3\n4\n5\n6\n7\n8\n9\n\nb1rwzyc3\n" >collision-super/sm1/a0blgqsjc &&
> +       git -C collision-super/sm1 add a0blgqsjc &&
> +       tree=$(git -C collision-super/sm1 write-tree) &&
> +       commit1=$(GIT_AUTHOR_DATE="1112911993 -0700" GIT_COMMITTER_DATE="1112911993 -0700" git -C collision-super/sm1 commit-tree "$tree" -m a2onsxbvj) &&
> +       commit2=$(printf "child\n" | GIT_AUTHOR_DATE="1112912053 -0700" GIT_COMMITTER_DATE="1112912053 -0700" git -C collision-super/sm1 commit-tree "$tree" -p "$commit1") &&
> +       git -C collision-super/sm1 checkout -q "$commit2" &&
> +       git -C collision-super update-index --add --cacheinfo 160000,"$commit1",sm1 &&
> +       git -C collision-super commit -q -m "Add submodule" &&
> +       git -C collision-super diff --submodule=log >actual &&
> +       printf "Submodule sm1 0000000000e..%s:\n  > child\n" \
> +               "$(git -C collision-super/sm1 rev-parse --short "$commit2")" >expected &&
> +       test_cmp expected actual
> +'
> +
>  # Test non-UTF-8 encoding in case iconv is available.
>  if test_have_prereq ICONV
>  then
>
> base-commit: 50fe5cc63f9cd6e89b87a2c9ccf96a444f4cfdb7
> --
> gitgitgadget
>