Skip to content

Drop support for git rebase --preserve-merges #195

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 11 commits into from
Closed
1 change: 0 additions & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ jobs:
shell: bash
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):


On Wed, Sep 01 2021, Johannes Schindelin via GitGitGadget wrote:


> diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
> index e78c7e37969..3bb023d7669 100755
> --- a/t/t3427-rebase-subtree.sh
> +++ b/t/t3427-rebase-subtree.sh
> @@ -69,25 +69,6 @@ test_expect_success 'setup' '
>  	git commit -m "Empty commit" --allow-empty
>  '

The comment at the start should be changed too, e.g. something like this
to squash in:

diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
index e78c7e37969..74e67b817f6 100755
--- a/t/t3427-rebase-subtree.sh
+++ b/t/t3427-rebase-subtree.sh
@@ -36,11 +36,8 @@ commit_message() {
 # where the root commit adds three files: topic_1.t, topic_2.t and topic_3.t.
 #
 # This commit history is then rebased onto `topic_3` with the
-# `-Xsubtree=files_subtree` option in three different ways:
-#
-# 1. using `--preserve-merges`
-# 2. using `--preserve-merges` and --keep-empty
-# 3. without specifying a rebase backend
+# `-Xsubtree=files_subtree` option, both with and without
+# specifying a rebase backend
 
 test_expect_success 'setup' '
        test_commit README &&

env:
NO_SVN_TESTS: 1
GIT_TEST_SKIP_REBASE_P: 1
run: ci/run-test-slice.sh ${{matrix.nr}} 10
- name: ci/print-test-failures.sh
if: failure()
Expand Down
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@
/git-range-diff
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):


On Wed, Sep 01 2021, Johannes Schindelin via GitGitGadget wrote:

> -	struct strbuf script_snippet = STRBUF_INIT, buf = STRBUF_INIT;
> +	struct strbuf script_snippet = STRBUF_INIT;

You end up removing all uses of script_snippet except the
strbuf_release() for it, so this & that can be removed too.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Johannes Schindelin wrote (reply to this):

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323328-1936974659-1630590872=:55
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Hi =C3=86var,

On Wed, 1 Sep 2021, =C3=86var Arnfj=C3=B6r=C3=B0 Bjarmason wrote:

>
> On Wed, Sep 01 2021, Johannes Schindelin via GitGitGadget wrote:
>
> > -	struct strbuf script_snippet =3D STRBUF_INIT, buf =3D STRBUF_INIT;
> > +	struct strbuf script_snippet =3D STRBUF_INIT;
>
> You end up removing all uses of script_snippet except the
> strbuf_release() for it, so this & that can be removed too.

Valid point, I missed it because compiling with `DEVELOPER=3D1` did not
complain (because `strbuf_release()` "uses" the variable).

Ciao,
Dscho

--8323328-1936974659-1630590872=:55--

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):


On Thu, Sep 02 2021, Johannes Schindelin wrote:

> Hi Ævar,
>
> On Wed, 1 Sep 2021, Ævar Arnfjörð Bjarmason wrote:
>
>>
>> On Wed, Sep 01 2021, Johannes Schindelin via GitGitGadget wrote:
>>
>> > -	struct strbuf script_snippet = STRBUF_INIT, buf = STRBUF_INIT;
>> > +	struct strbuf script_snippet = STRBUF_INIT;
>>
>> You end up removing all uses of script_snippet except the
>> strbuf_release() for it, so this & that can be removed too.
>
> Valid point, I missed it because compiling with `DEVELOPER=1` did not
> complain (because `strbuf_release()` "uses" the variable).

As an aside I was looking the other day to see if some variant of
-Wunused was smart enough to exclude (preferably a configurable) list of
functions. E.g.:

    void *some_var = malloc(123);
    free(some_var);

Could be unused if something was smart to see that a thing only
interacting with "malloc" and "free" wasn't otherwise used, and likewise
we could add "strbuf_release" and other such free() functions to such a
list.

But I didn't find anything...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):


On Wed, Sep 01 2021, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <[email protected]>
>
> This option was deprecated in favor of `--rebase-merges` some time ago,
> and now we retire it.

>  static int is_merge(struct rebase_options *opts)
>  {
> -	return opts->type == REBASE_MERGE ||
> -		opts->type == REBASE_PRESERVE_MERGES;
> +	return opts->type == REBASE_MERGE;
>  }

This leaves us with a rather pointless is_merge() function and
nonsensical control flow in parse_opt_merge(). Let's squash this in:

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 4e19f64b91b..043933ab771 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -383,11 +383,6 @@ static int parse_opt_keep_empty(const struct option *opt, const char *arg,
 	return 0;
 }
 
