Jeff King <peff@xxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html