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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 1, 2026, 21:45 UTC
Message-ID: <xmqqecd4z0mx.fsf@gitster.g>
In-Reply-To: <pull.2288.v3.git.1793474657981.gitgitgadget@gmail.com>

"Srujith A Poojary via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 9 quoted lines

> 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.

While the fallback behaviour does not make much sense (the commit object recorded in gitlink is not expected to be found in the superproject's repository, which is the reason why you are adding this fix in the first place), it would mean that for unpopulated submodules for which you haven't run "git submodule init", the behaviour would stay the ame as before.

Show 24 quoted lines

> 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

Quite straightforward.

> +test_expect_success SHA1 'submodule commit abbreviation uses submodule object database' '

Hmph. I wonder if there is a way to use the test_oid_cache facility to allow test with both SHA-1 and SHA-256 hashes?

Will queue.

Thanks.