-static int is_merge(struct rebase_options *opts)
-{
-	return opts->type == REBASE_MERGE;
-}
-
 static void imply_merge(struct rebase_options *opts, const char *option)
 {
 	switch (opts->type) {
@@ -909,8 +904,7 @@ static int parse_opt_merge(const struct option *opt, const char *arg, int unset)
 	BUG_ON_OPT_NEG(unset);
 	BUG_ON_OPT_ARG(arg);
 
-	if (!is_merge(opts))
-		opts->type = REBASE_MERGE;
+	opts->type = REBASE_MERGE;
 
 	return 0;
 }
@@ -988,7 +982,7 @@ static void set_reflog_action(struct rebase_options *options)
 	const char *env;
 	struct strbuf buf = STRBUF_INIT;
 
-	if (!is_merge(options))
+	if (options->type != REBASE_MERGE)
 		return;
 
 	env = getenv(GIT_REFLOG_ACTION_ENVIRONMENT);
@@ -1208,12 +1202,12 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		die(_("No rebase in progress?"));
 	setenv(GIT_REFLOG_ACTION_ENVIRONMENT, "rebase", 0);
 
-	if (action == ACTION_EDIT_TODO && !is_merge(&options))
+	if (action == ACTION_EDIT_TODO && options.type != REBASE_MERGE)
 		die(_("The --edit-todo action can only be used during "
 		      "interactive rebase."));
 
 	if (trace2_is_enabled()) {
-		if (is_merge(&options))
+		if (options.type == REBASE_MERGE)
 			trace2_cmd_mode("interactive");
 		else if (exec.nr)
 			trace2_cmd_mode("interactive-exec");
@@ -1465,7 +1459,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 				break;
 
 		if (i >= 0) {
-			if (is_merge(&options))
+			if (options.type == REBASE_MERGE)
 				die(_("cannot combine apply options with "
 				      "merge options"));
 			else
@@ -1507,7 +1501,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		else
 			options.empty = EMPTY_DROP;
 	}
-	if (reschedule_failed_exec > 0 && !is_merge(&options))
+	if (reschedule_failed_exec > 0 && options.type != REBASE_MERGE)
 		die(_("--reschedule-failed-exec requires "
 		      "--exec or --interactive"));
 	if (reschedule_failed_exec >= 0)
@@ -1746,7 +1740,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		diff_flush(&opts);
 	}
 
-	if (is_merge(&options))
+	if (options.type == REBASE_MERGE)
 		goto run_rebase;
 
 	/* Detach HEAD and reset the tree */

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Johannes Schindelin wrote (reply to this):

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323328-1123083655-1630591181=:55
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Hi =C3=86var,

On Wed, 1 Sep 2021, =C3=86var Arnfj=C3=B6r=C3=B0 Bjarmason wrote:

>
> On Wed, Sep 01 2021, Johannes Schindelin via GitGitGadget wrote:
>
> > From: Johannes Schindelin <[email protected]>
> >
> > This option was deprecated in favor of `--rebase-merges` some time ago=
,
> > and now we retire it.
>
> >  static int is_merge(struct rebase_options *opts)
> >  {
> > -	return opts->type =3D=3D REBASE_MERGE ||
> > -		opts->type =3D=3D REBASE_PRESERVE_MERGES;
> > +	return opts->type =3D=3D REBASE_MERGE;
> >  }
>
> This leaves us with a rather pointless is_merge() function and
> nonsensical control flow in parse_opt_merge().

Thank you for offering your perspective.

=46rom a readability point of view, I disagree with your assessment. Just
because it can be written shorter does not mean that it is clearer. Quite
the contrary, if you ask me. And since _I_ am contributing this patch
series, I will respectfully disagree and keep the version I find more
intuitive.

You could potentially talk me into adding a patch that renames that
function to `is_merge_backend()`, but that's as far as I would go. And I
am not really certain that that would improve things, either.

Ciao,
Dscho

--8323328-1123083655-1630591181=:55--

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):


On Thu, Sep 02 2021, Johannes Schindelin wrote:

> Hi Ævar,
>
> On Wed, 1 Sep 2021, Ævar Arnfjörð Bjarmason wrote:
>
>>
>> On Wed, Sep 01 2021, Johannes Schindelin via GitGitGadget wrote:
>>
>> > From: Johannes Schindelin <[email protected]>
>> >
>> > This option was deprecated in favor of `--rebase-merges` some time ago,
>> > and now we retire it.
>>
>> >  static int is_merge(struct rebase_options *opts)
>> >  {
>> > -	return opts->type == REBASE_MERGE ||
>> > -		opts->type == REBASE_PRESERVE_MERGES;
>> > +	return opts->type == REBASE_MERGE;
>> >  }
>>
>> This leaves us with a rather pointless is_merge() function and
>> nonsensical control flow in parse_opt_merge().
>
> Thank you for offering your perspective.
>
> From a readability point of view, I disagree with your assessment. Just
> because it can be written shorter does not mean that it is clearer. Quite
> the contrary, if you ask me. And since _I_ am contributing this patch
> series, I will respectfully disagree and keep the version I find more
> intuitive.
>
> You could potentially talk me into adding a patch that renames that
> function to `is_merge_backend()`, but that's as far as I would go. And I
> am not really certain that that would improve things, either.

I should have phrased that "Let's squash this in" less forcefully, to be
clear it was just a suggestion. I'm fine with you not taking that
squash. This series is good either way.

But just to poke a bit at the rationale: I'd be totally on-board with
keeping it because we'd like the smallest possible diff here, even if
that ends up including this oddity.

But in terms of making the end-result more readable I don't really see
that.

We're left with a mismatch of some things comparing the enum label
directly, and some using this function, and then there's
REBASE_APPLY. To make it consistent we could alternatively have the diff
here at the end, but having some mixture of both seems like the worst
end-result, shouldn't we pick one way of inspecting these flags and use
that consistently?

(I did not convert the few REBASE_UNSPECIFIED, but those comparisons
could similarly be is_unspecified(), and all three could be named
is_*_backend() per your suggestion).

diff --git a/builtin/rebase.c b/builtin/rebase.c
index d707e3604e7..ef82a5f1668 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -388,6 +388,11 @@ static int is_merge(struct rebase_options *opts)
 	return opts->type == REBASE_MERGE;
 }
 
