FETCH_HEAD documentation vs reality

[Joey Hess <joey@kitenet.net>]

[-- Attachment #1.1: Type: text/plain, Size: 2428 bytes --]

While trying to find some documentation of the format of .git/FETCH_HEAD,
I found this example in git-read-tree.txt, which I think will no longer
work. Probably when this was written, .git/FETCH_HEAD contained only a single
SHA; it's much more complicated now.

        $ JC=`git rev-parse --verify "HEAD^0"`
        $ git checkout-index -f -u -a $JC
        ...
        $ git fetch git://.... linus
        $ LT=`cat .git/FETCH_HEAD`
        ...
        $ git read-tree -m -u `git merge-base $JC $LT` $JC $LT

It's also common for the first line of .git/FETCH_HEAD to be an
arbitrary branch that was fetched (as part of an unqualified "git
pull"), marked not-for-merge. So using "FETCH_HEAD" as a refname will
refer to such a branch unintentionally. There are several places in the
docs that seem to expect FETCH_HEAD to always refer to the one that was
fetched and will be merged (ie, master):

revisions.txt:

	'FETCH_HEAD' records the branch which you fetched from a remote repository
	with your last `git fetch` invocation.

git-pull.txt:

	In its default mode, `git pull` is shorthand for
	`git fetch` followed by `git merge FETCH_HEAD`.

gittutorial.txt:

	alice$ git log -p HEAD..FETCH_HEAD
	$ gitk HEAD..FETCH_HEAD

howto/rebase-from-internal-branch.txt:

	You fetch from upstream, but not merge.
	
	    $ git fetch upstream
	
	This leaves the updated upstream head in .git/FETCH_HEAD but
	does not touch your .git/HEAD nor .git/refs/heads/master.
	You run "git rebase" now.
	
	    $ git rebase FETCH_HEAD master

All this documentation could be changed, or resolve_ref_unsafe in refs.c
could be changed to have a special case parser for .git/FETCH_HEAD,
that finds the first branch that is marked for merge, where it now has
this minor special case for it:

        /* Please note that FETCH_HEAD has a second line containing other data. */
        if (get_sha1_hex(buffer, sha1) || (buffer[40] != '\0' && !isspace(buffer[40]))) {

Or yet another way to fix it would be to make git fetch always write the
intended FETCH_HEAD first into .git/FETCH_HEAD. (When not in --append mode.)
This seems like perhaps the best fix, although it does mean that if a
fetch is done of only not-for-merge refs, without --append, FETCH_HEAD
will still refer to one of them. 

I've attached a minimal proof-of-concept patch implementing this last
option.

-- 
see shy jo

[-- Attachment #1.2: patch --]
[-- Type: text/plain, Size: 1221 bytes --]

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 33ad3aa..e2f2c69 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -376,6 +376,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 	struct strbuf note = STRBUF_INIT;
 	const char *what, *kind;
 	struct ref *rm;
+	int top = 1;
 	char *url, *filename = dry_run ? "/dev/null" : git_path("FETCH_HEAD");
 
 	fp = fopen(filename, "a");
@@ -393,6 +394,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 		goto abort;
 	}
 
+ write:
 	for (rm = ref_map; rm; rm = rm->next) {
 		struct ref *ref = NULL;
 
@@ -408,6 +410,9 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 		if (!commit)
 			rm->merge = 0;
 
+		if (top != rm->merge)
+			continue;
+
 		if (!strcmp(rm->name, "HEAD")) {
 			kind = "";
 			what = "";
@@ -474,6 +479,11 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 		}
 	}
 
+	if (top) {
+		top = 0;
+		goto write;
+	}
+
 	if (rc & STORE_REF_ERROR_DF_CONFLICT)
 		error(_("some local refs could not be updated; try running\n"
 		      " 'git remote prune %s' to remove any old, conflicting "

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]