[PATCH] HEAD, ORIG_HEAD and FETCH_HEAD are really special.
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 7, 2007, 11:21 UTC
- Message-ID
- <7vd4wu67qs.fsf_-_@gitster.siamese.dyndns.org>
- In-Reply-To
- <1189133898.30308.58.camel@koto.keithp.com>
Keith Packard <keithp@keithp.com> writes:
Show 5 quoted lines
> 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 masterThe 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