{"thread":{"id":"54334","subject":"[PATCH v2 0/1] teach format.useAutoBase \"whenAble\" option","startedAt":"2020-10-01T21:47:57Z","lastAt":"2020-10-01T22:22:04Z","messageCount":3,"participants":["Jacob Keller","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":1},"messages":[{"id":"406791","messageId":"20201001214653.1609405-1-jacob.e.keller@intel.com","threadId":"54334","inReplyTo":null,"subject":"[PATCH v2 0/1] teach format.useAutoBase \"whenAble\" option","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2020-10-01T21:46:52Z","receivedAt":"2020-10-01T21:47:57Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nThis is a v2 of [1] to address comments on the mailing list. Primarily the\nchange is to remove the option for --base=if-able, since it is not as useful\nas the configuration option. If we think it is desirable, adding back a\nwhenable or whenAble option is easy enough.\n\nHere's the range diff since the v1.\n\n1:  3c7f89213158 !  1:  cb24cf3fe8b5 format-patch: teach format.useAutoBase \"whenAble\" option\n    @@ Commit message\n         Teach format.useAutoBase a new mode, \"whenAble\". This mode will cause\n         format-patch to attempt to include a base commit when it can. However,\n         if no valid base commit can be found, then format-patch will continue\n    -    formatting the patch without a base commit. --base also learns the same\n    -    mode using the term \"if-able\".\n    +    formatting the patch without a base commit.\n    +\n    +    In order to avoid making yet another branch name unusable with --base,\n    +    do not teach --base=whenAble or --base=whenable.\n    +\n    +    Instead, refactor the base_commit option to use a callback, and rely on\n    +    the global configuration variable auto_base.\n    +\n    +    This does mean that a user cannot request this optional base commit\n    +    generation from the command line. However, this is likely not too\n    +    valuable. If the user requests base information manually, they will be\n    +    immediately informed of the failure to acquire a suitable base commit.\n    +    This allows the user to make an informed choice about whether to\n    +    continue the format.\n     \n         Add tests to cover the new mode of operation for --base.\n     \n         Signed-off-by: Jacob Keller <jacob.keller@gmail.com>\n    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>\n     \n      ## Documentation/config/format.txt ##\n     @@ Documentation/config/format.txt: format.outputDirectory::\n    @@ Documentation/config/format.txt: format.outputDirectory::\n      format.useAutoBase::\n      \tA boolean value which lets you enable the `--base=auto` option of\n     -\tformat-patch by default.\n    -+\tformat-patch by default. Can also be set to \"whenAble\" to set\n    -+\t`--base=if-able`. This causes format-patch to include the base\n    -+\tcommit information if it can be determined, but skip it otherwise\n    -+\twithout dying.\n    ++\tformat-patch by default. Can also be set to \"whenAble\" to allow\n    ++\tenabling `--base=auto` if a suitable base is available, but to skip\n    ++\tadding base info otherwise without the format dying.\n      \n      format.notes::\n      \tProvides the default value for the `--notes` option to\n     \n    - ## Documentation/git-format-patch.txt ##\n    -@@ Documentation/git-format-patch.txt: you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.\n    - \tRecord the base tree information to identify the state the\n    - \tpatch series applies to.  See the BASE TREE INFORMATION section\n    - \tbelow for details. If <commit> is \"auto\", a base commit is\n    --\tautomatically chosen. The `--no-base` option overrides a\n    -+\tautomatically chosen. If <commit> is \"if-able\", a base commit is\n    -+\tincluded if available, however format-patch won't die if it cannot\n    -+\tfind a valid base commit. The `--no-base` option overrides a\n    - \t`format.useAutoBase` configuration.\n    - \n    - --root::\n    -\n      ## builtin/log.c ##\n     @@ builtin/log.c: enum cover_from_description {\n      \tCOVER_FROM_AUTO\n    @@ builtin/log.c: static int git_format_config(const char *var, const char *value,\n      \t\treturn 0;\n      \t}\n      \tif (!strcmp(var, \"format.from\")) {\n    +@@ builtin/log.c: static int from_callback(const struct option *opt, const char *arg, int unset)\n    + \treturn 0;\n    + }\n    + \n    ++static int base_callback(const struct option *opt, const char *arg, int unset)\n    ++{\n    ++\tconst char **base_commit = opt->value;\n    ++\n    ++\tif (unset) {\n    ++\t\tauto_base = AUTO_BASE_NEVER;\n    ++\t\t*base_commit = NULL;\n    ++\t} else if (!strcmp(arg, \"auto\")) {\n    ++\t\tauto_base = AUTO_BASE_ALWAYS;\n    ++\t\t*base_commit = NULL;\n    ++\t} else {\n    ++\t\tauto_base = AUTO_BASE_NEVER;\n    ++\t\t*base_commit = arg;\n    ++\t}\n    ++\treturn 0;\n    ++}\n    ++\n    + struct base_tree_info {\n    + \tstruct object_id base_commit;\n    + \tint nr_patch_id, alloc_patch_id;\n     @@ builtin/log.c: static struct commit *get_base_commit(const char *base_commit,\n      {\n      \tstruct commit *base = NULL;\n    @@ builtin/log.c: static struct commit *get_base_commit(const char *base_commit,\n     +\tint i = 0, rev_nr = 0, auto_select, die_on_failure;\n      \n     -\tif (base_commit && strcmp(base_commit, \"auto\")) {\n    -+\tif (!strcmp(base_commit, \"auto\")) {\n    -+\t\tauto_select = 1;\n    -+\t\tdie_on_failure = 1;\n    -+\t} else if (!strcmp(base_commit, \"if-able\")) {\n    -+\t\tauto_select = 1;\n    -+\t\tdie_on_failure = 0;\n    -+\t} else {\n    -+\t\tauto_select = 0;\n    -+\t\tdie_on_failure = 1;\n    ++\tswitch (auto_base) {\n    ++\tcase AUTO_BASE_NEVER:\n    ++\t\tif (base_commit) {\n    ++\t\t\tauto_select = 0;\n    ++\t\t\tdie_on_failure = 1;\n    ++\t\t} else {\n    ++\t\t\t/* no base information is requested */\n    ++\t\t\treturn NULL;\n    ++\t\t}\n    ++\t\tbreak;\n    ++\tcase AUTO_BASE_ALWAYS:\n    ++\tcase AUTO_BASE_WHEN_ABLE:\n    ++\t\tif (base_commit) {\n    ++\t\t\tBUG(\"requested automatic base selection but a commit was provided\");\n    ++\t\t} else {\n    ++\t\t\tauto_select = 1;\n    ++\t\t\tdie_on_failure = auto_base == AUTO_BASE_ALWAYS;\n    ++\t\t}\n    ++\t\tbreak;\n    ++\tdefault:\n    ++\t\tBUG(\"unexpected automatic base selection method\");\n     +\t}\n     +\n     +\tif (!auto_select) {\n    @@ builtin/log.c: static struct commit *get_base_commit(const char *base_commit,\n      \t}\n      \n      \tfree(rev);\n    +@@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *prefix)\n    + \tchar *branch_name = NULL;\n    + \tchar *base_commit = NULL;\n    + \tstruct base_tree_info bases;\n    ++\tstruct commit *base;\n    + \tint show_progress = 0;\n    + \tstruct progress *progress = NULL;\n    + \tstruct oid_array idiff_prev = OID_ARRAY_INIT;\n    +@@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *prefix)\n    + \t\t\t    PARSE_OPT_OPTARG, thread_callback),\n    + \t\tOPT_STRING(0, \"signature\", &signature, N_(\"signature\"),\n    + \t\t\t    N_(\"add a signature\")),\n    +-\t\tOPT_STRING(0, \"base\", &base_commit, N_(\"base-commit\"),\n    +-\t\t\t   N_(\"add prerequisite tree info to the patch series\")),\n    ++\t\tOPT_CALLBACK_F(0, \"base\", &base_commit, N_(\"base-commit\"),\n    ++\t\t\t       N_(\"add prerequisite tree info to the patch series\"),\n    ++\t\t\t       0, base_callback),\n    + \t\tOPT_FILENAME(0, \"signature-file\", &signature_file,\n    + \t\t\t\tN_(\"add a signature from a file\")),\n    + \t\tOPT__QUIET(&quiet, N_(\"don't print the patch filenames\")),\n     @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *prefix)\n      \ts_r_opt.def = \"HEAD\";\n      \ts_r_opt.revarg_opt = REVARG_COMMITTISH;\n      \n     -\tif (base_auto)\n    -+\tif (auto_base == AUTO_BASE_ALWAYS)\n    - \t\tbase_commit = \"auto\";\n    -+\telse if (auto_base == AUTO_BASE_WHEN_ABLE)\n    -+\t\tbase_commit = \"if-able\";\n    - \n    +-\t\tbase_commit = \"auto\";\n    +-\n      \tif (default_attach) {\n      \t\trev.mime_boundary = default_attach;\n    + \t\trev.no_inline = 1;\n     @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *prefix)\n    - \tmemset(&bases, 0, sizeof(bases));\n    - \tif (base_commit) {\n    - \t\tstruct commit *base = get_base_commit(base_commit, list, nr);\n    --\t\treset_revision_walk();\n    --\t\tclear_object_flags(UNINTERESTING);\n    --\t\tprepare_bases(&bases, base, list, nr);\n    -+\t\tif (base) {\n    -+\t\t\treset_revision_walk();\n    -+\t\t\tclear_object_flags(UNINTERESTING);\n    -+\t\t\tprepare_bases(&bases, base, list, nr);\n    -+\t\t}\n      \t}\n      \n    - \tif (in_reply_to || thread || cover_letter)\n    + \tmemset(&bases, 0, sizeof(bases));\n    +-\tif (base_commit) {\n    +-\t\tstruct commit *base = get_base_commit(base_commit, list, nr);\n    ++\tbase = get_base_commit(base_commit, list, nr);\n    ++\tif (base) {\n    + \t\treset_revision_walk();\n    + \t\tclear_object_flags(UNINTERESTING);\n    + \t\tprepare_bases(&bases, base, list, nr);\n     \n      ## t/t4014-format-patch.sh ##\n     @@ t/t4014-format-patch.sh: test_expect_success 'format-patch errors out when history involves criss-cross'\n      \ttest_must_fail \tgit format-patch --base=auto -1\n      '\n      \n    -+test_expect_success 'format-patch disable base=if-able when history involves criss-cross' '\n    -+\tgit format-patch --base=if-able -1 >patch &&\n    -+\t! grep \"^base-commit:\" patch\n    -+'\n    -+\n     +test_expect_success 'format-patch format.useAutoBase whenAble history involves criss-cross' '\n     +\ttest_config format.useAutoBase whenAble &&\n     +\tgit format-patch -1 >patch &&\n\nJacob Keller (1):\n  format-patch: teach format.useAutoBase \"whenAble\" option\n\n Documentation/config/format.txt |   4 +-\n builtin/log.c                   | 130 ++++++++++++++++++++++++++------\n t/t4014-format-patch.sh         |  22 ++++++\n 3 files changed, 130 insertions(+), 26 deletions(-)\n\n-- \n2.28.0.497.g54e85e7af1ac\n\n"},{"id":"406792","messageId":"20201001214653.1609405-2-jacob.e.keller@intel.com","threadId":"54334","inReplyTo":"20201001214653.1609405-1-jacob.e.keller@intel.com","subject":"[PATCH v2 1/1] format-patch: teach format.useAutoBase \"whenAble\" option","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2020-10-01T21:46:53Z","receivedAt":"2020-10-01T21:48:00Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nThe format.useAutoBase configuration option exists to allow users to\nenable '--base=auto' for format-patch by default.\n\nThis can sometimes lead to poor workflow, due to unexpected failures\nwhen attempting to format an ancient patch:\n\n    $ git format-patch -1 <an old commit>\n    fatal: base commit shouldn't be in revision list\n\nThis can be very confusing, as it is not necessarily immediately obvious\nthat the user requested a --base (since this was in the configuration,\nnot on the command line).\n\nWe do want --base=auto to fail when it cannot provide a suitable base,\nas it would be equally confusing if a formatted patch did not include\nthe base information when it was requested.\n\nTeach format.useAutoBase a new mode, \"whenAble\". This mode will cause\nformat-patch to attempt to include a base commit when it can. However,\nif no valid base commit can be found, then format-patch will continue\nformatting the patch without a base commit.\n\nIn order to avoid making yet another branch name unusable with --base,\ndo not teach --base=whenAble or --base=whenable.\n\nInstead, refactor the base_commit option to use a callback, and rely on\nthe global configuration variable auto_base.\n\nThis does mean that a user cannot request this optional base commit\ngeneration from the command line. However, this is likely not too\nvaluable. If the user requests base information manually, they will be\nimmediately informed of the failure to acquire a suitable base commit.\nThis allows the user to make an informed choice about whether to\ncontinue the format.\n\nAdd tests to cover the new mode of operation for --base.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\n Documentation/config/format.txt |   4 +-\n builtin/log.c                   | 130 ++++++++++++++++++++++++++------\n t/t4014-format-patch.sh         |  22 ++++++\n 3 files changed, 130 insertions(+), 26 deletions(-)\n\ndiff --git a/Documentation/config/format.txt b/Documentation/config/format.txt\nindex 564e8091ba5c..c2efd8758a58 100644\n--- a/Documentation/config/format.txt\n+++ b/Documentation/config/format.txt\n@@ -96,7 +96,9 @@ format.outputDirectory::\n \n format.useAutoBase::\n \tA boolean value which lets you enable the `--base=auto` option of\n-\tformat-patch by default.\n+\tformat-patch by default. Can also be set to \"whenAble\" to allow\n+\tenabling `--base=auto` if a suitable base is available, but to skip\n+\tadding base info otherwise without the format dying.\n \n format.notes::\n \tProvides the default value for the `--notes` option to\ndiff --git a/builtin/log.c b/builtin/log.c\nindex b8824d898f49..55a4fd9eafeb 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -805,9 +805,15 @@ enum cover_from_description {\n \tCOVER_FROM_AUTO\n };\n \n+enum auto_base_setting {\n+\tAUTO_BASE_NEVER,\n+\tAUTO_BASE_ALWAYS,\n+\tAUTO_BASE_WHEN_ABLE\n+};\n+\n static enum thread_level thread;\n static int do_signoff;\n-static int base_auto;\n+static enum auto_base_setting auto_base;\n static char *from;\n static const char *signature = git_version_string;\n static const char *signature_file;\n@@ -906,7 +912,11 @@ static int git_format_config(const char *var, const char *value, void *cb)\n \tif (!strcmp(var, \"format.outputdirectory\"))\n \t\treturn git_config_string(&config_output_directory, var, value);\n \tif (!strcmp(var, \"format.useautobase\")) {\n-\t\tbase_auto = git_config_bool(var, value);\n+\t\tif (value && !strcasecmp(value, \"whenAble\")) {\n+\t\t\tauto_base = AUTO_BASE_WHEN_ABLE;\n+\t\t\treturn 0;\n+\t\t}\n+\t\tauto_base = git_config_bool(var, value) ? AUTO_BASE_ALWAYS : AUTO_BASE_NEVER;\n \t\treturn 0;\n \t}\n \tif (!strcmp(var, \"format.from\")) {\n@@ -1424,6 +1434,23 @@ static int from_callback(const struct option *opt, const char *arg, int unset)\n \treturn 0;\n }\n \n+static int base_callback(const struct option *opt, const char *arg, int unset)\n+{\n+\tconst char **base_commit = opt->value;\n+\n+\tif (unset) {\n+\t\tauto_base = AUTO_BASE_NEVER;\n+\t\t*base_commit = NULL;\n+\t} else if (!strcmp(arg, \"auto\")) {\n+\t\tauto_base = AUTO_BASE_ALWAYS;\n+\t\t*base_commit = NULL;\n+\t} else {\n+\t\tauto_base = AUTO_BASE_NEVER;\n+\t\t*base_commit = arg;\n+\t}\n+\treturn 0;\n+}\n+\n struct base_tree_info {\n \tstruct object_id base_commit;\n \tint nr_patch_id, alloc_patch_id;\n@@ -1436,13 +1463,36 @@ static struct commit *get_base_commit(const char *base_commit,\n {\n \tstruct commit *base = NULL;\n \tstruct commit **rev;\n-\tint i = 0, rev_nr = 0;\n+\tint i = 0, rev_nr = 0, auto_select, die_on_failure;\n \n-\tif (base_commit && strcmp(base_commit, \"auto\")) {\n+\tswitch (auto_base) {\n+\tcase AUTO_BASE_NEVER:\n+\t\tif (base_commit) {\n+\t\t\tauto_select = 0;\n+\t\t\tdie_on_failure = 1;\n+\t\t} else {\n+\t\t\t/* no base information is requested */\n+\t\t\treturn NULL;\n+\t\t}\n+\t\tbreak;\n+\tcase AUTO_BASE_ALWAYS:\n+\tcase AUTO_BASE_WHEN_ABLE:\n+\t\tif (base_commit) {\n+\t\t\tBUG(\"requested automatic base selection but a commit was provided\");\n+\t\t} else {\n+\t\t\tauto_select = 1;\n+\t\t\tdie_on_failure = auto_base == AUTO_BASE_ALWAYS;\n+\t\t}\n+\t\tbreak;\n+\tdefault:\n+\t\tBUG(\"unexpected automatic base selection method\");\n+\t}\n+\n+\tif (!auto_select) {\n \t\tbase = lookup_commit_reference_by_name(base_commit);\n \t\tif (!base)\n \t\t\tdie(_(\"unknown commit %s\"), base_commit);\n-\t} else if ((base_commit && !strcmp(base_commit, \"auto\"))) {\n+\t} else {\n \t\tstruct branch *curr_branch = branch_get(NULL);\n \t\tconst char *upstream = branch_get_upstream(curr_branch, NULL);\n \t\tif (upstream) {\n@@ -1450,19 +1500,32 @@ static struct commit *get_base_commit(const char *base_commit,\n \t\t\tstruct commit *commit;\n \t\t\tstruct object_id oid;\n \n-\t\t\tif (get_oid(upstream, &oid))\n-\t\t\t\tdie(_(\"failed to resolve '%s' as a valid ref\"), upstream);\n+\t\t\tif (get_oid(upstream, &oid)) {\n+\t\t\t\tif (die_on_failure)\n+\t\t\t\t\tdie(_(\"failed to resolve '%s' as a valid ref\"), upstream);\n+\t\t\t\telse\n+\t\t\t\t\treturn NULL;\n+\t\t\t}\n \t\t\tcommit = lookup_commit_or_die(&oid, \"upstream base\");\n \t\t\tbase_list = get_merge_bases_many(commit, total, list);\n \t\t\t/* There should be one and only one merge base. */\n-\t\t\tif (!base_list || base_list->next)\n-\t\t\t\tdie(_(\"could not find exact merge base\"));\n+\t\t\tif (!base_list || base_list->next) {\n+\t\t\t\tif (die_on_failure) {\n+\t\t\t\t\tdie(_(\"could not find exact merge base\"));\n+\t\t\t\t} else {\n+\t\t\t\t\tfree_commit_list(base_list);\n+\t\t\t\t\treturn NULL;\n+\t\t\t\t}\n+\t\t\t}\n \t\t\tbase = base_list->item;\n \t\t\tfree_commit_list(base_list);\n \t\t} else {\n-\t\t\tdie(_(\"failed to get upstream, if you want to record base commit automatically,\\n\"\n-\t\t\t      \"please use git branch --set-upstream-to to track a remote branch.\\n\"\n-\t\t\t      \"Or you could specify base commit by --base=<base-commit-id> manually\"));\n+\t\t\tif (die_on_failure)\n+\t\t\t\tdie(_(\"failed to get upstream, if you want to record base commit automatically,\\n\"\n+\t\t\t\t      \"please use git branch --set-upstream-to to track a remote branch.\\n\"\n+\t\t\t\t      \"Or you could specify base commit by --base=<base-commit-id> manually\"));\n+\t\t\telse\n+\t\t\t\treturn NULL;\n \t\t}\n \t}\n \n@@ -1479,8 +1542,14 @@ static struct commit *get_base_commit(const char *base_commit,\n \t\tfor (i = 0; i < rev_nr / 2; i++) {\n \t\t\tstruct commit_list *merge_base;\n \t\t\tmerge_base = get_merge_bases(rev[2 * i], rev[2 * i + 1]);\n-\t\t\tif (!merge_base || merge_base->next)\n-\t\t\t\tdie(_(\"failed to find exact merge base\"));\n+\t\t\tif (!merge_base || merge_base->next) {\n+\t\t\t\tif (die_on_failure) {\n+\t\t\t\t\tdie(_(\"failed to find exact merge base\"));\n+\t\t\t\t} else {\n+\t\t\t\t\tfree(rev);\n+\t\t\t\t\treturn NULL;\n+\t\t\t\t}\n+\t\t\t}\n \n \t\t\trev[i] = merge_base->item;\n \t\t}\n@@ -1490,12 +1559,24 @@ static struct commit *get_base_commit(const char *base_commit,\n \t\trev_nr = DIV_ROUND_UP(rev_nr, 2);\n \t}\n \n-\tif (!in_merge_bases(base, rev[0]))\n-\t\tdie(_(\"base commit should be the ancestor of revision list\"));\n+\tif (!in_merge_bases(base, rev[0])) {\n+\t\tif (die_on_failure) {\n+\t\t\tdie(_(\"base commit should be the ancestor of revision list\"));\n+\t\t} else {\n+\t\t\tfree(rev);\n+\t\t\treturn NULL;\n+\t\t}\n+\t}\n \n \tfor (i = 0; i < total; i++) {\n-\t\tif (base == list[i])\n-\t\t\tdie(_(\"base commit shouldn't be in revision list\"));\n+\t\tif (base == list[i]) {\n+\t\t\tif (die_on_failure) {\n+\t\t\t\tdie(_(\"base commit shouldn't be in revision list\"));\n+\t\t\t} else {\n+\t\t\t\tfree(rev);\n+\t\t\t\treturn NULL;\n+\t\t\t}\n+\t\t}\n \t}\n \n \tfree(rev);\n@@ -1638,6 +1719,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tchar *branch_name = NULL;\n \tchar *base_commit = NULL;\n \tstruct base_tree_info bases;\n+\tstruct commit *base;\n \tint show_progress = 0;\n \tstruct progress *progress = NULL;\n \tstruct oid_array idiff_prev = OID_ARRAY_INIT;\n@@ -1714,8 +1796,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t    PARSE_OPT_OPTARG, thread_callback),\n \t\tOPT_STRING(0, \"signature\", &signature, N_(\"signature\"),\n \t\t\t    N_(\"add a signature\")),\n-\t\tOPT_STRING(0, \"base\", &base_commit, N_(\"base-commit\"),\n-\t\t\t   N_(\"add prerequisite tree info to the patch series\")),\n+\t\tOPT_CALLBACK_F(0, \"base\", &base_commit, N_(\"base-commit\"),\n+\t\t\t       N_(\"add prerequisite tree info to the patch series\"),\n+\t\t\t       0, base_callback),\n \t\tOPT_FILENAME(0, \"signature-file\", &signature_file,\n \t\t\t\tN_(\"add a signature from a file\")),\n \t\tOPT__QUIET(&quiet, N_(\"don't print the patch filenames\")),\n@@ -1752,9 +1835,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \ts_r_opt.def = \"HEAD\";\n \ts_r_opt.revarg_opt = REVARG_COMMITTISH;\n \n-\tif (base_auto)\n-\t\tbase_commit = \"auto\";\n-\n \tif (default_attach) {\n \t\trev.mime_boundary = default_attach;\n \t\trev.no_inline = 1;\n@@ -2018,8 +2098,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t}\n \n \tmemset(&bases, 0, sizeof(bases));\n-\tif (base_commit) {\n-\t\tstruct commit *base = get_base_commit(base_commit, list, nr);\n+\tbase = get_base_commit(base_commit, list, nr);\n+\tif (base) {\n \t\treset_revision_walk();\n \t\tclear_object_flags(UNINTERESTING);\n \t\tprepare_bases(&bases, base, list, nr);\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 958c2da56ec6..294e76c860cf 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -2037,6 +2037,12 @@ test_expect_success 'format-patch errors out when history involves criss-cross'\n \ttest_must_fail \tgit format-patch --base=auto -1\n '\n \n+test_expect_success 'format-patch format.useAutoBase whenAble history involves criss-cross' '\n+\ttest_config format.useAutoBase whenAble &&\n+\tgit format-patch -1 >patch &&\n+\t! grep \"^base-commit:\" patch\n+'\n+\n test_expect_success 'format-patch format.useAutoBase option' '\n \tgit checkout local &&\n \ttest_config format.useAutoBase true &&\n@@ -2047,6 +2053,16 @@ test_expect_success 'format-patch format.useAutoBase option' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'format-patch format.useAutoBase option with whenAble' '\n+\tgit checkout local &&\n+\ttest_config format.useAutoBase whenAble &&\n+\tgit format-patch --stdout -1 >patch &&\n+\tgrep \"^base-commit:\" patch >actual &&\n+\tgit rev-parse upstream >commit-id-base &&\n+\techo \"base-commit: $(cat commit-id-base)\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'format-patch --base overrides format.useAutoBase' '\n \ttest_config format.useAutoBase true &&\n \tgit format-patch --stdout --base=HEAD~1 -1 >patch &&\n@@ -2062,6 +2078,12 @@ test_expect_success 'format-patch --no-base overrides format.useAutoBase' '\n \t! grep \"^base-commit:\" patch\n '\n \n+test_expect_success 'format-patch --no-base overrides format.useAutoBase whenAble' '\n+\ttest_config format.useAutoBase whenAble &&\n+\tgit format-patch --stdout --no-base -1 >patch &&\n+\t! grep \"^base-commit:\" patch\n+'\n+\n test_expect_success 'format-patch --base with --attach' '\n \tgit format-patch --attach=mimemime --stdout --base=HEAD~ -1 >patch &&\n \tsed -n -e \"/^base-commit:/s/.*/1/p\" -e \"/^---*mimemime--$/s/.*/2/p\" \\\n-- \n2.28.0.497.g54e85e7af1ac\n\n"},{"id":"406793","messageId":"xmqqft6xbn97.fsf@gitster.c.googlers.com","threadId":"54334","inReplyTo":"20201001214653.1609405-1-jacob.e.keller@intel.com","subject":"Re: [PATCH v2 0/1] teach format.useAutoBase \"whenAble\" option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-01T22:21:56Z","receivedAt":"2020-10-01T22:22:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller <jacob.e.keller@intel.com> writes:\n\n> From: Jacob Keller <jacob.keller@gmail.com>\n>\n> This is a v2 of [1] to address comments on the mailing list. Primarily the\n> change is to remove the option for --base=if-able, since it is not as useful\n> as the configuration option. If we think it is desirable, adding back a\n> whenable or whenAble option is easy enough.\n\nThe simplification is a great thing to do, and I agree with the\nrationale you wrote ...\n\n>     +    This does mean that a user cannot request this optional base commit\n>     +    generation from the command line. However, this is likely not too\n>     +    valuable. If the user requests base information manually, they will be\n>     +    immediately informed of the failure to acquire a suitable base commit.\n>     +    This allows the user to make an informed choice about whether to\n>     +    continue the format.\n\n... which is sensible.\n\nThanks, will queue.\n"}]}