+static int is_apply(struct rebase_options *opts)
+{
+	return opts->type == REBASE_APPLY;
+}
+
 static void imply_merge(struct rebase_options *opts, const char *option)
 {
 	switch (opts->type) {
@@ -558,7 +563,7 @@ static int finish_rebase(struct rebase_options *opts)
 	 * user should see them.
 	 */
 	run_auto_maintenance(!(opts->flags & (REBASE_NO_QUIET|REBASE_VERBOSE)));
-	if (opts->type == REBASE_MERGE) {
+	if (is_merge(opts)) {
 		struct replay_opts replay = REPLAY_OPTS_INIT;
 
 		replay.action = REPLAY_INTERACTIVE_REBASE;
@@ -744,7 +749,7 @@ static int run_specific_rebase(struct rebase_options *opts, enum action action)
 	struct strbuf script_snippet = STRBUF_INIT;
 	int status;
 
-	if (opts->type == REBASE_MERGE) {
+	if (is_merge(opts)) {
 		/* Run sequencer-based rebase */
 		setenv("GIT_CHERRY_PICK_HELP", resolvemsg, 1);
 		if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT)) {
@@ -759,14 +764,14 @@ static int run_specific_rebase(struct rebase_options *opts, enum action action)
 		}
 
 		status = run_sequencer_rebase(opts, action);
-	} else if (opts->type == REBASE_APPLY)
+	} else if (is_apply(opts))
 		status = run_am(opts);
 	else
 		BUG("Unhandled rebase type %d", opts->type);
 
 	if (opts->dont_finish_rebase)
 		; /* do nothing */
-	else if (opts->type == REBASE_MERGE)
+	else if (is_merge(opts))
 		; /* merge backend cleans up after itself */
 	else if (status == 0) {
 		if (!file_exists(state_dir_path("stopped-sha", opts)))
@@ -1293,7 +1298,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	}
 	case ACTION_QUIT: {
 		save_autostash(state_dir_path("autostash", &options));
-		if (options.type == REBASE_MERGE) {
+		if (is_merge(&options)) {
 			struct replay_opts replay = REPLAY_OPTS_INIT;
 
 			replay.action = REPLAY_INTERACTIVE_REBASE;
@@ -1406,7 +1411,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		imply_merge(&options, "--rebase-merges");
 	}
 
-	if (options.type == REBASE_APPLY) {
+	if (is_apply(&options)) {
 		if (ignore_whitespace)
 			strvec_push(&options.git_am_opts,
 				    "--ignore-whitespace");
@@ -1452,7 +1457,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		}
 	}
 
-	if (options.type == REBASE_MERGE)
+	if (is_merge(&options))
 		imply_merge(&options, "--merge");
 
 	if (options.root && !options.onto_name)
@@ -1461,7 +1466,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	if (isatty(2) && options.flags & REBASE_NO_QUIET)
 		strbuf_addstr(&options.git_format_patch_opt, " --progress");
 
-	if (options.git_am_opts.nr || options.type == REBASE_APPLY) {
+	if (options.git_am_opts.nr || is_apply(&options)) {
 		/* all am options except -q are compatible only with --apply */
 		for (i = options.git_am_opts.nr - 1; i >= 0; i--)
 			if (strcmp(options.git_am_opts.v[i], "-q"))
@@ -1486,7 +1491,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 			    options.default_backend);
 	}
 
-	if (options.type == REBASE_MERGE &&
+	if (is_merge(&options) &&
 	    !options.strategy &&
 	    getenv("GIT_TEST_MERGE_ALGORITHM"))
 		options.strategy = xstrdup(getenv("GIT_TEST_MERGE_ALGORITHM"));

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):


On Wed, Sep 01 2021, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <[email protected]>
>
> This option was deprecated in favor of `--rebase-merges` some time ago,
> and now we retire it.
>
> Signed-off-by: Johannes Schindelin <[email protected]>
> ---
>  .gitignore                     |    1 -
>  Documentation/git-rebase.txt   |   51 --
>  Makefile                       |    2 -
>  builtin/rebase.c               |  133 +---
>  git-rebase--preserve-merges.sh | 1057 --------------------------------
>  5 files changed, 6 insertions(+), 1238 deletions(-)
>  delete mode 100644 git-rebase--preserve-merges.sh
>
> diff --git a/.gitignore b/.gitignore
> index 311841f9bed..98d6275b20d 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -125,7 +125,6 @@
>  /git-range-diff
>  /git-read-tree
>  /git-rebase
> -/git-rebase--preserve-merges
>  /git-receive-pack
>  /git-reflog
>  /git-remote
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 55af6fd24e2..1382dc6f005 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -525,29 +525,12 @@ i.e. commits that would be excluded by linkgit:git-log[1]'s
>  the `rebase-cousins` mode is turned on, such commits are instead rebased
>  onto `<upstream>` (or `<onto>`, if specified).
>  +
> -The `--rebase-merges` mode is similar in spirit to the deprecated
> -`--preserve-merges` but works with interactive rebases,
> -where commits can be reordered, inserted and dropped at will.
> -+
>  It is currently only possible to recreate the merge commits using the
>  `recursive` merge strategy; Different merge strategies can be used only via
>  explicit `exec git merge -s <strategy> [...]` commands.
>  +
>  See also REBASING MERGES and INCOMPATIBLE OPTIONS below.
>  
> --p::
> ---preserve-merges::
> -	[DEPRECATED: use `--rebase-merges` instead] Recreate merge commits
> -	instead of flattening the history by replaying commits a merge commit
> -	introduces. Merge conflict resolutions or manual amendments to merge
> -	commits are not preserved.
> -+
> -This uses the `--interactive` machinery internally, but combining it
> -with the `--interactive` option explicitly is generally not a good
> -idea unless you know what you are doing (see BUGS below).
> -+
> -See also INCOMPATIBLE OPTIONS below.
> -

I wonder if we should leave some variant of this in, just as a last
resort to help users who have some ancient version / finger memory &
wonder why it doesn't work, something like;

-p::
--preserve-merges:
	These options were used by the long-deprecated and now removed
	"rebase merges" backend. The `--rebase-merges` option is the
	similar in spirit replacement mode.

Also, per your ea8b7be1476 (git svn: stop using `rebase
--preserve-merges`, 2019-11-22) the use in "git svn" couldn't really be
directly compared to this, but there we usurped the "-p" flag for
--rebase-merges. Perhaps we could/should do the same here (eventually,
doesn't need to be in this series).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):


On Wed, Sep 01 2021, Johannes Schindelin via GitGitGadget wrote:

>  git-rebase--preserve-merges.sh | 1057 --------------------------------

You could, but certainly don't have to, squash in the below. I.e. this
is the last user of eval_ngettext!

I mentioned in <[email protected]> that I'd recently
come up with pretty much the same diff, that was because I was seeing
how much of the shellscript gettext support I could rip out.

This is a major step to getting a whole section of git's shellscript
support infrastructure out-of-tree, i.e. git-sh-i18n.sh, and the
sh-i18n--envsubst built-in helper.

If I then merge mirucam/git-bisect-work-part4-v6 &
gitster/ar/submodule-run-update-procedure along with this we'll have a
relatively small amount of gettext-in-sh code left, which should be
entirely gone before too long. Yay!

diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
index e3d9f4836db..a15c0620db6 100644
--- a/git-sh-i18n.sh
+++ b/git-sh-i18n.sh
@@ -51,12 +51,6 @@ gettext_without_eval_gettext)
 		)
 	}
 
-	eval_ngettext () {
-		ngettext "$1" "$2" "$3" | (
-			export PATH $(git sh-i18n--envsubst --variables "$2");
-			git sh-i18n--envsubst "$2"
-		)
-	}
 	;;
 *)
 	gettext () {
@@ -70,12 +64,6 @@ gettext_without_eval_gettext)
 		)
 	}
 
