All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v3 0/2] Make git rebase work with --rebase-merges and --exec
Date: Thu, 09 Aug 2018 02:41:08 -0700 (PDT)	[thread overview]
Message-ID: <pull.13.v3.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.13.v2.git.gitgitgadget@gmail.com>

It was reported via IRC that the exec lines are inserted in the wrong spots
when using --rebase-merges.

The reason is that we used a simple, incorrect implementation that happened
to work as long as the generated todo list only contains pick, fixup and 
squash commands. Which is not the case with--rebase-merges.

Fix this issue by using a correct implementation instead, that even takes
into account merge commands in the --rebase-merges mode.

Changes since v1:

 * Replaced the "look-ahead" design by a "keep looking" one: instead of
   having a nested loop that looks for the end of the fixup/squash chain, we
   continue the loop, delaying the insertion until we know where the
   fixup/squash chain ends, if any.

Johannes Schindelin (2):
  t3430: demonstrate what -r, --autosquash & --exec should do
  rebase --exec: make it work with --rebase-merges

 sequencer.c              | 42 +++++++++++++++++++++++++++++-----------
 t/t3430-rebase-merges.sh | 17 ++++++++++++++++
 2 files changed, 48 insertions(+), 11 deletions(-)


base-commit: 1d89318c48d233d52f1db230cf622935ac3c69fa
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-13%2Fdscho%2Frebase-merges-and-exec-commands-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-13/dscho/rebase-merges-and-exec-commands-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/13

Range-diff vs v2:

 1:  1d82eb450 = 1:  1d82eb450 t3430: demonstrate what -r, --autosquash & --exec should do
 2:  7ca441a89 ! 2:  b436f67ba rebase --exec: make it work with --rebase-merges
     @@ -22,6 +22,11 @@
          `pick` lines, skip any fixup/squash chains, and then insert the `exec`
          line. Lather, rinse, repeat.
      
     +    Note: we take pains to insert *before* comment lines whenever possible,
     +    as empty commits are represented by commented-out pick lines (and we
     +    want to insert a preceding pick's exec line *before* such a line, not
     +    afterward).
     +
          While at it, also add `exec` lines after `merge` commands, because they
          are similar in spirit to `pick` commands: they add new commits.
      
     @@ -81,9 +86,13 @@
      +			insert = i + 1;
       	}
       
     - 	/* append final <commands> */
     +-	/* append final <commands> */
      -	strbuf_add(buf, commands, commands_len);
     -+	if (insert >= 0 || !offset)
     ++	/* insert or append final <commands> */
     ++	if (insert >= 0 && insert < todo_list.nr)
     ++		strbuf_insert(buf, todo_list.items[insert].offset_in_buf +
     ++			      offset, commands, commands_len);
     ++	else if (insert >= 0 || !offset)
      +		strbuf_add(buf, commands, commands_len);
       
       	i = write_message(buf->buf, buf->len, todo_file, 0);

-- 
gitgitgadget

  parent reply	other threads:[~2018-08-09  9:41 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-03 17:42 [PATCH 0/2] Make git rebase work with --rebase-merges and --exec Johannes Schindelin via GitGitGadget
2018-08-03 17:42 ` [PATCH 1/2] t3430: demonstrate what -r, --autosquash & --exec should do Johannes Schindelin via GitGitGadget
2018-08-03 17:42 ` [PATCH 2/2] rebase --exec: make it work with --rebase-merges Johannes Schindelin via GitGitGadget
2018-08-03 18:28   ` Junio C Hamano
2018-08-06  9:34     ` Johannes Schindelin
2018-08-06  9:50     ` Johannes Schindelin
2018-08-06 15:12       ` Junio C Hamano
2018-08-03 20:26   ` Junio C Hamano
2018-08-06  9:36     ` Johannes Schindelin
2018-08-06  9:52 ` [PATCH v2 0/2] Make git rebase work with --rebase-merges and --exec Johannes Schindelin via GitGitGadget
2018-08-06  9:52   ` [PATCH v2 1/2] t3430: demonstrate what -r, --autosquash & --exec should do Johannes Schindelin via GitGitGadget
2018-08-06  9:52   ` [PATCH v2 2/2] rebase --exec: make it work with --rebase-merges Johannes Schindelin via GitGitGadget
2018-08-06 15:23     ` Phillip Wood
2018-08-06 16:00       ` Phillip Wood
2018-08-09  9:22       ` Johannes Schindelin
2018-08-09 10:04         ` Phillip Wood
2018-08-09 13:30           ` Johannes Schindelin
2018-08-06 10:08   ` [PATCH v2 0/2] Make git rebase work with --rebase-merges and --exec Johannes Schindelin
2018-08-09  9:41   ` Johannes Schindelin via GitGitGadget [this message]
2018-08-09  9:41     ` [PATCH v3 1/2] t3430: demonstrate what -r, --autosquash & --exec should do Johannes Schindelin via GitGitGadget
2018-08-09  9:41     ` [PATCH v3 2/2] rebase --exec: make it work with --rebase-merges Johannes Schindelin via GitGitGadget

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=pull.13.v3.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.