Re: [PATCH v2 1/6] transport-helper: clarify *:* refspec
- From
Felipe Contreras <felipe.contreras@gmail.com>
- Date
- Apr 18, 2013, 09:27 UTC
- Message-ID
- <CAMP44s0cJwHSJrWKkWAwKSv8OE=OqVRdKEtBNdAvQ7YAE5-JKw@mail.gmail.com>
- In-Reply-To
- <20130418082441.GR2278@serenity.lan>
On Thu, Apr 18, 2013 at 3:24 AM, John Keeping <john@keeping.me.uk> wrote:
Show 7 quoted lines
> On Wed, Apr 17, 2013 at 11:14:28PM -0500, Felipe Contreras wrote: >> 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). > > In what way doesn't it work? If I specify that refspec then I do get > output that appears sensible.
% git checkout origin/master % make -C t t5801-remote-helpers.sh
not ok 15 - pushing with straight refspec # TODO known breakage
It fails for me here.
Show 19 quoted lines
>> Documentation/gitremote-helpers.txt | 4 ++-- >> t/t5801-remote-helpers.sh | 15 --------------- >> transport-helper.c | 2 +- >> 3 files changed, 3 insertions(+), 18 deletions(-) >> >> diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt >> index f506031..0c91aba 100644 >> --- a/Documentation/gitremote-helpers.txt >> +++ b/Documentation/gitremote-helpers.txt >> @@ -174,8 +174,8 @@ ref. >> This capability can be advertised multiple times. The first >> 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 *:*`. > > This is wrong.
It has been like that since v1.7.0.
Show 7 quoted lines
> As your later patch makes clearer, there is no implied > refspec for push - it only works for fetch. I found the wording you've > reverted to extremely misleading. How about something like this: > > For historical reasons, 'import' treats the absence of a 'refspec' > line as equivalent to `refspec *:*`; remote helpers should always > specify an explicit refspec.
Maybe my version was "misleading" because it didn't mention it was for historical reasons, but yours is plain wrong; remote helpers might not be using 'import' or 'export' at all, so not all remote helpers should always specify an explicit refspec. And if the previous text "It is recommended that all importers providing the 'import' capability use this. It's mandatory for 'export'." does not convey the idea these remote helpers should always specify an explicit refspec, I don't know what it does.
-- Felipe Contreras