Re: [PATCH 2/2] Make local branches behave like remote branches when --tracked

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 27, 2009, 08:08 UTC
Message-ID: <7vprg3fkw8.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1238100805-19619-3-git-send-email-git@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

Show 26 quoted lines

> This makes sure that local branches, when followed using --track, behave
> the same as remote ones (e.g. differences being reported by git status
> and git checkout). This fixes 1 known failure.
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
>  remote.c                 |    9 +++++----
>  t/t6040-tracking-info.sh |    2 +-
>  2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/remote.c b/remote.c
> index 2b037f1..5d2d7a1 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1170,8 +1170,9 @@ struct branch *branch_get(const char *name)
>  			for (i = 0; i < ret->merge_nr; i++) {
>  				ret->merge[i] = xcalloc(1, sizeof(**ret->merge));
>  				ret->merge[i]->src = xstrdup(ret->merge_name[i]);
> -				remote_find_tracking(ret->remote,
> -						     ret->merge[i]);
> +				if(remote_find_tracking(ret->remote,
> +						     ret->merge[i]) && !strcmp(ret->remote_name, "."))
> +					ret->merge[i]->dst = xstrdup(ret->merge_name[i]);
>  			}
>  		}
>  	}

Yuck; please have a SP betweeen "if" and "(", and also have a decency to break a long line at a more sensible place, like:

			if (remote_find_tracking(ret->remote, ret->merge[i])
			    && !strcmp(...))
                            	then do this;

A naïve question from me to this change is why this "fix-up" is done here.

The remote_find_tracking() function is given a half-filled refspec (this caller fills the src side, and asks to find the dst side to the function). After it fails to find a fetch refspec that copies remote refs to tracking refs in the local repository that match the criteria, it returns -1 to signal an error, otherwise it returns 0 after updating the other half of the refspec.

After calling r-f-t, because this new code assumes that for the "." remote (aka "local repository"), r-f-t lies and does not give back what it expects, fixes what it got back from r-f-t. Shouldn't we be fixing this inside r-f-t?

Show 8 quoted lines

> @@ -1449,8 +1450,8 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
>  		return 0;
>  
>  	base = branch->merge[0]->dst;
> -	if (!prefixcmp(base, "refs/remotes/")) {
> -		base += strlen("refs/remotes/");
> +	if (!prefixcmp(base, "refs/")) {
> +		base += strlen("refs/");

I am not sure if this is a good change. The majority of the case would be remotes/ and we would be better off not repeating them. Can't you limit the use of longer refs only when disambiguation is necessary?