Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:

>> I don't know if it's worth worrying about or not. The usual solution is
>> something like:
>>
>>   env_git_dir=$2
>>   env='GIT_DIR=$env_git_dir; export GIT_DIR'
>>   ...
>>   eval "$env"
>
> Makes sense; I wasn't quite happy with having $2 interpolated
> unquoted. Like you, though, I don't know if it's worth worrying
> about...

This is a good change, including the quoting of $env.

> I flip-flopped on this one several times, quoting, and not quoting.
> Documentation for 'eval' says:
>
>     The args are read and concatenated together into a single
>     command.

Which means the two eval's give different results:

    $ e='echo "a  b"'
    $ eval $e
    $ eval "$e"

>> This will set up the sane_unset regardless of whether $env does
>> anything. Would it make more sense to stick the test_when_finished
>> inside $env? You could use regular unset then, too, since you know the
>> variable would be set.
>
> I didn't worry about it too much because the end result is effectively
> the same and, with all the 'case' arms being short one-liners, I think
> the code is a bit easier to read as-is.

Yeah, this is OK.
--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]