Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 7, 2014, 20:36 UTC
- Message-ID
- <xmqq8uurcyw2.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1389126588-3663-1-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
Show 18 quoted lines
> A very common workflow for preparing patches involves working off a > topic branch and generating patches against 'master' to send off to the > maintainer. However, a plain > > $ git format-patch -o outgoing > > is a no-op on a topic branch, and the user has to remember to specify > 'master' explicitly everytime. This problem is not unique to > format-patch; even a > > $ git rebase -i > > is a no-op because the branch to rebase against isn't specified. > > To tackle this problem, introduce branch.*.forkedFrom which can specify > the parent branch of a topic branch. Future patches will build > functionality around this new configuration variable. >
I do not mind allowing laziness by defaulting to something, but I am not enthused by an approach that adds the new variable whose value is questionable. The description does not justify at all why @{upstream} is not a good default (unless the workflow is screwed up and @{upstream} is set to point at somewhere that is _not_ a true upstream, that is).
Show 90 quoted lines
> Cc: Jeff King <peff@peff.net>
> Cc: Junio C Hamano <gister@pobox.com>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
> Since -M, -C, -D are left in the argc, checking argc < 2 isn't
> sufficient.
>
> I wanted to get an early reaction before wiring up checkout and
> rebase.
>
> But I wanted to discuss the overall idea of the patch.
> builtin/log.c | 21 +++++++++++++++++++++
> t/t4014-format-patch.sh | 20 ++++++++++++++++++++
> 2 files changed, 41 insertions(+)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index b97373d..525e696 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -674,6 +674,7 @@ static int thread;
> static int do_signoff;
> static const char *signature = git_version_string;
> static int config_cover_letter;
> +static const char *config_base_branch;
>
> enum {
> COVER_UNSET,
> @@ -750,6 +751,22 @@ static int git_format_config(const char *var, const char *value, void *cb)
> config_cover_letter = git_config_bool(var, value) ? COVER_ON : COVER_OFF;
> return 0;
> }
> + if (starts_with(var, "branch.")) {
> + const char *name = var + 7;
> + const char *subkey = strrchr(name, '.');
> + struct strbuf buf = STRBUF_INIT;
> +
> + if (!subkey)
> + return 0;
> + strbuf_add(&buf, name, subkey - name);
> + if (branch_get(buf.buf) != branch_get(NULL))
> + return 0;
> + strbuf_release(&buf);
> + if (!strcmp(subkey, ".forkedfrom")) {
> + if (git_config_string(&config_base_branch, var, value))
> + return -1;
> + }
> + }
>
> return git_log_config(var, value, cb);
> }
> @@ -1324,6 +1341,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
> die (_("--subject-prefix and -k are mutually exclusive."));
> rev.preserve_subject = keep_subject;
>
> + if (argc < 2 && config_base_branch) {
> + argv[1] = config_base_branch;
> + argc++;
> + }
> argc = setup_revisions(argc, argv, &rev, &s_r_opt);
> if (argc > 1)
> die (_("unrecognized argument: %s"), argv[1]);
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index 73194b2..2ea94af 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -1370,4 +1370,24 @@ test_expect_success 'cover letter auto user override' '
> test_line_count = 2 list
> '
>
> +test_expect_success 'branch.*.forkedFrom matches' '
> + mkdir -p tmp &&
> + test_when_finished "rm -rf tmp;
> + git config --unset branch.rebuild-1.forkedFrom" &&
> +
> + git config branch.rebuild-1.forkedFrom master &&
> + git format-patch -o tmp >list &&
> + test_line_count = 2 list
> +'
> +
> +test_expect_success 'branch.*.forkedFrom does not match' '
> + mkdir -p tmp &&
> + test_when_finished "rm -rf tmp;
> + git config --unset branch.foo.forkedFrom" &&
> +
> + git config branch.foo.forkedFrom master &&
> + git format-patch -o tmp >list &&
> + test_line_count = 0 list
> +'
> +
> test_done