Git without morning coffee

Git without morning coffee

[Michael J Gruber]

Is this my lack of morning coffee, or git's:

git rev-parse origin/next origin/pu
7931f38a2fd882b0f75a4d6f0eb60c3b1b094178
edf014b39ded8ee8a67f2a0cd8d7926b33f3d578
# OK

git checkout origin/next
HEAD is now at 7931f38... Merge branch 'master' into next
# OK

git log --oneline --no-walk ":/Merge branch 'jk/generation-numbers' into pu"
0ac76f9 Merge branch 'jk/generation-numbers' into pu
# Sure it is.

git merge ":/Merge branch 'jk/generation-numbers' into pu"
fatal: ':/Merge branch 'jk/generation-numbers' into pu' does not point
to a commit
# Huh?

git rev-parse ":/Merge branch 'jk/generation-numbers' into pu"
0ac76f96a94a46bce7ec9dcc6c67abcf9d36adac
# It does point to a commit!

git merge $(git rev-parse ":/Merge branch 'jk/generation-numbers' into pu")
error: addinfo_cache failed for path 't/t7810-grep.sh'
Performing inexact rename detection: 100% (91476/91476), done.
error: addinfo_cache failed for path 't/t7810-grep.sh'
error: addinfo_cache failed for path 'gitweb/static/gitweb.js'
error: addinfo_cache failed for path 'gitweb/static/gitweb.css'

## hundreds more

error: addinfo_cache failed for path 't/t7810-grep.sh'
error: addinfo_cache failed for path 'gitweb/static/js/blame_incremental.js'
Auto-merging cache.h
Merge made by the 'recursive' strategy.


Double Huh? Finally, the merge succeeds, despite thy myriad of errors.

I mean, I'm merging a commit from origin/pu to origin/next when the
latter is basically contained in the former (except for some merge
commits). And what are these addinfo_cache errors?

git --version
git version 1.7.7.rc0.328.g9d6c7

which is last week's next with a few additions which I've been carrying
along for weeks. Stumped.

Michael

Re: Git without morning coffee

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> git merge ":/Merge branch 'jk/generation-numbers' into pu"
> fatal: ':/Merge branch 'jk/generation-numbers' into pu' does not point
> to a commit
> # Huh?

Interesting.

> git merge $(git rev-parse ":/Merge branch 'jk/generation-numbers' into pu")
> error: addinfo_cache failed for path 't/t7810-grep.sh'
> Performing inexact rename detection: 100% (91476/91476), done.
> error: addinfo_cache failed for path 't/t7810-grep.sh'

Smells like another case where merge-recursive is looking at the work tree
when it shouldn't inside an inner merge or something.

> I mean, I'm merging a commit from origin/pu to origin/next when the
> latter is basically contained in the former (except for some merge
> commits).

This falls into the "side note" category, but these days 'next' and 'pu'
do not share any history beyond the tip of 'master'. Every time I update
the 'pu' branch, it is rebuilt directly on top of 'master' from scratch by
merging the topics in 'next' (and at this point I make sure its tree
matches that of 'next') and then merging remaining topics on top of the
result. A topic often goes through multiple iterations of fix-ups while in
'next', and these fix-ups result in multiple incremental merges of the
same topic into 'next'; I do not want to see an incorrect merge when such
a topic is merged in a single-step into 'master', and it is one way to
ensure the health of the merge fixup machinery (including the rerere
database) to attempt from-scratch-the-whole-topic-at-once merges and
verify the result.

The merge you attempted will have a lot of "the history leading to the
current commit added/modified in a certain way and the history being
merged did the same modification independently" kind of conflicts that
should resolve to no-op.

Re: Git without morning coffee

[Michael Witten]

[Sorry for the repeat, Junio]