-	eval_ngettext () {
-		(test "$3" = 1 && printf "%s" "$1" || printf "%s" "$2") | (
-			export PATH $(git sh-i18n--envsubst --variables "$2");
-			git sh-i18n--envsubst "$2"
-		)
-	}
 	;;
 esac
 

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):


On Thu, Sep 02 2021, Ævar Arnfjörð Bjarmason wrote:

> On Wed, Sep 01 2021, Johannes Schindelin via GitGitGadget wrote:
>
>>  git-rebase--preserve-merges.sh | 1057 --------------------------------
>
> You could, but certainly don't have to, squash in the below. I.e. this
> is the last user of eval_ngettext!

Also get_author_ident_from_commit() in git-sh-setup.sh and
Documentation/git-sh-setup.txt can be removed,
git-rebase--preserve-merges.sh is the last user of it.

I looked over the other git-sh-setup functions, it seems that's the only
one that could be removed along with git-rebase--preserve-merges.sh.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):


On Thu, Sep 02 2021, Ævar Arnfjörð Bjarmason wrote:

> On Thu, Sep 02 2021, Ævar Arnfjörð Bjarmason wrote:
>
>> On Wed, Sep 01 2021, Johannes Schindelin via GitGitGadget wrote:
>>
>>>  git-rebase--preserve-merges.sh | 1057 --------------------------------
>>
>> You could, but certainly don't have to, squash in the below. I.e. this
>> is the last user of eval_ngettext!
>
> Also get_author_ident_from_commit() in git-sh-setup.sh and
> Documentation/git-sh-setup.txt can be removed,
> git-rebase--preserve-merges.sh is the last user of it.
>
> I looked over the other git-sh-setup functions, it seems that's the only
> one that could be removed along with git-rebase--preserve-merges.sh.

Although with git-rebase--preserve-merges.sh gone the
get_author_ident_from_commit(), parse_ident_from_commit() and
pick_ident_script() can all be moved to its last user:
git-filter-branch.sh

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Johannes Schindelin wrote (reply to this):

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323328-1550222341-1630784513=:55
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Hi =C3=86var,

On Thu, 2 Sep 2021, =C3=86var Arnfj=C3=B6r=C3=B0 Bjarmason wrote:

> On Wed, Sep 01 2021, Johannes Schindelin via GitGitGadget wrote:
>
> >  git-rebase--preserve-merges.sh | 1057 -------------------------------=
-
>
> You could, but certainly don't have to, squash in the below. I.e. this
> is the last user of eval_ngettext!

s/last user/last in-tree user/

Since we install `git-sh-i18n` and semi-advertised it as something to use
in user scripts (which makes removing it somewhat questionable a goal,
certainly when you do not even offer a deprecation period), and since the
removal of such things is completely orthogonal to the intention of this
patch series, I will not include this patch, let alone squash it into a
commit whose purpose has nothing to do with gettext whatsoever.

Ciao,
Johannes

--8323328-1550222341-1630784513=:55--

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):


On Sat, Sep 04 2021, Johannes Schindelin wrote:

> Hi Ævar,
>
> On Thu, 2 Sep 2021, Ævar Arnfjörð Bjarmason wrote:
>
>> On Wed, Sep 01 2021, Johannes Schindelin via GitGitGadget wrote:
>>
>> >  git-rebase--preserve-merges.sh | 1057 --------------------------------
>>
>> You could, but certainly don't have to, squash in the below. I.e. this
>> is the last user of eval_ngettext!
>
> s/last user/last in-tree user/
>
> Since we install `git-sh-i18n` and semi-advertised it as something to use
> in user scripts (which makes removing it somewhat questionable a goal,
> certainly when you do not even offer a deprecation period), and since the
> removal of such things is completely orthogonal to the intention of this
> patch series, I will not include this patch, let alone squash it into a
> commit whose purpose has nothing to do with gettext whatsoever.

Unlike whatever controversy we're having over git-sh-setup being removed
(see <[email protected]>[1]), the
git-sh-i18n's gettext() and eval_gettext() can't be useful to anyone out
of git.git's tree, since they accept strings that are expected to be
found in git.git's generated *.mo files.

So it's certainly possible that someone's used out out-of-tree, but that
use of eval_gettext() won't have been doing anything useful in terms of
translation, and if it did that would have been because someone copied a
to-be-translated string as-is, and expected git.git to keep it
byte-for-byte the same as their sources.

