git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: "SZEDER Gábor" <szeder.dev@gmail.com>
Cc: gitster@pobox.com, git@vger.kernel.org
Subject: Re: What's cooking in git.git (Apr 2018, #04; Mon, 30)
Date: Tue, 1 May 2018 15:46:58 +0200 (DST)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.1805011439580.79@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <CAM0VKjmkn7eyooKheOEQnS=6HMZSTbhejoxQdKB7W+n=7D5KuQ@mail.gmail.com>

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

  reply	other threads:[~2018-05-01 13:47 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-30  3:25 What's cooking in git.git (Apr 2018, #04; Mon, 30) Junio C Hamano
2018-04-30 15:33 ` SZEDER Gábor
2018-05-01 13:46   ` Johannes Schindelin [this message]
2018-05-02  3:59     ` Junio C Hamano
2018-05-02  6:53       ` Johannes Schindelin
2018-05-02 10:46         ` Junio C Hamano
2018-05-02  1:36   ` Junio C Hamano
2018-04-30 19:17 ` js/no-pager-shorthand [was: What's cooking in git.git (Apr 2018, #04; Mon, 30)] Johannes Sixt
2018-05-01 11:57   ` Johannes Schindelin
2018-05-01 14:58     ` Johannes Sixt
2018-05-01 18:10       ` Eric Sunshine
2018-05-02  1:43         ` js/no-pager-shorthand [ Junio C Hamano
2018-05-01 18:46     ` js/no-pager-shorthand [was: What's cooking in git.git (Apr 2018, #04; Mon, 30)] Ævar Arnfjörð Bjarmason
2018-05-02 23:17 ` What's cooking in git.git (Apr 2018, #04; Mon, 30) Brandon Williams
2018-05-04 18:04 ` Elijah Newren
2018-05-07  4:39   ` Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=nycvar.QRO.7.76.6.1805011439580.79@tvgsbejvaqbjf.bet \
    --to=johannes.schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=szeder.dev@gmail.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).