On 3/24/2022 2:58 PM, Taylor Blau wrote:
> On Thu, Mar 24, 2022 at 06:34:56PM +0000, Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <derrickstolee@xxxxxxxxxx>
>> diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
>> index 770d1432046..73452e23896 100755
>> --- a/t/t7700-repack.sh
>> +++ b/t/t7700-repack.sh
>> @@ -369,10 +369,56 @@ test_expect_success '--write-midx with preferred bitmap tips' '
>> )
>> '
>>
>> +get_sorted_objects_from_packs () {
>> + git show-index <$(cat) >raw &&
>
> It seems a little odd to me to pass the name of a single file as input
> to get_sorted_objects_from_packs over stdin. I probably would have
> expected something like `git show-index <"$1" >raw && ...` instead.
Based on the way we are creating a file whose contents is the name
of the .idx file, we would at least use '$(cat "$1")'. I kind of like
the symmetry of the input/output redirection when using the helper, but
I can easily change this.
> We may also want to s/packs/pack, since this function only will handle
> one index at a time.
Yes.
>> + cut -d" " -f2 raw | sort
>
> Having the sort in there is my fault, but after reading this more
> carefully it's definitely unnecessary, since show-index will give us
> the results in lexical order by object name already.
Cool. Will drop.
>> +}
>> +
>> test_expect_success '--write-midx -b packs non-kept objects' '
>> - GIT_TRACE2_EVENT="$(pwd)/trace.txt" \
>> - git repack --write-midx -a -b &&
>> - test_subcommand_inexact git pack-objects --honor-pack-keep <trace.txt
>> + git init repo &&
>> + test_when_finished "rm -fr repo" &&
>> + (
>> + cd repo &&
>> +
>> + # Create a kept pack-file
>> + test_commit base &&
>> + git repack -ad &&
>> + find $objdir/pack -name "*.idx" >before &&
>
> I thought that here it might be easier to say:
>
> before="$(find $objdir/pack -name "*.idx")"
>
>> + >$objdir/pack/$(basename $(cat before) .idx).keep &&
>
> ...and then replace "$(cat before)" with "$before", along with the
> other uses of the before file below. But it gets a little funny when
> you want to discover which is the new pack, where it is more natural to
> dump the output of comm into a file.
For this reason, I'll continue to store the .idx names in files.
>> + # Get object list from the one non-kept pack-file
>> + comm -13 before after >new-pack &&
>
> You could write "new_pack=$(comm -13 before after)", but debugging this
> test would be difficult if the output of comm there contained more than
> one line.
>
>> + get_sorted_objects_from_packs \
>> + <new-pack \
>
> Though we probably want to check that we only get one line anyway here,
> since get_sorted_objects_from_packs will barf if we had more than one
> line in file new-pack here, too.
Thanks. Easy to add a test_line_count before this check.
-Stolee