Anyway, if you don't want to squash this in I can submit it as some sort
of follow-up, perhaps along with some of the suggestions in
<[email protected]>[2] if you're generally
aiming to keep the changes here to the depth of the first callstack,
i.e. remove the --preserve-merges use of a foo(), but not a foo()
function itself if that foo() then becomes orphaned as a result.

1. https://lore.kernel.org/git/[email protected]/
2. https://lore.kernel.org/git/[email protected]/

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

Ævar Arnfjörð Bjarmason <[email protected]> writes:

> ... the
> git-sh-i18n's gettext() and eval_gettext() can't be useful to anyone out
> of git.git's tree, since they accept strings that are expected to be
> found in git.git's generated *.mo files.

True but removing it is probably outside the scope of this series.
Removal of helper functions from sh-i18n can be discussed once the
dust from this topic settles ("now the last caller of the helper is
gone from 'master', we can remove X, Y and Z").

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):


On Tue, Sep 07 2021, Johannes Schindelin via GitGitGadget wrote:

> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 33e09619005..5f8d9f89ba4 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -48,8 +48,7 @@ static GIT_PATH_FUNC(merge_dir, "rebase-merge")
>  enum rebase_type {
>  	REBASE_UNSPECIFIED = -1,
>  	REBASE_APPLY,
> -	REBASE_MERGE,
> -	REBASE_PRESERVE_MERGES
> +	REBASE_MERGE
>  };

This definitely doesn't require a re-roll, but just in case you didn't
know, from CodingGuidelines:

   . since early 2012 with e1327023ea, we have been using an enum
     definition whose last element is followed by a comma.  This, like
     an array initializer that ends with a trailing comma, can be used
     to reduce the patch noise when adding a new identifier at the end.

(That was added in cc0c42975a2 (CodingGuidelines: spell out post-C89
rules, 2019-07-16))

I.e. in case you're slavisly following this particular bit of C syntax
for C89 compatibility it's not needed anymore, which helps to make diffs
smaller, and writing code generation loops less annoying.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):

On Tue, Sep 07 2021, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <[email protected]>
> [...]
> --p::
> ---preserve-merges::
> -	[DEPRECATED: use `--rebase-merges` instead] Recreate merge commits
> -	instead of flattening the history by replaying commits a merge commit
> -	introduces. Merge conflict resolutions or manual amendments to merge
> -	commits are not preserved.

[In reply to an old commit]

I opened "man git-rebase" today due to an on-list discussion and went
through pretty much:

 1. /preserve-merges # fails
 2. skimming the SYNOPSIS, forgetting what the new thing is called
 3. Paging down, eventually findinging & remembering the new thing is
    "--rebase-merges".

I wonder if there's objections to reinstating this in the docs
somewhere, just as something like:

	--preserve-merges:
		An old "rebase" backend which is no longer supported,
		and which was removed from git in version v2.35.0.

We don't do that with all flags that we've dropped, but perhaps this one
was well known enough to not leave readers hanging...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

Ævar Arnfjörð Bjarmason <[email protected]> writes:

> On Tue, Sep 07 2021, Johannes Schindelin via GitGitGadget wrote:
>
>> From: Johannes Schindelin <[email protected]>
>> [...]
>> --p::
>> ---preserve-merges::
>> -	[DEPRECATED: use `--rebase-merges` instead] Recreate merge commits
>> -	instead of flattening the history by replaying commits a merge commit
>> -	introduces. Merge conflict resolutions or manual amendments to merge
>> -	commits are not preserved.
>
> [In reply to an old commit]
>
> I opened "man git-rebase" today due to an on-list discussion and went
> through pretty much:
>
>  1. /preserve-merges # fails
>  2. skimming the SYNOPSIS, forgetting what the new thing is called
>  3. Paging down, eventually findinging & remembering the new thing is
>     "--rebase-merges".
>
> I wonder if there's objections to reinstating this in the docs
> somewhere, just as something like:
>
> 	--preserve-merges:
> 		An old "rebase" backend which is no longer supported,
> 		and which was removed from git in version v2.35.0.
>
> We don't do that with all flags that we've dropped, but perhaps this one
> was well known enough to not leave readers hanging...

My impression is that we consider that we have done so already for a
few releases by keeping "DEPRECATED: use rebase-merges", exactly
because "this one was well known enough", and now it is time to go
one step further, i.e. drop it from the document like the quoted
patch does, while recognising an attempt to use the option and
giving a custom message than the bog-standard "unknown option".

    $ git rebase --preserve-merges
    fatal: --preserve-merges was replaced by --rebase-merges
    Note: Your `pull.rebase` configuration may also be set to 'preserve',
    which is no longer supported; use 'merges' instead

The next step will be to drop that custom error support, I think.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Johannes Schindelin wrote (reply to this):

Hi Junio,