On Wed, Sep 7, 2011 at 16:46, Junio C Hamano <gitster@pobox.com> wrote:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> git merge ":/Merge branch 'jk/generation-numbers' into pu"
>> fatal: ':/Merge branch 'jk/generation-numbers' into pu' does not point
>> to a commit
>> # Huh?
>
> Interesting.
>
>> git merge $(git rev-parse ":/Merge branch 'jk/generation-numbers' into pu")
>> error: addinfo_cache failed for path 't/t7810-grep.sh'
>> Performing inexact rename detection: 100% (91476/91476), done.
>> error: addinfo_cache failed for path 't/t7810-grep.sh'
>
> Smells like another case where merge-recursive is looking at the work tree
> when it shouldn't inside an inner merge or something.
>
>> I mean, I'm merging a commit from origin/pu to origin/next when the
>> latter is basically contained in the former (except for some merge
>> commits).
>
> This falls into the "side note" category, but these days 'next' and 'pu'
> do not share any history beyond the tip of 'master'. Every time I update
> the 'pu' branch, it is rebuilt directly on top of 'master' from scratch by
> merging the topics in 'next' (and at this point I make sure its tree
> matches that of 'next') and then merging remaining topics on top of the
> result. A topic often goes through multiple iterations of fix-ups while in
> 'next', and these fix-ups result in multiple incremental merges of the
> same topic into 'next'; I do not want to see an incorrect merge when such
> a topic is merged in a single-step into 'master', and it is one way to
> ensure the health of the merge fixup machinery (including the rerere
> database) to attempt from-scratch-the-whole-topic-at-once merges and
> verify the result.
>
> The merge you attempted will have a lot of "the history leading to the
> current commit added/modified in a certain way and the history being
> merged did the same modification independently" kind of conflicts that
> should resolve to no-op.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

I think it would be great if at some point you could write a detailed
tutorial of how you maintain git (using example topics, example patch
series and pull requests, and follow-along command sequences, etc.).
Most importantly, it would be nice to see an explicit description of
the maintenance properties you are trying to achieve with your
workflow.

Re: Git without morning coffee

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

> I think it would be great if at some point you could write a detailed
> tutorial of how you maintain git...

Is MaintNotes[*1*] taken together with Documentation/howto/maintain-git.txt
insufficient?

[Reference]
*1* http://git-blame.blogspot.com/p/note-from-maintainer.html

Re: Git without morning coffee

[Michael Witten]

On Wed, Sep 7, 2011 at 17:49, Junio C Hamano <gitster@pobox.com> wrote:
> Michael Witten <mfwitten@gmail.com> writes:
>
>> I think it would be great if at some point you could write a detailed
>> tutorial of how you maintain git...
>
> Is MaintNotes[*1*] taken together with Documentation/howto/maintain-git.txt
> insufficient?
>
> [Reference]
> *1* http://git-blame.blogspot.com/p/note-from-maintainer.html

