All of lore.kernel.org
 help / color / mirror / Atom feed
From: marcnarc@xiplink.com
To: git@vger.kernel.org
Cc: Jens Lehmann <Jens.Lehmann@web.de>
Subject: [PATCH 1/3] submodules: recursive fetch also checks new tags for submodule commits
Date: Mon, 16 Apr 2012 18:08:48 -0400	[thread overview]
Message-ID: <1334614130-31826-2-git-send-email-marcnarc@xiplink.com> (raw)
In-Reply-To: <1334614130-31826-1-git-send-email-marcnarc@xiplink.com>

From: Jens Lehmann <Jens.Lehmann@web.de>

Since 88a21979c (fetch/pull: recurse into submodules when necessary) all
fetched commits are examined if they contain submodule changes (unless
configuration or command line options inhibit that). If a newly recorded
submodule commit is not present in the submodule, a fetch is run inside
it to download that commit.

Checking new refs was done in an else branch where it wasn't executed for
tags. This normally isn't a problem because tags are only fetched with
the branches they live on, then checking the new commits in the fetched
branches for submodule commits will also process all tags. But when a
specific tag is fetched (or the refspec contains refs/tags/) commits only
reachable by tags won't be searched for submodule commits, which is a bug.

Fix that by moving the code outside the if/else construct to handle new
tags just like any other ref. The performance impact of adding tags that
most of the time lie on a branch which is checked anyway for new submodule
commit should be minimal, as since 6859de4 (fetch: avoid quadratic loop
checking for updated submodules) all ref-tips are collected first and then
fed to a single rev-list.

Spotted-by: Jeff King <peff@peff.net>
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 builtin/fetch.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 65f5f9b..cfb43df 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -300,11 +300,11 @@ static int update_local_ref(struct ref *ref,
 		else {
 			msg = "storing head";
 			what = _("[new branch]");
-			if ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&
-			    (recurse_submodules != RECURSE_SUBMODULES_ON))
-				check_for_new_submodule_commits(ref->new_sha1);
 		}
 
+		if ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&
+		    (recurse_submodules != RECURSE_SUBMODULES_ON))
+			check_for_new_submodule_commits(ref->new_sha1);
 		r = s_update_ref(msg, ref, 0);
 		strbuf_addf(display, "%c %-*s %-*s -> %s%s",
 			    r ? '!' : '*',
-- 
1.7.10.2.ge89da.dirty

  reply	other threads:[~2012-04-16 22:08 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-13 17:08 [PATCHv2] fetch: Only call a new ref a "branch" if it's under refs/heads/ marcnarc
2012-04-13 20:07 ` Jonathan Nieder
2012-04-16 14:26   ` Marc Branchaud
2012-04-13 21:13 ` Jeff King
2012-04-13 21:53   ` Jonathan Nieder
2012-04-13 22:39     ` Junio C Hamano
2012-04-16 14:58       ` Marc Branchaud
2012-04-16 15:00         ` Jeff King
2012-04-16 15:52           ` [PATCHv3] fetch: Use the remote's ref name to decide how to describe new refs marcnarc
2012-04-16 16:10             ` Jonathan Nieder
2012-04-16 17:59             ` Junio C Hamano
2012-04-16 20:21             ` Marc Branchaud
2012-04-16 22:08             ` [PATCHv4 0/3] fetch: Only call a new ref a "branch" if it's under refs/heads/ marcnarc
2012-04-16 22:08               ` marcnarc [this message]
2012-04-16 22:08               ` [PATCH 2/3] fetch: Pass both the full remote ref and its short name to update_local_ref() marcnarc
2012-04-16 22:08               ` [PATCH 3/3] fetch: Use the remote's ref name to decide how to describe new refs marcnarc
2012-04-16 22:34                 ` Jonathan Nieder
2012-04-17  7:53                   ` Zbigniew Jędrzejewski-Szmek
2012-04-17  7:57                     ` Jonathan Nieder
2012-04-17  8:39                       ` Zbigniew Jędrzejewski-Szmek
2012-04-17 14:23                     ` Marc Branchaud
2012-04-17 15:18                       ` Jonathan Nieder
2012-04-17 15:26               ` [PATCHv4 0/3] fetch: Only call a new ref a "branch" if it's under refs/heads/ Junio C Hamano
2012-04-17 15:28                 ` Junio C Hamano
2012-04-17 19:30                 ` Marc Branchaud
2012-04-17 22:29               ` Jeff King
2012-04-16 16:08           ` [PATCHv2] " Jonathan Nieder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1334614130-31826-2-git-send-email-marcnarc@xiplink.com \
    --to=marcnarc@xiplink.com \
    --cc=Jens.Lehmann@web.de \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.