On Thu, 21 Jul 2022, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <[email protected]> writes:
>
> > On Tue, Sep 07 2021, Johannes Schindelin via GitGitGadget wrote:
> >
> >> From: Johannes Schindelin <[email protected]>
> >> [...]
> >> --p::
> >> ---preserve-merges::
> >> -	[DEPRECATED: use `--rebase-merges` instead] Recreate merge commits
> >> -	instead of flattening the history by replaying commits a merge commit
> >> -	introduces. Merge conflict resolutions or manual amendments to merge
> >> -	commits are not preserved.
> >
> > [In reply to an old commit]
> >
> > I opened "man git-rebase" today due to an on-list discussion and went
> > through pretty much:
> >
> >  1. /preserve-merges # fails
> >  2. skimming the SYNOPSIS, forgetting what the new thing is called
> >  3. Paging down, eventually findinging & remembering the new thing is
> >     "--rebase-merges".
> >
> > I wonder if there's objections to reinstating this in the docs
> > somewhere, just as something like:
> >
> > 	--preserve-merges:
> > 		An old "rebase" backend which is no longer supported,
> > 		and which was removed from git in version v2.35.0.
> >
> > We don't do that with all flags that we've dropped, but perhaps this one
> > was well known enough to not leave readers hanging...
>
> My impression is that we consider that we have done so already for a
> few releases by keeping "DEPRECATED: use rebase-merges", exactly
> because "this one was well known enough", and now it is time to go
> one step further, i.e. drop it from the document like the quoted
> patch does, while recognising an attempt to use the option and
> giving a custom message than the bog-standard "unknown option".
>
>     $ git rebase --preserve-merges
>     fatal: --preserve-merges was replaced by --rebase-merges
>     Note: Your `pull.rebase` configuration may also be set to 'preserve',
>     which is no longer supported; use 'merges' instead
>
> The next step will be to drop that custom error support, I think.

Fully agree.

I _could_ see us introducing a sentence in the explanation of
`--rebase-merges` that leaves a historical note about superseding the
now-removed `--preserve-merges` option. But such historical notes tend to
go pretty stale pretty quickly, and eventually cause more confusion than
clarification.

So just like you said, I'd rather not re-introduce any text mentioning
`--preserve-merges` into the manual page.

Ciao,
Dscho

/git-read-tree
/git-rebase
/git-rebase--preserve-merges
/git-receive-pack
/git-reflog
/git-remote
Expand Down
4 changes: 0 additions & 4 deletions Documentation/config/branch.txt
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,6 @@ When `merges` (or just 'm'), pass the `--rebase-merges` option to 'git rebase'
so that the local merge commits are included in the rebase (see
linkgit:git-rebase[1] for details).
+
When `preserve` (or just 'p', deprecated in favor of `merges`), also pass
`--preserve-merges` along to 'git rebase' so that locally committed merge
commits will not be flattened by running 'git pull'.
+
When the value is `interactive` (or just 'i'), the rebase is run in interactive
mode.
+
Expand Down
4 changes: 0 additions & 4 deletions Documentation/config/pull.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ When `merges` (or just 'm'), pass the `--rebase-merges` option to 'git rebase'
so that the local merge commits are included in the rebase (see
linkgit:git-rebase[1] for details).
+
When `preserve` (or just 'p', deprecated in favor of `merges`), also pass
`--preserve-merges` along to 'git rebase' so that locally committed merge
commits will not be flattened by running 'git pull'.
+
When the value is `interactive` (or just 'i'), the rebase is run in interactive
mode.
+
Expand Down
6 changes: 1 addition & 5 deletions Documentation/git-pull.txt
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ Options related to merging
include::merge-options.txt[]

