# Bug with "git rebase" when format.useAutoBase is set 33 messages from 2019-11-26 to 2019-12-04. Participants: Christian Biesinger, Denton Liu, Eric Sunshine, Junio C Hamano, René Scharfe. Thread: https://gitlist.dev/t/52353 ## Christian Biesinger, 2019-11-26 20:50 Subject: Bug with "git rebase" when format.useAutoBase is set Message-ID: URL: https://gitlist.dev/e/CAPTJ0XEcMJEwPBEL4akncJ_A5XwyQyKL95fpf2tC0rNXJK65Og%40mail.gmail.com ``` Hello! I recently tried setting the format.useAutoBase setting to true so that patches I send with git format-patch identify their base commit. Unfortunately this seems to break "git rebase" completely, I always get the following error. Is this expected? This is with git version 2.24.0.432.g9d3f5f5b63-goog $ git rebase --onto foo 4a7dabea39556979cbf6bcbdd98a9413ce129fd0 First, rewinding head to replay your work on top of it... fatal: failed to get upstream, if you want to record base commit automatically, please use git branch --set-upstream-to to track a remote branch. Or you could specify base commit by --base= manually error: git encountered an error while preparing the patches to replay these revisions: 4a7dabea39556979cbf6bcbdd98a9413ce129fd0..12f4dc41bf9f018ecc1042830d5534de941f329e As a result, git cannot rebase them. ``` ## Denton Liu, 2019-11-27 02:09 Subject: [PATCH 0/5] rebase: fix breakage with `format.useAutoBase` Message-ID: URL: https://gitlist.dev/e/cover.1574820308.git.liu.denton%40gmail.com In-Reply-To: ``` Thanks for reporting the breakage, Christian. Apparently, this use case has been broken for a long time... Since bb52995f3e (format-patch: introduce format.useAutoBase configuration, 2016-04-26). I'm surprised it's only been reported now. This patchset fixes the breakage by teaching `git format-patch --no-base` and making rebase use it. Denton Liu (5): t3400: demonstrate failure with format.useAutoBase format-patch: fix indentation t4014: use `test_config` format-patch: teach --no-base rebase: fix `format.useAutoBase` breakage Documentation/git-format-patch.txt | 5 +++-- builtin/log.c | 26 ++++++++++++++++++++++---- builtin/rebase.c | 3 ++- t/t3400-rebase.sh | 6 ++++++ t/t4014-format-patch.sh | 14 +++++++++----- 5 files changed, 42 insertions(+), 12 deletions(-) -- 2.24.0.504.g3cd56eb17d ``` ## Denton Liu, 2019-11-27 02:09 Subject: [PATCH 1/5] t3400: demonstrate failure with format.useAutoBase Message-ID: URL: https://gitlist.dev/e/a1741e54346a387a65e272f0210118ce81475358.1574820308.git.liu.denton%40gmail.com In-Reply-To: ``` Ever since bb52995f3e (format-patch: introduce format.useAutoBase configuration, 2016-04-26), `git rebase` has been broken when `format.useAutoBase = true`. Demonstrate that failure here. Reported-by: Christian Biesinger Signed-off-by: Denton Liu --- t/t3400-rebase.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index ab18ac5f28..ca99e8c6c4 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -159,6 +159,12 @@ test_expect_success 'fail when upstream arg is missing and not configured' ' test_must_fail git rebase ' +test_expect_failure 'rebase works with format.useAutoBase' ' + test_config format.useAutoBase true && + git checkout topic && + git rebase master +' + test_expect_success 'default to common base in @{upstream}s reflog if no upstream arg' ' git checkout -b default-base master && git checkout -b default topic && -- 2.24.0.504.g3cd56eb17d ``` ## Denton Liu, 2019-11-27 02:09 Subject: [PATCH 2/5] format-patch: fix indentation Message-ID: <46fd4113aad5bd6c9925304eca35c808274982ba.1574820308.git.liu.denton@gmail.com> URL: https://gitlist.dev/e/46fd4113aad5bd6c9925304eca35c808274982ba.1574820308.git.liu.denton%40gmail.com In-Reply-To: ``` Signed-off-by: Denton Liu --- builtin/log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/log.c b/builtin/log.c index a26f223ab4..9c44682f61 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1350,7 +1350,7 @@ static int header_callback(const struct option *opt, const char *arg, int unset) string_list_clear(&extra_to, 0); string_list_clear(&extra_cc, 0); } else { - add_header(arg); + add_header(arg); } return 0; } -- 2.24.0.504.g3cd56eb17d ``` ## Denton Liu, 2019-11-27 02:09 Subject: [PATCH 3/5] t4014: use `test_config` Message-ID: <22b1fb14f9ac841e93b6177cec216530a6756d7c.1574820308.git.liu.denton@gmail.com> URL: https://gitlist.dev/e/22b1fb14f9ac841e93b6177cec216530a6756d7c.1574820308.git.liu.denton%40gmail.com In-Reply-To: ``` Instead of manually unsetting the config after the test case is done, use `test_config` to do it automatically. While we're at it, fix a typo in a test case name. Signed-off-by: Denton Liu --- t/t4014-format-patch.sh | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index 69267b16f0..c7cc643adf 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -1939,10 +1939,9 @@ 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 format.useAutoBaseoption' ' - test_when_finished "git config --unset format.useAutoBase" && +test_expect_success 'format-patch format.useAutoBase option' ' git checkout local && - git config format.useAutoBase true && + test_config format.useAutoBase true && git format-patch --stdout -1 >patch && grep "^base-commit:" patch >actual && git rev-parse upstream >commit-id-base && @@ -1951,8 +1950,7 @@ test_expect_success 'format-patch format.useAutoBaseoption' ' ' test_expect_success 'format-patch --base overrides format.useAutoBase' ' - test_when_finished "git config --unset format.useAutoBase" && - git config format.useAutoBase true && + test_config format.useAutoBase true && git format-patch --stdout --base=HEAD~1 -1 >patch && grep "^base-commit:" patch >actual && git rev-parse HEAD~1 >commit-id-base && -- 2.24.0.504.g3cd56eb17d ``` ## Denton Liu, 2019-11-27 02:09 Subject: [PATCH 4/5] format-patch: teach --no-base Message-ID: URL: https://gitlist.dev/e/e072c36e6a41ef70d2a15c052c672f94b54ce930.1574820308.git.liu.denton%40gmail.com In-Reply-To: ``` If `format.useAutoBase = true`, there was no way to override this from the command-line. Teach format-patch the `--no-base` option which overrides `format.useAutoBase`. Signed-off-by: Denton Liu --- Documentation/git-format-patch.txt | 5 +++-- builtin/log.c | 24 +++++++++++++++++++++--- t/t4014-format-patch.sh | 6 ++++++ 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 00bdf9b125..0d4f8951bb 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -333,11 +333,12 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`. Output an all-zero hash in each patch's From header instead of the hash of the commit. ---base=:: +--[no-]base[=]:: Record the base tree information to identify the state the patch series applies to. See the BASE TREE INFORMATION section below for details. If is "auto", a base commit is - automatically chosen. + automatically chosen. The `--no-base` option overrides a + `format.useAutoBase` configuration. --root:: Treat the revision argument as a , even if it diff --git a/builtin/log.c b/builtin/log.c index 9c44682f61..c017df4056 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1388,6 +1388,23 @@ 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) +{ + char **base_commit = opt->value; + + free(*base_commit); + + if (unset) { + base_auto = 0; + *base_commit = NULL; + } else if (arg) { + *base_commit = xstrdup(arg); + } else { + BUG("arg is NULL"); + } + return 0; +} + struct base_tree_info { struct object_id base_commit; int nr_patch_id, alloc_patch_id; @@ -1676,10 +1693,11 @@ 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")), + { OPTION_CALLBACK, 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")), + N_("add a signature from a file")), OPT__QUIET(&quiet, N_("don't print the patch filenames")), OPT_BOOL(0, "progress", &show_progress, N_("show progress while generating patches")), diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index c7cc643adf..a5b6302a1c 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -1958,6 +1958,12 @@ test_expect_success 'format-patch --base overrides format.useAutoBase' ' test_cmp expect actual ' +test_expect_success 'format-patch --no-base overrides format.useAutoBase' ' + test_config format.useAutoBase true && + 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.24.0.504.g3cd56eb17d ``` ## Denton Liu, 2019-11-27 02:09 Subject: [PATCH 5/5] rebase: fix `format.useAutoBase` breakage Message-ID: <15e6ccb2037d1e27b461d2344a13ebf1d5e10136.1574820308.git.liu.denton@gmail.com> URL: https://gitlist.dev/e/15e6ccb2037d1e27b461d2344a13ebf1d5e10136.1574820308.git.liu.denton%40gmail.com In-Reply-To: ``` With `format.useAutoBase = true`, running rebase resulted in an error when an upstream was't set: fatal: failed to get upstream, if you want to record base commit automatically, please use git branch --set-upstream-to to track a remote branch. Or you could specify base commit by --base= manually error: git encountered an error while preparing the patches to replay these revisions: ede2467cdedc63784887b587a61c36b7850ebfac..d8f581194799ae29bf5fa72a98cbae98a1198b12 As a result, git cannot rebase them. Fix this by always passing `--no-base` to format-patch from rebase so that the effect of `format.useAutoBase` is negated. Reported-by: Christian Biesinger Signed-off-by: Denton Liu --- builtin/rebase.c | 3 ++- t/t3400-rebase.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index e755087b0f..51980ab63d 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1012,7 +1012,8 @@ static int run_am(struct rebase_options *opts) argv_array_pushl(&format_patch.args, "format-patch", "-k", "--stdout", "--full-index", "--cherry-pick", "--right-only", "--src-prefix=a/", "--dst-prefix=b/", "--no-renames", - "--no-cover-letter", "--pretty=mboxrd", "--topo-order", NULL); + "--no-cover-letter", "--pretty=mboxrd", "--topo-order", + "--no-base", NULL); if (opts->git_format_patch_opt.len) argv_array_split(&format_patch.args, opts->git_format_patch_opt.buf); diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index ca99e8c6c4..1323f30fee 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -159,7 +159,7 @@ test_expect_success 'fail when upstream arg is missing and not configured' ' test_must_fail git rebase ' -test_expect_failure 'rebase works with format.useAutoBase' ' +test_expect_success 'rebase works with format.useAutoBase' ' test_config format.useAutoBase true && git checkout topic && git rebase master -- 2.24.0.504.g3cd56eb17d ``` ## Eric Sunshine, 2019-11-27 02:26 Subject: Re: [PATCH 1/5] t3400: demonstrate failure with format.useAutoBase Message-ID: URL: https://gitlist.dev/e/CAPig%2BcTqRcZ6R7bFqeNG_bvwqgAKP7NVEHBSQxgpVk%2BPgGVMVg%40mail.gmail.com In-Reply-To: ``` On Tue, Nov 26, 2019 at 9:09 PM Denton Liu wrote: > Ever since bb52995f3e (format-patch: introduce format.useAutoBase > configuration, 2016-04-26), `git rebase` has been broken when > `format.useAutoBase = true`. Demonstrate that failure here. What specifically does "broken" mean here? > Reported-by: Christian Biesinger > Signed-off-by: Denton Liu > --- > diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh > @@ -159,6 +159,12 @@ test_expect_success 'fail when upstream arg is missing and not configured' ' > +test_expect_failure 'rebase works with format.useAutoBase' ' > + test_config format.useAutoBase true && > + git checkout topic && > + git rebase master > +' Having read both the commit message and the test itself, I'm not wiser about what is actually "broken" or what this is demonstrating. ``` ## Eric Sunshine, 2019-11-27 02:42 Subject: Re: [PATCH 5/5] rebase: fix `format.useAutoBase` breakage Message-ID: URL: https://gitlist.dev/e/CAPig%2BcSH36aPwUDhtkiknpSsas-osCVnJ%3DcyNUzOgKXdvOCnmw%40mail.gmail.com In-Reply-To: <15e6ccb2037d1e27b461d2344a13ebf1d5e10136.1574820308.git.liu.denton@gmail.com> ``` On Tue, Nov 26, 2019 at 9:09 PM Denton Liu wrote: > With `format.useAutoBase = true`, running rebase resulted in an > error when an upstream was't set: s/was't/wasn't/ > fatal: failed to get upstream, if you want to record base commit automatically, > please use git branch --set-upstream-to to track a remote branch. > Or you could specify base commit by --base= manually > error: > git encountered an error while preparing the patches to replay > these revisions: > > ede2467cdedc63784887b587a61c36b7850ebfac..d8f581194799ae29bf5fa72a98cbae98a1198b12 > > As a result, git cannot rebase them. > > Fix this by always passing `--no-base` to format-patch from rebase so > that the effect of `format.useAutoBase` is negated. > > Reported-by: Christian Biesinger > Signed-off-by: Denton Liu ``` ## Christian Biesinger, 2019-11-27 17:14 Subject: Re: [PATCH 0/5] rebase: fix breakage with `format.useAutoBase` Message-ID: URL: https://gitlist.dev/e/CAPTJ0XH62P_-R4XCy5Jv-egQ06H8eJZ4YhLdRGAt721WVD5bXQ%40mail.gmail.com In-Reply-To: ``` On Tue, Nov 26, 2019 at 8:09 PM Denton Liu wrote: > > Thanks for reporting the breakage, Christian. > > Apparently, this use case has been broken for a long time... Since > bb52995f3e (format-patch: introduce format.useAutoBase configuration, > 2016-04-26). I'm surprised it's only been reported now. Thanks for fixing! Maybe few people use this option as it's not documented in the format-patch manpage? (A separate thing to fix, I guess) Christian > This patchset fixes the breakage by teaching > `git format-patch --no-base` and making rebase use it. > > Denton Liu (5): > t3400: demonstrate failure with format.useAutoBase > format-patch: fix indentation > t4014: use `test_config` > format-patch: teach --no-base > rebase: fix `format.useAutoBase` breakage > > Documentation/git-format-patch.txt | 5 +++-- > builtin/log.c | 26 ++++++++++++++++++++++---- > builtin/rebase.c | 3 ++- > t/t3400-rebase.sh | 6 ++++++ > t/t4014-format-patch.sh | 14 +++++++++----- > 5 files changed, 42 insertions(+), 12 deletions(-) > > -- > 2.24.0.504.g3cd56eb17d > ``` ## Denton Liu, 2019-11-27 18:13 Subject: [PATCH v2 0/5] rebase: fix breakage with `format.useAutoBase` Message-ID: URL: https://gitlist.dev/e/cover.1574878089.git.liu.denton%40gmail.com In-Reply-To: ``` Apparently, this use case has been broken for a long time... Since bb52995f3e (format-patch: introduce format.useAutoBase configuration, 2016-04-26). I'm surprised it's only been reported now. This patchset fixes the breakage by teaching `git format-patch --no-base` and making rebase use it. Changes since v1: * Update some log messages Denton Liu (5): t3400: demonstrate failure with format.useAutoBase format-patch: fix indentation t4014: use test_config() format-patch: teach --no-base rebase: fix format.useAutoBase breakage Documentation/git-format-patch.txt | 5 +++-- builtin/log.c | 26 ++++++++++++++++++++++---- builtin/rebase.c | 3 ++- t/t3400-rebase.sh | 6 ++++++ t/t4014-format-patch.sh | 14 +++++++++----- 5 files changed, 42 insertions(+), 12 deletions(-) Range-diff against v1: 1: a1741e5434 ! 1: 4089e51041 t3400: demonstrate failure with format.useAutoBase @@ Commit message Ever since bb52995f3e (format-patch: introduce format.useAutoBase configuration, 2016-04-26), `git rebase` has been broken when - `format.useAutoBase = true`. Demonstrate that failure here. + `format.useAutoBase = true`. It fails when rebasing a branch that + doesn't have an upstream set: + + fatal: failed to get upstream, if you want to record base commit automatically, + please use git branch --set-upstream-to to track a remote branch. + Or you could specify base commit by --base= manually + error: + git encountered an error while preparing the patches to replay + these revisions: + + ede2467cdedc63784887b587a61c36b7850ebfac..d8f581194799ae29bf5fa72a98cbae98a1198b12 + + As a result, git cannot rebase them. + + Demonstrate that failure here. Reported-by: Christian Biesinger 2: 46fd4113aa = 2: d288d6c3a5 format-patch: fix indentation 3: 22b1fb14f9 ! 3: 196b5d8dbc t4014: use `test_config` @@ Metadata Author: Denton Liu ## Commit message ## - t4014: use `test_config` + t4014: use test_config() Instead of manually unsetting the config after the test case is done, - use `test_config` to do it automatically. While we're at it, fix a typo + use test_config() to do it automatically. While we're at it, fix a typo in a test case name. ## t/t4014-format-patch.sh ## 4: e072c36e6a = 4: f7e5325cc0 format-patch: teach --no-base 5: 15e6ccb203 ! 5: 62c59c12e3 rebase: fix `format.useAutoBase` breakage @@ Metadata Author: Denton Liu ## Commit message ## - rebase: fix `format.useAutoBase` breakage + rebase: fix format.useAutoBase breakage With `format.useAutoBase = true`, running rebase resulted in an - error when an upstream was't set: + error when an upstream wasn't set: fatal: failed to get upstream, if you want to record base commit automatically, please use git branch --set-upstream-to to track a remote branch. -- 2.24.0.504.g3cd56eb17d ``` ## Denton Liu, 2019-11-27 18:13 Subject: [PATCH v2 1/5] t3400: demonstrate failure with format.useAutoBase Message-ID: <4089e51041a9cbd15e62882c370ed254d8e63cfd.1574878089.git.liu.denton@gmail.com> URL: https://gitlist.dev/e/4089e51041a9cbd15e62882c370ed254d8e63cfd.1574878089.git.liu.denton%40gmail.com In-Reply-To: ``` Ever since bb52995f3e (format-patch: introduce format.useAutoBase configuration, 2016-04-26), `git rebase` has been broken when `format.useAutoBase = true`. It fails when rebasing a branch that doesn't have an upstream set: fatal: failed to get upstream, if you want to record base commit automatically, please use git branch --set-upstream-to to track a remote branch. Or you could specify base commit by --base= manually error: git encountered an error while preparing the patches to replay these revisions: ede2467cdedc63784887b587a61c36b7850ebfac..d8f581194799ae29bf5fa72a98cbae98a1198b12 As a result, git cannot rebase them. Demonstrate that failure here. Reported-by: Christian Biesinger Signed-off-by: Denton Liu --- t/t3400-rebase.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index ab18ac5f28..ca99e8c6c4 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -159,6 +159,12 @@ test_expect_success 'fail when upstream arg is missing and not configured' ' test_must_fail git rebase ' +test_expect_failure 'rebase works with format.useAutoBase' ' + test_config format.useAutoBase true && + git checkout topic && + git rebase master +' + test_expect_success 'default to common base in @{upstream}s reflog if no upstream arg' ' git checkout -b default-base master && git checkout -b default topic && -- 2.24.0.504.g3cd56eb17d ``` ## Denton Liu, 2019-11-27 18:13 Subject: [PATCH v2 2/5] format-patch: fix indentation Message-ID: URL: https://gitlist.dev/e/d288d6c3a56fab339b2dfed59579c1acecda4559.1574878089.git.liu.denton%40gmail.com In-Reply-To: ``` Signed-off-by: Denton Liu --- builtin/log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/log.c b/builtin/log.c index a26f223ab4..9c44682f61 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1350,7 +1350,7 @@ static int header_callback(const struct option *opt, const char *arg, int unset) string_list_clear(&extra_to, 0); string_list_clear(&extra_cc, 0); } else { - add_header(arg); + add_header(arg); } return 0; } -- 2.24.0.504.g3cd56eb17d ``` ## Denton Liu, 2019-11-27 18:13 Subject: [PATCH v2 3/5] t4014: use test_config() Message-ID: <196b5d8dbccfa550608c9a6c19ae4b7743089f41.1574878089.git.liu.denton@gmail.com> URL: https://gitlist.dev/e/196b5d8dbccfa550608c9a6c19ae4b7743089f41.1574878089.git.liu.denton%40gmail.com In-Reply-To: ``` Instead of manually unsetting the config after the test case is done, use test_config() to do it automatically. While we're at it, fix a typo in a test case name. Signed-off-by: Denton Liu --- t/t4014-format-patch.sh | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index 69267b16f0..c7cc643adf 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -1939,10 +1939,9 @@ 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 format.useAutoBaseoption' ' - test_when_finished "git config --unset format.useAutoBase" && +test_expect_success 'format-patch format.useAutoBase option' ' git checkout local && - git config format.useAutoBase true && + test_config format.useAutoBase true && git format-patch --stdout -1 >patch && grep "^base-commit:" patch >actual && git rev-parse upstream >commit-id-base && @@ -1951,8 +1950,7 @@ test_expect_success 'format-patch format.useAutoBaseoption' ' ' test_expect_success 'format-patch --base overrides format.useAutoBase' ' - test_when_finished "git config --unset format.useAutoBase" && - git config format.useAutoBase true && + test_config format.useAutoBase true && git format-patch --stdout --base=HEAD~1 -1 >patch && grep "^base-commit:" patch >actual && git rev-parse HEAD~1 >commit-id-base && -- 2.24.0.504.g3cd56eb17d ``` ## Denton Liu, 2019-11-27 18:13 Subject: [PATCH v2 4/5] format-patch: teach --no-base Message-ID: URL: https://gitlist.dev/e/f7e5325cc0a7c29b62abe361e188c7cf9c59a9ca.1574878089.git.liu.denton%40gmail.com In-Reply-To: ``` If `format.useAutoBase = true`, there was no way to override this from the command-line. Teach format-patch the `--no-base` option which overrides `format.useAutoBase`. Signed-off-by: Denton Liu --- Documentation/git-format-patch.txt | 5 +++-- builtin/log.c | 24 +++++++++++++++++++++--- t/t4014-format-patch.sh | 6 ++++++ 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 00bdf9b125..0d4f8951bb 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -333,11 +333,12 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`. Output an all-zero hash in each patch's From header instead of the hash of the commit. ---base=:: +--[no-]base[=]:: Record the base tree information to identify the state the patch series applies to. See the BASE TREE INFORMATION section below for details. If is "auto", a base commit is - automatically chosen. + automatically chosen. The `--no-base` option overrides a + `format.useAutoBase` configuration. --root:: Treat the revision argument as a , even if it diff --git a/builtin/log.c b/builtin/log.c index 9c44682f61..c017df4056 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1388,6 +1388,23 @@ 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) +{ + char **base_commit = opt->value; + + free(*base_commit); + + if (unset) { + base_auto = 0; + *base_commit = NULL; + } else if (arg) { + *base_commit = xstrdup(arg); + } else { + BUG("arg is NULL"); + } + return 0; +} + struct base_tree_info { struct object_id base_commit; int nr_patch_id, alloc_patch_id; @@ -1676,10 +1693,11 @@ 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")), + { OPTION_CALLBACK, 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")), + N_("add a signature from a file")), OPT__QUIET(&quiet, N_("don't print the patch filenames")), OPT_BOOL(0, "progress", &show_progress, N_("show progress while generating patches")), diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index c7cc643adf..a5b6302a1c 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -1958,6 +1958,12 @@ test_expect_success 'format-patch --base overrides format.useAutoBase' ' test_cmp expect actual ' +test_expect_success 'format-patch --no-base overrides format.useAutoBase' ' + test_config format.useAutoBase true && + 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.24.0.504.g3cd56eb17d ``` ## Denton Liu, 2019-11-27 18:13 Subject: [PATCH v2 5/5] rebase: fix format.useAutoBase breakage Message-ID: <62c59c12e376dd73317a7e5d78275bca12875340.1574878089.git.liu.denton@gmail.com> URL: https://gitlist.dev/e/62c59c12e376dd73317a7e5d78275bca12875340.1574878089.git.liu.denton%40gmail.com In-Reply-To: ``` With `format.useAutoBase = true`, running rebase resulted in an error when an upstream wasn't set: fatal: failed to get upstream, if you want to record base commit automatically, please use git branch --set-upstream-to to track a remote branch. Or you could specify base commit by --base= manually error: git encountered an error while preparing the patches to replay these revisions: ede2467cdedc63784887b587a61c36b7850ebfac..d8f581194799ae29bf5fa72a98cbae98a1198b12 As a result, git cannot rebase them. Fix this by always passing `--no-base` to format-patch from rebase so that the effect of `format.useAutoBase` is negated. Reported-by: Christian Biesinger Signed-off-by: Denton Liu --- builtin/rebase.c | 3 ++- t/t3400-rebase.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index e755087b0f..51980ab63d 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1012,7 +1012,8 @@ static int run_am(struct rebase_options *opts) argv_array_pushl(&format_patch.args, "format-patch", "-k", "--stdout", "--full-index", "--cherry-pick", "--right-only", "--src-prefix=a/", "--dst-prefix=b/", "--no-renames", - "--no-cover-letter", "--pretty=mboxrd", "--topo-order", NULL); + "--no-cover-letter", "--pretty=mboxrd", "--topo-order", + "--no-base", NULL); if (opts->git_format_patch_opt.len) argv_array_split(&format_patch.args, opts->git_format_patch_opt.buf); diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index ca99e8c6c4..1323f30fee 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -159,7 +159,7 @@ test_expect_success 'fail when upstream arg is missing and not configured' ' test_must_fail git rebase ' -test_expect_failure 'rebase works with format.useAutoBase' ' +test_expect_success 'rebase works with format.useAutoBase' ' test_config format.useAutoBase true && git checkout topic && git rebase master -- 2.24.0.504.g3cd56eb17d ``` ## Denton Liu, 2019-11-27 19:56 Subject: Re: [PATCH v2 4/5] format-patch: teach --no-base Message-ID: <20191127195615.GA7413@generichostname> URL: https://gitlist.dev/e/20191127195615.GA7413%40generichostname In-Reply-To: ``` On Wed, Nov 27, 2019 at 10:13:46AM -0800, Denton Liu wrote: > If `format.useAutoBase = true`, there was no way to override this from > the command-line. Teach format-patch the `--no-base` option which > overrides `format.useAutoBase`. > > Signed-off-by: Denton Liu > --- > Documentation/git-format-patch.txt | 5 +++-- > builtin/log.c | 24 +++++++++++++++++++++--- > t/t4014-format-patch.sh | 6 ++++++ > 3 files changed, 30 insertions(+), 5 deletions(-) > > diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt > index 00bdf9b125..0d4f8951bb 100644 > --- a/Documentation/git-format-patch.txt > +++ b/Documentation/git-format-patch.txt > @@ -333,11 +333,12 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`. > Output an all-zero hash in each patch's From header instead > of the hash of the commit. > > ---base=:: > +--[no-]base[=]:: > Record the base tree information to identify the state the > patch series applies to. See the BASE TREE INFORMATION section > below for details. If is "auto", a base commit is > - automatically chosen. > + automatically chosen. The `--no-base` option overrides a > + `format.useAutoBase` configuration. > > --root:: > Treat the revision argument as a , even if it > diff --git a/builtin/log.c b/builtin/log.c > index 9c44682f61..c017df4056 100644 > --- a/builtin/log.c > +++ b/builtin/log.c > @@ -1388,6 +1388,23 @@ 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) > +{ > + char **base_commit = opt->value; > + > + free(*base_commit); > + > + if (unset) { > + base_auto = 0; > + *base_commit = NULL; > + } else if (arg) { > + *base_commit = xstrdup(arg); > + } else { > + BUG("arg is NULL"); > + } > + return 0; > +} > + > struct base_tree_info { > struct object_id base_commit; > int nr_patch_id, alloc_patch_id; > @@ -1676,10 +1693,11 @@ 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")), > + { OPTION_CALLBACK, 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")), > + N_("add a signature from a file")), Oops, this is reindentation is a spurious change and should be dropped. > OPT__QUIET(&quiet, N_("don't print the patch filenames")), > OPT_BOOL(0, "progress", &show_progress, > N_("show progress while generating patches")), > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > index c7cc643adf..a5b6302a1c 100755 > --- a/t/t4014-format-patch.sh > +++ b/t/t4014-format-patch.sh > @@ -1958,6 +1958,12 @@ test_expect_success 'format-patch --base overrides format.useAutoBase' ' > test_cmp expect actual > ' > > +test_expect_success 'format-patch --no-base overrides format.useAutoBase' ' > + test_config format.useAutoBase true && > + 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.24.0.504.g3cd56eb17d > ``` ## Junio C Hamano, 2019-11-30 17:25 Subject: Re: [PATCH 1/5] t3400: demonstrate failure with format.useAutoBase Message-ID: URL: https://gitlist.dev/e/xmqqimn12s5l.fsf%40gitster-ct.c.googlers.com In-Reply-To: ``` Eric Sunshine writes: > On Tue, Nov 26, 2019 at 9:09 PM Denton Liu wrote: >> Ever since bb52995f3e (format-patch: introduce format.useAutoBase >> configuration, 2016-04-26), `git rebase` has been broken when >> `format.useAutoBase = true`. Demonstrate that failure here. > > What specifically does "broken" mean here? > >> Reported-by: Christian Biesinger >> Signed-off-by: Denton Liu >> --- >> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh >> @@ -159,6 +159,12 @@ test_expect_success 'fail when upstream arg is missing and not configured' ' >> +test_expect_failure 'rebase works with format.useAutoBase' ' >> + test_config format.useAutoBase true && >> + git checkout topic && >> + git rebase master >> +' > > Having read both the commit message and the test itself, I'm not wiser > about what is actually "broken" or what this is demonstrating. True. The tests must be crystal clear what kind of brokenness it is demonstrating, not just "this test is expected to fail", especially when the "expect failure in one step, fix and flip expectation in a separate step" pattern is used; otherwise it becomes doubly puzzling. Thanks. ``` ## Denton Liu, 2019-12-04 07:47 Subject: [PATCH v3 0/5] rebase: fix breakage with `format.useAutoBase` Message-ID: URL: https://gitlist.dev/e/cover.1575445582.git.liu.denton%40gmail.com In-Reply-To: ``` Apparently, this use case has been broken for a long time... Since bb52995f3e (format-patch: introduce format.useAutoBase configuration, 2016-04-26). I'm surprised it's only been reported now. This patchset fixes the breakage by teaching `git format-patch --no-base` and making rebase use it. This patch is based on the latest master since it's such an old bug that only got noticed recently, I'm not sure if it's worth fixing in 'maint'. The series does not apply cleanly on top of 'maint' because it relies on c1a6f21cd4 (Doc: add more detail for git-format-patch, 2019-08-27) but the conflicts are relatively minor. Changes since v2: * Remove spurious indentation change * Rebase onto the latest master Changes since v1: * Update some log messages Denton Liu (5): t3400: demonstrate failure with format.useAutoBase format-patch: fix indentation t4014: use test_config() format-patch: teach --no-base rebase: fix format.useAutoBase breakage Documentation/git-format-patch.txt | 5 +++-- builtin/log.c | 24 +++++++++++++++++++++--- builtin/rebase.c | 3 ++- t/t3400-rebase.sh | 6 ++++++ t/t4014-format-patch.sh | 14 +++++++++----- 5 files changed, 41 insertions(+), 11 deletions(-) Range-diff against v2: 1: 4089e51041 = 1: 8d67bbe5bf t3400: demonstrate failure with format.useAutoBase 2: d288d6c3a5 = 2: 8cfde9f98e format-patch: fix indentation 3: 196b5d8dbc = 3: 638c4add00 t4014: use test_config() 4: f7e5325cc0 ! 4: 6cba51ca24 format-patch: teach --no-base @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre + 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")), -+ N_("add a signature from a file")), + N_("add a signature from a file")), OPT__QUIET(&quiet, N_("don't print the patch filenames")), - OPT_BOOL(0, "progress", &show_progress, - N_("show progress while generating patches")), ## t/t4014-format-patch.sh ## @@ t/t4014-format-patch.sh: test_expect_success 'format-patch --base overrides format.useAutoBase' ' 5: 62c59c12e3 = 5: eb266aaedc rebase: fix format.useAutoBase breakage -- 2.24.0.578.g4820254054 ``` ## Denton Liu, 2019-12-04 07:47 Subject: [PATCH v3 1/5] t3400: demonstrate failure with format.useAutoBase Message-ID: <8d67bbe5bfb7b09dff9687a06acb92fbf8202699.1575445583.git.liu.denton@gmail.com> URL: https://gitlist.dev/e/8d67bbe5bfb7b09dff9687a06acb92fbf8202699.1575445583.git.liu.denton%40gmail.com In-Reply-To: ``` Ever since bb52995f3e (format-patch: introduce format.useAutoBase configuration, 2016-04-26), `git rebase` has been broken when `format.useAutoBase = true`. It fails when rebasing a branch that doesn't have an upstream set: fatal: failed to get upstream, if you want to record base commit automatically, please use git branch --set-upstream-to to track a remote branch. Or you could specify base commit by --base= manually error: git encountered an error while preparing the patches to replay these revisions: ede2467cdedc63784887b587a61c36b7850ebfac..d8f581194799ae29bf5fa72a98cbae98a1198b12 As a result, git cannot rebase them. Demonstrate that failure here. Reported-by: Christian Biesinger Signed-off-by: Denton Liu --- t/t3400-rebase.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index ab18ac5f28..ca99e8c6c4 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -159,6 +159,12 @@ test_expect_success 'fail when upstream arg is missing and not configured' ' test_must_fail git rebase ' +test_expect_failure 'rebase works with format.useAutoBase' ' + test_config format.useAutoBase true && + git checkout topic && + git rebase master +' + test_expect_success 'default to common base in @{upstream}s reflog if no upstream arg' ' git checkout -b default-base master && git checkout -b default topic && -- 2.24.0.578.g4820254054 ``` ## Denton Liu, 2019-12-04 07:47 Subject: [PATCH v3 2/5] format-patch: fix indentation Message-ID: <8cfde9f98e416aa44bc27e71d200990fd4d83ae6.1575445583.git.liu.denton@gmail.com> URL: https://gitlist.dev/e/8cfde9f98e416aa44bc27e71d200990fd4d83ae6.1575445583.git.liu.denton%40gmail.com In-Reply-To: ``` Signed-off-by: Denton Liu --- builtin/log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/log.c b/builtin/log.c index a26f223ab4..9c44682f61 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1350,7 +1350,7 @@ static int header_callback(const struct option *opt, const char *arg, int unset) string_list_clear(&extra_to, 0); string_list_clear(&extra_cc, 0); } else { - add_header(arg); + add_header(arg); } return 0; } -- 2.24.0.578.g4820254054 ``` ## Denton Liu, 2019-12-04 07:47 Subject: [PATCH v3 4/5] format-patch: teach --no-base Message-ID: <6cba51ca247423c76bda498152c162900aba1b59.1575445583.git.liu.denton@gmail.com> URL: https://gitlist.dev/e/6cba51ca247423c76bda498152c162900aba1b59.1575445583.git.liu.denton%40gmail.com In-Reply-To: ``` If `format.useAutoBase = true`, there was no way to override this from the command-line. Teach format-patch the `--no-base` option which overrides `format.useAutoBase`. Signed-off-by: Denton Liu --- Documentation/git-format-patch.txt | 5 +++-- builtin/log.c | 22 ++++++++++++++++++++-- t/t4014-format-patch.sh | 6 ++++++ 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 00bdf9b125..0d4f8951bb 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -333,11 +333,12 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`. Output an all-zero hash in each patch's From header instead of the hash of the commit. ---base=:: +--[no-]base[=]:: Record the base tree information to identify the state the patch series applies to. See the BASE TREE INFORMATION section below for details. If is "auto", a base commit is - automatically chosen. + automatically chosen. The `--no-base` option overrides a + `format.useAutoBase` configuration. --root:: Treat the revision argument as a , even if it diff --git a/builtin/log.c b/builtin/log.c index 9c44682f61..645d6db7cc 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1388,6 +1388,23 @@ 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) +{ + char **base_commit = opt->value; + + free(*base_commit); + + if (unset) { + base_auto = 0; + *base_commit = NULL; + } else if (arg) { + *base_commit = xstrdup(arg); + } else { + BUG("arg is NULL"); + } + return 0; +} + struct base_tree_info { struct object_id base_commit; int nr_patch_id, alloc_patch_id; @@ -1676,8 +1693,9 @@ 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")), + { OPTION_CALLBACK, 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")), diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index c7cc643adf..a5b6302a1c 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -1958,6 +1958,12 @@ test_expect_success 'format-patch --base overrides format.useAutoBase' ' test_cmp expect actual ' +test_expect_success 'format-patch --no-base overrides format.useAutoBase' ' + test_config format.useAutoBase true && + 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.24.0.578.g4820254054 ``` ## Denton Liu, 2019-12-04 07:47 Subject: [PATCH v3 3/5] t4014: use test_config() Message-ID: <638c4add00657daa9b7a4fa50882053335d5caef.1575445583.git.liu.denton@gmail.com> URL: https://gitlist.dev/e/638c4add00657daa9b7a4fa50882053335d5caef.1575445583.git.liu.denton%40gmail.com In-Reply-To: ``` Instead of manually unsetting the config after the test case is done, use test_config() to do it automatically. While we're at it, fix a typo in a test case name. Signed-off-by: Denton Liu --- t/t4014-format-patch.sh | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index 69267b16f0..c7cc643adf 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -1939,10 +1939,9 @@ 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 format.useAutoBaseoption' ' - test_when_finished "git config --unset format.useAutoBase" && +test_expect_success 'format-patch format.useAutoBase option' ' git checkout local && - git config format.useAutoBase true && + test_config format.useAutoBase true && git format-patch --stdout -1 >patch && grep "^base-commit:" patch >actual && git rev-parse upstream >commit-id-base && @@ -1951,8 +1950,7 @@ test_expect_success 'format-patch format.useAutoBaseoption' ' ' test_expect_success 'format-patch --base overrides format.useAutoBase' ' - test_when_finished "git config --unset format.useAutoBase" && - git config format.useAutoBase true && + test_config format.useAutoBase true && git format-patch --stdout --base=HEAD~1 -1 >patch && grep "^base-commit:" patch >actual && git rev-parse HEAD~1 >commit-id-base && -- 2.24.0.578.g4820254054 ``` ## Denton Liu, 2019-12-04 07:48 Subject: [PATCH v3 5/5] rebase: fix format.useAutoBase breakage Message-ID: URL: https://gitlist.dev/e/eb266aaedcfb346b405a7a7073caa6b6482480fe.1575445583.git.liu.denton%40gmail.com In-Reply-To: ``` With `format.useAutoBase = true`, running rebase resulted in an error when an upstream wasn't set: fatal: failed to get upstream, if you want to record base commit automatically, please use git branch --set-upstream-to to track a remote branch. Or you could specify base commit by --base= manually error: git encountered an error while preparing the patches to replay these revisions: ede2467cdedc63784887b587a61c36b7850ebfac..d8f581194799ae29bf5fa72a98cbae98a1198b12 As a result, git cannot rebase them. Fix this by always passing `--no-base` to format-patch from rebase so that the effect of `format.useAutoBase` is negated. Reported-by: Christian Biesinger Signed-off-by: Denton Liu --- builtin/rebase.c | 3 ++- t/t3400-rebase.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index e755087b0f..51980ab63d 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1012,7 +1012,8 @@ static int run_am(struct rebase_options *opts) argv_array_pushl(&format_patch.args, "format-patch", "-k", "--stdout", "--full-index", "--cherry-pick", "--right-only", "--src-prefix=a/", "--dst-prefix=b/", "--no-renames", - "--no-cover-letter", "--pretty=mboxrd", "--topo-order", NULL); + "--no-cover-letter", "--pretty=mboxrd", "--topo-order", + "--no-base", NULL); if (opts->git_format_patch_opt.len) argv_array_split(&format_patch.args, opts->git_format_patch_opt.buf); diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index ca99e8c6c4..1323f30fee 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -159,7 +159,7 @@ test_expect_success 'fail when upstream arg is missing and not configured' ' test_must_fail git rebase ' -test_expect_failure 'rebase works with format.useAutoBase' ' +test_expect_success 'rebase works with format.useAutoBase' ' test_config format.useAutoBase true && git checkout topic && git rebase master -- 2.24.0.578.g4820254054 ``` ## René Scharfe, 2019-12-04 10:36 Subject: Re: [PATCH v3 4/5] format-patch: teach --no-base Message-ID: <3d355c0c-805a-a55d-eb4e-e4918585e30c@web.de> URL: https://gitlist.dev/e/3d355c0c-805a-a55d-eb4e-e4918585e30c%40web.de In-Reply-To: <6cba51ca247423c76bda498152c162900aba1b59.1575445583.git.liu.denton@gmail.com> ``` Am 04.12.19 um 08:47 schrieb Denton Liu: > If `format.useAutoBase = true`, there was no way to override this from > the command-line. Teach format-patch the `--no-base` option which > overrides `format.useAutoBase`. > diff --git a/builtin/log.c b/builtin/log.c > index 9c44682f61..645d6db7cc 100644 > --- a/builtin/log.c > +++ b/builtin/log.c > @@ -1388,6 +1388,23 @@ 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) > +{ > + char **base_commit = opt->value; > + > + free(*base_commit); > + > + if (unset) { > + base_auto = 0; > + *base_commit = NULL; > + } else if (arg) { > + *base_commit = xstrdup(arg); > + } else { > + BUG("arg is NULL"); > + } > + return 0; > +} > + > struct base_tree_info { > struct object_id base_commit; > int nr_patch_id, alloc_patch_id; > @@ -1676,8 +1693,9 @@ 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")), > + { OPTION_CALLBACK, 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")), Clearing the global variable base_auto feels unclean to me, as does the introduction of a callback for that purpose. Why not set base_commit after reading the config and before parsing command line options to reflect base_auto? That would achieve the intended precedence in a simpler way, something like this: diff --git a/builtin/log.c b/builtin/log.c index a26f223ab4..af1b0d0209 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1714,6 +1714,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) rev.mime_boundary = default_attach; rev.no_inline = 1; } + if (base_auto) + base_commit = "auto"; /* * Parse the arguments before setup_revisions(), or something @@ -1973,7 +1975,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) } memset(&bases, 0, sizeof(bases)); - if (base_commit || base_auto) { + if (base_commit) { struct commit *base = get_base_commit(base_commit, list, nr); reset_revision_walk(); clear_object_flags(UNINTERESTING); ``` ## Christian Biesinger, 2019-12-04 16:21 Subject: Re: [PATCH v2 5/5] rebase: fix format.useAutoBase breakage Message-ID: URL: https://gitlist.dev/e/CAPTJ0XEZ9ydM-EkuJfarK6epChDLsj9tZsb_KxH8BMYmv-PB7g%40mail.gmail.com In-Reply-To: <62c59c12e376dd73317a7e5d78275bca12875340.1574878089.git.liu.denton@gmail.com> ``` On Wed, Nov 27, 2019 at 12:13 PM Denton Liu wrote: > > With `format.useAutoBase = true`, running rebase resulted in an > error when an upstream wasn't set: FWIW, this also failed for me when an upstream was set. Christian ``` ## Junio C Hamano, 2019-12-04 17:26 Subject: Re: [PATCH v3 4/5] format-patch: teach --no-base Message-ID: URL: https://gitlist.dev/e/xmqq5ziw57ew.fsf%40gitster-ct.c.googlers.com In-Reply-To: <3d355c0c-805a-a55d-eb4e-e4918585e30c@web.de> ``` René Scharfe writes: > Clearing the global variable base_auto feels unclean to me, as does the > introduction of a callback for that purpose. Why not set base_commit > after reading the config and before parsing command line options to > reflect base_auto? That would achieve the intended precedence in a > simpler way, something like this: Nice. > > diff --git a/builtin/log.c b/builtin/log.c > index a26f223ab4..af1b0d0209 100644 > --- a/builtin/log.c > +++ b/builtin/log.c > @@ -1714,6 +1714,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > rev.mime_boundary = default_attach; > rev.no_inline = 1; > } > + if (base_auto) > + base_commit = "auto"; > > /* > * Parse the arguments before setup_revisions(), or something > @@ -1973,7 +1975,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > } > > memset(&bases, 0, sizeof(bases)); > - if (base_commit || base_auto) { > + if (base_commit) { > struct commit *base = get_base_commit(base_commit, list, nr); > reset_revision_walk(); > clear_object_flags(UNINTERESTING); ``` ## Denton Liu, 2019-12-04 21:24 Subject: [PATCH v4 0/5] rebase: fix breakage with `format.useAutoBase` Message-ID: URL: https://gitlist.dev/e/cover.1575494617.git.liu.denton%40gmail.com In-Reply-To: ``` Apparently, this use case has been broken for a long time... Since bb52995f3e (format-patch: introduce format.useAutoBase configuration, 2016-04-26). I'm surprised it's only been reported now. This patchset fixes the breakage by teaching `git format-patch --no-base` and making rebase use it. This patch is based on the latest master since it's such an old bug that only got noticed recently, I'm not sure if it's worth fixing in 'maint'. The series does not apply cleanly on top of 'maint' because it relies on c1a6f21cd4 (Doc: add more detail for git-format-patch, 2019-08-27) but the conflicts are relatively minor. Changes since v3: * Correct error in log messages more precise by saying that rebases fail in general (even if an upstream is set) * Change ugly use of OPT_CALLBACK into a more elegant form Changes since v2: * Remove spurious indentation change * Rebase onto the latest master Changes since v1: * Update some log messages Denton Liu (5): t3400: demonstrate failure with format.useAutoBase format-patch: fix indentation t4014: use test_config() format-patch: teach --no-base rebase: fix format.useAutoBase breakage Documentation/git-format-patch.txt | 5 +++-- builtin/log.c | 9 ++++++--- builtin/rebase.c | 3 ++- t/t3400-rebase.sh | 6 ++++++ t/t4014-format-patch.sh | 14 +++++++++----- 5 files changed, 26 insertions(+), 11 deletions(-) Range-diff against v3: 1: 78b928bf49 ! 1: 386148a550 t3400: demonstrate failure with format.useAutoBase @@ Commit message Ever since bb52995f3e (format-patch: introduce format.useAutoBase configuration, 2016-04-26), `git rebase` has been broken when - `format.useAutoBase = true`. It fails when rebasing a branch that - doesn't have an upstream set: + `format.useAutoBase = true`. It fails when rebasing a branch: fatal: failed to get upstream, if you want to record base commit automatically, please use git branch --set-upstream-to to track a remote branch. 2: 5435a04427 = 2: 0464bd61c2 format-patch: fix indentation 3: 455f2df08d = 3: a55eacbad7 t4014: use test_config() 4: 7c7b94c0c4 ! 4: eb35de8f49 format-patch: teach --no-base @@ Commit message format-patch: teach --no-base If `format.useAutoBase = true`, there was no way to override this from - the command-line. Teach format-patch the `--no-base` option which - overrides `format.useAutoBase`. + the command-line. Teach the `--no-base` option in format-patch to + override `format.useAutoBase`. + + Helped-by: René Scharfe ## Documentation/git-format-patch.txt ## @@ Documentation/git-format-patch.txt: you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`. @@ Documentation/git-format-patch.txt: you can use `--suffix=-patch` to get `0001-d Treat the revision argument as a , even if it ## builtin/log.c ## -@@ 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) -+{ -+ char **base_commit = opt->value; -+ -+ free(*base_commit); -+ -+ if (unset) { -+ base_auto = 0; -+ *base_commit = NULL; -+ } else if (arg) { -+ *base_commit = xstrdup(arg); -+ } else { -+ BUG("arg is NULL"); -+ } -+ 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, + base = lookup_commit_reference_by_name(base_commit); + if (!base) + die(_("unknown commit %s"), base_commit); +- } else if ((base_commit && !strcmp(base_commit, "auto")) || base_auto) { ++ } else if ((base_commit && !strcmp(base_commit, "auto"))) { + struct branch *curr_branch = branch_get(NULL); + const char *upstream = branch_get_upstream(curr_branch, NULL); + if (upstream) { @@ 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")), -+ { OPTION_CALLBACK, 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")), -+ N_("add a signature from a file")), - OPT__QUIET(&quiet, N_("don't print the patch filenames")), - OPT_BOOL(0, "progress", &show_progress, - N_("show progress while generating patches")), + s_r_opt.def = "HEAD"; + s_r_opt.revarg_opt = REVARG_COMMITTISH; + ++ if (base_auto) ++ 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 || base_auto) { ++ if (base_commit) { + struct commit *base = get_base_commit(base_commit, list, nr); + reset_revision_walk(); + clear_object_flags(UNINTERESTING); ## t/t4014-format-patch.sh ## @@ t/t4014-format-patch.sh: test_expect_success 'format-patch --base overrides format.useAutoBase' ' 5: 2b4166e371 ! 5: 210905f163 rebase: fix format.useAutoBase breakage @@ Commit message rebase: fix format.useAutoBase breakage With `format.useAutoBase = true`, running rebase resulted in an - error when an upstream wasn't set: + error: fatal: failed to get upstream, if you want to record base commit automatically, please use git branch --set-upstream-to to track a remote branch. -- 2.24.0.578.g4820254054 ``` ## Denton Liu, 2019-12-04 21:24 Subject: [PATCH v4 1/5] t3400: demonstrate failure with format.useAutoBase Message-ID: <386148a550bdbd06e3c35cd506f69c229a4df664.1575494618.git.liu.denton@gmail.com> URL: https://gitlist.dev/e/386148a550bdbd06e3c35cd506f69c229a4df664.1575494618.git.liu.denton%40gmail.com In-Reply-To: ``` Ever since bb52995f3e (format-patch: introduce format.useAutoBase configuration, 2016-04-26), `git rebase` has been broken when `format.useAutoBase = true`. It fails when rebasing a branch: fatal: failed to get upstream, if you want to record base commit automatically, please use git branch --set-upstream-to to track a remote branch. Or you could specify base commit by --base= manually error: git encountered an error while preparing the patches to replay these revisions: ede2467cdedc63784887b587a61c36b7850ebfac..d8f581194799ae29bf5fa72a98cbae98a1198b12 As a result, git cannot rebase them. Demonstrate that failure here. Reported-by: Christian Biesinger Signed-off-by: Denton Liu --- t/t3400-rebase.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index ab18ac5f28..ca99e8c6c4 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -159,6 +159,12 @@ test_expect_success 'fail when upstream arg is missing and not configured' ' test_must_fail git rebase ' +test_expect_failure 'rebase works with format.useAutoBase' ' + test_config format.useAutoBase true && + git checkout topic && + git rebase master +' + test_expect_success 'default to common base in @{upstream}s reflog if no upstream arg' ' git checkout -b default-base master && git checkout -b default topic && -- 2.24.0.578.g4820254054 ``` ## Denton Liu, 2019-12-04 21:24 Subject: [PATCH v4 2/5] format-patch: fix indentation Message-ID: <0464bd61c26109bc6d98ac56bb8bf1fe9d1a6864.1575494618.git.liu.denton@gmail.com> URL: https://gitlist.dev/e/0464bd61c26109bc6d98ac56bb8bf1fe9d1a6864.1575494618.git.liu.denton%40gmail.com In-Reply-To: ``` Signed-off-by: Denton Liu --- builtin/log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/log.c b/builtin/log.c index a26f223ab4..9c44682f61 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1350,7 +1350,7 @@ static int header_callback(const struct option *opt, const char *arg, int unset) string_list_clear(&extra_to, 0); string_list_clear(&extra_cc, 0); } else { - add_header(arg); + add_header(arg); } return 0; } -- 2.24.0.578.g4820254054 ``` ## Denton Liu, 2019-12-04 21:25 Subject: [PATCH v4 3/5] t4014: use test_config() Message-ID: URL: https://gitlist.dev/e/a55eacbad7ca0f15d038fa61d33190b1beb663a7.1575494618.git.liu.denton%40gmail.com In-Reply-To: ``` Instead of manually unsetting the config after the test case is done, use test_config() to do it automatically. While we're at it, fix a typo in a test case name. Signed-off-by: Denton Liu --- t/t4014-format-patch.sh | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index 69267b16f0..c7cc643adf 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -1939,10 +1939,9 @@ 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 format.useAutoBaseoption' ' - test_when_finished "git config --unset format.useAutoBase" && +test_expect_success 'format-patch format.useAutoBase option' ' git checkout local && - git config format.useAutoBase true && + test_config format.useAutoBase true && git format-patch --stdout -1 >patch && grep "^base-commit:" patch >actual && git rev-parse upstream >commit-id-base && @@ -1951,8 +1950,7 @@ test_expect_success 'format-patch format.useAutoBaseoption' ' ' test_expect_success 'format-patch --base overrides format.useAutoBase' ' - test_when_finished "git config --unset format.useAutoBase" && - git config format.useAutoBase true && + test_config format.useAutoBase true && git format-patch --stdout --base=HEAD~1 -1 >patch && grep "^base-commit:" patch >actual && git rev-parse HEAD~1 >commit-id-base && -- 2.24.0.578.g4820254054 ``` ## Denton Liu, 2019-12-04 21:25 Subject: [PATCH v4 4/5] format-patch: teach --no-base Message-ID: URL: https://gitlist.dev/e/eb35de8f4917a406d886a6da4180d2d90e95e633.1575494618.git.liu.denton%40gmail.com In-Reply-To: ``` If `format.useAutoBase = true`, there was no way to override this from the command-line. Teach the `--no-base` option in format-patch to override `format.useAutoBase`. Helped-by: René Scharfe Signed-off-by: Denton Liu --- Documentation/git-format-patch.txt | 5 +++-- builtin/log.c | 7 +++++-- t/t4014-format-patch.sh | 6 ++++++ 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 00bdf9b125..0d4f8951bb 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -333,11 +333,12 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`. Output an all-zero hash in each patch's From header instead of the hash of the commit. ---base=:: +--[no-]base[=]:: Record the base tree information to identify the state the patch series applies to. See the BASE TREE INFORMATION section below for details. If is "auto", a base commit is - automatically chosen. + automatically chosen. The `--no-base` option overrides a + `format.useAutoBase` configuration. --root:: Treat the revision argument as a , even if it diff --git a/builtin/log.c b/builtin/log.c index 9c44682f61..bf904e887f 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1406,7 +1406,7 @@ static struct commit *get_base_commit(const char *base_commit, base = lookup_commit_reference_by_name(base_commit); if (!base) die(_("unknown commit %s"), base_commit); - } else if ((base_commit && !strcmp(base_commit, "auto")) || base_auto) { + } else if ((base_commit && !strcmp(base_commit, "auto"))) { struct branch *curr_branch = branch_get(NULL); const char *upstream = branch_get_upstream(curr_branch, NULL); if (upstream) { @@ -1710,6 +1710,9 @@ 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) + base_commit = "auto"; + if (default_attach) { rev.mime_boundary = default_attach; rev.no_inline = 1; @@ -1973,7 +1976,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) } memset(&bases, 0, sizeof(bases)); - if (base_commit || base_auto) { + if (base_commit) { struct commit *base = get_base_commit(base_commit, list, nr); reset_revision_walk(); clear_object_flags(UNINTERESTING); diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index c7cc643adf..a5b6302a1c 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -1958,6 +1958,12 @@ test_expect_success 'format-patch --base overrides format.useAutoBase' ' test_cmp expect actual ' +test_expect_success 'format-patch --no-base overrides format.useAutoBase' ' + test_config format.useAutoBase true && + 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.24.0.578.g4820254054 ``` ## Denton Liu, 2019-12-04 21:25 Subject: [PATCH v4 5/5] rebase: fix format.useAutoBase breakage Message-ID: <210905f16303e9ce7ae3845e252bc07e288f70c1.1575494618.git.liu.denton@gmail.com> URL: https://gitlist.dev/e/210905f16303e9ce7ae3845e252bc07e288f70c1.1575494618.git.liu.denton%40gmail.com In-Reply-To: ``` With `format.useAutoBase = true`, running rebase resulted in an error: fatal: failed to get upstream, if you want to record base commit automatically, please use git branch --set-upstream-to to track a remote branch. Or you could specify base commit by --base= manually error: git encountered an error while preparing the patches to replay these revisions: ede2467cdedc63784887b587a61c36b7850ebfac..d8f581194799ae29bf5fa72a98cbae98a1198b12 As a result, git cannot rebase them. Fix this by always passing `--no-base` to format-patch from rebase so that the effect of `format.useAutoBase` is negated. Reported-by: Christian Biesinger Signed-off-by: Denton Liu --- builtin/rebase.c | 3 ++- t/t3400-rebase.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index e755087b0f..51980ab63d 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1012,7 +1012,8 @@ static int run_am(struct rebase_options *opts) argv_array_pushl(&format_patch.args, "format-patch", "-k", "--stdout", "--full-index", "--cherry-pick", "--right-only", "--src-prefix=a/", "--dst-prefix=b/", "--no-renames", - "--no-cover-letter", "--pretty=mboxrd", "--topo-order", NULL); + "--no-cover-letter", "--pretty=mboxrd", "--topo-order", + "--no-base", NULL); if (opts->git_format_patch_opt.len) argv_array_split(&format_patch.args, opts->git_format_patch_opt.buf); diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index ca99e8c6c4..1323f30fee 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -159,7 +159,7 @@ test_expect_success 'fail when upstream arg is missing and not configured' ' test_must_fail git rebase ' -test_expect_failure 'rebase works with format.useAutoBase' ' +test_expect_success 'rebase works with format.useAutoBase' ' test_config format.useAutoBase true && git checkout topic && git rebase master -- 2.24.0.578.g4820254054 ```