Re: [PATCH] fetch: refuse to fetch into the current branch in a non-bare repository

From: Daniel Barkalow <barkalow@iabervon.org>
Date: Oct 12, 2008, 20:37 UTC
Message-ID: <alpine.LNX.1.00.0810121501590.19665@iabervon.org>
In-Reply-To: <alpine.DEB.1.00.0810111336350.22125@pacific.mpi-cbg.de.mpi-cbg.de>

On Sat, 11 Oct 2008, Johannes Schindelin wrote:

Show 10 quoted lines

> Some confusing tutorials suggest that it would be a good idea to call
> something like this:
> 
> 	git pull origin master:master
> 
> While it might make sense to store what you want to merge, it typically
> is plain wrong.  Especially so when the current branch is "master".
> 
> Be at least a little bit helpful by refusing to fetch something into
> the current branch.

I think this is the right thing to do.

Show 28 quoted lines

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  builtin-fetch.c   |   20 ++++++++++++++++++++
>  t/t5505-remote.sh |    2 +-
>  t/t5510-fetch.sh  |    6 ++++++
>  3 files changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/builtin-fetch.c b/builtin-fetch.c
> index ee93d3a..d701550 100644
> --- a/builtin-fetch.c
> +++ b/builtin-fetch.c
> @@ -534,6 +534,25 @@ static void find_non_local_tags(struct transport *transport,
>  	string_list_clear(&new_refs, 0);
>  }
>  
> +static void check_ref_map(struct ref *ref_map)
> +{
> +	int flag;
> +	unsigned char sha1[20];
> +	const char *HEAD;
> +
> +	if (is_bare_repository())
> +		return;
> +
> +	HEAD = resolve_ref("HEAD", sha1, 1, &flag);
> +
> +	if (!HEAD || !(flag & REF_ISSYMREF))
> +		return;

remote.h has a function for getting "the current branch", which would save 5 lines here:

	struct branch *current_branch = branch_get(NULL);
	if (!current_branch || is_bare_repository())
		return;

> +
> +	for (; ref_map; ref_map = ref_map->next)
> +		if (ref_map->peer_ref && !strcmp(HEAD, ref_map->peer_ref->name))

		!strcmp(current_branch->ref_name, ref_map->peer_ref->name)

(untested, and might be off by a "refs/" or something)

Show 14 quoted lines

> +			die("Refusing to fetch into current branch");
> +}
> +
>  static int do_fetch(struct transport *transport,
>  		    struct refspec *refs, int ref_count)
>  {
> @@ -558,6 +577,7 @@ static int do_fetch(struct transport *transport,
>  	}
>  
>  	ref_map = get_ref_map(transport, refs, ref_count, tags, &autotags);
> +	check_ref_map(ref_map);
>  
>  	for (rm = ref_map; rm; rm = rm->next) {
>  		if (rm->peer_ref)