Re: [PATCH 1/3] show-index: implement automatic hash detection
- From
Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
- Date
- Jan 21, 2026, 10:28 UTC
- Message-ID
- <20260121102956.759871-1-shreyanshpaliwalcmsmn@gmail.com>
- In-Reply-To
- <xmqqzf68yx75.fsf@gitster.g>
Show 15 quoted lines
> Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:
>
> > @@ -71,6 +60,40 @@ int cmd_show_index(int argc,
> > die("corrupt index file");
> > nr = n;
> > }
> > +
> > + /* detection of hash algorithm
> > + Only works for small files, i.e without large offsets */
> > + if(!the_hash_algo && version == 2) {
>
> We have one SP between "if" (and other syntactic elements like
> "while") and the open parenthesis "(". End-user controlled function
> names lack this SP between <word> and "(".
> Got it, I will keep this in mind.
Show 16 quoted lines
> For example, ...
>
> > + struct stat st;
> > + size_t file_base_size;
> > + size_t table_size;
> > + size_t size_rem;
> > + size_t hash_size;
> > +
> > + if(fstat(0, &st) || !S_ISREG(st.st_mode))
> > + die(_("unable to detect hash from non-regular file"));
>
> ... this "die()" does not have to be here. We can just return
> GIT_HASH_UNKNOWN and let the caller fallback. Does the existing
> code correctly complain when the filestream is opened for a
> non-regular file, or it just gets totally confused?
> I believe there is no explicit check for irregular files in the current implementation.
Show 36 quoted lines
> > + file_base_size = 8 + (256 * 4);
> > + table_size = file_base_size + (nr * 4 * 4);
> > + size_rem = st.st_size - table_size;
> > + hash_size = size_rem / (nr + 2);
> > +
> > + if(hash_size == GIT_SHA1_RAWSZ) {
> > + repo_set_hash_algo(the_repository, GIT_HASH_SHA1);
> > + } else if(hash_size == GIT_SHA256_RAWSZ) {
> > + repo_set_hash_algo(the_repository, GIT_HASH_SHA256);
>
> And instead of calling repo_set_hash_algo(), just return the
> constants so that the caller can handle it. And
>
> > + } else {
> > + die(_("unable to detect hash algorithm, "
> > + "use --object-format option"));
>
> ... this also can return GIT_HASH_UNKNOWN, without complaining
> anything.
>
> > + }
> > + }
>
> So, instead of inserting all of the above lines in cmd_show_index(),
> we'd have something like the following ...
>
> hash_func = auto_detect_hash_function(0);
> if (hash_func == GIT_HASH_UNKNOWN) {
> warning(_("assuming SHA-1; use --object-format to override"));
> hash_func = GIT_HASH_SHA1;
> }
> repo_set_hash_algo(the_repository, hash_func);
> hashsz = the_hash_algo->rawsz;
>
> ... there.
> Yes this is surely a much better approach than before, I will implement this as a seperate helper function in v2 if we go ahead with the detection logic.