git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Bug with "git rebase" when format.useAutoBase is set
@ 2019-11-26 20:50 Christian Biesinger
  2019-11-27  2:09 ` [PATCH 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
  0 siblings, 1 reply; 33+ messages in thread
From: Christian Biesinger @ 2019-11-26 20:50 UTC (permalink / raw)
  To: git

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=<base-commit-id> manually
error:
git encountered an error while preparing the patches to replay
these revisions:

    4a7dabea39556979cbf6bcbdd98a9413ce129fd0..12f4dc41bf9f018ecc1042830d5534de941f329e

As a result, git cannot rebase them.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 0/5] rebase: fix breakage with `format.useAutoBase`
  2019-11-26 20:50 Bug with "git rebase" when format.useAutoBase is set Christian Biesinger
@ 2019-11-27  2:09 ` Denton Liu
  2019-11-27  2:09   ` [PATCH 1/5] t3400: demonstrate failure with format.useAutoBase Denton Liu
                     ` (6 more replies)
  0 siblings, 7 replies; 33+ messages in thread
From: Denton Liu @ 2019-11-27  2:09 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Christian Biesinger

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


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 1/5] t3400: demonstrate failure with format.useAutoBase
  2019-11-27  2:09 ` [PATCH 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
@ 2019-11-27  2:09   ` Denton Liu
  2019-11-27  2:26     ` Eric Sunshine
  2019-11-27  2:09   ` [PATCH 2/5] format-patch: fix indentation Denton Liu
                     ` (5 subsequent siblings)
  6 siblings, 1 reply; 33+ messages in thread
From: Denton Liu @ 2019-11-27  2:09 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Christian Biesinger

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 <cbiesinger@google.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 2/5] format-patch: fix indentation
  2019-11-27  2:09 ` [PATCH 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
  2019-11-27  2:09   ` [PATCH 1/5] t3400: demonstrate failure with format.useAutoBase Denton Liu
@ 2019-11-27  2:09   ` Denton Liu
  2019-11-27  2:09   ` [PATCH 3/5] t4014: use `test_config` Denton Liu
                     ` (4 subsequent siblings)
  6 siblings, 0 replies; 33+ messages in thread
From: Denton Liu @ 2019-11-27  2:09 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Christian Biesinger

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 3/5] t4014: use `test_config`
  2019-11-27  2:09 ` [PATCH 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
  2019-11-27  2:09   ` [PATCH 1/5] t3400: demonstrate failure with format.useAutoBase Denton Liu
  2019-11-27  2:09   ` [PATCH 2/5] format-patch: fix indentation Denton Liu
@ 2019-11-27  2:09   ` Denton Liu
  2019-11-27  2:09   ` [PATCH 4/5] format-patch: teach --no-base Denton Liu
                     ` (3 subsequent siblings)
  6 siblings, 0 replies; 33+ messages in thread
From: Denton Liu @ 2019-11-27  2:09 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Christian Biesinger

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 <liu.denton@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 4/5] format-patch: teach --no-base
  2019-11-27  2:09 ` [PATCH 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
                     ` (2 preceding siblings ...)
  2019-11-27  2:09   ` [PATCH 3/5] t4014: use `test_config` Denton Liu
@ 2019-11-27  2:09   ` Denton Liu
  2019-11-27  2:09   ` [PATCH 5/5] rebase: fix `format.useAutoBase` breakage Denton Liu
                     ` (2 subsequent siblings)
  6 siblings, 0 replies; 33+ messages in thread
From: Denton Liu @ 2019-11-27  2:09 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Christian Biesinger

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 <liu.denton@gmail.com>
---
 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=<commit>::
+--[no-]base[=<commit>]::
 	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.
+	automatically chosen. The `--no-base` option overrides a
+	`format.useAutoBase` configuration.
 
 --root::
 	Treat the revision argument as a <revision range>, 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 5/5] rebase: fix `format.useAutoBase` breakage
  2019-11-27  2:09 ` [PATCH 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
                     ` (3 preceding siblings ...)
  2019-11-27  2:09   ` [PATCH 4/5] format-patch: teach --no-base Denton Liu
@ 2019-11-27  2:09   ` Denton Liu
  2019-11-27  2:42     ` Eric Sunshine
  2019-11-27 17:14   ` [PATCH 0/5] rebase: fix breakage with `format.useAutoBase` Christian Biesinger
  2019-11-27 18:13   ` [PATCH v2 " Denton Liu
  6 siblings, 1 reply; 33+ messages in thread
From: Denton Liu @ 2019-11-27  2:09 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Christian Biesinger

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=<base-commit-id> 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 <cbiesinger@google.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/5] t3400: demonstrate failure with format.useAutoBase
  2019-11-27  2:09   ` [PATCH 1/5] t3400: demonstrate failure with format.useAutoBase Denton Liu
