Hi,

On Mon, 30 Apr 2018, SZEDER Gábor wrote:

On Mon, Apr 30, 2018 at 5:25 AM, Junio C Hamano [off-list ref] wrote:

quoted

* js/rebase-i-clean-msg-after-fixup-continue (2018-04-30) 4 commits
  - rebase --skip: clean up commit message after a failed fixup/squash
  - sequencer: always commit without editing when asked for
  - rebase -i: Handle "combination of <n> commits" with GETTEXT_POISON
  - rebase -i: demonstrate bugs with fixup!/squash! commit messages

quoted

  "git rebase -i" sometimes left intermediate "# This is a
  combination of N commits" message meant for the human consumption
  inside an editor in the final result in certain corner cases, which
  has been fixed.

quoted

  Will merge to 'next'.

This topic branches off from v2.16.3.  However, its last patch uses
the sequencer's parse_head() function, which was only added in
v2.17.0-rc0~110^2~6 (sequencer: try to commit without forking 'git
commit', 2017-11-24), in topic 'pw/sequencer-in-process-commit',
leading to compilation errors.

Great find.

As luck has it, I recently played with tbdiff and compared what Junio
applied vs what I have, and found only context line changes (and Junio's
extra Signed-off-by: lines).

So the problem you found is not a problem with *my* branch, of course, as
I did not fork off of v2.16.3 (which is a bit arbitrary, I must say: if
you want to fix these bugs for reals, you'd have to go all the way back to
where fixup!/squash!  support was introduced).

However, as a maintainer I am sympathetic to the goal of wanting to have
*one* branch that does not need to be backported.

The patch to make all of this work is most likely this:

-- snip --
sequencer: backport parse_head()

This function exists in v2.17.0, and will be used in the upcoming fixes
for bugs when running `git rebase --skip` after a fixup/squash failed.

Signed-off-by: Johannes Schindelin <redacted>

--

diff --git a/sequencer.c b/sequencer.c
index a766796d1a7..f9c478a4d79 100644
--- a/sequencer.c
+++ b/sequencer.c

@@ -695,6 +695,29 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,
 	return run_command(&cmd);
 }
 
+static int parse_head(struct commit **head)
+{
+	struct commit *current_head;
+	struct object_id oid;
+
+	if (get_oid("HEAD", &oid)) {
+		current_head = NULL;
+	} else {
+		current_head = lookup_commit_reference(&oid);
+		if (!current_head)
+			return error(_("could not parse HEAD"));
+		if (oidcmp(&oid, &current_head->object.oid)) {
+			warning(_("HEAD %s is not a commit!"),
+				oid_to_hex(&oid));
+		}
+		if (parse_commit(current_head))
+			return error(_("could not parse HEAD commit"));
+	}
+	*head = current_head;
+
+	return 0;
+}
+
 static int is_original_commit_empty(struct commit *commit)
 {
 	const struct object_id *ptree_oid;
-- snap --

I also had to apply this to make things compile with DEVELOPER=1:

-- snip --

diff --git a/sequencer.c b/sequencer.c
index f9c478a4d79..44b1874f459 100644
--- a/sequencer.c
+++ b/sequencer.c

@@ -2345,7 +2345,7 @@ static int commit_staged_changes(struct replay_opts *opts,
 				 * We need to update the squash message to skip
 				 * the latest commit message.
 				 */
-				struct commit *commit;
+				struct commit *commit = NULL;
 				const char *path = rebase_path_squash_msg();
 
 				if (parse_head(&commit) ||
-- snap --

This should most likely be squashed into "rebase --skip: clean up commit
message after a failed fixup/squash".

Junio, for your convenience, I pushed what I have here to the
`clean-msg-after-fixup-continue-backport-v2.16.3` branch on
https://github.com/dscho/git

For shiggles, I also looked how far back I could push this, and backported
to v2.15.1, v2.14.3, v2.13.6, v2.12.5

Things got painful only really with v2.12.5. Like, really painful. And in
the end not even worth it... I managed to backport the changes, sure, but
that code path is not even used ;-)

For your convenience, I pushed all of the backports, just in case you want
to use them. I did leave a couple of fixup! commits in place, to show a
*little* better what I did and where.

Ciao,
Dscho