[RFC/PATCH] format-patch: introduce branch.*.forkedFrom
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.
Cc: Jeff King p...@peff.net
Cc: Junio C Hamano gis...@pobox.com
Signed-off-by: Ramkumar Ramachandra artag...@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
--
1.8.5.2.234.gba2dde8.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom
[Fixed typo in Junio's address]
On Wed, Jan 8, 2014 at 1:59 AM, Ramkumar Ramachandra artag...@gmail.com wrote:
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.
Cc: Jeff King p...@peff.net
Cc: Junio C Hamano gis...@pobox.com
Signed-off-by: Ramkumar Ramachandra artag...@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
--
1.8.5.2.234.gba2dde8.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom
Ramkumar Ramachandra artag...@gmail.com writes:
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).
Cc: Jeff King p...@peff.net
Cc: Junio C Hamano gis...@pobox.com
Signed-off-by: Ramkumar Ramachandra artag...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom
On Wed, Jan 08, 2014 at 02:00:44AM +0530, Ramkumar Ramachandra wrote:
On Wed, Jan 8, 2014 at 1:59 AM, Ramkumar Ramachandra artag...@gmail.com
wrote:
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.
Cc: Jeff King p...@peff.net
Cc: Junio C Hamano gis...@pobox.com
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
I have not carefully read some of the later bits of the discussion from
last night / this morning, so maybe I am missing something, but this
seems backwards to me from what Junio and I were discussing earlier.
The point was that the meaning of @{upstream} (and branch.*.merge)
is _already_ forked-from, and push -u and push.default=upstream
are the odd men out. If we are going to add an option to distinguish the
two branch relationships:
1. Where you forked from
2. Where you push to
we should leave @{upstream} as (1), and add a new option to represent
(2). Not the other way around.
Am I missing something?
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom
Junio C Hamano wrote:
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).
Did you find the explanation I gave in
http://article.gmane.org/gmane.comp.version-control.git/240077
reasonable? I don't know why label the respin-workflow as being
screwed up.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom
Ramkumar Ramachandra artag...@gmail.com writes:
Junio C Hamano wrote:
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).
Did you find the explanation I gave in
http://article.gmane.org/gmane.comp.version-control.git/240077
reasonable?
No.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom
Jeff King p...@peff.net writes:
The point was that the meaning of @{upstream} (and branch.*.merge)
is _already_ forked-from, and push -u and push.default=upstream
are the odd men out. If we are going to add an option to distinguish the
two branch relationships:
1. Where you forked from
2. Where you push to
we should leave @{upstream} as (1), and add a new option to represent
(2). Not the other way around.
That matches my feeling as well.
I am not sure if push -u is truly odd man out, though. It was an
invention back in the you fetch from and push to the same place and
there is no other workflow support days, and in that context, the
upstream meant just that: the place you fetch from, which happens
to be the same as where you are pushing to right now. If push -u
suddenly stopped setting the configuration to merge back from where
it is pushing, that would regress for centralized folks, so I am not
sure how it could be extended to also support triangular folks, but
I do think @{upstream} should mean this is where I sync with to
stay abreast with others.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom
Jeff King wrote:
I have not carefully read some of the later bits of the discussion from
last night / this morning, so maybe I am missing something, but this
seems backwards to me from what Junio and I were discussing earlier.
The point was that the meaning of @{upstream} (and branch.*.merge)
is _already_ forked-from, and push -u and push.default=upstream
are the odd men out. If we are going to add an option to distinguish the
two branch relationships:
1. Where you forked from
2. Where you push to
we should leave @{upstream} as (1), and add a new option to represent
(2). Not the other way around.
I have a local branch 'forkedfrom' that has two sources: 'master'
and 'ram/forkedfrom'. 'ram/forkedfrom' isn't a dumb publish-point:
the relationship information I get between 'forkedfrom' and
'ram/forkedfrom' is very useful; it's what helps me tell how my
re-roll is doing with respect to the original series; I'd often want
to cherry-pick commits/ messages from my original series to prepare
the re-roll, so interaction with this source is quite high. On the
other hand, the relationship information I get between 'forkedfrom'
and 'master' is practically useless: 'forkedfrom' is always ahead of
'master', and a divergence indicates that I need to rebase; I'll never
really need to interact with this source.
I'm only thinking in terms of what infrastructure we've already built:
if @{u} is set to 'ram/forkedfrom', we get a lot of information for
free _now_. If @{u} is set to 'master', the current git-status is
unhelpful.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom
On Wed, Jan 08, 2014 at 02:32:10AM +0530, Ramkumar Ramachandra wrote:
we should leave @{upstream} as (1), and add a new option to represent
(2). Not the other way around.
I have a local branch 'forkedfrom' that has two sources: 'master'
and 'ram/forkedfrom'. 'ram/forkedfrom' isn't a dumb publish-point:
the relationship information I get between 'forkedfrom' and
'ram/forkedfrom' is very useful; it's what helps me tell how my
re-roll is doing with respect to the original series; I'd often want
to cherry-pick commits/ messages from my original series to prepare
the re-roll, so interaction with this source is quite high. On the
other hand, the relationship information I get between 'forkedfrom'
and 'master' is practically useless: 'forkedfrom' is always ahead of
'master', and a divergence indicates that I need to rebase; I'll never
really need to interact with this source.
Thanks for a concrete example.
I definitely respect the desire to reuse the existing tooling we have
for @{u}. At the same time, I think you are warping the meaning of
@{u} somewhat. It is _not_ your upstream here, but rather another
version of the branch that has useful changes in it. That might be
splitting hairs a bit, but I think you will find that the differences
leak through in inconvenient spots (like format-patch, where you really
_do_ want to default to the true upstream).
If we add @{publish} (and @{pu}), then it becomes very convenient to
refer to the ram/ version of your branch. That seems like an obvious
first step to me. We don't have to add new config, because
branch.*.pushremote already handles this.
Now you can do git rebase @{pu} which is nice, but not _quite_ as nice
as git rebase, which defaults to @{u}. That first step might be
enough, and I'd hold off there and try it out for a few days or weeks
first. But if you find in your workflow that you are having to specify
@{pu} a lot, then maybe it is worth adding an option to default rebase
to @{pu} instead of @{u}.
You end up in the same place (git rebase without options does what you
want), but I think the underlying data more accurately represents what
is going on (and there is no need to teach format-patch anything
special).
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom
Jeff King wrote:
I definitely respect the desire to reuse the existing tooling we have
for @{u}. At the same time, I think you are warping the meaning of
@{u} somewhat. It is _not_ your upstream here, but rather another
version of the branch that has useful changes in it. That might be
splitting hairs a bit, but I think you will find that the differences
leak through in inconvenient spots (like format-patch, where you really
_do_ want to default to the true upstream).
Thanks for the clear reasoning.
If we add @{publish} (and @{pu}), then it becomes very convenient to
refer to the ram/ version of your branch. That seems like an obvious
first step to me. We don't have to add new config, because
branch.*.pushremote already handles this.
Agreed. I'll start working on @{publish}. It's going to take quite a
bit of effort, because I won't actually start using it until my prompt
is @{publish}-aware.
Now you can do git rebase @{pu} which is nice, but not _quite_ as nice
as git rebase, which defaults to @{u}. That first step might be
enough, and I'd hold off there and try it out for a few days or weeks
first. But if you find in your workflow that you are having to specify
@{pu} a lot, then maybe it is worth adding an option to default rebase
to @{pu} instead of @{u}.
Actually, I'm not sure I'd use git rebase @{pu}; for me @{pu} is
mainly a source of information for taking apart to build a new series.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
