Hi,
On Mon, 30 Apr 2018, SZEDER Gábor wrote:
> On Mon, Apr 30, 2018 at 5:25 AM, Junio C Hamano <gitster@xxxxxxxxx> 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@xxxxxx>
--
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, ¤t_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
