Re: [PATCH] fetch: Only call a new ref a "branch" if it's under refs/heads/.
- From
Marc Branchaud <marcnarc@xiplink.com>
- Date
- Apr 12, 2012, 20:36 UTC
- Message-ID
- <4F873CD0.4050204@xiplink.com>
- In-Reply-To
- <4F8737C8.1020501@web.de>
On 12-04-12 04:15 PM, Jens Lehmann wrote:
Show 47 quoted lines
> 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.