Re: [PATCH v2 4/6] transport-helper: warn when refspec is not used

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

 



Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:

> For the modes that need it. In the future we should probably error out,
> instead of providing half-assed support.
>
> The reason we want to do this is because if it's not present, the remote
> helper might be updating refs/heads/*, or refs/remotes/origin/*,
> directly, and in the process fetch will get confused trying to update
> refs that are already updated, or older than what they should be. We
> shouldn't be messing with the rest of git.

So that answers my question in the response to an earlier one in
this series.  We expect the ref updates to be done by the fetch or
push that drives the helper, and do not want the helper to interfere
with its ref updates.

So it is not just 'refspec' _allows_ the refs to be constrained to a
private namespace, like the earlier updates made the documentation
say; it _is_ mandatory to use refspecs to constrain them to avoid
touching refs/heads and refs/remotes namespace.

Am I reading you correctly?

Assuming I am, the patch in this message looks reasonable.

It makes the documentation updates a few patches ago look a bit
wanting, though.

Thanks.

> Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
> ---
>  t/t5801-remote-helpers.sh | 6 ++++--
>  transport-helper.c        | 2 ++
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
> index 3eeb309..1bb7529 100755
> --- a/t/t5801-remote-helpers.sh
> +++ b/t/t5801-remote-helpers.sh
> @@ -100,14 +100,16 @@ test_expect_failure 'push new branch with old:new refspec' '
>  
>  test_expect_success 'cloning without refspec' '
>  	GIT_REMOTE_TESTGIT_REFSPEC="" \
> -	git clone "testgit::${PWD}/server" local2 &&
> +	git clone "testgit::${PWD}/server" local2 2> error &&
> +	grep "This remote helper should implement refspec capability" error &&
>  	compare_refs local2 HEAD server HEAD
>  '
>  
>  test_expect_success 'pulling without refspecs' '
>  	(cd local2 &&
>  	git reset --hard &&
> -	GIT_REMOTE_TESTGIT_REFSPEC="" git pull) &&
> +	GIT_REMOTE_TESTGIT_REFSPEC="" git pull 2> ../error) &&
> +	grep "This remote helper should implement refspec capability" error &&
>  	compare_refs local2 HEAD server HEAD
>  '
>  
> diff --git a/transport-helper.c b/transport-helper.c
> index 4d98567..573eaf7 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -215,6 +215,8 @@ static struct child_process *get_helper(struct transport *transport)
>  			free((char *)refspecs[i]);
>  		}
>  		free(refspecs);
> +	} else if (data->import || data->bidi_import || data->export) {
> +		warning("This remote helper should implement refspec capability.");
>  	}
>  	strbuf_release(&buf);
>  	if (debug)
--
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]