Hi Alex
On 21/02/2023 05:58, Alex Henrie wrote:
The purpose of the new option is to accommodate users who would like
--rebase-merges to be on by default and to facilitate possibly turning
on --rebase-merges by default without configuration in a future version
of Git.
Thanks for improving the commit message, this seems like a reasonable
change.
Signed-off-by: Alex Henrie <alexhenrie24@xxxxxxxxx>
---
Documentation/config/rebase.txt | 3 ++
Documentation/git-rebase.txt | 3 +-
builtin/rebase.c | 11 +++++
t/t3430-rebase-merges.sh | 81 +++++++++++++++++++++++++++++++++
4 files changed, 97 insertions(+), 1 deletion(-)
diff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt
index f19bd0e040..d956ec4441 100644
--- a/Documentation/config/rebase.txt
+++ b/Documentation/config/rebase.txt
@@ -67,3 +67,6 @@ rebase.rescheduleFailedExec::
rebase.forkPoint::
If set to false set `--no-fork-point` option by default.
+
+rebase.merges::
+ Default value of `--rebase-merges` option.
Below I see this takes a boolean in addition to [no-]rebase-cousins, it
would be nice to document that, especially what "true" means.
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index c98784a0d2..b02f9cbb8c 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -537,7 +537,8 @@ See also INCOMPATIBLE OPTIONS below.
by recreating the merge commits. Any resolved merge conflicts or
manual amendments in these merge commits will have to be
resolved/re-applied manually. `--no-rebase-merges` can be used to
- countermand a previous `--rebase-merges`.
+ countermand both the `rebase.merges` config option and a previous
+ `--rebase-merges`.
+
By default, or when `no-rebase-cousins` was specified, commits which do not
have `<upstream>` as direct ancestor will keep their original branch point,
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 0a8366f30f..35f3837f43 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -829,6 +829,17 @@ static int rebase_config(const char *var, const char *value, void *data)
return 0;
}
+ if (!strcmp(var, "rebase.merges")) {
+ const char *rebase_merges;
+ if (!git_config_string(&rebase_merges, var, value) &&
+ rebase_merges && *rebase_merges) {
rebase_merges is guaranteed to be non-NULL if git_config_string returns 0.
+ opts->rebase_merges = git_parse_maybe_bool(rebase_merges);
+ if (opts->rebase_merges < 0)
+ parse_merges_value(opts, rebase_merges);
+ }
+ return 0;
+ }
I think this leaks rebase_merges as git_config_string() returns a copy
despite taking a "const char*".
If we're adding a new config option that is incompatible with the apply
backend we should add some logic to error out if the config is set and
any of the apply options are given - see eddfcd8ece (rebase: provide
better error message for apply options vs. merge config, 2023-01-25)
if (!strcmp(var, "rebase.backend")) {
return git_config_string(&opts->default_backend, var, value);
}
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index b8ba323dbc..4a7193d501 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -299,6 +299,86 @@ test_expect_success '--rebase-merges="" is invalid syntax' '
test_cmp expect actual
'
+test_expect_success 'rebase.merges="" is equivalent to not passing --rebase-merges' '
+ git config rebase.merges "" &&
test_config is generally preferred as it clears the value at the end of
the test. Then you don't need the final hunk of this patch.
+ git checkout -b config-merges-blank E &&
+ git rebase C &&
+ test_cmp_graph C.. <<-\EOF
+ * B
+ * D
+ o C
+ EOF
+'
+
+test_expect_success 'rebase.merges=rebase-cousins is equivalent to --rebase-merges=rebase-cousins' '
+ git config rebase.merges rebase-cousins &&
+ git checkout -b config-rebase-cousins main &&
+ git rebase HEAD^ &&
+ test_cmp_graph HEAD^.. <<-\EOF
+ * Merge the topic branch '\''onebranch'\''
+ |\
+ | * D
+ | * G
+ |/
+ o H
+ EOF
+'
+
+test_expect_success '--no-rebase-merges overrides rebase.merges=no-rebase-cousins' '
+ git config rebase.merges no-rebase-cousins &&
+ git checkout -b override-config-no-rebase-cousins E &&
+ git rebase --no-rebase-merges C &&
+ test_cmp_graph C.. <<-\EOF
+ * B
+ * D
+ o C
+ EOF
+'
+
+test_expect_success '--rebase-merges=no-rebase-cousins overrides rebase.merges=rebase-cousins' '
+ git config rebase.merges rebase-cousins &&
+ git checkout -b override-config-rebase-cousins main &&
+ git rebase --rebase-merges=no-rebase-cousins HEAD^ &&
+ test_cmp_graph HEAD^.. <<-\EOF
+ * Merge the topic branch '\''onebranch'\''
+ |\
+ | * D
+ | * G
+ o | H
+ |/
+ o A
+ EOF
+'
+
+test_expect_success '--rebase-merges overrides rebase.merges=false' '
+ git config rebase.merges false &&
+ git checkout -b override-config-merges-false main &&
+ git rebase --rebase-merges HEAD^ &&
+ test_cmp_graph HEAD^.. <<-\EOF
+ * Merge the topic branch '\''onebranch'\''
+ |\
+ | * D
+ | * G
+ o | H
+ |/
+ o A
+ EOF
+'
+
+test_expect_success '--rebase-merges does not override rebase.merges=rebase-cousins' '
+ git config rebase.merges rebase-cousins &&
+ git checkout -b no-override-config-rebase-cousins main &&
+ git rebase --rebase-merges HEAD^ &&
+ test_cmp_graph HEAD^.. <<-\EOF
+ * Merge the topic branch '\''onebranch'\''
+ |\
+ | * D
+ | * G
+ |/
+ o H
+ EOF
+'
Thanks for the tests, I think they provide good coverage of the various
combinations of config and commandline settings
Best Wishes
Phillip
test_expect_success 'refs/rewritten/* is worktree-local' '
git worktree add wt &&
cat >wt/script-from-scratch <<-\EOF &&
@@ -409,6 +489,7 @@ test_expect_success 'a "merge" into a root commit is a fast-forward' '
'
test_expect_success 'A root commit can be a cousin, treat it that way' '
+ git config --unset rebase.merges &&
git checkout --orphan khnum &&
test_commit yama &&
git checkout -b asherah main &&
