Re: [PATCH v9 1/6] t5323: test cases for git-pack-redundant

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

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Fri, Feb 1, 2019 at 11:22 AM Jiang Xin <worldhello.net@gmail.com> wrote:
>> Add test cases for git pack-redundant to validate new algorithm for git
>> pack-redundant.
>>
>> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>> ---
>> diff --git a/t/t5323-pack-redundant.sh b/t/t5323-pack-redundant.sh
>> @@ -0,0 +1,510 @@
>> +# Note: DO NOT run it in a subshell, otherwise the variables will not be set
>
> Which variables won't be set? It's not clear what this restriction is about.

>> +       git -C "$repo" update-ref refs/heads/master $oid
>> +}
>> +
>> +# Note: DO NOT run it in a subshell, otherwise the variables will not be set
>> +create_pack_1 () {
>> +       P1=$(git -C "$master_repo/objects/pack" pack-objects -q pack <<-EOF
>
> Which variables? Note that you can capture output of a subshell into a
> variable, if necessary.

These helper functions set a bunch of variables $P1, $P2, etc. as
well as variables whose name begin with P and followed by 40-hex.
The script wants to use them later when preparing expected output,
and with the most natural way to organize the code, that "later"
happens in the process that would have spawned a subshell to run
this function.

It would have been easier for you to grok if the note instead said
"this function sets two global shell variables" or something,
perhaps?  Such a variable would certainly not be visible if this
function is called inside a subshell to the main process.