Re: [PATCH 2/2] builtin/fetch.c: ignore merge config when not fetching from branch's remote

From: Brandon Casey <casey@nrlssc.navy.mil>
Date: Aug 25, 2010, 21:41 UTC
Message-ID: <TDHEkC5Y5b2p-yjw_5mixlQb49a7TMinj2d5CgfEHHH9Dq6lNFV7dQ@cipher.nrlssc.navy.mil>
In-Reply-To: <20100825211641.GC2319@burratino>

On 08/25/2010 04:16 PM, Jonathan Nieder wrote:

Show 21 quoted lines

> Brandon Casey wrote:
> 
>> If the argument to fetch does not specify a named remote, or if the name
>> supplied does not match the remote configured for the current branch, then
>> the current branch's merge configuration should not be considered.
> 
> Thanks for a fix.
> 
>> +++ b/builtin/fetch.c
>> @@ -146,7 +146,8 @@ static struct ref *get_ref_map(struct transport *transport,
>>  		struct remote *remote = transport->remote;
>>  		struct branch *branch = branch_get(NULL);
>>  		int has_merge = branch_has_merge_config(branch);
>> -		if (remote && (remote->fetch_refspec_nr || has_merge)) {
>> +		if (remote && (remote->fetch_refspec_nr || (has_merge &&
>> +				!strcmp(branch->remote_name, remote->name)))) {
> 
> What will happen with this (invalid) branch?
> 
> 	[branch "tmp"]
> 		merge = refs/heads/tmp

The same thing that would have happened before, since a few lines further down there is this:

   if (has_merge &&
       !strcmp(branch->remote_name, remote->name))
           add_merge_config(&ref_map, remote_refs, branch, &tail);

I didn't trace branch_get() to check whether it returns an object with remote_name initialized in all cases. I relied on the form of the existing code. Perhaps it's worth investigating. If something needs to be fixed, then it was already broken and deserves a separate patch anyway.

-Brandon