Re: [PATCH v2 1/6] transport-helper: clarify *:* refspec
- From
Felipe Contreras <felipe.contreras@gmail.com>
- Date
- Apr 18, 2013, 19:07 UTC
- Message-ID
- <CAMP44s1NRMbk2DpgyWqnyEKV+WkxAg1Gjn3UZQKMopJUDBZ4jw@mail.gmail.com>
- In-Reply-To
- <7vhaj37nr5.fsf@alter.siamese.dyndns.org>
On Thu, Apr 18, 2013 at 12:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 18 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> 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.
Show 24 quoted lines
>> 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