[RFC/PATCH 0/4] update tracking refs on explicit fetch

[RFC/PATCH 0/4] update tracking refs on explicit fetch

[Jeff King]

This is a cleaned-up version of the oft-discussed[1] concept that "git
pull origin master" should update "refs/remotes/origin/master".

It is a little bit of a risky change, in that anybody who deeply cares
about when their tracking ref branches are updated would be impacted.
But I think the general consensus is that while you can come up with a
hypothetical use case, the logic is quite tortured, and you could
accomplish the same thing by keeping a local branch. Most people seem to
have the mental model that remote tracking branches are basically a
cache for what's on the remote, updated opportunistically as
appropriate.

  [1/4]: t5510: start tracking-ref tests from a known state
  [2/4]: fetch/pull doc: untangle meaning of bare <ref>
  [3/4]: refactor "ref->merge" flag
  [4/4]: fetch: opportunistically update tracking refs

The final patch is the moral equivalent of the patch I've posted before
when this topic comes up (and referenced in the threads below). But that
patch did not take care not to write duplicate (mergable!) entries into
FETCH_HEAD, leading to great confusion from git-pull.  This series deals
with that, and takes care to write the exact same FETCH_HEAD we would
have written before the series.

However, Jonathan raised an interesting question: what is the point of
not-for-merge entries at all? We are not going to merge them, so none of
the pull/merge machinery cares about them. We do not consider extra
entries in FETCH_HEAD for reachability or anything like that. Is there
anybody who reads them?

If we do not care about them, we can simplify this much further, and
either write the duplicates as not-for-merge, or even omit not-for-merge
entries entirely. However, it is possible that some third-party scripts
process FETCH_HEAD, so I took the conservative route.

Note that this patch is as greedy as possible[2]; whenever we see the
LHS is fetched, we update the RHS side, even if the user did:

  git fetch origin master:foobar

Some of the discussions below argue that the behavior should not kick in
for such a case. I am not sure I agree (and argue against it in those
discussions). I think at this point that is the only potentially
contentious item.

-Peff

[1] Past discussions:

    http://thread.gmane.org/gmane.comp.version-control.git/127163/focus=127215

    http://thread.gmane.org/gmane.comp.version-control.git/192252

    http://thread.gmane.org/gmane.comp.version-control.git/203357/focus=203442

    http://article.gmane.org/gmane.comp.version-control.git/221234

[PATCH 1/4] t5510: start tracking-ref tests from a known state

[Jeff King]

We have three sequential tests for for whether tracking refs
are updated by various fetches and pulls; the first two
should not update the ref, and the third should. Each test
depends on the state left by the test before.

This is fragile (a failing early test will confuse later
tests), and means we cannot add more "should update" tests
after the third one.

Let's instead save the initial state before these tests, and
then reset to a known state before running each test.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t5510-fetch.sh | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index d7a19a1..789c228 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -370,12 +370,20 @@ test_expect_success 'explicit fetch should not update tracking' '
 
 '
 