@ 2019-11-27  2:26     ` Eric Sunshine
  2019-11-30 17:25       ` Junio C Hamano
  0 siblings, 1 reply; 33+ messages in thread
From: Eric Sunshine @ 2019-11-27  2:26 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List, Christian Biesinger

On Tue, Nov 26, 2019 at 9:09 PM Denton Liu <liu.denton@gmail.com> 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 <cbiesinger@google.com>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> 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.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 5/5] rebase: fix `format.useAutoBase` breakage
  2019-11-27  2:09   ` [PATCH 5/5] rebase: fix `format.useAutoBase` breakage Denton Liu
@ 2019-11-27  2:42     ` Eric Sunshine
  0 siblings, 0 replies; 33+ messages in thread
From: Eric Sunshine @ 2019-11-27  2:42 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List, Christian Biesinger

On Tue, Nov 26, 2019 at 9:09 PM Denton Liu <liu.denton@gmail.com> 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=<base-commit-id> 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 <cbiesinger@google.com>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 0/5] rebase: fix breakage with `format.useAutoBase`
  2019-11-27  2:09 ` [PATCH 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
                     ` (4 preceding siblings ...)
  2019-11-27  2:09   ` [PATCH 5/5] rebase: fix `format.useAutoBase` breakage Denton Liu
@ 2019-11-27 17:14   ` Christian Biesinger
  2019-11-27 18:13   ` [PATCH v2 " Denton Liu
  6 siblings, 0 replies; 33+ messages in thread
From: Christian Biesinger @ 2019-11-27 17:14 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List

On Tue, Nov 26, 2019 at 8:09 PM Denton Liu <liu.denton@gmail.com> 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
>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v2 0/5] rebase: fix breakage with `format.useAutoBase`
  2019-11-27  2:09 ` [PATCH 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
                     ` (5 preceding siblings ...)
  2019-11-27 17:14   ` [PATCH 0/5] rebase: fix breakage with `format.useAutoBase` Christian Biesinger
@ 2019-11-27 18:13   ` Denton Liu
  2019-11-27 18:13     ` [PATCH v2 1/5] t3400: demonstrate failure with format.useAutoBase Denton Liu
                       ` (5 more replies)
  6 siblings, 6 replies; 33+ messages in thread
From: Denton Liu @ 2019-11-27 18:13 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Christian Biesinger, Eric Sunshine

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=<base-commit-id> 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 <cbiesinger@google.com>
     
2:  46fd4113aa = 2:  d288d6c3a5 format-patch: fix indentation
3:  22b1fb14f9 ! 3:  196b5d8dbc t4014: use `test_config`
    @@ Metadata
     Author: Denton Liu <liu.denton@gmail.com>
     
      ## 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 <liu.denton@gmail.com>
     
      ## 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


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v2 1/5] t3400: demonstrate failure with format.useAutoBase
  2019-11-27 18:13   ` [PATCH v2 " Denton Liu
@ 2019-11-27 18:13     ` Denton Liu
  2019-11-27 18:13     ` [PATCH v2 2/5] format-patch: fix indentation Denton Liu
                       ` (4 subsequent siblings)
  5 siblings, 0 replies; 33+ messages in thread
From: Denton Liu @ 2019-11-27 18:13 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Christian Biesinger, Eric Sunshine

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=<base-commit-id> 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 <cbiesinger@google.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v2 2/5] format-patch: fix indentation
  2019-11-27 18:13   ` [PATCH v2 " Denton Liu
  2019-11-27 18:13     ` [PATCH v2 1/5] t3400: demonstrate failure with format.useAutoBase Denton Liu
@ 2019-11-27 18:13     ` Denton Liu
  2019-11-27 18:13     ` [PATCH v2 3/5] t4014: use test_config() Denton Liu
                       ` (3 subsequent siblings)
  5 siblings, 0 replies; 33+ messages in thread
From: Denton Liu @ 2019-11-27 18:13 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Christian Biesinger, Eric Sunshine

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v2 3/5] t4014: use test_config()
  2019-11-27 18:13   ` [PATCH v2 " Denton Liu
  2019-11-27 18:13     ` [PATCH v2 1/5] t3400: demonstrate failure with format.useAutoBase Denton Liu
  2019-11-27 18:13     ` [PATCH v2 2/5] format-patch: fix indentation Denton Liu
@ 2019-11-27 18:13     ` Denton Liu
  2019-11-27 18:13     ` [PATCH v2 4/5] format-patch: teach --no-base Denton Liu
                       ` (2 subsequent siblings)
  5 siblings, 0 replies; 33+ messages in thread
From: Denton Liu @ 2019-11-27 18:13 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Christian Biesinger, Eric Sunshine

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 <liu.denton@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v2 4/5] format-patch: teach --no-base
  2019-11-27 18:13   ` [PATCH v2 " Denton Liu
                       ` (2 preceding siblings ...)
  2019-11-27 18:13     ` [PATCH v2 3/5] t4014: use test_config() Denton Liu
@ 2019-11-27 18:13     ` Denton Liu
  2019-11-27 19:56       ` Denton Liu
  2019-11-27 18:13     ` [PATCH v2 5/5] rebase: fix format.useAutoBase breakage Denton Liu
  2019-12-04  7:47     ` [PATCH v3 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
  5 siblings, 1 reply; 33+ messages in thread
From: Denton Liu @ 2019-11-27 18:13 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Christian Biesinger, Eric Sunshine

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 <liu.denton@gmail.com>
---
 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=<commit>::
+--[no-]base[=<commit>]::
 	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.
+	automatically chosen. The `--no-base` option overrides a
+	`format.useAutoBase` configuration.
 
 --root::
 	Treat the revision argument as a <revision range>, 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v2 5/5] rebase: fix format.useAutoBase breakage
  2019-11-27 18:13   ` [PATCH v2 " Denton Liu
                       ` (3 preceding siblings ...)
  2019-11-27 18:13     ` [PATCH v2 4/5] format-patch: teach --no-base Denton Liu
@ 2019-11-27 18:13     ` Denton Liu
  2019-12-04 16:21       ` Christian Biesinger
  2019-12-04  7:47     ` [PATCH v3 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
  5 siblings, 1 reply; 33+ messages in thread
From: Denton Liu @ 2019-11-27 18:13 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Christian Biesinger, Eric Sunshine

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=<base-commit-id> 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 <cbiesinger@google.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH v2 4/5] format-patch: teach --no-base
  2019-11-27 18:13     ` [PATCH v2 4/5] format-patch: teach --no-base Denton Liu
@ 2019-11-27 19:56       ` Denton Liu
  0 siblings, 0 replies; 33+ messages in thread
From: Denton Liu @ 2019-11-27 19:56 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Christian Biesinger, Eric Sunshine

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 <liu.denton@gmail.com>
> ---
>  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=<commit>::
> +--[no-]base[=<commit>]::
>  	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.
> +	automatically chosen. The `--no-base` option overrides a
> +	`format.useAutoBase` configuration.
>  
>  --root::
>  	Treat the revision argument as a <revision range>, 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
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/5] t3400: demonstrate failure with format.useAutoBase
  2019-11-27  2:26     ` Eric Sunshine
@ 2019-11-30 17:25       ` Junio C Hamano
  0 siblings, 0 replies; 33+ messages in thread
From: Junio C Hamano @ 2019-11-30 17:25 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Denton Liu, Git Mailing List, Christian Biesinger

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Tue, Nov 26, 2019 at 9:09 PM Denton Liu <liu.denton@gmail.com> 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 <cbiesinger@google.com>
>> Signed-off-by: Denton Liu <liu.denton@gmail.com>
>> ---
>> 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.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v3 0/5] rebase: fix breakage with `format.useAutoBase`
  2019-11-27 18:13   ` [PATCH v2 " Denton Liu
                       ` (4 preceding siblings ...)
  2019-11-27 18:13     ` [PATCH v2 5/5] rebase: fix format.useAutoBase breakage Denton Liu
@ 2019-12-04  7:47     ` Denton Liu
  2019-12-04  7:47       ` [PATCH v3 1/5] t3400: demonstrate failure with format.useAutoBase Denton Liu
                         ` (5 more replies)
  5 siblings, 6 replies; 33+ messages in thread
From: Denton Liu @ 2019-12-04  7:47 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Christian Biesinger, Eric Sunshine, Junio C Hamano

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


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v3 1/5] t3400: demonstrate failure with format.useAutoBase
  2019-12-04  7:47     ` [PATCH v3 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
@ 2019-12-04  7:47       ` Denton Liu
  2019-12-04  7:47       ` [PATCH v3 2/5] format-patch: fix indentation Denton Liu
                         ` (4 subsequent siblings)
  5 siblings, 0 replies; 33+ messages in thread
From: Denton Liu @ 2019-12-04  7:47 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Christian Biesinger, Eric Sunshine, Junio C Hamano

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=<base-commit-id> 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 <cbiesinger@google.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 2/5] format-patch: fix indentation
  2019-12-04  7:47     ` [PATCH v3 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
  2019-12-04  7:47       ` [PATCH v3 1/5] t3400: demonstrate failure with format.useAutoBase Denton Liu
@ 2019-12-04  7:47       ` Denton Liu
  2019-12-04  7:47       ` [PATCH v3 3/5] t4014: use test_config() Denton Liu
                         ` (3 subsequent siblings)
  5 siblings, 0 replies; 33+ messages in thread
From: Denton Liu @ 2019-12-04  7:47 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Christian Biesinger, Eric Sunshine, Junio C Hamano

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 3/5] t4014: use test_config()
  2019-12-04  7:47     ` [PATCH v3 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
  2019-12-04  7:47       ` [PATCH v3 1/5] t3400: demonstrate failure with format.useAutoBase Denton Liu
  2019-12-04  7:47       ` [PATCH v3 2/5] format-patch: fix indentation Denton Liu
@ 2019-12-04  7:47       ` Denton Liu
  2019-12-04  7:47       ` [PATCH v3 4/5] format-patch: teach --no-base Denton Liu
                         ` (2 subsequent siblings)
  5 siblings, 0 replies; 33+ messages in thread
From: Denton Liu @ 2019-12-04  7:47 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Christian Biesinger, Eric Sunshine, Junio C Hamano

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 <liu.denton@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 4/5] format-patch: teach --no-base
  2019-12-04  7:47     ` [PATCH v3 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
                         ` (2 preceding siblings ...)
  2019-12-04  7:47       ` [PATCH v3 3/5] t4014: use test_config() Denton Liu
@ 2019-12-04  7:47       ` Denton Liu
  2019-12-04 10:36         ` René Scharfe
  2019-12-04  7:48       ` [PATCH v3 5/5] rebase: fix format.useAutoBase breakage Denton Liu
  2019-12-04 21:24       ` [PATCH v4 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
  5 siblings, 1 reply; 33+ messages in thread
From: Denton Liu @ 2019-12-04  7:47 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Christian Biesinger, Eric Sunshine, Junio C Hamano

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 <liu.denton@gmail.com>
---
 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=<commit>::
+--[no-]base[=<commit>]::
 	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.
+	automatically chosen. The `--no-base` option overrides a
+	`format.useAutoBase` configuration.
 
 --root::
 	Treat the revision argument as a <revision range>, 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 5/5] rebase: fix format.useAutoBase breakage
  2019-12-04  7:47     ` [PATCH v3 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
                         ` (3 preceding siblings ...)
  2019-12-04  7:47       ` [PATCH v3 4/5] format-patch: teach --no-base Denton Liu
@ 2019-12-04  7:48       ` Denton Liu
  2019-12-04 21:24       ` [PATCH v4 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
  5 siblings, 0 replies; 33+ messages in thread
From: Denton Liu @ 2019-12-04  7:48 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Christian Biesinger, Eric Sunshine, Junio C Hamano

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=<base-commit-id> 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 <cbiesinger@google.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 4/5] format-patch: teach --no-base
  2019-12-04  7:47       ` [PATCH v3 4/5] format-patch: teach --no-base Denton Liu
@ 2019-12-04 10:36         ` René Scharfe
  2019-12-04 17:26           ` Junio C Hamano
  0 siblings, 1 reply; 33+ messages in thread
From: René Scharfe @ 2019-12-04 10:36 UTC (permalink / raw)
  To: Denton Liu, Git Mailing List
  Cc: Christian Biesinger, Eric Sunshine, Junio C Hamano

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);


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH v2 5/5] rebase: fix format.useAutoBase breakage
  2019-11-27 18:13     ` [PATCH v2 5/5] rebase: fix format.useAutoBase breakage Denton Liu
@ 2019-12-04 16:21       ` Christian Biesinger
  0 siblings, 0 replies; 33+ messages in thread
From: Christian Biesinger @ 2019-12-04 16:21 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List, Eric Sunshine

On Wed, Nov 27, 2019 at 12:13 PM Denton Liu <liu.denton@gmail.com> 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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 4/5] format-patch: teach --no-base
  2019-12-04 10:36         ` René Scharfe
@ 2019-12-04 17:26           ` Junio C Hamano
  0 siblings, 0 replies; 33+ messages in thread
From: Junio C Hamano @ 2019-12-04 17:26 UTC (permalink / raw)
  To: René Scharfe
  Cc: Denton Liu, Git Mailing List, Christian Biesinger, Eric Sunshine

René Scharfe <l.s.r@web.de> 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);

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v4 0/5] rebase: fix breakage with `format.useAutoBase`
  2019-12-04  7:47     ` [PATCH v3 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
                         ` (4 preceding siblings ...)
  2019-12-04  7:48       ` [PATCH v3 5/5] rebase: fix format.useAutoBase breakage Denton Liu
@ 2019-12-04 21:24       ` Denton Liu
  2019-12-04 21:24         ` [PATCH v4 1/5] t3400: demonstrate failure with format.useAutoBase Denton Liu
                           ` (4 more replies)
  5 siblings, 5 replies; 33+ messages in thread
From: Denton Liu @ 2019-12-04 21:24 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Christian Biesinger, Eric Sunshine, Junio C Hamano, René Scharfe

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 <l.s.r@web.de>
     
      ## 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 <revision range>, 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


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v4 1/5] t3400: demonstrate failure with format.useAutoBase
  2019-12-04 21:24       ` [PATCH v4 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
@ 2019-12-04 21:24         ` Denton Liu
  2019-12-04 21:24         ` [PATCH v4 2/5] format-patch: fix indentation Denton Liu
                           ` (3 subsequent siblings)
  4 siblings, 0 replies; 33+ messages in thread
From: Denton Liu @ 2019-12-04 21:24 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Christian Biesinger, Eric Sunshine, Junio C Hamano, René Scharfe

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=<base-commit-id> 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 <cbiesinger@google.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 2/5] format-patch: fix indentation
  2019-12-04 21:24       ` [PATCH v4 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
  2019-12-04 21:24         ` [PATCH v4 1/5] t3400: demonstrate failure with format.useAutoBase Denton Liu
@ 2019-12-04 21:24         ` Denton Liu
  2019-12-04 21:25         ` [PATCH v4 3/5] t4014: use test_config() Denton Liu
                           ` (2 subsequent siblings)
  4 siblings, 0 replies; 33+ messages in thread
From: Denton Liu @ 2019-12-04 21:24 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Christian Biesinger, Eric Sunshine, Junio C Hamano, René Scharfe

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 3/5] t4014: use test_config()
  2019-12-04 21:24       ` [PATCH v4 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
  2019-12-04 21:24         ` [PATCH v4 1/5] t3400: demonstrate failure with format.useAutoBase Denton Liu
  2019-12-04 21:24         ` [PATCH v4 2/5] format-patch: fix indentation Denton Liu
@ 2019-12-04 21:25         ` Denton Liu
  2019-12-04 21:25         ` [PATCH v4 4/5] format-patch: teach --no-base Denton Liu
  2019-12-04 21:25         ` [PATCH v4 5/5] rebase: fix format.useAutoBase breakage Denton Liu
  4 siblings, 0 replies; 33+ messages in thread
From: Denton Liu @ 2019-12-04 21:25 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Christian Biesinger, Eric Sunshine, Junio C Hamano, René Scharfe

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 <liu.denton@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 4/5] format-patch: teach --no-base
  2019-12-04 21:24       ` [PATCH v4 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
                           ` (2 preceding siblings ...)
  2019-12-04 21:25         ` [PATCH v4 3/5] t4014: use test_config() Denton Liu
@ 2019-12-04 21:25         ` Denton Liu
  2019-12-04 21:25         ` [PATCH v4 5/5] rebase: fix format.useAutoBase breakage Denton Liu
  4 siblings, 0 replies; 33+ messages in thread
From: Denton Liu @ 2019-12-04 21:25 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Christian Biesinger, Eric Sunshine, Junio C Hamano, René Scharfe

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 <l.s.r@web.de>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 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=<commit>::
+--[no-]base[=<commit>]::
 	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.
+	automatically chosen. The `--no-base` option overrides a
+	`format.useAutoBase` configuration.
 
 --root::
 	Treat the revision argument as a <revision range>, 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 5/5] rebase: fix format.useAutoBase breakage
  2019-12-04 21:24       ` [PATCH v4 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
                           ` (3 preceding siblings ...)
  2019-12-04 21:25         ` [PATCH v4 4/5] format-patch: teach --no-base Denton Liu
@ 2019-12-04 21:25         ` Denton Liu
  4 siblings, 0 replies; 33+ messages in thread
From: Denton Liu @ 2019-12-04 21:25 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Christian Biesinger, Eric Sunshine, Junio C Hamano, René Scharfe

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=<base-commit-id> 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 <cbiesinger@google.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2019-12-04 21:25 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-26 20:50 Bug with "git rebase" when format.useAutoBase is set Christian Biesinger
2019-11-27  2:09 ` [PATCH 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
2019-11-27  2:09   ` [PATCH 1/5] t3400: demonstrate failure with format.useAutoBase Denton Liu
2019-11-27  2:26     ` Eric Sunshine
2019-11-30 17:25       ` Junio C Hamano
2019-11-27  2:09   ` [PATCH 2/5] format-patch: fix indentation Denton Liu
2019-11-27  2:09   ` [PATCH 3/5] t4014: use `test_config` Denton Liu
2019-11-27  2:09   ` [PATCH 4/5] format-patch: teach --no-base Denton Liu
2019-11-27  2:09   ` [PATCH 5/5] rebase: fix `format.useAutoBase` breakage Denton Liu
2019-11-27  2:42     ` Eric Sunshine
2019-11-27 17:14   ` [PATCH 0/5] rebase: fix breakage with `format.useAutoBase` Christian Biesinger
2019-11-27 18:13   ` [PATCH v2 " Denton Liu
2019-11-27 18:13     ` [PATCH v2 1/5] t3400: demonstrate failure with format.useAutoBase Denton Liu
2019-11-27 18:13     ` [PATCH v2 2/5] format-patch: fix indentation Denton Liu
2019-11-27 18:13     ` [PATCH v2 3/5] t4014: use test_config() Denton Liu
2019-11-27 18:13     ` [PATCH v2 4/5] format-patch: teach --no-base Denton Liu
2019-11-27 19:56       ` Denton Liu
2019-11-27 18:13     ` [PATCH v2 5/5] rebase: fix format.useAutoBase breakage Denton Liu
2019-12-04 16:21       ` Christian Biesinger
2019-12-04  7:47     ` [PATCH v3 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
2019-12-04  7:47       ` [PATCH v3 1/5] t3400: demonstrate failure with format.useAutoBase Denton Liu
2019-12-04  7:47       ` [PATCH v3 2/5] format-patch: fix indentation Denton Liu
2019-12-04  7:47       ` [PATCH v3 3/5] t4014: use test_config() Denton Liu
2019-12-04  7:47       ` [PATCH v3 4/5] format-patch: teach --no-base Denton Liu
2019-12-04 10:36         ` René Scharfe
2019-12-04 17:26           ` Junio C Hamano
2019-12-04  7:48       ` [PATCH v3 5/5] rebase: fix format.useAutoBase breakage Denton Liu
2019-12-04 21:24       ` [PATCH v4 0/5] rebase: fix breakage with `format.useAutoBase` Denton Liu
2019-12-04 21:24         ` [PATCH v4 1/5] t3400: demonstrate failure with format.useAutoBase Denton Liu
2019-12-04 21:24         ` [PATCH v4 2/5] format-patch: fix indentation Denton Liu
2019-12-04 21:25         ` [PATCH v4 3/5] t4014: use test_config() Denton Liu
2019-12-04 21:25         ` [PATCH v4 4/5] format-patch: teach --no-base Denton Liu
2019-12-04 21:25         ` [PATCH v4 5/5] rebase: fix format.useAutoBase breakage Denton Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).