-r::
--rebase[=false|true|merges|preserve|interactive]::
--rebase[=false|true|merges|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
Expand All @@ -113,10 +113,6 @@ When set to `merges`, rebase using `git rebase --rebase-merges` so that
the local merge commits are included in the rebase (see
linkgit:git-rebase[1] for details).
+
When set to `preserve` (deprecated in favor of `merges`), rebase with the
`--preserve-merges` option passed to `git rebase` so that locally created
merge commits will not be flattened.
+
When false, merge the upstream branch into the current branch.
+
When `interactive`, enable the interactive mode of rebase.
Expand Down
51 changes: 0 additions & 51 deletions Documentation/git-rebase.txt
Original file line number Diff line number Diff line change
Expand Up @@ -525,29 +525,12 @@ i.e. commits that would be excluded by linkgit:git-log[1]'s
the `rebase-cousins` mode is turned on, such commits are instead rebased
onto `<upstream>` (or `<onto>`, if specified).
+
The `--rebase-merges` mode is similar in spirit to the deprecated
`--preserve-merges` but works with interactive rebases,
where commits can be reordered, inserted and dropped at will.
+
It is currently only possible to recreate the merge commits using the
`recursive` merge strategy; Different merge strategies can be used only via
explicit `exec git merge -s <strategy> [...]` commands.
+
See also REBASING MERGES and INCOMPATIBLE OPTIONS below.

-p::
--preserve-merges::
[DEPRECATED: use `--rebase-merges` instead] Recreate merge commits
instead of flattening the history by replaying commits a merge commit
introduces. Merge conflict resolutions or manual amendments to merge
commits are not preserved.
+
This uses the `--interactive` machinery internally, but combining it
with the `--interactive` option explicitly is generally not a good
idea unless you know what you are doing (see BUGS below).
+
See also INCOMPATIBLE OPTIONS below.

-x <cmd>::
--exec <cmd>::
Append "exec <cmd>" after each line creating a commit in the
Expand Down Expand Up @@ -579,9 +562,6 @@ See also INCOMPATIBLE OPTIONS below.
the root commit(s) on a branch. When used with --onto, it
will skip changes already contained in <newbase> (instead of
<upstream>) whereas without --onto it will operate on every change.
When used together with both --onto and --preserve-merges,
'all' root commits will be rewritten to have <newbase> as parent
instead.
+
See also INCOMPATIBLE OPTIONS below.

Expand Down Expand Up @@ -643,7 +623,6 @@ are incompatible with the following options:
* --allow-empty-message
* --[no-]autosquash
* --rebase-merges
* --preserve-merges
* --interactive
* --exec
* --no-keep-empty
Expand All @@ -654,13 +633,6 @@ are incompatible with the following options:

In addition, the following pairs of options are incompatible:

* --preserve-merges and --interactive
* --preserve-merges and --signoff
* --preserve-merges and --rebase-merges
* --preserve-merges and --empty=
* --preserve-merges and --ignore-whitespace
* --preserve-merges and --committer-date-is-author-date
* --preserve-merges and --ignore-date
* --keep-base and --onto
* --keep-base and --root
* --fork-point and --root
Expand Down Expand Up @@ -1274,29 +1246,6 @@ CONFIGURATION
include::config/rebase.txt[]
include::config/sequencer.txt[]

BUGS
----
The todo list presented by the deprecated `--preserve-merges --interactive`
does not represent the topology of the revision graph (use `--rebase-merges`
instead). Editing commits and rewording their commit messages should work
fine, but attempts to reorder commits tend to produce counterintuitive results.
Use `--rebase-merges` in such scenarios instead.

For example, an attempt to rearrange
------------
1 --- 2 --- 3 --- 4 --- 5
------------
to
------------
1 --- 2 --- 4 --- 3 --- 5
------------
by moving the "pick 4" line will result in the following history:
------------
3
/
1 --- 2 --- 4 --- 5
------------

GIT
---
Part of the linkgit:git[1] suite
1 change: 0 additions & 1 deletion Documentation/git-svn.txt
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,6 @@ config key: svn.authorsProg
--strategy=<strategy>::
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Eric Wong wrote (reply to this):

Johannes Schindelin via GitGitGadget <[email protected]> wrote:
> We already passed the `--rebase-merges` option to `git rebase` instead,
> now we make this move permanent.

> diff --git a/git-svn.perl b/git-svn.perl
> index 4aa208ff5f..f1fa1bc7f7 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -271,7 +271,6 @@ sub _req_svn {
>  			  'fetch-all|all' => \$_fetch_all,
>  			  'dry-run|n' => \$_dry_run,
>  			  'rebase-merges|p' => \$_rebase_merges,
> -			  'preserve-merges|p' => \$_rebase_merges,
>  			  %fc_opts } ],
>  	'commit-diff' => [ \&cmd_commit_diff,
>  	                   'Commit a diff between two trees',

Nack, it breaks existing usages.   Why the urgency with removal?

I don't know a whole lot about this rebase feature in
particular, but deprecation periods should be measured in years
or even decades because of LTS distros.  Not months, especially
for things which have been around for a long while.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Johannes Schindelin wrote (reply to this):

Hi Eric,

On Sat, 23 Nov 2019, Eric Wong wrote:

> Johannes Schindelin via GitGitGadget <[email protected]> wrote:
> > We already passed the `--rebase-merges` option to `git rebase` instead=
,
> > now we make this move permanent.
>
> > diff --git a/git-svn.perl b/git-svn.perl
> > index 4aa208ff5f..f1fa1bc7f7 100755
> > --- a/git-svn.perl
> > +++ b/git-svn.perl
> > @@ -271,7 +271,6 @@ sub _req_svn {
> >  			  'fetch-all|all' =3D> \$_fetch_all,
> >  			  'dry-run|n' =3D> \$_dry_run,
> >  			  'rebase-merges|p' =3D> \$_rebase_merges,
> > -			  'preserve-merges|p' =3D> \$_rebase_merges,
> >  			  %fc_opts } ],
> >  	'commit-diff' =3D> [ \&cmd_commit_diff,
> >  	                   'Commit a diff between two trees',
>
> Nack, it breaks existing usages.   Why the urgency with removal?

Which urgency? The cover letter spells it out quite clearly that this is
not even intended for v2.25.0, which is still over 2 months out.

The reason I submitted this patch series now is so that we can avoid
inadvertent new users of the `--preserve-merges` backend.

> I don't know a whole lot about this rebase feature in
> particular, but deprecation periods should be measured in years
> or even decades because of LTS distros.  Not months, especially
> for things which have been around for a long while.

The LTS distros will not even pick up this patch. So that's a red herring.

But yes, you're right, v2.25.0 will probably be the first version to even
have the `--rebase-merges` option in `git svn`, and therefore v2.26.0
would be awfully early a time to drop `--preserve-merges` in `git svn`.
Question is whether we want to split this patch series, or just rather
wait with merging it to `master` until a year from now, or something like
that?

Ciao,
Dscho

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Eric Wong wrote (reply to this):

Johannes Schindelin <[email protected]> wrote:
> > Johannes Schindelin via GitGitGadget <[email protected]> wrote:
> > > We already passed the `--rebase-merges` option to `git rebase` instead,
> > > now we make this move permanent.
> >
> > > diff --git a/git-svn.perl b/git-svn.perl
> > > index 4aa208ff5f..f1fa1bc7f7 100755
> > > --- a/git-svn.perl
> > > +++ b/git-svn.perl
> > > @@ -271,7 +271,6 @@ sub _req_svn {
> > >  			  'fetch-all|all' => \$_fetch_all,
> > >  			  'dry-run|n' => \$_dry_run,
> > >  			  'rebase-merges|p' => \$_rebase_merges,
> > > -			  'preserve-merges|p' => \$_rebase_merges,
> > >  			  %fc_opts } ],
> > >  	'commit-diff' => [ \&cmd_commit_diff,
> > >  	                   'Commit a diff between two trees',
> >
> > Nack, it breaks existing usages.   Why the urgency with removal?
> 
> Which urgency? The cover letter spells it out quite clearly that this is
> not even intended for v2.25.0, which is still over 2 months out.

"Months" a blink of an eye when it comes to deprecations and removals.

> The reason I submitted this patch series now is so that we can avoid
> inadvertent new users of the `--preserve-merges` backend.

Then documenting it as deprecated and warning is all that's
needed.

> > I don't know a whole lot about this rebase feature in
> > particular, but deprecation periods should be measured in years
> > or even decades because of LTS distros.  Not months, especially
> > for things which have been around for a long while.
> 
> The LTS distros will not even pick up this patch. So that's a red herring.
> 
> But yes, you're right, v2.25.0 will probably be the first version to even
> have the `--rebase-merges` option in `git svn`, and therefore v2.26.0
> would be awfully early a time to drop `--preserve-merges` in `git svn`.
> Question is whether we want to split this patch series, or just rather
> wait with merging it to `master` until a year from now, or something like
> that?

Fwiw, I object to the regressions to all the other commands
(rebase/pull/remote) in this series, too, but I mainly do Perl.

--preserve-merges was only deprecated in v2.22.0 (2019-06-07).
LTS distro users are very likely on pre-v2.22.0, more likely
v2.1x.0 and maybe even v2.x.0.

Their next LTS release could be several years from now.  We
could be on git 2.[345]x.0 by then and that's when the LTS
packagers could package the next version.  LTS users are likely
to never see the entire period from v2.22.0..v2.25.0 and thus
never see a deprecation warning.

Even Debian stable (not exactly LTS, but still on the slower
side) went from v2.11.0 in Debian 9 all the way to v2.20.1
in Debian 10.  Actual LTS users will see bigger jumps.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):


