On Sun, Oct 27, 2024 at 08:10:16PM -0400, Taylor Blau wrote:
> On Sat, Oct 26, 2024 at 05:39:50PM +0530, Abhijeet Sonar wrote:
> > diff --git a/builtin/show-index.c b/builtin/show-index.c
> > index f164c01bbe..978ae70470 100644
> > --- a/builtin/show-index.c
> > +++ b/builtin/show-index.c
> > @@ -38,6 +38,9 @@ int cmd_show_index(int argc,
> > repo_set_hash_algo(the_repository, hash_algo);
> > }
> >
> > + if (!the_hash_algo)
> > + repo_set_hash_algo(the_repository, GIT_HASH_SHA1);
Let's add a todo-comment here. The behaviour with this patch is somewhat
broken as you cannot inspect indices that use any other object hash than
SHA256 outside of a repository. This is fine from my point of view and
nothing that you have to fix here, as you simply fix up the broken
behaviour. But in the future, we should either:
- Add logic to detect the format of the passed-in index and set that
up as the hash algorithm.
- If that is impossible, add a command line option to pick the hash
algo.
> > hashsz = the_hash_algo->rawsz;
> >
> > if (fread(top_index, 2 * 4, 1, stdin) != 1)
> > diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
> > index 3b9dae331a..51fed26cc4 100755
> > --- a/t/t5300-pack-object.sh
> > +++ b/t/t5300-pack-object.sh
> > @@ -523,6 +523,10 @@ test_expect_success 'index-pack --strict <pack> works in non-repo' '
> > test_path_is_file foo.idx
> > '
> >
> > +test_expect_success SHA1 'show-index works OK outside a repository' '
> > + nongit git show-index <foo.idx
> > +'
So how does this behave with SHA256? Does it raise an error? Does it
segfault?
I think it's okay to fail with SHA256 for now, but I'd like the
failure behaviour to be cleanish. So I'd prefer to not skip the test
completely, but adapt our expectations based on the hash algo. Or have
two separate tests, one for each hash, that explicitly init the repo
with `git init --ref-format=$hash`, and then exercise the behaviour for
each of them.
> > test_expect_success !PTHREADS,!FAIL_PREREQS \
> > 'index-pack --threads=N or pack.threads=N warns when no pthreads' '
> > test_must_fail git index-pack --threads=2 2>err &&
> > --
> > 2.47.0.107.g34b6ce9b30
>
> These all look reasonable and as-expected to me. Patrick (CC'd) has been
> reviewing similar changes elsewhere, so I'd like him to chime in as well
> on whether or not this looks good to go.
Ah, thanks. I've missed this topic somehow.
Patrick