+test_expect_success 'mark initial state of origin/master' '
+	(
+		cd three &&
+		git tag base-origin-master refs/remotes/origin/master
+	)
+'
+
 test_expect_success 'explicit fetch should not update tracking' '
 
 	cd "$D" &&
 	git branch -f side &&
 	(
 		cd three &&
+		git update-ref refs/remotes/origin/master base-origin-master &&
 		o=$(git rev-parse --verify refs/remotes/origin/master) &&
 		git fetch origin master &&
 		n=$(git rev-parse --verify refs/remotes/origin/master) &&
@@ -390,6 +398,7 @@ test_expect_success 'explicit pull should not update tracking' '
 	git branch -f side &&
 	(
 		cd three &&
+		git update-ref refs/remotes/origin/master base-origin-master &&
 		o=$(git rev-parse --verify refs/remotes/origin/master) &&
 		git pull origin master &&
 		n=$(git rev-parse --verify refs/remotes/origin/master) &&
@@ -404,6 +413,7 @@ test_expect_success 'configured fetch updates tracking' '
 	git branch -f side &&
 	(
 		cd three &&
+		git update-ref refs/remotes/origin/master base-origin-master &&
 		o=$(git rev-parse --verify refs/remotes/origin/master) &&
 		git fetch origin &&
 		n=$(git rev-parse --verify refs/remotes/origin/master) &&
-- 
1.8.3.rc1.2.g12db477

Re: [PATCH 1/4] t5510: start tracking-ref tests from a known state

[Eric Sunshine]

On Sat, May 11, 2013 at 12:14 PM, Jeff King <peff@peff.net> wrote:
> We have three sequential tests for for whether tracking refs

s/for for/for/
[or]
s/for for/for checking/

> are updated by various fetches and pulls; the first two
> should not update the ref, and the third should. Each test
> depends on the state left by the test before.
>
> This is fragile (a failing early test will confuse later
> tests), and means we cannot add more "should update" tests
> after the third one.
>
> Let's instead save the initial state before these tests, and
> then reset to a known state before running each test.
>
> Signed-off-by: Jeff King <peff@peff.net>

[PATCH 2/4] fetch/pull doc: untangle meaning of bare <ref>

[Jeff King]

From: Thomas Rast <trast@inf.ethz.ch>

The documentation erroneously used the same wording for both fetch and
pull, stating that something will be merged even in git-fetch(1).

In addition, saying that "<ref> is equivalent to <ref>:" doesn't
really help anyone who still needs to read manpages.  Clarify what is
actually going on.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/pull-fetch-param.txt | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/pull-fetch-param.txt b/Documentation/pull-fetch-param.txt
index 94a9d32..6f5ca21 100644
--- a/Documentation/pull-fetch-param.txt
+++ b/Documentation/pull-fetch-param.txt
@@ -68,6 +68,11 @@ Some short-cut notations are also supported.
 +
 * `tag <tag>` means the same as `refs/tags/<tag>:refs/tags/<tag>`;
   it requests fetching everything up to the given tag.
-* A parameter <ref> without a colon is equivalent to
-  <ref>: when pulling/fetching, so it merges <ref> into the current
-  branch without storing the remote branch anywhere locally
+ifndef::git-pull[]
+* A parameter <ref> without a colon fetches that ref into FETCH_HEAD,
+endif::git-pull[]
+ifdef::git-pull[]
+* A parameter <ref> without a colon merges <ref> into the current
+  branch,
+endif::git-pull[]
+  while not storing the branch anywhere locally.
-- 
1.8.3.rc1.2.g12db477

[PATCH 3/4] refactor "ref->merge" flag

[Jeff King]

Each "struct ref" has a boolean flag that is set by the
fetch code to determine whether the ref should be marked as
"not-for-merge" or not when we write it out to FETCH_HEAD.

It would be useful to turn this boolean into a tri-state,
with the third state meaning "do not bother writing it out
to FETCH_HEAD at all". That would let us add extra refs to
the set of refs to be stored (e.g., to store copies of
things we fetched) without impacting FETCH_HEAD.

This patch turns it into an enum that covers the tri-state
case, and hopefully makes the code more explicit and easier
to read.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/fetch.c | 57 +++++++++++++++++++++++++++++++++++----------------------
 cache.h         | 14 +++++++++++++-
 2 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 4b6b1df..287cf4c 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -119,7 +119,7 @@ static void add_merge_config(struct ref **head,
 
 		for (rm = *head; rm; rm = rm->next) {
 			if (branch_merge_matches(branch, i, rm->name)) {
-				rm->merge = 1;
+				rm->fetch_head_status = FETCH_HEAD_MERGE;
 				break;
 			}
 		}
@@ -140,7 +140,7 @@ static void add_merge_config(struct ref **head,
 		refspec.src = branch->merge[i]->src;
 		get_fetch_map(remote_refs, &refspec, tail, 1);
 		for (rm = *old_tail; rm; rm = rm->next)
-			rm->merge = 1;
+			rm->fetch_head_status = FETCH_HEAD_MERGE;
 	}
 }
 
@@ -167,7 +167,7 @@ static struct ref *get_ref_map(struct transport *transport,
 		}
 		/* Merge everything on the command line, but not --tags */
 		for (rm = ref_map; rm; rm = rm->next)
-			rm->merge = 1;
+			rm->fetch_head_status = FETCH_HEAD_MERGE;
 		if (tags == TAGS_SET)
 			get_fetch_map(remote_refs, tag_refspec, &tail, 0);
 	} else {
@@ -186,7 +186,7 @@ static struct ref *get_ref_map(struct transport *transport,
 					*autotags = 1;
 				if (!i && !has_merge && ref_map &&
 				    !remote->fetch[0].pattern)
-					ref_map->merge = 1;
+					ref_map->fetch_head_status = FETCH_HEAD_MERGE;
 			}
 			/*
 			 * if the remote we're fetching from is the same
@@ -202,7 +202,7 @@ static struct ref *get_ref_map(struct transport *transport,
 			ref_map = get_remote_ref(remote_refs, "HEAD");
 			if (!ref_map)
 				die(_("Couldn't find remote ref HEAD"));
-			ref_map->merge = 1;
+			ref_map->fetch_head_status = FETCH_HEAD_MERGE;
 			tail = &ref_map->next;
 		}
 	}
@@ -389,7 +389,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 	const char *what, *kind;
 	struct ref *rm;
 	char *url, *filename = dry_run ? "/dev/null" : git_path("FETCH_HEAD");
-	int want_merge;
+	int want_status;
 
 	fp = fopen(filename, "a");
 	if (!fp)
@@ -407,19 +407,22 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 	}
 
 	/*
-	 * The first pass writes objects to be merged and then the
-	 * second pass writes the rest, in order to allow using
-	 * FETCH_HEAD as a refname to refer to the ref to be merged.
+	 * We do a pass for each fetch_head_status type in their enum order, so
+	 * merged entries are written before not-for-merge. That lets readers
+	 * use FETCH_HEAD as a refname to refer to the ref to be merged.
 	 */
-	for (want_merge = 1; 0 <= want_merge; want_merge--) {
+	for (want_status = FETCH_HEAD_MERGE;
+	     want_status <= FETCH_HEAD_IGNORE;
+	     want_status++) {
 		for (rm = ref_map; rm; rm = rm->next) {
 			struct ref *ref = NULL;
+			const char *merge_status_marker = "";
 
 			commit = lookup_commit_reference_gently(rm->old_sha1, 1);
 			if (!commit)
-				rm->merge = 0;
+				rm->fetch_head_status = FETCH_HEAD_NOT_FOR_MERGE;
 
-			if (rm->merge != want_merge)
+			if (rm->fetch_head_status != want_status)
 				continue;
 
 			if (rm->peer_ref) {
@@ -465,16 +468,26 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 					strbuf_addf(&note, "%s ", kind);
 				strbuf_addf(&note, "'%s' of ", what);
 			}
-			fprintf(fp, "%s\t%s\t%s",
-				sha1_to_hex(rm->old_sha1),
-				rm->merge ? "" : "not-for-merge",
-				note.buf);
-			for (i = 0; i < url_len; ++i)
-				if ('\n' == url[i])
-					fputs("\\n", fp);
-				else
-					fputc(url[i], fp);
-			fputc('\n', fp);
+			switch (rm->fetch_head_status) {
+			case FETCH_HEAD_NOT_FOR_MERGE:
+				merge_status_marker = "not-for-merge";
+				/* fall-through */
+			case FETCH_HEAD_MERGE:
+				fprintf(fp, "%s\t%s\t%s",
+					sha1_to_hex(rm->old_sha1),
+					merge_status_marker,
+					note.buf);
+				for (i = 0; i < url_len; ++i)
+					if ('\n' == url[i])
+						fputs("\\n", fp);
+					else
+						fputc(url[i], fp);
+				fputc('\n', fp);
+				break;
+			default:
+				/* do not write anything to FETCH_HEAD */
+				break;
+			}
 
 			strbuf_reset(&note);
 			if (ref) {
diff --git a/cache.h b/cache.h
index 94ca1ac..9670d99 100644
--- a/cache.h
+++ b/cache.h
@@ -1024,9 +1024,21 @@ struct ref {
 	unsigned int
 		force:1,
 		forced_update:1,
-		merge:1,
 		deletion:1,
 		matched:1;
+
+	/*
+	 * Order is important here, as we write to FETCH_HEAD
+	 * in numeric order. And the default NOT_FOR_MERGE
+	 * should be 0, so that xcalloc'd structures get it
+	 * by default.
+	 */
+	enum {
+		FETCH_HEAD_MERGE = -1,
+		FETCH_HEAD_NOT_FOR_MERGE = 0,
+		FETCH_HEAD_IGNORE = 1
+	} fetch_head_status;
+
 	enum {
 		REF_STATUS_NONE = 0,
 		REF_STATUS_OK,
-- 
1.8.3.rc1.2.g12db477

[PATCH 4/4] fetch: opportunistically update tracking refs

[Jeff King]

When we run a regular "git fetch" without arguments, we
update the tracking refs according to the configured
refspec. However, when we run "git fetch origin master" (or
"git pull origin master"), we do not look at the configured
refspecs at all, and just update FETCH_HEAD.

We miss an opportunity to update "refs/remotes/origin/master"
(or whatever the user has configured). Some users find this
confusing, because they would want to do further comparisons
against the old state of the remote master, like:

  $ git pull origin master
  $ git log HEAD...origin/master

In the currnet code, they are comparing against whatever
commit happened to be in origin/master from the last time
they did a complete "git fetch".  This patch will update a
ref from the RHS of a configured refspec whenever we happen
to be fetching its LHS. That makes the case above work.

The downside is that any users who really care about whether
and when their tracking branches are updated may be
surprised.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/pull-fetch-param.txt |  2 +-
 builtin/fetch.c                    | 16 ++++++++++++++++
 t/t5510-fetch.sh                   |  8 ++++----
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/Documentation/pull-fetch-param.txt b/Documentation/pull-fetch-param.txt
index 6f5ca21..18cffc2 100644
--- a/Documentation/pull-fetch-param.txt
+++ b/Documentation/pull-fetch-param.txt
@@ -75,4 +75,4 @@ endif::git-pull[]
 * A parameter <ref> without a colon merges <ref> into the current
   branch,
 endif::git-pull[]
-  while not storing the branch anywhere locally.
+  and updates the remote-tracking branches (if any).
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 287cf4c..e41cc0d 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -160,6 +160,8 @@ static struct ref *get_ref_map(struct transport *transport,
 	const struct ref *remote_refs = transport_get_remote_refs(transport);
 
 	if (ref_count || tags == TAGS_SET) {
+		struct ref **old_tail;
+
 		for (i = 0; i < ref_count; i++) {
 			get_fetch_map(remote_refs, &refs[i], &tail, 0);
 			if (refs[i].dst && refs[i].dst[0])
@@ -170,6 +172,20 @@ static struct ref *get_ref_map(struct transport *transport,
 			rm->fetch_head_status = FETCH_HEAD_MERGE;
 		if (tags == TAGS_SET)
 			get_fetch_map(remote_refs, tag_refspec, &tail, 0);
+
+		/*
+		 * For any refs that we happen to be fetching via command-line
+		 * arguments, take the opportunity to update their configured
+		 * counterparts. However, we do not want to mention these
+		 * entries in FETCH_HEAD at all, as they would simply be
+		 * duplicates of existing entries.
+		 */
+		old_tail = tail;
+		for (i = 0; i < transport->remote->fetch_refspec_nr; i++)
+			get_fetch_map(ref_map, &transport->remote->fetch[i],
+				      &tail, 0);
+		for (rm = *old_tail; rm; rm = rm->next)
+			rm->fetch_head_status = FETCH_HEAD_IGNORE;
 	} else {
 		/* Use the defaults */
 		struct remote *remote = transport->remote;
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 789c228..ff43e08 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -377,7 +377,7 @@ test_expect_success 'mark initial state of origin/master' '
 	)
 '
 
-test_expect_success 'explicit fetch should not update tracking' '
+test_expect_success 'explicit fetch should update tracking' '
 
 	cd "$D" &&
 	git branch -f side &&
@@ -387,12 +387,12 @@ test_expect_success 'explicit fetch should not update tracking' '
 		o=$(git rev-parse --verify refs/remotes/origin/master) &&
 		git fetch origin master &&
 		n=$(git rev-parse --verify refs/remotes/origin/master) &&
-		test "$o" = "$n" &&
+		test "$o" != "$n" &&
 		test_must_fail git rev-parse --verify refs/remotes/origin/side
 	)
 '
 
-test_expect_success 'explicit pull should not update tracking' '
+test_expect_success 'explicit pull should update tracking' '
 
 	cd "$D" &&
 	git branch -f side &&
@@ -402,7 +402,7 @@ test_expect_success 'explicit pull should not update tracking' '
 		o=$(git rev-parse --verify refs/remotes/origin/master) &&
 		git pull origin master &&
 		n=$(git rev-parse --verify refs/remotes/origin/master) &&
-		test "$o" = "$n" &&
+		test "$o" != "$n" &&
 		test_must_fail git rev-parse --verify refs/remotes/origin/side
 	)
 '
-- 
1.8.3.rc1.2.g12db477

Re: [PATCH 4/4] fetch: opportunistically update tracking refs

[Thomas Rast]

Jeff King <peff@peff.net> writes:

> When we run a regular "git fetch" without arguments, we
> update the tracking refs according to the configured
> refspec. However, when we run "git fetch origin master" (or
> "git pull origin master"), we do not look at the configured
> refspecs at all, and just update FETCH_HEAD.
>
> We miss an opportunity to update "refs/remotes/origin/master"
> (or whatever the user has configured). Some users find this
> confusing, because they would want to do further comparisons
> against the old state of the remote master, like:
>
>   $ git pull origin master
>   $ git log HEAD...origin/master

I agree with the patch, but I would use a different reasoning. Your
example here is not even correct because the range in the second command
would be empty unless the merge conflicted.

I believe the more common confusion stems from the tracking displays in
git-checkout and others.  The issue there is that we *internally* treat
origin/masters as a kind of mirror, and the tracking displays all go
against user expectations once they have seen a newer state of origin's
master by explicit pull or fetch.

Michael also pointed out yesterday that short of

  git fetch origin master:refs/remotes/origin/master

there is no way to fetch only one branch.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH 4/4] fetch: opportunistically update tracking refs

[Thomas Rast]

Thomas Rast <trast@inf.ethz.ch> writes:

> Jeff King <peff@peff.net> writes:
>
>> When we run a regular "git fetch" without arguments, we
>> update the tracking refs according to the configured
>> refspec. However, when we run "git fetch origin master" (or
>> "git pull origin master"), we do not look at the configured
>> refspecs at all, and just update FETCH_HEAD.
>>
>> We miss an opportunity to update "refs/remotes/origin/master"
>> (or whatever the user has configured). Some users find this
>> confusing, because they would want to do further comparisons
>> against the old state of the remote master, like:
>>
>>   $ git pull origin master
>>   $ git log HEAD...origin/master
>
> I agree with the patch, but I would use a different reasoning. Your
> example here is not even correct because the range in the second command
> would be empty unless the merge conflicted.

Meh, I just read this again and saw that you actually had *three* dots.
Serves me right for writing a reply on the phone.

So the quoted part is indeed correct.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH 4/4] fetch: opportunistically update tracking refs

[Jeff King]

On Sun, May 12, 2013 at 11:41:45AM +0200, Thomas Rast wrote:

> >> We miss an opportunity to update "refs/remotes/origin/master"
> >> (or whatever the user has configured). Some users find this
> >> confusing, because they would want to do further comparisons
> >> against the old state of the remote master, like:
> >>
> >>   $ git pull origin master
> >>   $ git log HEAD...origin/master
> >
> > I agree with the patch, but I would use a different reasoning. Your
> > example here is not even correct because the range in the second command
> > would be empty unless the merge conflicted.
> 
> Meh, I just read this again and saw that you actually had *three* dots.
> Serves me right for writing a reply on the phone.
> 
> So the quoted part is indeed correct.

Yes, it's correct, but it is a bit subtle. A better example would
probably be:

  $ git status
  # On branch foo
  # Your branch is ahead of 'origin/master' by 2 commits.

  $ git pull origin master
  [pull 10 new commits]

  $ git status
  # On branch foo
  # Your branch is ahead of 'origin/master' by 13 commits.

That is technically true, but only because origin/master is now out of
date. The more interesting number is 3: the two commits we had already,
plus the new merge commit.

And of course there are infinite variations where you pull on one
branch, and then switch branches to compare on another. But they all
boil down to having an out-of-date view of origin/master that does not
reflect the most recent pull.

-Peff

Re: [PATCH 4/4] fetch: opportunistically update tracking refs

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> @@ -170,6 +172,20 @@ static struct ref *get_ref_map(struct transport *transport,
>  			rm->fetch_head_status = FETCH_HEAD_MERGE;
>  		if (tags == TAGS_SET)
>  			get_fetch_map(remote_refs, tag_refspec, &tail, 0);
> +
> +		/*
> +		 * For any refs that we happen to be fetching via command-line
> +		 * arguments, take the opportunity to update their configured
> +		 * counterparts. However, we do not want to mention these
> +		 * entries in FETCH_HEAD at all, as they would simply be
> +		 * duplicates of existing entries.
> +		 */
> +		old_tail = tail;
> +		for (i = 0; i < transport->remote->fetch_refspec_nr; i++)
> +			get_fetch_map(ref_map, &transport->remote->fetch[i],
> +				      &tail, 0);
> +		for (rm = *old_tail; rm; rm = rm->next)
> +			rm->fetch_head_status = FETCH_HEAD_IGNORE;

Was there a reason why this change was done by appending new ref at
the tail of the ref_map list?  I would have expected that a naïve
and obvious implementation would be to iterate existing refs over
ref_map to find refs with an empty RHS whose LHS is configured to
usually store the fetched result to somewhere and to update their
RHS in place.

Being curious.

Re: [PATCH 4/4] fetch: opportunistically update tracking refs

[Jeff King]

On Tue, Aug 06, 2013 at 09:28:28AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > @@ -170,6 +172,20 @@ static struct ref *get_ref_map(struct transport *transport,
> >  			rm->fetch_head_status = FETCH_HEAD_MERGE;
> >  		if (tags == TAGS_SET)
> >  			get_fetch_map(remote_refs, tag_refspec, &tail, 0);
> > +
> > +		/*
> > +		 * For any refs that we happen to be fetching via command-line
> > +		 * arguments, take the opportunity to update their configured
> > +		 * counterparts. However, we do not want to mention these
> > +		 * entries in FETCH_HEAD at all, as they would simply be
> > +		 * duplicates of existing entries.
> > +		 */
> > +		old_tail = tail;
> > +		for (i = 0; i < transport->remote->fetch_refspec_nr; i++)
> > +			get_fetch_map(ref_map, &transport->remote->fetch[i],
> > +				      &tail, 0);
> > +		for (rm = *old_tail; rm; rm = rm->next)
> > +			rm->fetch_head_status = FETCH_HEAD_IGNORE;
> 
> Was there a reason why this change was done by appending new ref at
> the tail of the ref_map list?  I would have expected that a naïve
> and obvious implementation would be to iterate existing refs over
> ref_map to find refs with an empty RHS whose LHS is configured to
> usually store the fetched result to somewhere and to update their
> RHS in place.
> 
> Being curious.

Two reasons:

  1. The implementation you suggest above behaves differently than the
     current code. It does not look for refspecs with an empty RHS. It
     looks for any LHS that matches our configured entries. So if you do
     "git fetch origin master:foobar", we will update both
     "refs/heads/foobar" as well as "refs/remotes/origin/master".

     That means it is purely an opportunistic "hey, during another
     operation we happened to find out something new about
     origin/master, so let's update our tracking branch". Whereas what
     you stated above is more like "we are fetching into FETCH_HEAD, so
     let's also update the tracking branch".

  2. The list of refs after get_ref_map is actually more of an
     instruction/command list for the rest of the code to follow. It
     gets fed to store_updated_ref, but also impacts the status table we
     show. You could implement it such that a single ref entry had
     multiple storage destinations, but that would require changes to
     all of the consumers of the instruction list. Since we already need
     to handle extra refspecs (e.g., you can say "master:foobar
     master:refs/remotes/origin/master" on the command line already), we
     can treat these the same way. We append to the instruction list,
     and the rest of the code treats it as if you specified it on the
     command line.

-Peff

Re: [PATCH 4/4] fetch: opportunistically update tracking refs

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Two reasons:

OK, both boils down to "Junio did not consider 'master:foobar'
case".

Thanks; it makes sense now.