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

[John Keeping <john@keeping.me.uk>]

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.

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  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.  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.

?

>  'bidi-import'::
>  	This modifies the 'import' capability.
> 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
> -'
> -
> -test_expect_failure 'pushing with straight refspec' '
> -	test_when_finished "(cd local2 && git reset --hard origin)" &&
> -	(cd local2 &&
> -	echo content >>file &&
> -	git commit -a -m eleven &&
> -	GIT_REMOTE_TESTGIT_REFSPEC="*:*" git push) &&
> -	compare_refs local2 HEAD server HEAD
> -'
> -
>  test_expect_success 'pulling without marks' '
>  	(cd local2 &&
>  	GIT_REMOTE_TESTGIT_NO_MARKS=1 git pull) &&
> diff --git a/transport-helper.c b/transport-helper.c
> index dcd8d97..cea787c 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -469,7 +469,7 @@ static int fetch_with_import(struct transport *transport,
>  	 * were fetching.
>  	 *
>  	 * (If no "refspec" capability was specified, for historical
> -	 * reasons we default to *:*.)
> +	 * reasons we default to the equivalent of *:*.)
>  	 *
>  	 * Store the result in to_fetch[i].old_sha1.  Callers such
>  	 * as "git fetch" can use the value to write feedback to the
> -- 
> 1.8.2.1.679.g509521a