Re: [PATCHv4 15/15] Replace "unset VAR" with "unset VAR;" in testsuite as per t/README
- From
Elijah Newren <newren@gmail.com>
- Date
- Sep 29, 2010, 20:30 UTC
- Message-ID
- <AANLkTimgTAerCNcSHBR9t-s-ThWHLMXdsb2T=E2w8gpB@mail.gmail.com>
- In-Reply-To
- <7viq1omiv8.fsf@alter.siamese.dyndns.org>
Hi,
Thanks for all the reviews. I'll fix things up. One question on this last one...
On Wed, Sep 29, 2010 at 1:48 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 37 quoted lines
> Elijah Newren <newren@gmail.com> writes:
>
>> Acked-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> Signed-off-by: Elijah Newren <newren@gmail.com>
>> ---
>> @@ -175,8 +175,8 @@ test_expect_success 'init with init.templatedir set' '
>> git config -f "$test_config" init.templatedir "${HOME}/templatedir-source" &&
>> mkdir templatedir-set &&
>> cd templatedir-set &&
>> - unset GIT_CONFIG_NOGLOBAL &&
>> - unset GIT_TEMPLATE_DIR &&
>> + unset GIT_CONFIG_NOGLOBAL;
>> + unset GIT_TEMPLATE_DIR;
>> NO_SET_GIT_TEMPLATE_DIR=t &&
>> export NO_SET_GIT_TEMPLATE_DIR &&
>> git init
>> @@ -187,7 +187,7 @@ test_expect_success 'init with init.templatedir set' '
>> test_expect_success 'init --bare/--shared overrides system/global config' '
>> (
>> test_config="$HOME"/.gitconfig &&
>> - unset GIT_CONFIG_NOGLOBAL &&
>> + unset GIT_CONFIG_NOGLOBAL;
>> git config -f "$test_config" core.bare false &&
>> git config -f "$test_config" core.sharedRepository 0640 &&
>> mkdir init-bare-shared-override &&
>> @@ -202,7 +202,7 @@ test_expect_success 'init --bare/--shared overrides system/global config' '
>> test_expect_success 'init honors global core.sharedRepository' '
>> (
>> test_config="$HOME"/.gitconfig &&
>> - unset GIT_CONFIG_NOGLOBAL &&
>> + unset GIT_CONFIG_NOGLOBAL;
>> git config -f "$test_config" core.sharedRepository 0666 &&
>> mkdir shared-honor-global &&
>> cd shared-honor-global &&
>
> These three hunks look wrong as they break the && cascades. All others
> (including the change to 7502) look Ok.They break the && cascades, but aren't the && cascades already inherently broken due to the unportable return value of unset?
Show 5 quoted lines
> Personally I do not agree that ";" at the end is explicit enough as > t/README seems to think. If we want to be explicit, I'd say we should do > so by saying something like: > > unset VAR ;# can fail
That makes sense. Alternatively, I asked in an earlier posting of
this series whether it would it make sense to add a
portable_unset () {
unset $* || true
}
helper function to test-lib, and just use it instead of unset? That
would allow proper chaining everywhere. Thoughts?