Re: [PATCH v2 1/6] transport-helper: clarify *:* refspec

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

 



On Thu, Apr 18, 2013 at 12:27 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:
>
>> The *:* refspec doesn't work, and never has, clarify the code and
>> documentation to reflect that. This in effect reverts commit 9e7673e
>> (gitremote-helpers(1): clarify refspec behaviour).
>> ...
>>  applicable refspec takes precedence.  The left-hand of refspecs
>>  advertised with this capability must cover all refs reported by
>> -the list command.  If a helper does not need a specific 'refspec'
>> -capability then it should advertise `refspec *:*`.
>> +the list command.  If no 'refspec' capability is advertised,
>> +there is an implied `refspec *:*`.
>
> I presume
>
>     s/.$/, but `*:*` does not work so do not use it./
>
> is implied?

I believe

  s/.$/, but you should specify an apropriate one as described above./

Would be better.

>> diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
>> index f387027..cd1873c 100755
>> --- a/t/t5801-remote-helpers.sh
>> +++ b/t/t5801-remote-helpers.sh
>> @@ -120,21 +120,6 @@ test_expect_failure 'pushing without refspecs' '
>>       compare_refs local2 HEAD server HEAD
>>  '
>>
>> -test_expect_success 'pulling with straight refspec' '
>> -     (cd local2 &&
>> -     GIT_REMOTE_TESTGIT_REFSPEC="*:*" git pull) &&
>> -     compare_refs local2 HEAD server HEAD
>> -'
>
> This one made me raise my eyebrows, first.
>
> The only reason this test "passes" is because this particular
> "pull", due to what local2 and server happens to have before this
> test runs, is an "Already up-to-date" and a no-op.  You are removing
> this because it is not really testing anything useful, and if you
> change it to test something real, e.g. by rewinding local2, it will
> reveal the breakage.
>
> Am I reading you correctly?

Not really. This particular fetch does work, and it's tricky to
explain all the reasons why, even if you update the server ref before
fetching. The reason it's written this way is that it comes from from
a branch of mine that has a hack to make the push above works, and
since the transport-helper's push doesn't update the ref in this
version, the test did actually do something and was working.

I am removing the test because we don't care if it works or not,
remote-helpers should not be doing that. The fact that the test wasn't
actually doing anything is icing on the cake.

Cheers.

-- 
Felipe Contreras
--
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]