Re: [PATCH] fetch: Only call a new ref a "branch" if it's under refs/heads/.

[Marc Branchaud <marcnarc@xiplink.com>]

On 12-04-12 04:15 PM, Jens Lehmann wrote:
> Am 12.04.2012 07:52, schrieb Jeff King:
>> On Wed, Apr 11, 2012 at 10:29:29AM -0400, marcnarc@xiplink.com wrote:
>>
>>>  builtin/fetch.c |    9 +++++++--
>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/builtin/fetch.c b/builtin/fetch.c
>>> index 65f5f9b..57be58a 100644
>>> --- a/builtin/fetch.c
>>> +++ b/builtin/fetch.c
>>> @@ -298,8 +298,13 @@ static int update_local_ref(struct ref *ref,
>>>  			what = _("[new tag]");
>>>  		}
>>>  		else {
>>> -			msg = "storing head";
>>> -			what = _("[new branch]");
>>> +			if (!prefixcmp(ref->name, "refs/heads/")) {
>>> +				msg = "storing head";
>>> +				what = _("[new branch]");
>>> +			} else {
>>> +				msg = "storing ref";
>>> +				what = _("[new ref]");
>>> +			}
>>>  			if ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&
>>>  			    (recurse_submodules != RECURSE_SUBMODULES_ON))
>>>  				check_for_new_submodule_commits(ref->new_sha1);
>>
>> It looks like you kept the behavior the same with respect to
>> recurse_submodules, which will continue to run for everything except
>> tags. Which is probably a good choice, since your patch only wanted to
>> deal with the status message, but I am left to wonder: if submodules
>> were intended to be recursed for branches but not tags, what should
>> happen for other types of refs? Was it intentional that they fell into
>> the "branch" category here, or were they following the same failure to
>> distinguish that the message-writing code had?
>>
>> This code block handles only new refs.  If you look at the code below,
>> updates of existing refs (forced or not) will happen for all refs,
>> including tags.
>>
>> Jens, can you double-check the intended logic?
> 
> Thanks for spotting this inconsistency. I think it makes sense to
> check for new submodule commits no matter if we fetched a new tag,
> branch or other ref. I can't remember a reason why I put that code
> into the refs & branches part instead of doing that for every new
> ref. Patch following ...

I assumed it was an optimization of some sort -- that since tags are normally
only fetched when they're part of a requested branch's history (right?),
there was no point in doing submodule recursion on the fetched tags since
those tagged tree-ishes had already been submodule-recursed.

Or something like that.  :)

		M.