On Wed, Sep 01 2021, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <[email protected]>
>
> We already passed the `--rebase-merges` option to `git rebase` instead,
> now we make this move permanent.

Unlike the cover letter's claim of deprecated since v2.22, this in
particular has only been deprecated since v2.25, i.e. your follow-up
ea8b7be1476 (git svn: stop using `rebase --preserve-merges`,
2019-11-22).

I think it's fine to do this, but at least updating the commit message
to note the difference would be good.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Johannes Schindelin wrote (reply to this):

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323328-254040796-1630591226=:55
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Hi =C3=86var,

On Wed, 1 Sep 2021, =C3=86var Arnfj=C3=B6r=C3=B0 Bjarmason wrote:

>
> On Wed, Sep 01 2021, Johannes Schindelin via GitGitGadget wrote:
>
> > From: Johannes Schindelin <[email protected]>
> >
> > We already passed the `--rebase-merges` option to `git rebase` instead=
,
> > now we make this move permanent.
>
> Unlike the cover letter's claim of deprecated since v2.22, this in
> particular has only been deprecated since v2.25, i.e. your follow-up
> ea8b7be1476 (git svn: stop using `rebase --preserve-merges`,
> 2019-11-22).
>
> I think it's fine to do this, but at least updating the commit message
> to note the difference would be good.

I will update the commit message.

Ciao,
Dscho

--8323328-254040796-1630591226=:55--

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Johannes Schindelin wrote (reply to this):

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323328-1240559053-1630591698=:55
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Hi =C3=86var,

On Wed, 1 Sep 2021, =C3=86var Arnfj=C3=B6r=C3=B0 Bjarmason wrote:

>
> On Wed, Sep 01 2021, Johannes Schindelin via GitGitGadget wrote:
>
> > From: Johannes Schindelin <[email protected]>
> >
> > We already passed the `--rebase-merges` option to `git rebase` instead=
,
> > now we make this move permanent.
>
> Unlike the cover letter's claim of deprecated since v2.22, this in
> particular has only been deprecated since v2.25, i.e. your follow-up
> ea8b7be1476 (git svn: stop using `rebase --preserve-merges`,
> 2019-11-22).
>
> I think it's fine to do this, but at least updating the commit message
> to note the difference would be good.

Thank you for being super rigorous.

Ciao,
Dscho

--8323328-1240559053-1630591698=:55--

-p::
--rebase-merges::
--preserve-merges (DEPRECATED)::
These are only used with the 'dcommit' and 'rebase' commands.
+
Passed directly to 'git rebase' when using 'dcommit' if a
Expand Down
2 changes: 0 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,6 @@ SCRIPT_SH += git-submodule.sh
SCRIPT_SH += git-web--browse.sh

SCRIPT_LIB += git-mergetool--lib
SCRIPT_LIB += git-rebase--preserve-merges
SCRIPT_LIB += git-sh-i18n
SCRIPT_LIB += git-sh-setup

Expand Down Expand Up @@ -2649,7 +2648,6 @@ XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --language=Perl \
--keyword=__ --keyword=N__ --keyword="__n:1,2"
LOCALIZED_C = $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H)
LOCALIZED_SH = $(SCRIPT_SH)
LOCALIZED_SH += git-rebase--preserve-merges.sh
LOCALIZED_SH += git-sh-setup.sh
LOCALIZED_PERL = $(SCRIPT_PERL)

Expand Down
9 changes: 3 additions & 6 deletions builtin/pull.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,8 @@
/**
* 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
* "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.
* "merges", returns REBASE_MERGES. If value is a invalid value, dies with
* a fatal error if fatal is true, otherwise returns REBASE_INVALID.
*/
static enum rebase_type parse_config_rebase(const char *key, const char *value,
int fatal)
Expand Down Expand Up @@ -126,7 +125,7 @@ static struct option pull_options[] = {
/* Options passed to git-merge or git-rebase */
OPT_GROUP(N_("Options related to merging")),
OPT_CALLBACK_F('r', "rebase", &opt_rebase,
"(false|true|merges|preserve|interactive)",
"(false|true|merges|interactive)",
N_("incorporate changes by rebasing rather than merging"),
PARSE_OPT_OPTARG, parse_opt_rebase),
OPT_PASSTHRU('n', NULL, &opt_diffstat, NULL,
Expand Down Expand Up @@ -883,8 +882,6 @@ static int run_rebase(const struct object_id *newbase,
/* Options passed to git-rebase */
if (opt_rebase == REBASE_MERGES)
strvec_push(&args, "--rebase-merges");
else if (opt_rebase == REBASE_PRESERVE)
strvec_push(&args, "--preserve-merges");
else if (opt_rebase == REBASE_INTERACTIVE)
strvec_push(&args, "--interactive");
if (opt_diffstat)
Expand Down
Loading