On Tue, Aug 14, 2018 at 01:47:21PM +0200, SZEDER Gábor wrote: > The test 'pack-objects to file can use bitmap' added in 645c432d61 > (pack-objects: use reachability bitmap index when generating > non-stdout pack, 2016-09-10) is silently buggy and doesn't check what > it's supposed to. > > In 't5310-pack-bitmaps.sh', the 'list_packed_objects' helper function > does what its name implies by running: > > git show-index <"$1" | cut -d' ' -f2 > > The test in question invokes this function like this: > > list_packed_objects <packa-$packasha1.idx >packa.objects && > list_packed_objects <packb-$packbsha1.idx >packb.objects && > test_cmp packa.objects packb.objects > > Note how these two callsites don't specify the name of the pack index > file as the function's parameter, but redirect the function's standard > input from it. This triggers an error message from the shell, as it > has no filename to redirect from in the function, but this error is > ignored, because it happens upstream of a pipe. Consequently, both > invocations produce empty 'pack{a,b}.objects' files, and the > subsequent 'test_cmp' happily finds those two empty files identical. > > Fix these two 'list_packed_objects' invocations by specifying the pack > index files as parameters. Furthermore, eliminate the pipe in that > function by replacing it with an &&-chained pair of commands using an > intermediate file, so a failure of 'git show-index' or the shell > redirection will fail the test. Good catch, and nicely explained. The patch itself looks obviously correct. -Peff