Re: [PATCH] pack-objects: name pack files after trailer hash

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> The second half would be to simplify git-repack. The current behavior is
> to replace the old packfile with a tricky rename dance. Which is still
> correct, but overly complicated. We should be able to just drop the new
> packfile, since we know the bytes are identical (or rename the new one
> over the old, though I think keeping the old is probably kinder to the
> disk cache, especially if another process already has it mmap'd).

Concurred.

> One test needs to be updated, because it actually corrupts a
> pack and expects that re-packing the corrupted bytes will
> use the same name. It won't anymore, but we can easily just
> use the name that pack-objects hands back.

Re-reading the tests in that script, I am not sure if keeping these
tests is even a sane thing to do, by the way.  It "expects" that
certain breakages are propagated, and anybody who breaks that
expectation by improving pack-objects etc. to catch such breakages
will be yelled at by breaking the test that used to pass.

Seeing that the way the test scripts are line-wrapped follows the
ancient convention, I suspect that this may be because it predates
our more recent best practice to document known breakages with
test_expect_failure.

> diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh
> index fe82025..4bbb718 100755
> --- a/t/t5302-pack-index.sh
> +++ b/t/t5302-pack-index.sh
> @@ -174,11 +174,11 @@ test_expect_success \
>  test_expect_success \
>      '[index v1] 5) pack-objects happily reuses corrupted data' \
>      'pack4=$(git pack-objects test-4 <obj-list) &&
> -     test -f "test-4-${pack1}.pack"'
> +     test -f "test-4-${pack4}.pack"'
>  
>  test_expect_success \
>      '[index v1] 6) newly created pack is BAD !' \
> -    'test_must_fail git verify-pack -v "test-4-${pack1}.pack"'
> +    'test_must_fail git verify-pack -v "test-4-${pack4}.pack"'

A good thing is that the above hunks are the right thing to do, even
if we are to modernise these tests so that they document a known
breakage with expect-failure.

Thanks.