git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [BUG] "git pull" will regress between 'master' and 'pu'
Date: Sun, 19 Apr 2015 09:07:46 -0400	[thread overview]
Message-ID: <20150419130745.GA20476@peff.net> (raw)
In-Reply-To: <xmqqbnikoq0n.fsf@gitster.dls.corp.google.com>

On Sat, Apr 18, 2015 at 06:39:20PM -0700, Junio C Hamano wrote:

> This is primarily note-to-self; even though I haven't got around
> bisecting yet, I think I know I did some bad change myself.
> 
> "git pull $URL $tag" seems to:
> 
>  * fail to invoke the editor without "--edit".
>  * show the summary merge log message twice.

I think that "git merge -m $msg $commit" is not quite the same as "git
merge $msg HEAD $commit".

The former suppresses the editor. This makes sense, as it's consistent
with "commit -m", etc. You can override that with "--edit".

But the latter also respects merge.log. Which also makes sense, because
the contents of "-m" are generally user-created, not the output of
fmt-merge-msg. So it is merge's job to format the content appropriately.

It is tempting to "solve" both by dropping the call to git-fmt-merge-msg
right before calling git-merge. Then "git-merge" will call fmt-merge-msg
itself. However, we feed it only the sha1 of the merge-head, whereas our
fmt-merge-msg call gets to see FETCH_HEAD. So we would lose the nice
"Merge tag 'v1.0' of git://..." part of the message. Instead we get
"Merge tag '1234abcd'".

So this is _almost_ enough:

diff --git a/git-pull.sh b/git-pull.sh
index 252969e..63493ee 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -323,7 +323,7 @@ then
 	fi
 fi
 
-merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
+merge_name=$(git fmt-merge-msg --no-log <"$GIT_DIR/FETCH_HEAD") || exit
 case "$rebase" in
 true)
 	eval="git-rebase $diffstat $strategy_args $merge_args $rebase_args $verbosity"
@@ -334,7 +334,7 @@ true)
 	eval="git-merge $diffstat $no_commit $verify_signatures $edit $squash $no_ff $ff_only"
 	eval="$eval $log_arg $strategy_args $merge_args $verbosity $progress"
 	eval="$eval $gpg_sign_args"
-	eval="$eval -m \"\$merge_name\" $merge_head"
+	eval="$eval --edit -m \"\$merge_name\" $merge_head"
 	;;
 esac
 eval "exec $eval"

But that is starting to feel pretty hacky.  Moreover, both fmt-merge-msg
and "merge -m" will verify the tag signature and output the tag message.
I don't see a way to suppress that. So it really would be nice to be
able to just drop the extra fmt-merge-msg call and have git-merge do it
all internally, which would mean telling it to use FETCH_HEAD and not
$merge_name.

Which I guess is just:

diff --git a/git-pull.sh b/git-pull.sh
index 252969e..15d9431 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -323,7 +323,6 @@ then
 	fi
 fi
 
-merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
 case "$rebase" in
 true)
 	eval="git-rebase $diffstat $strategy_args $merge_args $rebase_args $verbosity"
@@ -334,7 +333,7 @@ true)
 	eval="git-merge $diffstat $no_commit $verify_signatures $edit $squash $no_ff $ff_only"
 	eval="$eval $log_arg $strategy_args $merge_args $verbosity $progress"
 	eval="$eval $gpg_sign_args"
-	eval="$eval -m \"\$merge_name\" $merge_head"
+	eval="$eval FETCH_HEAD"
 	;;
 esac
 eval "exec $eval"

as we seem to special-case the name FETCH_HEAD. It assumes that
git-merge's parsing of FETCH_HEAD is the same as what we do in git-pull,
but that seems safe. Unfortunately we still have to compute $merge_head
ourselves here for the "git pull --rebase" case.

-Peff

  reply	other threads:[~2015-04-19 13:08 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-19  1:39 [BUG] "git pull" will regress between 'master' and 'pu' Junio C Hamano
2015-04-19 13:07 ` Jeff King [this message]
2015-04-19 17:38   ` brian m. carlson
2015-04-19 18:19     ` Jeff King
2015-04-20 18:59   ` Junio C Hamano
2015-04-20 19:10     ` Jeff King
2015-04-20 19:24       ` Junio C Hamano
2015-04-26  5:25         ` [PATCH 00/14] Teach "git merge FETCH_HEAD" octopus merges Junio C Hamano
2015-04-26  5:25           ` [PATCH 01/14] merge: simplify code flow Junio C Hamano
2015-04-26  5:25           ` [PATCH 02/14] t5520: style fixes Junio C Hamano
2015-04-26  5:25           ` [PATCH 03/14] t5520: test pulling an octopus into an unborn branch Junio C Hamano
2015-04-26  5:25           ` [PATCH 04/14] merge: clarify "pulling into void" special case Junio C Hamano
2015-04-26  5:25           ` [PATCH 05/14] merge: do not check argc to determine number of remote heads Junio C Hamano
2015-04-26  5:25           ` [PATCH 06/14] merge: small leakfix and code simplification Junio C Hamano
2015-04-26  5:26           ` [PATCH 07/14] merge: clarify collect_parents() logic Junio C Hamano
2015-04-26  5:26           ` [PATCH 08/14] merge: split reduce_parents() out of collect_parents() Junio C Hamano
2015-04-26  5:26           ` [PATCH 09/14] merge: narrow scope of merge_names Junio C Hamano
2015-04-26  5:26           ` [PATCH 10/14] merge: extract prepare_merge_message() logic out Junio C Hamano
2015-04-26  5:26           ` [PATCH 11/14] merge: make collect_parents() auto-generate the merge message Junio C Hamano
2015-04-26  5:26           ` [PATCH 12/14] merge: decide if we auto-generate the message early in collect_parents() Junio C Hamano
2015-04-26  5:26           ` [PATCH 13/14] merge: handle FETCH_HEAD internally Junio C Hamano
2015-04-26  5:26           ` [PATCH 14/14] merge: deprecate 'git merge <message> HEAD <commit>' syntax Junio C Hamano
2015-04-29 21:29           ` [PATCH v2 00/15] Teach "git merge FETCH_HEAD" octopus merges Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 01/15] merge: test the top-level merge driver Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 02/15] merge: simplify code flow Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 03/15] t5520: style fixes Junio C Hamano
2015-05-01  8:35               ` Paul Tan
2015-05-03  1:57                 ` Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 04/15] t5520: test pulling an octopus into an unborn branch Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 05/15] merge: clarify "pulling into void" special case Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 06/15] merge: do not check argc to determine number of remote heads Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 07/15] merge: small leakfix and code simplification Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 08/15] merge: clarify collect_parents() logic Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 09/15] merge: split reduce_parents() out of collect_parents() Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 10/15] merge: narrow scope of merge_names Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 11/15] merge: extract prepare_merge_message() logic out Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 12/15] merge: make collect_parents() auto-generate the merge message Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 13/15] merge: decide if we auto-generate the message early in collect_parents() Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 14/15] merge: handle FETCH_HEAD internally Junio C Hamano
2015-04-29 21:29             ` [PATCH v2 15/15] merge: deprecate 'git merge <message> HEAD <commit>' syntax Junio C Hamano
2015-04-20 19:28 ` [BUG] "git pull" will regress between 'master' and 'pu' Junio C Hamano
2015-04-21  7:23   ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150419130745.GA20476@peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).