On Fri, Feb 09 2018, Eric Sunshine jotted:
> On Thu, Feb 8, 2018 at 11:19 AM, Ævar Arnfjörð Bjarmason
> <avarab@xxxxxxxxx> wrote:
>> When a remote URL is supplied on the command-line the internals of the
>> fetch are different, in particular the code in get_ref_map(). An
>> earlier version of the subsequent fetch.pruneTags patch hid a segfault
>> because the difference wasn't tested for.
>>
>> Now all the tests are run as both of the variants of:
>>
>> git fetch
>> git -c [...] fetch $(git config remote.origin.url) $(git config remote.origin.fetch)
>>
>> I'm using -c because while the [fetch] config just set by
>> set_config_tristate will be picked up, the remote.origin.* config
>> won't override it as intended.
>>
>> Work around that and turn this into a purely command-line test by
>> always setting the variables on the command-line, and translate any
>> setting of remote.origin.X into fetch.X.
>> [...]
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
>> ---
>> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
>> @@ -548,18 +548,52 @@ set_config_tristate () {
>> *)
>> git config "$1" "$2"
>> + key=$(echo $1 | sed -e 's/^remote\.origin/fetch/')
>
> Faster (thus more Windows-friendly) assuming that $1 always starts
> with "remote.origin":
>
> key=fetch${u#remote.origin}
Tests fail with this and I'm not excited to be the first user in git's
test suite to use some novel shell feature, no existing uses of
${u[...].
I also think stuff like this is on the wrong side of cleverness
v.s. benefit. I can't find any reference to this syntax in bash or dash
manpages (forward-search "${u"), but echo | sed is obvious, and it's not
going to make a big difference for Windows.
>> + git_fetch_c="$git_fetch_c -c $key=$2"
>> ;;
>> esac
>> }
>>
>> +test_configured_prune_type () {
>> fetch_prune=$1
>> remote_origin_prune=$2
>> expected_branch=$3
>> expected_tag=$4
>> cmdline=$5
>> -
>> - test_expect_success "prune fetch.prune=$1 remote.origin.prune=$2${5:+ $5}; branch:$3 tag:$4" '
>> + mode=$6
>> +
>> + if ! test -e prune-type-setup-done
>> + then
>> + test_expect_success 'prune_type setup' '
>> + git -C one config remote.origin.url >one.remote-url &&
>> + git -C one config remote.origin.fetch >one.remote-fetch &&
>> + remote_url="file://$(cat one.remote-url)" &&
>> + remote_fetch="$(cat one.remote-fetch)" &&
>
> Is there a reason that these values need to be captured to files
> (which are otherwise not used) before being assigned to variables?
> That is, wouldn't this work?
>
> remote_url="file://$(git -C one config remote.origin.url)" &&
> remote_fetch="$(git -C one config remote.origin.fetch)" &&
Nope, I'll just do that. This was cruft left over from an earlier
version which I didn't clean up.
>> + cmdline_setup="\"$remote_url\" \"$remote_fetch\"" &&
>> + touch prune-type-setup-done
>
> Why does "prune-type-setup-done" need to be a file rather than a
> simple shell variable (which is global by default even when assigned
> inside test_expect_success)?
>
> Also, since the purpose of this code seems to compute 'cmdline_setup'
> just once, can't you do away with 'prune-type-setup-done' altogether
> and change:
>
> if ! test -e prune-type-setup-done
>
> to:
>
> if test -z "$cmdline_setup"
>
>> + '
>> + fi
Yup, that's much simpler. Thanks.
