[PATCH v6 00/15] rebase -i: offer to recreate commit topology
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Apr 10, 2018, 12:29 UTC
- Message-ID
- <cover.1523362469.git.johannes.schindelin@gmx.de>
- In-Reply-To
- <cover.1519680483.git.johannes.schindelin@gmx.de>
Once upon a time, I dreamt of an interactive rebase that would not flatten branch structure, but instead recreate the commit topology faithfully.
My original attempt was --preserve-merges, but that design was so limited that I did not even enable it in interactive mode.
Subsequently, it *was* enabled in interactive mode, with the predictable consequences: as the --preserve-merges design does not allow for specifying the parents of merge commits explicitly, all the new commits' parents are defined *implicitly* by the previous commit history, and hence it is *not possible to even reorder commits*.
This design flaw cannot be fixed. Not without a complete re-design, at least. This patch series offers such a re-design.
Think of --rebase-merges as "--preserve-merges done right". It introduces new verbs for the todo list, `label`, `reset` and `merge`. For a commit topology like this:
A - B - C
\ /
Dthe generated todo list would look like this:
# branch D
pick 0123 A
label branch-point
pick 1234 D
label D reset branch-point
pick 2345 B
merge -C 3456 D # CThere are more patches in the pipeline, based on this patch series, but left for later in the interest of reviewable patch series: one mini series to use the sequencer even for `git rebase -i --root`, and another one to add support for octopus merges to --rebase-merges. And then one to allow for rebasing merge commits in a smarter way (this one will need a bit more work, though, as it can result in very complicated, nested merge conflicts *very* easily).
Changes since v5 (sorry, this one is big, and so is the interdiff):
- rebased to `master`, resolving conflicts with `ws/rebase-p` and `pw/rebase-keep-empty-fixes` (these changes are not reflected in the interdiff because I still did not find a good way to represent such fixups).
- just like `git merge` refuses to merge ancestors of HEAD, so does now the todo command `merge`.
- `git remote -v`'s output now differs when pulling with --rebase-merges vs pulling with --interactive.
- the `merge` command now also gives rerere a chance (just like `pick` already does).
- simplified test for rebase-cousins (no need to run --rebase-merges interactively, so there is no need to override the editor either).
- fixed `safe_append()` to roll back the lock file even when *reading* failed.
- used `reflog_message()` in `do_reset()` rather than duplicating the logic.
- reworded misleading commit message talking about fast-forwarding merge commits, when we just fast-forward `merge` commands *to* those merge commits whenever possible.
- removed duplicate `if (can_fast_forward)` clause.
- stopped promising support for octopus merges in --make-script in this patch series (it will be added in a later patch series).
- fixed grammar error in the message of the commit adding support for post-rewrite hooks to handle commits processed via the `merge` command.
- folded TODO_MERGE_AND_EDIT into TODO_MERGE by using a new `flags` field.
- the code of `do_merge()` has been made more obvious by using a variable `oneline_offset` instead of the non-descriptive `p`.
- renamed the option to `rebase-merges`, in preparation for doing it smarter using Phillip Wood's strategy (this will be contributed in a follow-up patch series after two others that add support for octopus merges and for handling --root via the sequencer).
- included Phillip Wood's test for --keep-empty with the new mode (and folded in a fix into the code of the `merge` command).
- added -r as shortcut for --rebase-merges
- added an entire section about "REBASING MERGES" to git-rebase's man page.
Johannes Schindelin (13): sequencer: avoid using errno clobbered by rollback_lock_file() sequencer: make rearrange_squash() a bit more obvious sequencer: introduce new commands to reset the revision sequencer: introduce the `merge` command sequencer: fast-forward `merge` commands, if possible rebase-helper --make-script: introduce a flag to rebase merges rebase: introduce the --rebase-merges option sequencer: make refs generated by the `label` command worktree-local sequencer: handle post-rewrite for merge commands rebase --rebase-merges: avoid "empty merges" pull: accept --rebase=merges to recreate the branch topology rebase -i: introduce --rebase-merges=[no-]rebase-cousins rebase -i --rebase-merges: add a section to the man page
Phillip Wood (1): rebase --rebase-merges: add test for --keep-empty
Stefan Beller (1): git-rebase--interactive: clarify arguments
Documentation/config.txt | 8 + Documentation/git-pull.txt | 5 +- Documentation/git-rebase.txt | 140 ++++- builtin/pull.c | 14 +- builtin/rebase--helper.c | 13 +- builtin/remote.c | 18 +- contrib/completion/git-completion.bash | 4 +- git-rebase--interactive.sh | 22 +- git-rebase.sh | 16 + refs.c | 3 +- sequencer.c | 775 ++++++++++++++++++++++++- sequencer.h | 7 + t/t3421-rebase-topology-linear.sh | 1 + t/t3430-rebase-merges.sh | 211 +++++++ 14 files changed, 1203 insertions(+), 34 deletions(-) create mode 100755 t/t3430-rebase-merges.sh
base-commit: 0b0cc9f86731f894cff8dd25299a9b38c254569e Published-As: https://github.com/dscho/git/releases/tag/recreate-merges-v6 Fetch-It-Via: git fetch https://github.com/dscho/git recreate-merges-v6
Interdiff vs v5:
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 85dc3a0c429..45916ea8104 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1058,7 +1058,7 @@ branch.<name>.rebase::
"git pull" is run. See "pull.rebase" for doing this in a non
branch-specific manner.
+
-When recreate, also pass `--recreate-merges` along to 'git rebase'
+When `merges`, pass the `--rebase-merges` option to 'git rebase'
so that locally committed merge commits will not be flattened
by running 'git pull'.
+
@@ -2620,7 +2620,7 @@ pull.rebase::
pull" is run. See "branch.<name>.rebase" for setting this on a
per-branch basis.
+
-When recreate, also pass `--recreate-merges` along to 'git rebase'
+When `merges`, pass the `--rebase-merges` option to 'git rebase'
so that locally committed merge commits will not be flattened
by running 'git pull'.
+
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index b4f9f057ea9..6f76d815dd3 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -101,15 +101,15 @@ Options related to merging
include::merge-options.txt[]
-r::
---rebase[=false|true|recreate|preserve|interactive]::
+--rebase[=false|true|merges|preserve|interactive]::
When true, rebase the current branch on top of the upstream
branch after fetching. If there is a remote-tracking branch
corresponding to the upstream branch and the upstream branch
was rebased since last fetched, the rebase uses that information
to avoid rebasing non-local changes.
+
-When set to recreate, rebase with the `--recreate-merges` option passed
-to `git rebase` so that locally created merge commits will not be flattened.
+When set to `merges`, rebase using `git rebase --rebase-merges` so that
+locally created merge commits will not be flattened.
+
When set to preserve, rebase with the `--preserve-merges` option passed
to `git rebase` so that locally created merge commits will not be flattened.
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 2b85416f969..be946de2efb 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -378,16 +378,19 @@ The commit list format can be changed by setting the configuration option
rebase.instructionFormat. A customized instruction format will automatically
have the long commit hash prepended to the format.
---recreate-merges[=(rebase-cousins|no-rebase-cousins)]::
- Recreate merge commits instead of flattening the history by replaying
+-r::
+--rebase-merges[=(rebase-cousins|no-rebase-cousins)]::
+ Rebase merge commits instead of flattening the history by replaying
merges. Merge conflict resolutions or manual amendments to merge
- commits are not recreated automatically, but have to be recreated
+ commits are not rebased automatically, but have to be applied
manually.
+
By default, or when `no-rebase-cousins` was specified, commits which do not
-have `<upstream>` as direct ancestor keep their original branch point.
+have `<upstream>` as direct ancestor will keep their original branch point.
If the `rebase-cousins` mode is turned on, such commits are rebased onto
`<upstream>` (or `<onto>`, if specified).
++
+See also REBASING MERGES below.
-p::
--preserve-merges::
@@ -786,13 +789,136 @@ The ripple effect of a "hard case" recovery is especially bad:
'everyone' downstream from 'topic' will now have to perform a "hard
case" recovery too!
+REBASING MERGES
+-----------------
+
+The interactive rebase command was originally designed to handle
+individual patch series. As such, it makes sense to exclude merge
+commits from the todo list, as the developer may have merged the
+current `master` while working on the branch, only to eventually
+rebase all the commits onto `master` (skipping the merge commits).
+
+However, there are legitimate reasons why a developer may want to
+recreate merge commits: to keep the branch structure (or "commit
+topology") when working on multiple, inter-related branches.
+
+In the following example, the developer works on a topic branch that
+refactors the way buttons are defined, and on another topic branch
+that uses that refactoring to implement a "Report a bug" button. The
+output of `git log --graph --format=%s -5` may look like this:
+
+------------
+* Merge branch 'report-a-bug'
+|\
+| * Add the feedback button
+* | Merge branch 'refactor-button'
+|\ \
+| |/
+| * Use the Button class for all buttons
+| * Extract a generic Button class from the DownloadButton one
+------------
+
+The developer might want to rebase those commits to a newer `master`
+while keeping the branch topology, for example when the first topic
+branch is expected to be integrated into `master` much earlier than the
+second one, say, to resolve merge conflicts with changes to the
+DownloadButton class that made it into `master`.
+
+This rebase can be performed using the `--rebase-merges` option.
+It will generate a todo list looking like this:
+
+------------
+label onto
+
+# Branch: refactor-button
+reset onto
+pick 123456 Extract a generic Button class from the DownloadButton one
+pick 654321 Use the Button class for all buttons
+label refactor-button
+
+# Branch: report-a-bug
+reset refactor-button # Use the Button class for all buttons
+pick abcdef Add the feedback button
+label report-a-bug
+
+reset onto
+merge -C a1b2c3 refactor-button # Merge 'refactor-button'
+merge -C 6f5e4d report-a-bug # Merge 'report-a-bug'
+------------
+
+In contrast to a regular interactive rebase, there are `label`, `reset` and
+`merge` commands in addition to `pick` ones.
+
+The `label` command puts a label to whatever will be the current
+revision when that command is executed. Internally, these labels are
+worktree-local refs that will be deleted when the rebase finishes or
+when it is aborted. That way, rebase operations in multiple worktrees
+linked to the same repository do not interfere with one another.
+
+The `reset` command is essentially a `git reset --hard` to the specified
+revision (typically a previously-labeled one).
+
+The `merge` command will merge the specified revision into whatever is
+HEAD at that time. With `-C <original-commit>`, the commit message of
+the specified merge commit will be used. When the `-C` is changed to
+a lower-case `-c`, the message will be opened in an editor after a
+successful merge so that the user can edit the message.
+
+At this time, the `merge` command will *always* use the `recursive`
+merge strategy, with no way to choose a different one. To work around
+this, an `exec` command can be used to call `git merge` explicitly,
+using the fact that the labels are worktree-local refs (the ref
+`refs/rewritten/onto` would correspond to the label `onto`).
+
+Note: the first command (`reset onto`) labels the revision onto which
+the commits are rebased; The name `onto` is just a convention, as a nod
+to the `--onto` option.
+
+It is also possible to introduce completely new merge commits from scratch
+by adding a command of the form `merge <merge-head>`. This form will
+generate a tentative commit message and always open an editor to let the
+user edit it. This can be useful e.g. when a topic branch turns out to
+address more than a single concern and wants to be split into two or
+even more topic branches. Consider this todo list:
+
+------------
+pick 192837 Switch from GNU Makefiles to CMake
+pick 5a6c7e Document the switch to CMake
+pick 918273 Fix detection of OpenSSL in CMake
+pick afbecd http: add support for TLS v1.3
+pick fdbaec Fix detection of cURL in CMake on Windows
+------------
+
+The one commit in this list that is not related to CMake may very well
+have been motivated by working on fixing all those bugs introduced by
+switching to CMake, but it addresses a different concern. To split this
+branch into two topic branches, the todo list could be edited like this:
+
+------------
+label onto
+
+pick afbecd http: add support for TLS v1.3
+label tlsv1.3
+
+reset onto
+pick 192837 Switch from GNU Makefiles to CMake
+pick 918273 Fix detection of OpenSSL in CMake
+pick fdbaec Fix detection of cURL in CMake on Windows
+pick 5a6c7e Document the switch to CMake
+label cmake
+
+reset onto
+merge tlsv1.3
+merge cmake
+------------
+
BUGS
----
The todo list presented by `--preserve-merges --interactive` does not
represent the topology of the revision graph. Editing commits and
rewording their commit messages should work fine, but attempts to
reorder commits tend to produce counterintuitive results. Use
---recreate-merges for a more faithful representation.
+--rebase-merges for a more faithful representation.
For example, an attempt to rearrange
------------
diff --git a/builtin/pull.c b/builtin/pull.c
index 3d1cc60eed6..70b44146ce4 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -27,14 +27,14 @@ enum rebase_type {
REBASE_FALSE = 0,
REBASE_TRUE,
REBASE_PRESERVE,
- REBASE_RECREATE,
+ REBASE_MERGES,
REBASE_INTERACTIVE
};
/**
* Parses the value of --rebase. If value is a false value, returns
* REBASE_FALSE. If value is a true value, returns REBASE_TRUE. If value is
- * "recreate", returns REBASE_RECREATE. If value is "preserve", returns
+ * "merges", returns REBASE_MERGES. If value is "preserve", returns
* REBASE_PRESERVE. If value is a invalid value, dies with a fatal error if
* fatal is true, otherwise returns REBASE_INVALID.
*/
@@ -49,8 +49,8 @@ static enum rebase_type parse_config_rebase(const char *key, const char *value,
return REBASE_TRUE;
else if (!strcmp(value, "preserve"))
return REBASE_PRESERVE;
- else if (!strcmp(value, "recreate"))
- return REBASE_RECREATE;
+ else if (!strcmp(value, "merges"))
+ return REBASE_MERGES;
else if (!strcmp(value, "interactive"))
return REBASE_INTERACTIVE;
@@ -134,7 +134,7 @@ static struct option pull_options[] = {
/* Options passed to git-merge or git-rebase */
OPT_GROUP(N_("Options related to merging")),
{ OPTION_CALLBACK, 'r', "rebase", &opt_rebase,
- "false|true|recreate|preserve|interactive",
+ "false|true|merges|preserve|interactive",
N_("incorporate changes by rebasing rather than merging"),
PARSE_OPT_OPTARG, parse_opt_rebase },
OPT_PASSTHRU('n', NULL, &opt_diffstat, NULL,
@@ -804,8 +804,8 @@ static int run_rebase(const struct object_id *curr_head,
argv_push_verbosity(&args);
/* Options passed to git-rebase */
- if (opt_rebase == REBASE_RECREATE)
- argv_array_push(&args, "--recreate-merges");
+ if (opt_rebase == REBASE_MERGES)
+ argv_array_push(&args, "--rebase-merges");
else if (opt_rebase == REBASE_PRESERVE)
argv_array_push(&args, "--preserve-merges");
else if (opt_rebase == REBASE_INTERACTIVE)
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 5d1f12de57b..f7c2a5fdc81 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -12,7 +12,7 @@ static const char * const builtin_rebase_helper_usage[] = {
int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
{
struct replay_opts opts = REPLAY_OPTS_INIT;
- unsigned flags = 0, keep_empty = 0, recreate_merges = 0;
+ unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
int abbreviate_commands = 0, rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
@@ -24,7 +24,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
OPT_BOOL(0, "keep-empty", &keep_empty, N_("keep empty commits")),
OPT_BOOL(0, "allow-empty-message", &opts.allow_empty_message,
N_("allow commits with empty messages")),
- OPT_BOOL(0, "recreate-merges", &recreate_merges, N_("recreate merge commits")),
+ OPT_BOOL(0, "rebase-merges", &rebase_merges, N_("rebase merge commits")),
OPT_BOOL(0, "rebase-cousins", &rebase_cousins,
N_("keep original branch points of cousins")),
OPT_CMDMODE(0, "continue", &command, N_("continue rebase"),
@@ -60,13 +60,13 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
flags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0;
flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0;
- flags |= recreate_merges ? TODO_LIST_RECREATE_MERGES : 0;
+ flags |= rebase_merges ? TODO_LIST_REBASE_MERGES : 0;
flags |= rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0;
flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;
- if (rebase_cousins >= 0 && !recreate_merges)
+ if (rebase_cousins >= 0 && !rebase_merges)
warning(_("--[no-]rebase-cousins has no effect without "
- "--recreate-merges"));
+ "--rebase-merges"));
if (command == CONTINUE && argc == 1)
return !!sequencer_continue(&opts);
diff --git a/builtin/remote.c b/builtin/remote.c
index 210890c8a8e..45c9219e07a 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -245,7 +245,9 @@ static int add(int argc, const char **argv)
struct branch_info {
char *remote_name;
struct string_list merge;
- enum { NO_REBASE, NORMAL_REBASE, INTERACTIVE_REBASE } rebase;
+ enum {
+ NO_REBASE, NORMAL_REBASE, INTERACTIVE_REBASE, REBASE_MERGES
+ } rebase;
};
static struct string_list branch_list = STRING_LIST_INIT_NODUP;
@@ -306,8 +308,8 @@ static int config_read_branches(const char *key, const char *value, void *cb)
info->rebase = v;
else if (!strcmp(value, "preserve"))
info->rebase = NORMAL_REBASE;
- else if (!strcmp(value, "recreate"))
- info->rebase = NORMAL_REBASE;
+ else if (!strcmp(value, "merges"))
+ info->rebase = REBASE_MERGES;
else if (!strcmp(value, "interactive"))
info->rebase = INTERACTIVE_REBASE;
}
@@ -965,9 +967,15 @@ static int show_local_info_item(struct string_list_item *item, void *cb_data)
printf(" %-*s ", show_info->width, item->string);
if (branch_info->rebase) {
- printf_ln(branch_info->rebase == INTERACTIVE_REBASE
- ? _("rebases interactively onto remote %s")
- : _("rebases onto remote %s"), merge->items[0].string);
+ const char *msg;
+ if (branch_info->rebase == INTERACTIVE_REBASE)
+ msg = _("rebases interactively onto remote %s");
+ else if (branch_info->rebase == REBASE_MERGES)
+ msg = _("rebases interactively (with merges) onto "
+ "remote %s");
+ else
+ msg = _("rebases onto remote %s");
+ printf_ln(msg, merge->items[0].string);
return 0;
} else if (show_info->any_rebase) {
printf_ln(_(" merges with remote %s"), merge->items[0].string);
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 7d2e7062919..6af65155c59 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1949,7 +1949,7 @@ _git_rebase ()
--*)
__gitcomp "
--onto --merge --strategy --interactive
- --recreate-merges --preserve-merges --stat --no-stat
+ --rebase-merges --preserve-merges --stat --no-stat
--committer-date-is-author-date --ignore-date
--ignore-whitespace --whitespace=
--autosquash --no-autosquash
@@ -2120,7 +2120,7 @@ _git_config ()
return
;;
branch.*.rebase)
- __gitcomp "false true recreate preserve interactive"
+ __gitcomp "false true merges preserve interactive"
return
;;
remote.pushdefault)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 4c21faaccb1..b4ad130e8b1 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -970,7 +970,7 @@ git_rebase__interactive () {
init_revisions_and_shortrevisions
git rebase--helper --make-script ${keep_empty:+--keep-empty} \
- ${recreate_merges:+--recreate-merges} \
+ ${rebase_merges:+--rebase-merges} \
${rebase_cousins:+--rebase-cousins} \
$revisions ${restrict_revision+^$restrict_revision} >"$todo" ||
die "$(gettext "Could not generate todo list")"
diff --git a/git-rebase.sh b/git-rebase.sh
index dd39dfb1112..157705d2a72 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -17,7 +17,7 @@ q,quiet! be quiet. implies --no-stat
autostash automatically stash/stash pop before and after
fork-point use 'merge-base --fork-point' to refine upstream
onto=! rebase onto given branch instead of upstream
-recreate-merges? try to recreate merges instead of skipping them
+r,rebase-merges? try to rebase merges instead of skipping them
p,preserve-merges! try to recreate merges instead of ignoring them
s,strategy=! use the given merge strategy
no-ff! cherry-pick all commits, even if unchanged
@@ -89,7 +89,7 @@ type=
state_dir=
# One of {'', continue, skip, abort}, as parsed from command line
action=
-recreate_merges=
+rebase_merges=
rebase_cousins=
preserve_merges=
autosquash=
@@ -273,12 +273,12 @@ do
--allow-empty-message)
allow_empty_message=--allow-empty-message
;;
- --recreate-merges)
- recreate_merges=t
+ --rebase-merges)
+ rebase_merges=t
test -z "$interactive_rebase" && interactive_rebase=implied
;;
- --recreate-merges=*)
- recreate_merges=t
+ --rebase-merges=*)
+ rebase_merges=t
case "${1#*=}" in
rebase-cousins) rebase_cousins=t;;
no-rebase-cousins) rebase_cousins=;;
diff --git a/sequencer.c b/sequencer.c
index 0b6aaced9a5..809df1ce484 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1308,7 +1308,6 @@ enum todo_command {
TODO_LABEL,
TODO_RESET,
TODO_MERGE,
- TODO_MERGE_AND_EDIT,
/* commands that do nothing but are counted for reporting progress */
TODO_NOOP,
TODO_DROP,
@@ -1330,7 +1329,6 @@ static struct {
{ 'l', "label" },
{ 't', "reset" },
{ 'm', "merge" },
- { 0, "merge" }, /* MERGE_AND_EDIT */
{ 0, "noop" },
{ 'd', "drop" },
{ 0, NULL }
@@ -1758,9 +1756,14 @@ static int read_and_refresh_cache(struct replay_opts *opts)
return 0;
}
+enum todo_item_flags {
+ TODO_EDIT_MERGE_MSG = 1
+};
+
struct todo_item {
enum todo_command command;
struct commit *commit;
+ unsigned int flags;
const char *arg;
int arg_len;
size_t offset_in_buf;
@@ -1795,6 +1798,8 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)
char *end_of_object_name;
int i, saved, status, padding;
+ item->flags = 0;
+
/* left-trim */
bol += strspn(bol, " \t");
@@ -1849,9 +1854,9 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)
bol += strspn(bol, " \t");
else if (skip_prefix(bol, "-c", &bol)) {
bol += strspn(bol, " \t");
- item->command = TODO_MERGE_AND_EDIT;
+ item->flags |= TODO_EDIT_MERGE_MSG;
} else {
- item->command = TODO_MERGE_AND_EDIT;
+ item->flags |= TODO_EDIT_MERGE_MSG;
item->commit = NULL;
item->arg = bol;
item->arg_len = (int)(eol - bol);
@@ -2511,8 +2516,11 @@ static int safe_append(const char *filename, const char *fmt, ...)
if (fd < 0)
return -1;
- if (strbuf_read_file(&buf, filename, 0) < 0 && errno != ENOENT)
- return error_errno(_("could not read '%s'"), filename);
+ if (strbuf_read_file(&buf, filename, 0) < 0 && errno != ENOENT) {
+ error_errno(_("could not read '%s'"), filename);
+ rollback_lock_file(&lock);
+ return -1;
+ }
strbuf_complete(&buf, '\n');
va_start(ap, fmt);
strbuf_vaddf(&buf, fmt, ap);
@@ -2574,6 +2582,9 @@ static int do_label(const char *name, int len)
return ret;
}
+static const char *reflog_message(struct replay_opts *opts,
+ const char *sub_action, const char *fmt, ...);
+
static int do_reset(const char *name, int len, struct replay_opts *opts)
{
struct strbuf ref_name = STRBUF_INIT;
@@ -2638,33 +2649,50 @@ static int do_reset(const char *name, int len, struct replay_opts *opts)
ret = error(_("could not write index"));
free((void *)desc.buffer);
- if (!ret) {
- struct strbuf msg = STRBUF_INIT;
-
- strbuf_addf(&msg, "(rebase -i) reset '%.*s'", len, name);
- ret = update_ref(msg.buf, "HEAD", &oid, NULL, 0,
- UPDATE_REFS_MSG_ON_ERR);
- strbuf_release(&msg);
- }
+ if (!ret)
+ ret = update_ref(reflog_message(opts, "reset", "'%.*s'",
+ len, name), "HEAD", &oid,
+ NULL, 0, UPDATE_REFS_MSG_ON_ERR);
strbuf_release(&ref_name);
return ret;
}
static int do_merge(struct commit *commit, const char *arg, int arg_len,
- int run_commit_flags, struct replay_opts *opts)
+ int flags, struct replay_opts *opts)
{
- int merge_arg_len;
+ int run_commit_flags = (flags & TODO_EDIT_MERGE_MSG) ?
+ EDIT_MSG | VERIFY_MSG : 0;
struct strbuf ref_name = STRBUF_INIT;
struct commit *head_commit, *merge_commit, *i;
- struct commit_list *common, *j, *reversed = NULL;
+ struct commit_list *bases, *j, *reversed = NULL;
struct merge_options o;
- int can_fast_forward, ret;
+ int merge_arg_len, oneline_offset, can_fast_forward, ret;
static struct lock_file lock;
+ const char *p;
- for (merge_arg_len = 0; merge_arg_len < arg_len; merge_arg_len++)
- if (isspace(arg[merge_arg_len]))
- break;
+ oneline_offset = arg_len;
+ merge_arg_len = strcspn(arg, " \t\n");
+ p = arg + merge_arg_len;
+ p += strspn(p, " \t\n");
+ if (*p == '#' && (!p[1] || isspace(p[1]))) {
+ p += 1 + strspn(p + 1, " \t\n");
+ oneline_offset = p - arg;
+ } else if (p - arg < arg_len)
+ BUG("octopus merges are not supported yet: '%s'", p);
+
+ strbuf_addf(&ref_name, "refs/rewritten/%.*s", merge_arg_len, arg);
+ merge_commit = lookup_commit_reference_by_name(ref_name.buf);
+ if (!merge_commit) {
+ /* fall back to non-rewritten ref or commit */
+ strbuf_splice(&ref_name, 0, strlen("refs/rewritten/"), "", 0);
+ merge_commit = lookup_commit_reference_by_name(ref_name.buf);
+ }
+ if (!merge_commit) {
+ error(_("could not resolve '%s'"), ref_name.buf);
+ strbuf_release(&ref_name);
+ return -1;
+ }
if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0)
return -1;
@@ -2697,7 +2725,6 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,
}
unuse_commit_buffer(commit, message);
} else {
- const char *p = arg + merge_arg_len;
struct strbuf buf = STRBUF_INIT;
int len;
@@ -2705,12 +2732,10 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,
write_author_script(buf.buf);
strbuf_reset(&buf);
- p += strspn(p, " \t");
- if (*p == '#' && isspace(p[1]))
- p += 1 + strspn(p + 1, " \t");
- if (*p)
- len = strlen(p);
- else {
+ if (oneline_offset < arg_len) {
+ p = arg + oneline_offset;
+ len = arg_len - oneline_offset;
+ } else {
strbuf_addf(&buf, "Merge branch '%.*s'",
merge_arg_len, arg);
p = buf.buf;
@@ -2728,25 +2753,24 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,
}
/*
- * If HEAD is not identical to the parent of the original merge commit,
- * we cannot fast-forward.
+ * If HEAD is not identical to the first parent of the original merge
+ * commit, we cannot fast-forward.
*/
can_fast_forward = opts->allow_ff && commit && commit->parents &&
!oidcmp(&commit->parents->item->object.oid,
&head_commit->object.oid);
- strbuf_addf(&ref_name, "refs/rewritten/%.*s", merge_arg_len, arg);
- merge_commit = lookup_commit_reference_by_name(ref_name.buf);
- if (!merge_commit) {
- /* fall back to non-rewritten ref or commit */
- strbuf_splice(&ref_name, 0, strlen("refs/rewritten/"), "", 0);
- merge_commit = lookup_commit_reference_by_name(ref_name.buf);
- }
- if (!merge_commit) {
- error(_("could not resolve '%s'"), ref_name.buf);
- strbuf_release(&ref_name);
- rollback_lock_file(&lock);
- return -1;
+ /*
+ * If the merge head is different from the original one, we cannot
+ * fast-forward.
+ */
+ if (can_fast_forward) {
+ struct commit_list *second_parent = commit->parents->next;
+
+ if (second_parent && !second_parent->next &&
+ oidcmp(&merge_commit->object.oid,
+ &second_parent->item->object.oid))
+ can_fast_forward = 0;
}
if (can_fast_forward && commit->parents->next &&
@@ -2763,10 +2787,18 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,
git_path_merge_head(), 0);
write_message("no-ff", 5, git_path_merge_mode(), 0);
- common = get_merge_bases(head_commit, merge_commit);
- for (j = common; j; j = j->next)
+ bases = get_merge_bases(head_commit, merge_commit);
+ if (bases && !oidcmp(&merge_commit->object.oid,
+ &bases->item->object.oid)) {
+ strbuf_release(&ref_name);
+ rollback_lock_file(&lock);
+ /* skip merging an ancestor of HEAD */
+ return 0;
+ }
+
+ for (j = bases; j; j = j->next)
commit_list_insert(j->item, &reversed);
- free_commit_list(common);
+ free_commit_list(bases);
read_cache();
init_merge_options(&o);
@@ -2775,6 +2807,8 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,
o.buffer_output = 2;
ret = merge_recursive(&o, head_commit, merge_commit, reversed, &i);
+ if (!ret)
+ rerere(opts->allow_rerere_auto);
if (ret <= 0)
fputs(o.obuf.buf, stdout);
strbuf_release(&o.obuf);
@@ -2986,11 +3020,9 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
res = do_label(item->arg, item->arg_len);
else if (item->command == TODO_RESET)
res = do_reset(item->arg, item->arg_len, opts);
- else if (item->command == TODO_MERGE ||
- item->command == TODO_MERGE_AND_EDIT) {
+ else if (item->command == TODO_MERGE) {
res = do_merge(item->commit, item->arg, item->arg_len,
- item->command == TODO_MERGE_AND_EDIT ?
- EDIT_MSG | VERIFY_MSG : 0, opts);
+ item->flags, opts);
if (item->commit)
record_in_rewritten(&item->commit->object.oid,
peek_command(todo_list, 1));
@@ -3404,7 +3436,7 @@ static const char *label_oid(struct object_id *oid, const char *label,
strbuf_grow(&state->buf, GIT_SHA1_HEXSZ);
label = p = state->buf.buf;
- find_unique_abbrev_r(p, oid->hash, default_abbrev);
+ find_unique_abbrev_r(p, oid, default_abbrev);
/*
* We may need to extend the abbreviated hash so that there is
@@ -3508,11 +3540,13 @@ static int make_script_with_merges(struct pretty_print_context *pp,
int is_octopus;
const char *p1, *p2;
struct object_id *oid;
+ int is_empty;
tail = &commit_list_insert(commit, tail)->next;
oidset_insert(&interesting, &commit->object.oid);
- if ((commit->object.flags & PATCHSAME))
+ is_empty = is_original_commit_empty(commit);
+ if (!is_empty && (commit->object.flags & PATCHSAME))
continue;
strbuf_reset(&oneline);
@@ -3522,7 +3556,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,
if (!to_merge) {
/* non-merge commit: easy case */
strbuf_reset(&buf);
- if (!keep_empty && is_original_commit_empty(commit))
+ if (!keep_empty && is_empty)
strbuf_addf(&buf, "%c ", comment_line_char);
strbuf_addf(&buf, "%s %s %s", cmd_pick,
oid_to_hex(&commit->object.oid),
@@ -3698,11 +3732,11 @@ int sequencer_make_script(FILE *out, int argc, const char **argv,
struct commit *commit;
int keep_empty = flags & TODO_LIST_KEEP_EMPTY;
const char *insn = flags & TODO_LIST_ABBREVIATE_CMDS ? "p" : "pick";
- int recreate_merges = flags & TODO_LIST_RECREATE_MERGES;
+ int rebase_merges = flags & TODO_LIST_REBASE_MERGES;
init_revisions(&revs, NULL);
revs.verbose_header = 1;
- if (recreate_merges)
+ if (rebase_merges)
revs.cherry_mark = 1;
else {
revs.max_parents = 1;
@@ -3731,7 +3765,7 @@ int sequencer_make_script(FILE *out, int argc, const char **argv,
if (prepare_revision_walk(&revs) < 0)
return error(_("make_script: error preparing revisions"));
- if (recreate_merges)
+ if (rebase_merges)
return make_script_with_merges(&pp, &revs, out, flags);
while ((commit = get_revision(&revs))) {
@@ -3823,10 +3857,12 @@ int transform_todos(unsigned flags)
short_commit_name(item->commit) :
oid_to_hex(&item->commit->object.oid);
- if (item->command == TODO_MERGE)
- strbuf_addstr(&buf, " -C");
- else if (item->command == TODO_MERGE_AND_EDIT)
- strbuf_addstr(&buf, " -c");
+ if (item->command == TODO_MERGE) {
+ if (item->flags & TODO_EDIT_MERGE_MSG)
+ strbuf_addstr(&buf, " -c");
+ else
+ strbuf_addstr(&buf, " -C");
+ }
strbuf_addf(&buf, " %s", oid);
}
diff --git a/sequencer.h b/sequencer.h
index 739dd0fa92b..d9570d92b11 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -59,9 +59,9 @@ int sequencer_remove_state(struct replay_opts *opts);
#define TODO_LIST_KEEP_EMPTY (1U << 0)
#define TODO_LIST_SHORTEN_IDS (1U << 1)
#define TODO_LIST_ABBREVIATE_CMDS (1U << 2)
-#define TODO_LIST_RECREATE_MERGES (1U << 3)
+#define TODO_LIST_REBASE_MERGES (1U << 3)
/*
- * When recreating merges, commits that do have the base commit as ancestor
+ * When rebasing merges, commits that do have the base commit as ancestor
* ("cousins") are *not* rebased onto the new base by default. If those
* commits should be rebased onto the new base, this flag needs to be passed.
*/
diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
index 68fe2003ef5..fbae5dab7e2 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -217,6 +217,7 @@ test_run_rebase success ''
test_run_rebase failure -m
test_run_rebase failure -i
test_run_rebase failure -p
+test_run_rebase success --rebase-merges
# m
# /
diff --git a/t/t3430-rebase-recreate-merges.sh b/t/t3430-rebase-merges.sh
similarity index 87%
rename from t/t3430-rebase-recreate-merges.sh
rename to t/t3430-rebase-merges.sh
index 9a59f12b670..ee006810573 100755
--- a/t/t3430-rebase-recreate-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -1,9 +1,9 @@
#!/bin/sh
#
-# Copyright (c) 2017 Johannes E. Schindelin
+# Copyright (c) 2018 Johannes E. Schindelin
#
-test_description='git rebase -i --recreate-merges
+test_description='git rebase -i --rebase-merges
This test runs git rebase "interactively", retaining the branch structure by
recreating merge commits.
@@ -19,6 +19,13 @@ Initial setup:
. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-rebase.sh
+test_cmp_graph () {
+ cat >expect &&
+ git log --graph --boundary --format=%s "$@" >output &&
+ sed "s/ *$//" <output >output.trimmed &&
+ test_cmp expect output.trimmed
+}
+
test_expect_success 'setup' '
write_script replace-editor.sh <<-\EOF &&
mv "$1" "$(git rev-parse --git-path ORIGINAL-TODO)"
@@ -63,17 +70,10 @@ merge -C H second
merge onebranch # Merge the topic branch 'onebranch'
EOF
-test_cmp_graph () {
- cat >expect &&
- git log --graph --boundary --format=%s "$@" >output &&
- sed "s/ *$//" <output >output.trimmed &&
- test_cmp expect output.trimmed
-}
-
test_expect_success 'create completely different structure' '
test_config sequence.editor \""$PWD"/replace-editor.sh\" &&
test_tick &&
- git rebase -i --recreate-merges A &&
+ git rebase -i -r A &&
test_cmp_graph <<-\EOF
* Merge the topic branch '\''onebranch'\''
|\
@@ -132,7 +132,7 @@ test_expect_success 'with a branch tip that was cherry-picked already' '
git checkout already-upstream &&
test_tick &&
- git rebase -i --recreate-merges upstream-with-a2 &&
+ git rebase -i -r upstream-with-a2 &&
test_cmp_graph upstream-with-a2.. <<-\EOF
* Merge branch A
|\
@@ -144,18 +144,13 @@ test_expect_success 'with a branch tip that was cherry-picked already' '
'
test_expect_success 'do not rebase cousins unless asked for' '
- write_script copy-editor.sh <<-\EOF &&
- cp "$1" "$(git rev-parse --git-path ORIGINAL-TODO)"
- EOF
-
- test_config sequence.editor \""$PWD"/copy-editor.sh\" &&
git checkout -b cousins master &&
before="$(git rev-parse --verify HEAD)" &&
test_tick &&
- git rebase -i --recreate-merges HEAD^ &&
+ git rebase -r HEAD^ &&
test_cmp_rev HEAD $before &&
test_tick &&
- git rebase -i --recreate-merges=rebase-cousins HEAD^ &&
+ git rebase --rebase-merges=rebase-cousins HEAD^ &&
test_cmp_graph HEAD^.. <<-\EOF
* Merge the topic branch '\''onebranch'\''
|\
@@ -196,7 +191,7 @@ test_expect_success 'post-rewrite hook and fixups work for merges' '
echo "cat >actual" | write_script .git/hooks/post-rewrite &&
test_tick &&
- git rebase -i --autosquash --recreate-merges HEAD^^^ &&
+ git rebase -i --autosquash -r HEAD^^^ &&
printf "%s %s\n%s %s\n%s %s\n%s %s\n" >expect $(git rev-parse \
$fixup^^2 HEAD^2 \
$fixup^^ HEAD^ \
@@ -205,4 +200,12 @@ test_expect_success 'post-rewrite hook and fixups work for merges' '
test_cmp expect actual
'
+test_expect_success 'refuse to merge ancestors of HEAD' '
+ echo "merge HEAD^" >script-from-scratch &&
+ test_config -C wt sequence.editor \""$PWD"/replace-editor.sh\" &&
+ before="$(git rev-parse HEAD)" &&
+ git rebase -i HEAD &&
+ test_cmp_rev HEAD $before
+'
+
test_done-- 2.17.0.windows.1.4.g7e4058d72e3