I was unaware of the `howto'; I look forward to reading it.

Re: Git without morning coffee

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> git merge ":/Merge branch 'jk/generation-numbers' into pu"
>> fatal: ':/Merge branch 'jk/generation-numbers' into pu' does not point
>> to a commit
>> # Huh?
>
> Interesting.

This is because 1c7b76b (Build in merge, 2008-07-07) grabs the name of the
commit to be merged using peel_to_type(), which was defined in 8177631
(expose a helper function peel_to_type()., 2007-12-24) in terms of
get_sha1_1(). It understands $commit~$n, $commit^$n and $ref@{$nth}, but
does not understand :/$str, $treeish:$path, and :$stage:$path.

Re* Git without morning coffee

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Michael J Gruber <git@drmicha.warpmail.net> writes:
>>
>>> git merge ":/Merge branch 'jk/generation-numbers' into pu"
>>> fatal: ':/Merge branch 'jk/generation-numbers' into pu' does not point
>>> to a commit
>>> # Huh?
>>
>> Interesting.
>
> This is because 1c7b76b (Build in merge, 2008-07-07) grabs the name of the
> commit to be merged using peel_to_type(), which was defined in 8177631
> (expose a helper function peel_to_type()., 2007-12-24) in terms of
> get_sha1_1(). It understands $commit~$n, $commit^$n and $ref@{$nth}, but
> does not understand :/$str, $treeish:$path, and :$stage:$path.

-- >8 --
Subject: Allow git merge ":/<pattern>"

It probably is not such a good idea to use ":/<pattern>" to specify which
commit to merge, as ":/<pattern>" can often hit unexpected commits, but
somebody tried it and got a nonsense error message:

	fatal: ':/Foo bar' does not point to a commit

So here is a for-the-sake-of-consistency update that is fairly useless
that allows users to carefully try not shooting in the foot.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c |   19 ++++++++++++++-----
 sha1_name.c     |    6 ------
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index ab4077f..ee56974 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -403,6 +403,16 @@ static void finish(const unsigned char *new_head, const char *msg)
 	strbuf_release(&reflog_message);
 }
 
+static struct object *want_commit(const char *name)
+{
+	struct object *obj;
+	unsigned char sha1[20];
+	if (get_sha1(name, sha1))
+		return NULL;
+	obj = parse_object(sha1);
+	return peel_to_type(name, 0, obj, OBJ_COMMIT);
+}
+
 /* Get the name for the merge commit's message. */
 static void merge_name(const char *remote, struct strbuf *msg)
 {
@@ -418,7 +428,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
 	remote = bname.buf;
 
 	memset(branch_head, 0, sizeof(branch_head));
-	remote_head = peel_to_type(remote, 0, NULL, OBJ_COMMIT);
+	remote_head = want_commit(remote);
 	if (!remote_head)
 		die(_("'%s' does not point to a commit"), remote);
 
@@ -1124,7 +1134,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		if (!allow_fast_forward)
 			die(_("Non-fast-forward commit does not make sense into "
 			    "an empty head"));
-		remote_head = peel_to_type(argv[0], 0, NULL, OBJ_COMMIT);
+		remote_head = want_commit(argv[0]);
 		if (!remote_head)
 			die(_("%s - not something we can merge"), argv[0]);
 		read_empty(remote_head->sha1, 0);
@@ -1170,7 +1180,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		struct object *o;
 		struct commit *commit;
 
-		o = peel_to_type(argv[i], 0, NULL, OBJ_COMMIT);
+		o = want_commit(argv[i]);
 		if (!o)
 			die(_("%s - not something we can merge"), argv[i]);
 		commit = lookup_commit(o->sha1);
@@ -1238,8 +1248,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		if (have_message)
 			strbuf_addstr(&msg,
 				" (no commit created; -m option ignored)");
-		o = peel_to_type(sha1_to_hex(remoteheads->item->object.sha1),
-			0, NULL, OBJ_COMMIT);
+		o = want_commit(sha1_to_hex(remoteheads->item->object.sha1));
 		if (!o)
 			return 1;
 
diff --git a/sha1_name.c b/sha1_name.c
index ff5992a..653b065 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -501,12 +501,6 @@ struct object *peel_to_type(const char *name, int namelen,
 {
 	if (name && !namelen)
 		namelen = strlen(name);
-	if (!o) {
-		unsigned char sha1[20];
-		if (get_sha1_1(name, namelen, sha1))
-			return NULL;
-		o = parse_object(sha1);
-	}
 	while (1) {
 		if (!o || (!o->parsed && !parse_object(o->sha1)))
 			return NULL;

Re: Re* Git without morning coffee

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 07.09.2011 20:16:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Michael J Gruber <git@drmicha.warpmail.net> writes:
>>>
>>>> git merge ":/Merge branch 'jk/generation-numbers' into pu"
>>>> fatal: ':/Merge branch 'jk/generation-numbers' into pu' does not point
>>>> to a commit
>>>> # Huh?
>>>
>>> Interesting.
>>
>> This is because 1c7b76b (Build in merge, 2008-07-07) grabs the name of the
>> commit to be merged using peel_to_type(), which was defined in 8177631
>> (expose a helper function peel_to_type()., 2007-12-24) in terms of
>> get_sha1_1(). It understands $commit~$n, $commit^$n and $ref@{$nth}, but
>> does not understand :/$str, $treeish:$path, and :$stage:$path.
> 
> -- >8 --
> Subject: Allow git merge ":/<pattern>"
> 
> It probably is not such a good idea to use ":/<pattern>" to specify which
> commit to merge, as ":/<pattern>" can often hit unexpected commits, but
> somebody tried it and got a nonsense error message:
> 
> 	fatal: ':/Foo bar' does not point to a commit
> 
> So here is a for-the-sake-of-consistency update that is fairly useless
> that allows users to carefully try not shooting in the foot.

Shooting in the foot can be a good thing, depending on the feet...

My concern is: If a command expects a commit(tish), a user expects to be
able to specify it in any way which git rev-parse can parse. I had no
idea we distinguish even further then what the command itself requires
("branch" needs a branch refname, diff-tree a treeish etc.).

So, for systematic reasons I think the below is an improvement.

> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  builtin/merge.c |   19 ++++++++++++++-----
>  sha1_name.c     |    6 ------
>  2 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/builtin/merge.c b/builtin/merge.c
> index ab4077f..ee56974 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -403,6 +403,16 @@ static void finish(const unsigned char *new_head, const char *msg)
>  	strbuf_release(&reflog_message);
>  }
>  
> +static struct object *want_commit(const char *name)
> +{
> +	struct object *obj;
> +	unsigned char sha1[20];
> +	if (get_sha1(name, sha1))
> +		return NULL;
> +	obj = parse_object(sha1);
> +	return peel_to_type(name, 0, obj, OBJ_COMMIT);
> +}
> +
>  /* Get the name for the merge commit's message. */
>  static void merge_name(const char *remote, struct strbuf *msg)
>  {
> @@ -418,7 +428,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
>  	remote = bname.buf;
>  
>  	memset(branch_head, 0, sizeof(branch_head));
> -	remote_head = peel_to_type(remote, 0, NULL, OBJ_COMMIT);
> +	remote_head = want_commit(remote);
>  	if (!remote_head)
>  		die(_("'%s' does not point to a commit"), remote);
>  
> @@ -1124,7 +1134,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  		if (!allow_fast_forward)
>  			die(_("Non-fast-forward commit does not make sense into "
>  			    "an empty head"));
> -		remote_head = peel_to_type(argv[0], 0, NULL, OBJ_COMMIT);
> +		remote_head = want_commit(argv[0]);
>  		if (!remote_head)
>  			die(_("%s - not something we can merge"), argv[0]);
>  		read_empty(remote_head->sha1, 0);
> @@ -1170,7 +1180,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  		struct object *o;
>  		struct commit *commit;
>  
> -		o = peel_to_type(argv[i], 0, NULL, OBJ_COMMIT);
> +		o = want_commit(argv[i]);
>  		if (!o)
>  			die(_("%s - not something we can merge"), argv[i]);
>  		commit = lookup_commit(o->sha1);
> @@ -1238,8 +1248,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  		if (have_message)
>  			strbuf_addstr(&msg,
>  				" (no commit created; -m option ignored)");
> -		o = peel_to_type(sha1_to_hex(remoteheads->item->object.sha1),
> -			0, NULL, OBJ_COMMIT);
> +		o = want_commit(sha1_to_hex(remoteheads->item->object.sha1));
>  		if (!o)
>  			return 1;
>  
> diff --git a/sha1_name.c b/sha1_name.c
> index ff5992a..653b065 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -501,12 +501,6 @@ struct object *peel_to_type(const char *name, int namelen,
>  {
>  	if (name && !namelen)
>  		namelen = strlen(name);
> -	if (!o) {
> -		unsigned char sha1[20];
> -		if (get_sha1_1(name, namelen, sha1))
> -			return NULL;
> -		o = parse_object(sha1);
> -	}
>  	while (1) {
>  		if (!o || (!o->parsed && !parse_object(o->sha1)))
>  			return NULL;