Re: [PATCH] ls-remote: add --show-object-format

[Adrian Ratiu <adrian.ratiu@collabora.com>]

Hi Patrick and thank you for the awesome review, as always.

I'll let this sit for about 1 week to also give others a chance to
review, then send v2 addresing all your feedback.

On Thu, 05 Nov 2026, Patrick Steinhardt <ps@pks.im> wrote:
<snip>
>> diff --git a/Documentation/git-ls-remote.adoc b/Documentation/git-ls-remote.adoc
>> index d71c4ab3e2..1abf7b60b7 100644
>> --- a/Documentation/git-ls-remote.adoc
>> +++ b/Documentation/git-ls-remote.adoc
>> @@ -64,6 +65,13 @@ OPTIONS
>>  	upload-pack only shows the symref HEAD, so it will be the only
>>  	one shown by ls-remote.
>>  
>> +--show-object-format::
>> +	Show the object format (hash algorithm, e.g. `sha1` or `sha256`) used
>> +	by the remote repository before printing the references, on a line of
>> +	its own. This also works for a remote repo without any refs, which is
>> +	useful to find out which object format to use for a local repository,
>
> A bit curious that you first say "repo" and then "repository". I'd just
> drop "repo".

Initially I wrote repository twice then the lines didn't fit, so I cut
repository -> repo to make them fit. :)

English is also not my first nor second languege, so I tend to keep
rewriting multiple times before sending. It's still not "good" on the
Nth read, tbh.

I'll try to improve this further to avoid redundancies, for e.g. I just
realized I wrote "--show-objoct-format" then immediately I repeated "Show
the object format on the next line". =)

>
>> diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
>> index fe77829557..6d961f9961 100644
>> --- a/builtin/ls-remote.c
>> +++ b/builtin/ls-remote.c
>> @@ -150,6 +154,8 @@ int cmd_ls_remote(int argc,
>>  
>>  	if (!dest && !quiet)
>>  		fprintf(stderr, "From %s\n", remote->url.v[0]);
>> +	if (show_object_format)
>> +		printf("%s\n", transport_get_hash_algo(transport)->name);
>
> Should we maybe make this a tiny bit more extensible by not printing the
> object format without any kind of prefix? We could for example instead
> print "object-format: %s", which would allow us to add more fields in
> the future without the need to make parsers be order-dependent.

Yes, this is a great idea, will do in v2.

(I just need to know the sha1 vs sha256 information, the format can be
anything at all, no strong preference on my side).

>
> Also, one more question I had is whether we know that the hash algo will
> always be populated here. I would assume so, as we have this logic a
> couple lines further up:
>
> 	ref = transport_get_remote_refs(transport, &transport_options);
> 	if (ref) {
> 		int hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport));
> 		repo_set_hash_algo(the_repository, hash_algo);
> 	}
>
> But in theory, this logic would also work alright if the hash was
> NULL.

Yes, the hash format will always be populated by the time we call
transport_get_hash_algo() here.

>> diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
>> index 2e1d4f7586..7e82154fe4 100755
>> --- a/t/t5512-ls-remote.sh
>> +++ b/t/t5512-ls-remote.sh
>> @@ -332,6 +332,52 @@ test_expect_success 'indicate no refs in v0 standards-compliant empty remote' '
>>  	test_expect_code 2 git ls-remote --exit-code --upload-pack=./cat-input .
>>  '
>>  
>> +# Run "ls-remote --show-object-format" with the given arguments over
>> +# protocol v0 and v2, and compare its output with "expect".
>> +test_show_object_format () {
>> +	for version in 0 2
>> +	do
>> +		git -c protocol.version=$version \
>> +			ls-remote --show-object-format "$@" >actual &&
>> +		test_cmp expect actual || return 1
>
> It's a bit weird that we expect the caller to already have written
> `expect`. A better interface would be to have them call the expected
> object format as another parameter.

Ack, will do in v2.

>> +	done
>> +}
>> +
>> +test_expect_success 'ls-remote --show-object-format with empty sha1 remote' '
>> +	git init --bare --object-format=sha1 empty-sha1.git &&
>> +	echo sha1 >expect &&
>> +	test_show_object_format empty-sha1.git
>> +'
>> +
>> +test_expect_success 'ls-remote --show-object-format with empty sha256 remote' '
>> +	git init --bare --object-format=sha256 empty-sha256.git &&
>> +	echo sha256 >expect &&
>> +	test_show_object_format empty-sha256.git
>> +'
>
> We could easily deduplicate these tests by looping around `for hash in
> sha1 sha256`.

Ack, will do in v2.