[PATCH] HEAD, ORIG_HEAD and FETCH_HEAD are really special.

[Junio C Hamano <gitster@pobox.com>]

Keith Packard <keithp@keithp.com> writes:

> On Thu, 2007-09-06 at 16:26 -0700, Junio C Hamano wrote:
> ...
>> Perhaps you have ".git/master" by mistake?
>
> oops.

Ok, that explains it; git is doing exactly what the user asked
it to do.

              5---6 side
             /
	1---2---3---4 master

The user has this history.  But he has a stray .git/master file,
perhaps created by hand by mistake (it would be very interesting
to find how that file got there in the first place), that points
at commit "3".  From a side branch, he says "git rebase master".

The first parameter to "git rebase" in a single parameter form
is "the commit on which to replay the changes my current branch
has".  It is not limited to a branch name.  IOW, it can be an
arbitrary object name, and one of the rules to translate a user
string that is supposed to mean an arbitrary object name goes
through this table:

        "%s", "refs/%s", "refs/tags/%s", "refs/heads/%s",
        "refs/remotes/%s", "refs/remotes/%s/HEAD"

For each entry in the above table, "%s" part of the entry is
replaced by the user string, and resulting string is used to see
if there is such a file under .git/ directory, and the first
match is used (this explanation is simplifying things a bit).

This rule has been there almost from the beginning.  The first
entry allowed you to have a tag A and a branch A at the same
time, while giving you a way to disambiguate them by spelling
them out as "refs/tags/A" and "refs/heads/A", respectively.
Also the first rule covered special "ref" names such as HEAD and
ORIG_HEAD.

Alas, there is one drawback of this rule.  His .git/master hides
refs/heads/master.  So essentially his command line said "I've
built a few commits since I forked from the history that leads
to commit 3; I want to replay my changes on top of that commit".
He meant to say 4, but said 3, and as a unfortunate consequence,
commit 4 is lost.

The above explanation is solely for understanding the situation
and, not meant to defend the current behaviour.  I think the
current behaviour is inviting mistakes and confusion.

This patch brings in a new world order by introducing a backward
incompatible change.  When the string the user gave us does not
contain any slash, we do not apply the first entry (i.e.
directly underneath .git/ without any "refs/***") unless the
name consists solely of uppercase letters or an underscore,
thereby ignoring .git/master.  The ones we often use, such as
HEAD and ORIG_HEAD are not affected by this change.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 sha1_name.c                |   35 ++++++++++++++++++++++++++++++++++
 t/t3407-rebase-confused.sh |   45 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 0 deletions(-)
 create mode 100755 t/t3407-rebase-confused.sh

diff --git a/sha1_name.c b/sha1_name.c
index 2d727d5..1b980ca 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -239,6 +239,35 @@ static int ambiguous_path(const char *path, int len)
 	return slash;
 }
 
+static int confused_ref(const char *str)
+{
+	char ch;
+	int seen_non_uppercase = 0;
+
+	/*
+	 * People create .git/master by mistake using hand-rolled
+	 * scripts and confuse themselves utterly.  Make sure this
+	 * does not match such a path.
+	 */
+	while ((ch = *str++) != '\0') {
+		if (ch == '/')
+			return 0;
+		if (!((ch == '_') ||
+		      ('A' <= ch && ch <= 'Z') ||
+		      ('0' <= ch && ch <= '9')))
+			seen_non_uppercase = 1;
+	}
+
+	/*
+	 * str did not have any slash and we are checking when
+	 * it hangs directly underneath .git/; the only valid
+	 * cases we currently have are HEAD, ORIG_HEAD, MERGE_HEAD and
+	 * FETCH_HEAD.  If we saw any non uppercase, non underscore,
+	 * we should ignore this string.
+	 */
+	return seen_non_uppercase;
+}
+
 static const char *ref_fmt[] = {
 	"%.*s",
 	"refs/%.*s",
@@ -259,6 +288,9 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
 		unsigned char sha1_from_ref[20];
 		unsigned char *this_result;
 
+		if ((p == ref_fmt) && confused_ref(str))
+			continue;
+
 		this_result = refs_found ? sha1_from_ref : sha1;
 		r = resolve_ref(mkpath(*p, len, str), this_result, 1, NULL);
 		if (r) {
@@ -283,6 +315,9 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 		char path[PATH_MAX];
 		const char *ref, *it;
 
+		if (p == ref_fmt && confused_ref(str))
+			continue;
+
 		strcpy(path, mkpath(*p, len, str));
 		ref = resolve_ref(path, hash, 0, NULL);
 		if (!ref)
diff --git a/t/t3407-rebase-confused.sh b/t/t3407-rebase-confused.sh
new file mode 100755
index 0000000..5254da6
--- /dev/null
+++ b/t/t3407-rebase-confused.sh
@@ -0,0 +1,45 @@
+#!/bin/sh
+
+test_description="Keithp's .git/master problem
+
+              5---6 side
+             /
+	1---2---3---4 master
+
+"
+
+. ./test-lib.sh
+
+build () {
+	echo "$1" >"$1" && git add "$1" && test_tick && git commit -m "$1"
+}
+
+test_expect_success setup '
+
+	build one &&
+	build two &&
+	git branch side &&
+	build three &&
+	git tag anchor &&
+	build four &&
+	git-checkout side &&
+	build five &&
+	build six &&
+	git update-ref master anchor &&
+	git tag -d anchor
+
+'
+
+test_expect_success rebase '
+
+	git rebase master
+
+'
+
+test_expect_success 'everybody is still there' '
+
+	test 6 = $(git log --pretty=oneline HEAD | wc -l)
+
+'
+
+test_done
-- 
1.5.3.1.879.g4d83f