[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