Re: What's cooking in git.git (Apr 2018, #04; Mon, 30)

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 4253 bytes --]

Hi,

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

> On Mon, Apr 30, 2018 at 5:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
> > * 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
> 
> >   "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.
> 
> >   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 <johannes.schindelin@gmx.de>

--
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