From: Jacob Keller <jacob.keller@xxxxxxxxx>
This is a v2 of [1] to address comments on the mailing list. Primarily the
change is to remove the option for --base=if-able, since it is not as useful
as the configuration option. If we think it is desirable, adding back a
whenable or whenAble option is easy enough.
Here's the range diff since the v1.
1: 3c7f89213158 ! 1: cb24cf3fe8b5 format-patch: teach format.useAutoBase "whenAble" option
@@ Commit message
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".
+ formatting the patch without a base commit.
+
+ In order to avoid making yet another branch name unusable with --base,
+ do not teach --base=whenAble or --base=whenable.
+
+ Instead, refactor the base_commit option to use a callback, and rely on
+ the global configuration variable auto_base.
+
+ This does mean that a user cannot request this optional base commit
+ generation from the command line. However, this is likely not too
+ valuable. If the user requests base information manually, they will be
+ immediately informed of the failure to acquire a suitable base commit.
+ This allows the user to make an informed choice about whether to
+ continue the format.
Add tests to cover the new mode of operation for --base.
Signed-off-by: Jacob Keller <jacob.keller@xxxxxxxxx>
- Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
## Documentation/config/format.txt ##
@@ Documentation/config/format.txt: format.outputDirectory::
@@ Documentation/config/format.txt: 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-patch by default. Can also be set to "whenAble" to allow
++ enabling `--base=auto` if a suitable base is available, but to skip
++ adding base info otherwise without the format dying.
format.notes::
Provides the default value for the `--notes` option to
- ## Documentation/git-format-patch.txt ##
-@@ Documentation/git-format-patch.txt: 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::
-
## builtin/log.c ##
@@ builtin/log.c: enum cover_from_description {
COVER_FROM_AUTO
@@ builtin/log.c: static int git_format_config(const char *var, const char *value,
return 0;
}
if (!strcmp(var, "format.from")) {
+@@ builtin/log.c: static int from_callback(const struct option *opt, const char *arg, int unset)
+ return 0;
+ }
+
++static int base_callback(const struct option *opt, const char *arg, int unset)
++{
++ const char **base_commit = opt->value;
++
++ if (unset) {
++ auto_base = AUTO_BASE_NEVER;
++ *base_commit = NULL;
++ } else if (!strcmp(arg, "auto")) {
++ auto_base = AUTO_BASE_ALWAYS;
++ *base_commit = NULL;
++ } else {
++ auto_base = AUTO_BASE_NEVER;
++ *base_commit = arg;
++ }
++ return 0;
++}
++
+ struct base_tree_info {
+ struct object_id base_commit;
+ int nr_patch_id, alloc_patch_id;
@@ builtin/log.c: static struct commit *get_base_commit(const char *base_commit,
{
struct commit *base = NULL;
@@ builtin/log.c: static struct commit *get_base_commit(const char *base_commit,
+ 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;
++ switch (auto_base) {
++ case AUTO_BASE_NEVER:
++ if (base_commit) {
++ auto_select = 0;
++ die_on_failure = 1;
++ } else {
++ /* no base information is requested */
++ return NULL;
++ }
++ break;
++ case AUTO_BASE_ALWAYS:
++ case AUTO_BASE_WHEN_ABLE:
++ if (base_commit) {
++ BUG("requested automatic base selection but a commit was provided");
++ } else {
++ auto_select = 1;
++ die_on_failure = auto_base == AUTO_BASE_ALWAYS;
++ }
++ break;
++ default:
++ BUG("unexpected automatic base selection method");
+ }
+
+ if (!auto_select) {
@@ builtin/log.c: static struct commit *get_base_commit(const char *base_commit,
}
free(rev);
+@@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *prefix)
+ char *branch_name = NULL;
+ char *base_commit = NULL;
+ struct base_tree_info bases;
++ struct commit *base;
+ int show_progress = 0;
+ struct progress *progress = NULL;
+ struct oid_array idiff_prev = OID_ARRAY_INIT;
+@@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *prefix)
+ PARSE_OPT_OPTARG, thread_callback),
+ OPT_STRING(0, "signature", &signature, N_("signature"),
+ N_("add a signature")),
+- OPT_STRING(0, "base", &base_commit, N_("base-commit"),
+- N_("add prerequisite tree info to the patch series")),
++ OPT_CALLBACK_F(0, "base", &base_commit, N_("base-commit"),
++ N_("add prerequisite tree info to the patch series"),
++ 0, base_callback),
+ OPT_FILENAME(0, "signature-file", &signature_file,
+ N_("add a signature from a file")),
+ OPT__QUIET(&quiet, N_("don't print the patch filenames")),
@@ builtin/log.c: 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";
-
+- base_commit = "auto";
+-
if (default_attach) {
rev.mime_boundary = default_attach;
+ rev.no_inline = 1;
@@ builtin/log.c: 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)
+ memset(&bases, 0, sizeof(bases));
+- if (base_commit) {
+- struct commit *base = get_base_commit(base_commit, list, nr);
++ base = get_base_commit(base_commit, list, nr);
++ if (base) {
+ reset_revision_walk();
+ clear_object_flags(UNINTERESTING);
+ prepare_bases(&bases, base, list, nr);
## t/t4014-format-patch.sh ##
@@ t/t4014-format-patch.sh: 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 &&
Jacob Keller (1):
format-patch: teach format.useAutoBase "whenAble" option
Documentation/config/format.txt | 4 +-
builtin/log.c | 130 ++++++++++++++++++++++++++------
t/t4014-format-patch.sh | 22 ++++++
3 files changed, 130 insertions(+), 26 deletions(-)
--
2.28.0.497.g54e85e7af1ac
