From: Jacob Keller <jacob.keller@xxxxxxxxx>
The format.useAutoBase configuration option exists to allow users to
enable '--base=auto' for format-patch by default.
This can sometimes lead to poor workflow, due to unexpected failures
when attempting to format an ancient patch:
$ git format-patch -1 <an old commit>
fatal: base commit shouldn't be in revision list
This can be very confusing, as it is not necessarily immediately obvious
that the user requested a --base (since this was in the configuration,
not on the command line).
We do want --base=auto to fail when it cannot provide a suitable base,
as it would be equally confusing if a formatted patch did not include
the base information when it was requested.
Teach format.useAutoBase a new mode, "whenAble". This mode will cause
format-patch to attempt to include a base commit when it can. However,
if no valid base commit can be found, then format-patch will continue
formatting the patch without a base commit. --base also learns the same
mode using the term "if-able".
Add tests to cover the new mode of operation for --base.
Signed-off-by: Jacob Keller <jacob.keller@xxxxxxxxx>
---
Documentation/config/format.txt | 5 +-
Documentation/git-format-patch.txt | 4 +-
builtin/log.c | 100 ++++++++++++++++++++++-------
t/t4014-format-patch.sh | 27 ++++++++
4 files changed, 112 insertions(+), 24 deletions(-)
diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index 564e8091ba5c..e0760f16d6ad 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -96,7 +96,10 @@ format.outputDirectory::
format.useAutoBase::
A boolean value which lets you enable the `--base=auto` option of
- format-patch by default.
+ format-patch by default. Can also be set to "whenAble" to set
+ `--base=if-able`. This causes format-patch to include the base
+ commit information if it can be determined, but skip it otherwise
+ without dying.
format.notes::
Provides the default value for the `--notes` option to
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 0f81d0437bb6..b58f7cd13382 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -345,7 +345,9 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.
Record the base tree information to identify the state the
patch series applies to. See the BASE TREE INFORMATION section
below for details. If <commit> is "auto", a base commit is
- automatically chosen. The `--no-base` option overrides a
+ automatically chosen. If <commit> is "if-able", a base commit is
+ included if available, however format-patch won't die if it cannot
+ find a valid base commit. The `--no-base` option overrides a
`format.useAutoBase` configuration.
--root::
diff --git a/builtin/log.c b/builtin/log.c
index b8824d898f49..3b15d3cb87ea 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -805,9 +805,15 @@ enum cover_from_description {
COVER_FROM_AUTO
};
+enum auto_base_setting {
+ AUTO_BASE_NEVER,
+ AUTO_BASE_ALWAYS,
+ AUTO_BASE_WHEN_ABLE
+};
+
static enum thread_level thread;
static int do_signoff;
-static int base_auto;
+static enum auto_base_setting auto_base;
static char *from;
static const char *signature = git_version_string;
static const char *signature_file;
@@ -906,7 +912,11 @@ static int git_format_config(const char *var, const char *value, void *cb)
if (!strcmp(var, "format.outputdirectory"))
return git_config_string(&config_output_directory, var, value);
if (!strcmp(var, "format.useautobase")) {
- base_auto = git_config_bool(var, value);
+ if (value && !strcasecmp(value, "whenAble")) {
+ auto_base = AUTO_BASE_WHEN_ABLE;
+ return 0;
+ }
+ auto_base = git_config_bool(var, value) ? AUTO_BASE_ALWAYS : AUTO_BASE_NEVER;
return 0;
}
if (!strcmp(var, "format.from")) {
@@ -1436,13 +1446,24 @@ static struct commit *get_base_commit(const char *base_commit,
{
struct commit *base = NULL;
struct commit **rev;
- int i = 0, rev_nr = 0;
+ int i = 0, rev_nr = 0, auto_select, die_on_failure;
- if (base_commit && strcmp(base_commit, "auto")) {
+ if (!strcmp(base_commit, "auto")) {
+ auto_select = 1;
+ die_on_failure = 1;
+ } else if (!strcmp(base_commit, "if-able")) {
+ auto_select = 1;
+ die_on_failure = 0;
+ } else {
+ auto_select = 0;
+ die_on_failure = 1;
+ }
+
+ if (!auto_select) {
base = lookup_commit_reference_by_name(base_commit);
if (!base)
die(_("unknown commit %s"), base_commit);
- } else if ((base_commit && !strcmp(base_commit, "auto"))) {
+ } else {
struct branch *curr_branch = branch_get(NULL);
const char *upstream = branch_get_upstream(curr_branch, NULL);
if (upstream) {
@@ -1450,19 +1471,32 @@ static struct commit *get_base_commit(const char *base_commit,
struct commit *commit;
struct object_id oid;
- if (get_oid(upstream, &oid))
- die(_("failed to resolve '%s' as a valid ref"), upstream);
+ if (get_oid(upstream, &oid)) {
+ if (die_on_failure)
+ die(_("failed to resolve '%s' as a valid ref"), upstream);
+ else
+ return NULL;
+ }
commit = lookup_commit_or_die(&oid, "upstream base");
base_list = get_merge_bases_many(commit, total, list);
/* There should be one and only one merge base. */
- if (!base_list || base_list->next)
- die(_("could not find exact merge base"));
+ if (!base_list || base_list->next) {
+ if (die_on_failure) {
+ die(_("could not find exact merge base"));
+ } else {
+ free_commit_list(base_list);
+ return NULL;
+ }
+ }
base = base_list->item;
free_commit_list(base_list);
} else {
- die(_("failed to get upstream, if you want to record base commit automatically,\n"
- "please use git branch --set-upstream-to to track a remote branch.\n"
- "Or you could specify base commit by --base=<base-commit-id> manually"));
+ if (die_on_failure)
+ die(_("failed to get upstream, if you want to record base commit automatically,\n"
+ "please use git branch --set-upstream-to to track a remote branch.\n"
+ "Or you could specify base commit by --base=<base-commit-id> manually"));
+ else
+ return NULL;
}
}
@@ -1479,8 +1513,14 @@ static struct commit *get_base_commit(const char *base_commit,
for (i = 0; i < rev_nr / 2; i++) {
struct commit_list *merge_base;
merge_base = get_merge_bases(rev[2 * i], rev[2 * i + 1]);
- if (!merge_base || merge_base->next)
- die(_("failed to find exact merge base"));
+ if (!merge_base || merge_base->next) {
+ if (die_on_failure) {
+ die(_("failed to find exact merge base"));
+ } else {
+ free(rev);
+ return NULL;
+ }
+ }
rev[i] = merge_base->item;
}
@@ -1490,12 +1530,24 @@ static struct commit *get_base_commit(const char *base_commit,
rev_nr = DIV_ROUND_UP(rev_nr, 2);
}
- if (!in_merge_bases(base, rev[0]))
- die(_("base commit should be the ancestor of revision list"));
+ if (!in_merge_bases(base, rev[0])) {
+ if (die_on_failure) {
+ die(_("base commit should be the ancestor of revision list"));
+ } else {
+ free(rev);
+ return NULL;
+ }
+ }
for (i = 0; i < total; i++) {
- if (base == list[i])
- die(_("base commit shouldn't be in revision list"));
+ if (base == list[i]) {
+ if (die_on_failure) {
+ die(_("base commit shouldn't be in revision list"));
+ } else {
+ free(rev);
+ return NULL;
+ }
+ }
}
free(rev);
@@ -1752,8 +1804,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
s_r_opt.def = "HEAD";
s_r_opt.revarg_opt = REVARG_COMMITTISH;
- if (base_auto)
+ if (auto_base == AUTO_BASE_ALWAYS)
base_commit = "auto";
+ else if (auto_base == AUTO_BASE_WHEN_ABLE)
+ base_commit = "if-able";
if (default_attach) {
rev.mime_boundary = default_attach;
@@ -2020,9 +2074,11 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
memset(&bases, 0, sizeof(bases));
if (base_commit) {
struct commit *base = get_base_commit(base_commit, list, nr);
- reset_revision_walk();
- clear_object_flags(UNINTERESTING);
- prepare_bases(&bases, base, list, nr);
+ if (base) {
+ reset_revision_walk();
+ clear_object_flags(UNINTERESTING);
+ prepare_bases(&bases, base, list, nr);
+ }
}
if (in_reply_to || thread || cover_letter)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 958c2da56ec6..0c14619293bb 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -2037,6 +2037,17 @@ test_expect_success 'format-patch errors out when history involves criss-cross'
test_must_fail git format-patch --base=auto -1
'
+test_expect_success 'format-patch disable base=if-able when history involves criss-cross' '
+ git format-patch --base=if-able -1 >patch &&
+ ! grep "^base-commit:" patch
+'
+
+test_expect_success 'format-patch format.useAutoBase whenAble history involves criss-cross' '
+ test_config format.useAutoBase whenAble &&
+ git format-patch -1 >patch &&
+ ! grep "^base-commit:" patch
+'
+
test_expect_success 'format-patch format.useAutoBase option' '
git checkout local &&
test_config format.useAutoBase true &&
@@ -2047,6 +2058,16 @@ test_expect_success 'format-patch format.useAutoBase option' '
test_cmp expect actual
'
+test_expect_success 'format-patch format.useAutoBase option with whenAble' '
+ git checkout local &&
+ test_config format.useAutoBase whenAble &&
+ git format-patch --stdout -1 >patch &&
+ grep "^base-commit:" patch >actual &&
+ git rev-parse upstream >commit-id-base &&
+ echo "base-commit: $(cat commit-id-base)" >expect &&
+ test_cmp expect actual
+'
+
test_expect_success 'format-patch --base overrides format.useAutoBase' '
test_config format.useAutoBase true &&
git format-patch --stdout --base=HEAD~1 -1 >patch &&
@@ -2062,6 +2083,12 @@ test_expect_success 'format-patch --no-base overrides format.useAutoBase' '
! grep "^base-commit:" patch
'
+test_expect_success 'format-patch --no-base overrides format.useAutoBase whenAble' '
+ test_config format.useAutoBase whenAble &&
+ git format-patch --stdout --no-base -1 >patch &&
+ ! grep "^base-commit:" patch
+'
+
test_expect_success 'format-patch --base with --attach' '
git format-patch --attach=mimemime --stdout --base=HEAD~ -1 >patch &&
sed -n -e "/^base-commit:/s/.*/1/p" -e "/^---*mimemime--$/s/.*/2/p" \
--
2.28.0.497.g54e85e7af1ac
