* [PATCH v3 0/6] Support triangular workflows
@ 2013-03-22 7:52 Ramkumar Ramachandra
2013-03-22 7:52 ` [PATCH 1/6] remote.c: simplify a bit of code using git_config_string() Ramkumar Ramachandra
` (5 more replies)
0 siblings, 6 replies; 29+ messages in thread
From: Ramkumar Ramachandra @ 2013-03-22 7:52 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jeff King, Jonathan Nieder
After Jonathan's review of [1], I've decided to pick two changes to
apply in this iteration:
1. [1/6], [5/6] and [6/6] now use the return value of
git_config_string(), hence handling configuration errors properly.
I consider this a major and important improvement.
2. The commit message in [4/6] has been augmented with a small
example. This is mainly a nit, as full-blown examples are already
present in the next two patches in the series.
Thanks.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/218598
Ramkumar Ramachandra (6):
remote.c: simplify a bit of code using git_config_string()
t5516 (fetch-push): update test description
t5516 (fetch-push): introduce mk_test_with_name()
remote.c: introduce a way to have different remotes for fetch/push
remote.c: introduce remote.pushdefault
remote.c: introduce branch.<name>.pushremote
Documentation/config.txt | 23 +++++++++++++++---
builtin/push.c | 2 +-
remote.c | 39 ++++++++++++++++++++++++------
remote.h | 1 +
t/t5516-fetch-push.sh | 63 ++++++++++++++++++++++++++++++++++++++++--------
5 files changed, 107 insertions(+), 21 deletions(-)
--
1.8.2.62.ga35d936.dirty
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 1/6] remote.c: simplify a bit of code using git_config_string() 2013-03-22 7:52 [PATCH v3 0/6] Support triangular workflows Ramkumar Ramachandra @ 2013-03-22 7:52 ` Ramkumar Ramachandra 2013-03-22 18:26 ` Jonathan Nieder 2013-03-22 7:52 ` [PATCH 2/6] t5516 (fetch-push): update test description Ramkumar Ramachandra ` (4 subsequent siblings) 5 siblings, 1 reply; 29+ messages in thread From: Ramkumar Ramachandra @ 2013-03-22 7:52 UTC (permalink / raw) To: Git List; +Cc: Junio C Hamano, Jeff King, Jonathan Nieder A small segment where handle_config() parses the branch.remote configuration variable can be simplified using git_config_string(). Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com> --- remote.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/remote.c b/remote.c index e53a6eb..5bd59bb 100644 --- a/remote.c +++ b/remote.c @@ -356,9 +356,8 @@ static int handle_config(const char *key, const char *value, void *cb) return 0; branch = make_branch(name, subkey - name); if (!strcmp(subkey, ".remote")) { - if (!value) - return config_error_nonbool(key); - branch->remote_name = xstrdup(value); + if (git_config_string(&branch->remote_name, key, value)) + return -1; if (branch == current_branch) { default_remote_name = branch->remote_name; explicit_default_remote_name = 1; -- 1.8.2.62.ga35d936.dirty ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 1/6] remote.c: simplify a bit of code using git_config_string() 2013-03-22 7:52 ` [PATCH 1/6] remote.c: simplify a bit of code using git_config_string() Ramkumar Ramachandra @ 2013-03-22 18:26 ` Jonathan Nieder 0 siblings, 0 replies; 29+ messages in thread From: Jonathan Nieder @ 2013-03-22 18:26 UTC (permalink / raw) To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano, Jeff King Ramkumar Ramachandra wrote: > A small segment where handle_config() parses the branch.remote > configuration variable can be simplified using git_config_string(). Looks correct. ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 2/6] t5516 (fetch-push): update test description 2013-03-22 7:52 [PATCH v3 0/6] Support triangular workflows Ramkumar Ramachandra 2013-03-22 7:52 ` [PATCH 1/6] remote.c: simplify a bit of code using git_config_string() Ramkumar Ramachandra @ 2013-03-22 7:52 ` Ramkumar Ramachandra 2013-03-22 18:29 ` Jonathan Nieder 2013-03-22 7:52 ` [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name() Ramkumar Ramachandra ` (3 subsequent siblings) 5 siblings, 1 reply; 29+ messages in thread From: Ramkumar Ramachandra @ 2013-03-22 7:52 UTC (permalink / raw) To: Git List; +Cc: Junio C Hamano, Jeff King, Jonathan Nieder The file was originally created in bcdb34f (Test wildcard push/fetch, 2007-06-08), and only contained tests that exercised wildcard functionality at the time. In subsequent commits, many other tests unrelated to wildcards were added but the test description was never updated. Fix this. Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com> --- t/t5516-fetch-push.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index c31e5c1..bfeec60 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -1,6 +1,6 @@ #!/bin/sh -test_description='fetching and pushing, with or without wildcard' +test_description='fetching and pushing' . ./test-lib.sh -- 1.8.2.62.ga35d936.dirty ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 2/6] t5516 (fetch-push): update test description 2013-03-22 7:52 ` [PATCH 2/6] t5516 (fetch-push): update test description Ramkumar Ramachandra @ 2013-03-22 18:29 ` Jonathan Nieder 0 siblings, 0 replies; 29+ messages in thread From: Jonathan Nieder @ 2013-03-22 18:29 UTC (permalink / raw) To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano, Jeff King Ramkumar Ramachandra wrote: > --- a/t/t5516-fetch-push.sh > +++ b/t/t5516-fetch-push.sh > @@ -1,6 +1,6 @@ > #!/bin/sh > > -test_description='fetching and pushing, with or without wildcard' > +test_description='fetching and pushing' The description before and after are equally useless. You might as well use the following description: test_description='t5516-fetch-push.sh' (Please don't actually do that.) I gave a sketch of what I think might be a more useful description and got shot down without an explanation I could understand in reply. So, this one needs more work I guess. Hope that helps, Jonathan ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name() 2013-03-22 7:52 [PATCH v3 0/6] Support triangular workflows Ramkumar Ramachandra 2013-03-22 7:52 ` [PATCH 1/6] remote.c: simplify a bit of code using git_config_string() Ramkumar Ramachandra 2013-03-22 7:52 ` [PATCH 2/6] t5516 (fetch-push): update test description Ramkumar Ramachandra @ 2013-03-22 7:52 ` Ramkumar Ramachandra 2013-03-22 14:44 ` Jeff King 2013-03-22 14:54 ` Junio C Hamano 2013-03-22 7:52 ` [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push Ramkumar Ramachandra ` (2 subsequent siblings) 5 siblings, 2 replies; 29+ messages in thread From: Ramkumar Ramachandra @ 2013-03-22 7:52 UTC (permalink / raw) To: Git List; +Cc: Junio C Hamano, Jeff King, Jonathan Nieder mk_test() creates a repository with the constant name "testrepo", and this may be limiting for tests that need to create more than one repository for testing. To fix this, create a new mk_test_with_name() which accepts the repository name as $1. Reimplement mk_test() as a special case of this function, making sure that no tests need to be rewritten. Do the same thing for check_push_result(). Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com> --- t/t5516-fetch-push.sh | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index bfeec60..a546c2c 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -7,27 +7,32 @@ test_description='fetching and pushing' D=`pwd` mk_empty () { - rm -fr testrepo && - mkdir testrepo && + repo_name="$1" + test -z "$repo_name" && repo_name=testrepo + rm -fr $repo_name && + mkdir $repo_name && ( - cd testrepo && + cd $repo_name && git init && git config receive.denyCurrentBranch warn && mv .git/hooks .git/hooks-disabled ) } -mk_test () { - mk_empty && +mk_test_with_name () { + repo_name="$1" + shift + + mk_empty $repo_name && ( for ref in "$@" do - git push testrepo $the_first_commit:refs/$ref || { + git push $repo_name $the_first_commit:refs/$ref || { echo "Oops, push refs/$ref failure" exit 1 } done && - cd testrepo && + cd $repo_name && for ref in "$@" do r=$(git show-ref -s --verify refs/$ref) && @@ -40,6 +45,10 @@ mk_test () { ) } +mk_test () { + mk_test_with_name testrepo "$@" +} + mk_test_with_hooks() { mk_test "$@" && ( @@ -79,9 +88,12 @@ mk_child() { git clone testrepo "$1" } -check_push_result () { +check_push_result_with_name () { + repo_name="$1" + shift + ( - cd testrepo && + cd $repo_name && it="$1" && shift for ref in "$@" @@ -96,6 +108,10 @@ check_push_result () { ) } +check_push_result () { + check_push_result_with_name testrepo "$@" +} + test_expect_success setup ' >path1 && -- 1.8.2.62.ga35d936.dirty ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name() 2013-03-22 7:52 ` [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name() Ramkumar Ramachandra @ 2013-03-22 14:44 ` Jeff King 2013-03-22 14:52 ` Junio C Hamano 2013-03-28 13:03 ` Ramkumar Ramachandra 2013-03-22 14:54 ` Junio C Hamano 1 sibling, 2 replies; 29+ messages in thread From: Jeff King @ 2013-03-22 14:44 UTC (permalink / raw) To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano, Jonathan Nieder On Fri, Mar 22, 2013 at 01:22:33PM +0530, Ramkumar Ramachandra wrote: > mk_test() creates a repository with the constant name "testrepo", and > this may be limiting for tests that need to create more than one > repository for testing. To fix this, create a new mk_test_with_name() > which accepts the repository name as $1. Reimplement mk_test() as a > special case of this function, making sure that no tests need to be > rewritten. Do the same thing for check_push_result(). I think this is OK, and I do not mind if it gets applied. But what I was hinting at in my earlier mail was that we might want to do this (I have it as a separate patch on top of your 3/6 here, but it would make more sense squashed in): -- >8 -- Subject: [PATCH] t5516: drop implicit arguments from helper functions Many of the tests in t5516 look like: mk_empty && git push testrepo ... && check_push_result $commit heads/master It's reasonably easy to see what is being tested, with the exception that "testrepo" is a magic global name (it is implicitly used in the helpers, but we have to name it explicitly when calling git directly). Let's make it explicit when call the helpers, too. This is slightly more typing, but makes the test snippets read more naturally. It also makes it easy for future tests to use an alternate or multiple repositories, without a proliferation of helper functions. Signed-off-by: Jeff King <peff@peff.net> --- t/t5516-fetch-push.sh | 268 ++++++++++++++++++++++++-------------------------- 1 file changed, 130 insertions(+), 138 deletions(-) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 05579b6..d27b8d3 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -8,7 +8,6 @@ mk_empty () { mk_empty () { repo_name="$1" - test -z "$repo_name" && repo_name=testrepo rm -fr $repo_name && mkdir $repo_name && ( @@ -19,7 +18,7 @@ mk_empty () { ) } -mk_test_with_name () { +mk_test () { repo_name="$1" shift @@ -45,14 +44,11 @@ mk_test_with_hooks() { ) } -mk_test () { - mk_test_with_name testrepo "$@" -} - mk_test_with_hooks() { + repo_name=$1 mk_test "$@" && ( - cd testrepo && + cd $repo_name && mkdir .git/hooks && cd .git/hooks && @@ -84,11 +80,11 @@ mk_child() { } mk_child() { - rm -rf "$1" && - git clone testrepo "$1" + rm -rf "$2" && + git clone "$1" "$2" } -check_push_result_with_name () { +check_push_result () { repo_name="$1" shift @@ -108,10 +104,6 @@ check_push_result_with_name () { ) } -check_push_result () { - check_push_result_with_name testrepo "$@" -} - test_expect_success setup ' >path1 && @@ -129,7 +121,7 @@ test_expect_success 'fetch without wildcard' ' ' test_expect_success 'fetch without wildcard' ' - mk_empty && + mk_empty testrepo && ( cd testrepo && git fetch .. refs/heads/master:refs/remotes/origin/master && @@ -142,7 +134,7 @@ test_expect_success 'fetch with wildcard' ' ' test_expect_success 'fetch with wildcard' ' - mk_empty && + mk_empty testrepo && ( cd testrepo && git config remote.up.url .. && @@ -157,7 +149,7 @@ test_expect_success 'fetch with insteadOf' ' ' test_expect_success 'fetch with insteadOf' ' - mk_empty && + mk_empty testrepo && ( TRASH=$(pwd)/ && cd testrepo && @@ -174,7 +166,7 @@ test_expect_success 'fetch with pushInsteadOf (should not rewrite)' ' ' test_expect_success 'fetch with pushInsteadOf (should not rewrite)' ' - mk_empty && + mk_empty testrepo && ( TRASH=$(pwd)/ && cd testrepo && @@ -191,7 +183,7 @@ test_expect_success 'push without wildcard' ' ' test_expect_success 'push without wildcard' ' - mk_empty && + mk_empty testrepo && git push testrepo refs/heads/master:refs/remotes/origin/master && ( @@ -204,7 +196,7 @@ test_expect_success 'push with wildcard' ' ' test_expect_success 'push with wildcard' ' - mk_empty && + mk_empty testrepo && git push testrepo "refs/heads/*:refs/remotes/origin/*" && ( @@ -217,7 +209,7 @@ test_expect_success 'push with insteadOf' ' ' test_expect_success 'push with insteadOf' ' - mk_empty && + mk_empty testrepo && TRASH="$(pwd)/" && git config "url.$TRASH.insteadOf" trash/ && git push trash/testrepo refs/heads/master:refs/remotes/origin/master && @@ -231,7 +223,7 @@ test_expect_success 'push with pushInsteadOf' ' ' test_expect_success 'push with pushInsteadOf' ' - mk_empty && + mk_empty testrepo && TRASH="$(pwd)/" && git config "url.$TRASH.pushInsteadOf" trash/ && git push trash/testrepo refs/heads/master:refs/remotes/origin/master && @@ -245,7 +237,7 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf ' test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf should not rewrite)' ' - mk_empty && + mk_empty testrepo && TRASH="$(pwd)/" && git config "url.trash2/.pushInsteadOf" trash/ && git config remote.r.url trash/wrong && @@ -262,242 +254,242 @@ test_expect_success 'push with config remote.*.push = HEAD' ' test_expect_success 'push with matching heads' ' - mk_test heads/master && + mk_test testrepo heads/master && git push testrepo && - check_push_result $the_commit heads/master + check_push_result testrepo $the_commit heads/master ' test_expect_success 'push with matching heads on the command line' ' - mk_test heads/master && + mk_test testrepo heads/master && git push testrepo : && - check_push_result $the_commit heads/master + check_push_result testrepo $the_commit heads/master ' test_expect_success 'failed (non-fast-forward) push with matching heads' ' - mk_test heads/master && + mk_test testrepo heads/master && git push testrepo : && git commit --amend -massaged && test_must_fail git push testrepo && - check_push_result $the_commit heads/master && + check_push_result testrepo $the_commit heads/master && git reset --hard $the_commit ' test_expect_success 'push --force with matching heads' ' - mk_test heads/master && + mk_test testrepo heads/master && git push testrepo : && git commit --amend -massaged && git push --force testrepo && - ! check_push_result $the_commit heads/master && + ! check_push_result testrepo $the_commit heads/master && git reset --hard $the_commit ' test_expect_success 'push with matching heads and forced update' ' - mk_test heads/master && + mk_test testrepo heads/master && git push testrepo : && git commit --amend -massaged && git push testrepo +: && - ! check_push_result $the_commit heads/master && + ! check_push_result testrepo $the_commit heads/master && git reset --hard $the_commit ' test_expect_success 'push with no ambiguity (1)' ' - mk_test heads/master && + mk_test testrepo heads/master && git push testrepo master:master && - check_push_result $the_commit heads/master + check_push_result testrepo $the_commit heads/master ' test_expect_success 'push with no ambiguity (2)' ' - mk_test remotes/origin/master && + mk_test testrepo remotes/origin/master && git push testrepo master:origin/master && - check_push_result $the_commit remotes/origin/master + check_push_result testrepo $the_commit remotes/origin/master ' test_expect_success 'push with colon-less refspec, no ambiguity' ' - mk_test heads/master heads/t/master && + mk_test testrepo heads/master heads/t/master && git branch -f t/master master && git push testrepo master && - check_push_result $the_commit heads/master && - check_push_result $the_first_commit heads/t/master + check_push_result testrepo $the_commit heads/master && + check_push_result testrepo $the_first_commit heads/t/master ' test_expect_success 'push with weak ambiguity (1)' ' - mk_test heads/master remotes/origin/master && + mk_test testrepo heads/master remotes/origin/master && git push testrepo master:master && - check_push_result $the_commit heads/master && - check_push_result $the_first_commit remotes/origin/master + check_push_result testrepo $the_commit heads/master && + check_push_result testrepo $the_first_commit remotes/origin/master ' test_expect_success 'push with weak ambiguity (2)' ' - mk_test heads/master remotes/origin/master remotes/another/master && + mk_test testrepo heads/master remotes/origin/master remotes/another/master && git push testrepo master:master && - check_push_result $the_commit heads/master && - check_push_result $the_first_commit remotes/origin/master remotes/another/master + check_push_result testrepo $the_commit heads/master && + check_push_result testrepo $the_first_commit remotes/origin/master remotes/another/master ' test_expect_success 'push with ambiguity' ' - mk_test heads/frotz tags/frotz && + mk_test testrepo heads/frotz tags/frotz && if git push testrepo master:frotz then echo "Oops, should have failed" false else - check_push_result $the_first_commit heads/frotz tags/frotz + check_push_result testrepo $the_first_commit heads/frotz tags/frotz fi ' test_expect_success 'push with colon-less refspec (1)' ' - mk_test heads/frotz tags/frotz && + mk_test testrepo heads/frotz tags/frotz && git branch -f frotz master && git push testrepo frotz && - check_push_result $the_commit heads/frotz && - check_push_result $the_first_commit tags/frotz + check_push_result testrepo $the_commit heads/frotz && + check_push_result testrepo $the_first_commit tags/frotz ' test_expect_success 'push with colon-less refspec (2)' ' - mk_test heads/frotz tags/frotz && + mk_test testrepo heads/frotz tags/frotz && if git show-ref --verify -q refs/heads/frotz then git branch -D frotz fi && git tag -f frotz && git push -f testrepo frotz && - check_push_result $the_commit tags/frotz && - check_push_result $the_first_commit heads/frotz + check_push_result testrepo $the_commit tags/frotz && + check_push_result testrepo $the_first_commit heads/frotz ' test_expect_success 'push with colon-less refspec (3)' ' - mk_test && + mk_test testrepo && if git show-ref --verify -q refs/tags/frotz then git tag -d frotz fi && git branch -f frotz master && git push testrepo frotz && - check_push_result $the_commit heads/frotz && + check_push_result testrepo $the_commit heads/frotz && test 1 = $( cd testrepo && git show-ref | wc -l ) ' test_expect_success 'push with colon-less refspec (4)' ' - mk_test && + mk_test testrepo && if git show-ref --verify -q refs/heads/frotz then git branch -D frotz fi && git tag -f frotz && git push testrepo frotz && - check_push_result $the_commit tags/frotz && + check_push_result testrepo $the_commit tags/frotz && test 1 = $( cd testrepo && git show-ref | wc -l ) ' test_expect_success 'push head with non-existent, incomplete dest' ' - mk_test && + mk_test testrepo && git push testrepo master:branch && - check_push_result $the_commit heads/branch + check_push_result testrepo $the_commit heads/branch ' test_expect_success 'push tag with non-existent, incomplete dest' ' - mk_test && + mk_test testrepo && git tag -f v1.0 && git push testrepo v1.0:tag && - check_push_result $the_commit tags/tag + check_push_result testrepo $the_commit tags/tag ' test_expect_success 'push sha1 with non-existent, incomplete dest' ' - mk_test && + mk_test testrepo && test_must_fail git push testrepo `git rev-parse master`:foo ' test_expect_success 'push ref expression with non-existent, incomplete dest' ' - mk_test && + mk_test testrepo && test_must_fail git push testrepo master^:branch ' test_expect_success 'push with HEAD' ' - mk_test heads/master && + mk_test testrepo heads/master && git checkout master && git push testrepo HEAD && - check_push_result $the_commit heads/master + check_push_result testrepo $the_commit heads/master ' test_expect_success 'push with HEAD nonexisting at remote' ' - mk_test heads/master && + mk_test testrepo heads/master && git checkout -b local master && git push testrepo HEAD && - check_push_result $the_commit heads/local + check_push_result testrepo $the_commit heads/local ' test_expect_success 'push with +HEAD' ' - mk_test heads/master && + mk_test testrepo heads/master && git checkout master && git branch -D local && git checkout -b local && git push testrepo master local && - check_push_result $the_commit heads/master && - check_push_result $the_commit heads/local && + check_push_result testrepo $the_commit heads/master && + check_push_result testrepo $the_commit heads/local && # Without force rewinding should fail git reset --hard HEAD^ && test_must_fail git push testrepo HEAD && - check_push_result $the_commit heads/local && + check_push_result testrepo $the_commit heads/local && # With force rewinding should succeed git push testrepo +HEAD && - check_push_result $the_first_commit heads/local + check_push_result testrepo $the_first_commit heads/local ' test_expect_success 'push HEAD with non-existent, incomplete dest' ' - mk_test && + mk_test testrepo && git checkout master && git push testrepo HEAD:branch && - check_push_result $the_commit heads/branch + check_push_result testrepo $the_commit heads/branch ' test_expect_success 'push with config remote.*.push = HEAD' ' - mk_test heads/local && + mk_test testrepo heads/local && git checkout master && git branch -f local $the_commit && ( @@ -509,8 +501,8 @@ test_expect_success 'push with config remote.*.push = HEAD' ' git config remote.there.push HEAD && git config branch.master.remote there && git push && - check_push_result $the_commit heads/master && - check_push_result $the_first_commit heads/local + check_push_result testrepo $the_commit heads/master && + check_push_result testrepo $the_first_commit heads/local ' # clean up the cruft left with the previous one @@ -519,12 +511,12 @@ test_expect_success 'push with config remote.*.pushurl' ' test_expect_success 'push with config remote.*.pushurl' ' - mk_test heads/master && + mk_test testrepo heads/master && git checkout master && git config remote.there.url test2repo && git config remote.there.pushurl testrepo && git push there && - check_push_result $the_commit heads/master + check_push_result testrepo $the_commit heads/master ' # clean up the cruft left with the previous one @@ -532,19 +524,19 @@ test_expect_success 'push updates local refs' ' test_expect_success 'push with dry-run' ' - mk_test heads/master && + mk_test testrepo heads/master && ( cd testrepo && old_commit=$(git show-ref -s --verify refs/heads/master) ) && git push --dry-run testrepo && - check_push_result $old_commit heads/master + check_push_result testrepo $old_commit heads/master ' test_expect_success 'push updates local refs' ' - mk_test heads/master && - mk_child child && + mk_test testrepo heads/master && + mk_child testrepo child && ( cd child && git pull .. master && @@ -557,9 +549,9 @@ test_expect_success 'push updates up-to-date local refs' ' test_expect_success 'push updates up-to-date local refs' ' - mk_test heads/master && - mk_child child1 && - mk_child child2 && + mk_test testrepo heads/master && + mk_child testrepo child1 && + mk_child testrepo child2 && (cd child1 && git pull .. master && git push) && ( cd child2 && @@ -573,8 +565,8 @@ test_expect_success 'push preserves up-to-date packed refs' ' test_expect_success 'push preserves up-to-date packed refs' ' - mk_test heads/master && - mk_child child && + mk_test testrepo heads/master && + mk_child testrepo child && ( cd child && git push && @@ -585,8 +577,8 @@ test_expect_success 'push does not update local refs on failure' ' test_expect_success 'push does not update local refs on failure' ' - mk_test heads/master && - mk_child child && + mk_test testrepo heads/master && + mk_child testrepo child && mkdir testrepo/.git/hooks && echo "#!/no/frobnication/today" >testrepo/.git/hooks/pre-receive && chmod +x testrepo/.git/hooks/pre-receive && @@ -602,7 +594,7 @@ test_expect_success 'allow deleting an invalid remote ref' ' test_expect_success 'allow deleting an invalid remote ref' ' - mk_test heads/master && + mk_test testrepo heads/master && rm -f testrepo/.git/objects/??/* && git push testrepo :refs/heads/master && (cd testrepo && test_must_fail git rev-parse --verify refs/heads/master) @@ -610,7 +602,7 @@ test_expect_success 'pushing valid refs triggers post-receive and post-update ho ' test_expect_success 'pushing valid refs triggers post-receive and post-update hooks' ' - mk_test_with_hooks heads/master heads/next && + mk_test_with_hooks testrepo heads/master heads/next && orgmaster=$(cd testrepo && git show-ref -s --verify refs/heads/master) && newmaster=$(git show-ref -s --verify refs/heads/master) && orgnext=$(cd testrepo && git show-ref -s --verify refs/heads/next) && @@ -646,7 +638,7 @@ test_expect_success 'deleting dangling ref triggers hooks with correct args' ' ' test_expect_success 'deleting dangling ref triggers hooks with correct args' ' - mk_test_with_hooks heads/master && + mk_test_with_hooks testrepo heads/master && rm -f testrepo/.git/objects/??/* && git push testrepo :refs/heads/master && ( @@ -675,7 +667,7 @@ test_expect_success 'deletion of a non-existent ref is not fed to post-receive a ' test_expect_success 'deletion of a non-existent ref is not fed to post-receive and post-update hooks' ' - mk_test_with_hooks heads/master && + mk_test_with_hooks testrepo heads/master && orgmaster=$(cd testrepo && git show-ref -s --verify refs/heads/master) && newmaster=$(git show-ref -s --verify refs/heads/master) && git push testrepo master :refs/heads/nonexistent && @@ -707,7 +699,7 @@ test_expect_success 'deletion of a non-existent ref alone does trigger post-rece ' test_expect_success 'deletion of a non-existent ref alone does trigger post-receive and post-update hooks' ' - mk_test_with_hooks heads/master && + mk_test_with_hooks testrepo heads/master && git push testrepo :refs/heads/nonexistent && ( cd testrepo/.git && @@ -727,7 +719,7 @@ test_expect_success 'mixed ref updates, deletes, invalid deletes trigger hooks w ' test_expect_success 'mixed ref updates, deletes, invalid deletes trigger hooks with correct input' ' - mk_test_with_hooks heads/master heads/next heads/pu && + mk_test_with_hooks testrepo heads/master heads/next heads/pu && orgmaster=$(cd testrepo && git show-ref -s --verify refs/heads/master) && newmaster=$(git show-ref -s --verify refs/heads/master) && orgnext=$(cd testrepo && git show-ref -s --verify refs/heads/next) && @@ -773,14 +765,14 @@ test_expect_success 'allow deleting a tag using --delete' ' ' test_expect_success 'allow deleting a ref using --delete' ' - mk_test heads/master && + mk_test testrepo heads/master && (cd testrepo && git config receive.denyDeleteCurrent warn) && git push testrepo --delete master && (cd testrepo && test_must_fail git rev-parse --verify refs/heads/master) ' test_expect_success 'allow deleting a tag using --delete' ' - mk_test heads/master && + mk_test testrepo heads/master && git tag -a -m dummy_message deltag heads/master && git push testrepo --tags && (cd testrepo && git rev-parse --verify -q refs/tags/deltag) && @@ -789,17 +781,17 @@ test_expect_success 'warn on push to HEAD of non-bare repository' ' ' test_expect_success 'push --delete without args aborts' ' - mk_test heads/master && + mk_test testrepo heads/master && test_must_fail git push testrepo --delete ' test_expect_success 'push --delete refuses src:dest refspecs' ' - mk_test heads/master && + mk_test testrepo heads/master && test_must_fail git push testrepo --delete master:foo ' test_expect_success 'warn on push to HEAD of non-bare repository' ' - mk_test heads/master && + mk_test testrepo heads/master && ( cd testrepo && git checkout master && @@ -810,7 +802,7 @@ test_expect_success 'deny push to HEAD of non-bare repository' ' ' test_expect_success 'deny push to HEAD of non-bare repository' ' - mk_test heads/master && + mk_test testrepo heads/master && ( cd testrepo && git checkout master && @@ -820,7 +812,7 @@ test_expect_success 'allow push to HEAD of bare repository (bare)' ' ' test_expect_success 'allow push to HEAD of bare repository (bare)' ' - mk_test heads/master && + mk_test testrepo heads/master && ( cd testrepo && git checkout master && @@ -832,7 +824,7 @@ test_expect_success 'allow push to HEAD of non-bare repository (config)' ' ' test_expect_success 'allow push to HEAD of non-bare repository (config)' ' - mk_test heads/master && + mk_test testrepo heads/master && ( cd testrepo && git checkout master && @@ -843,7 +835,7 @@ test_expect_success 'fetch with branches' ' ' test_expect_success 'fetch with branches' ' - mk_empty && + mk_empty testrepo && git branch second $the_first_commit && git checkout second && echo ".." > testrepo/.git/branches/branch1 && @@ -858,7 +850,7 @@ test_expect_success 'fetch with branches containing #' ' ' test_expect_success 'fetch with branches containing #' ' - mk_empty && + mk_empty testrepo && echo "..#second" > testrepo/.git/branches/branch2 && ( cd testrepo && @@ -871,7 +863,7 @@ test_expect_success 'push with branches' ' ' test_expect_success 'push with branches' ' - mk_empty && + mk_empty testrepo && git checkout second && echo "testrepo" > .git/branches/branch1 && git push branch1 && @@ -884,7 +876,7 @@ test_expect_success 'push with branches containing #' ' ' test_expect_success 'push with branches containing #' ' - mk_empty && + mk_empty testrepo && echo "testrepo#branch3" > .git/branches/branch2 && git push branch2 && ( @@ -897,9 +889,9 @@ test_expect_success 'push into aliased refs (consistent)' ' ' test_expect_success 'push into aliased refs (consistent)' ' - mk_test heads/master && - mk_child child1 && - mk_child child2 && + mk_test testrepo heads/master && + mk_child testrepo child1 && + mk_child testrepo child2 && ( cd child1 && git branch foo && @@ -919,9 +911,9 @@ test_expect_success 'push into aliased refs (inconsistent)' ' ' test_expect_success 'push into aliased refs (inconsistent)' ' - mk_test heads/master && - mk_child child1 && - mk_child child2 && + mk_test testrepo heads/master && + mk_child testrepo child1 && + mk_child testrepo child2 && ( cd child1 && git branch foo && @@ -946,9 +938,9 @@ test_expect_success 'push requires --force to update lightweight tag' ' ' test_expect_success 'push requires --force to update lightweight tag' ' - mk_test heads/master && - mk_child child1 && - mk_child child2 && + mk_test testrepo heads/master && + mk_child testrepo child1 && + mk_child testrepo child2 && ( cd child1 && git tag Tag && @@ -967,7 +959,7 @@ test_expect_success 'push --porcelain' ' ' test_expect_success 'push --porcelain' ' - mk_empty && + mk_empty testrepo && echo >.git/foo "To testrepo" && echo >>.git/foo "* refs/heads/master:refs/remotes/origin/master [new branch]" && echo >>.git/foo "Done" && @@ -982,13 +974,13 @@ test_expect_success 'push --porcelain rejected' ' ' test_expect_success 'push --porcelain bad url' ' - mk_empty && + mk_empty testrepo && test_must_fail git push >.git/bar --porcelain asdfasdfasd refs/heads/master:refs/remotes/origin/master && test_must_fail grep -q Done .git/bar ' test_expect_success 'push --porcelain rejected' ' - mk_empty && + mk_empty testrepo && git push testrepo refs/heads/master:refs/remotes/origin/master && (cd testrepo && git reset --hard origin/master^ @@ -1002,7 +994,7 @@ test_expect_success 'push --porcelain --dry-run rejected' ' ' test_expect_success 'push --porcelain --dry-run rejected' ' - mk_empty && + mk_empty testrepo && git push testrepo refs/heads/master:refs/remotes/origin/master && (cd testrepo && git reset --hard origin/master @@ -1017,25 +1009,25 @@ do ' test_expect_success 'push --prune' ' - mk_test heads/master heads/second heads/foo heads/bar && + mk_test testrepo heads/master heads/second heads/foo heads/bar && git push --prune testrepo && - check_push_result $the_commit heads/master && - check_push_result $the_first_commit heads/second && - ! check_push_result $the_first_commit heads/foo heads/bar + check_push_result testrepo $the_commit heads/master && + check_push_result testrepo $the_first_commit heads/second && + ! check_push_result testrepo $the_first_commit heads/foo heads/bar ' test_expect_success 'push --prune refspec' ' - mk_test tmp/master tmp/second tmp/foo tmp/bar && + mk_test testrepo tmp/master tmp/second tmp/foo tmp/bar && git push --prune testrepo "refs/heads/*:refs/tmp/*" && - check_push_result $the_commit tmp/master && - check_push_result $the_first_commit tmp/second && - ! check_push_result $the_first_commit tmp/foo tmp/bar + check_push_result testrepo $the_commit tmp/master && + check_push_result testrepo $the_first_commit tmp/second && + ! check_push_result testrepo $the_first_commit tmp/foo tmp/bar ' for configsection in transfer receive do test_expect_success "push to update a ref hidden by $configsection.hiderefs" ' - mk_test heads/master hidden/one hidden/two hidden/three && + mk_test testrepo heads/master hidden/one hidden/two hidden/three && ( cd testrepo && git config $configsection.hiderefs refs/hidden @@ -1043,32 +1035,32 @@ test_expect_success 'fetch exact SHA1' ' # push to unhidden ref succeeds normally git push testrepo master:refs/heads/master && - check_push_result $the_commit heads/master && + check_push_result testrepo $the_commit heads/master && # push to update a hidden ref should fail test_must_fail git push testrepo master:refs/hidden/one && - check_push_result $the_first_commit hidden/one && + check_push_result testrepo $the_first_commit hidden/one && # push to delete a hidden ref should fail test_must_fail git push testrepo :refs/hidden/two && - check_push_result $the_first_commit hidden/two && + check_push_result testrepo $the_first_commit hidden/two && # idempotent push to update a hidden ref should fail test_must_fail git push testrepo $the_first_commit:refs/hidden/three && - check_push_result $the_first_commit hidden/three + check_push_result testrepo $the_first_commit hidden/three ' done test_expect_success 'fetch exact SHA1' ' - mk_test heads/master hidden/one && + mk_test testrepo heads/master hidden/one && git push testrepo master:refs/hidden/one && ( cd testrepo && git config transfer.hiderefs refs/hidden ) && - check_push_result $the_commit hidden/one && + check_push_result testrepo $the_commit hidden/one && - mk_child child && + mk_child testrepo child && ( cd child && -- 1.8.2.13.g0f18d3c ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name() 2013-03-22 14:44 ` Jeff King @ 2013-03-22 14:52 ` Junio C Hamano 2013-03-22 14:59 ` Jeff King 2013-03-22 21:14 ` Jonathan Nieder 2013-03-28 13:03 ` Ramkumar Ramachandra 1 sibling, 2 replies; 29+ messages in thread From: Junio C Hamano @ 2013-03-22 14:52 UTC (permalink / raw) To: Jeff King; +Cc: Ramkumar Ramachandra, Git List, Jonathan Nieder Jeff King <peff@peff.net> writes: > I think this is OK, and I do not mind if it gets applied. But what I was > hinting at in my earlier mail was that we might want to do this (I have > it as a separate patch on top of your 3/6 here, but it would make more > sense squashed in): I would prefer to see a preparatory patch to teach mk_test/mk_empty to _always_ take the new name (i.e. the result of your patch) and then do whatever new things on top. By the way, I am planning to _not_ look at new stuff today. I'd rather see known recent regressions addressed (and unknown ones discovered and squashed) before we move forward, and I would appreciate if regular contributors did the same. Thanks. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name() 2013-03-22 14:52 ` Junio C Hamano @ 2013-03-22 14:59 ` Jeff King 2013-03-22 21:14 ` Jonathan Nieder 1 sibling, 0 replies; 29+ messages in thread From: Jeff King @ 2013-03-22 14:59 UTC (permalink / raw) To: Junio C Hamano; +Cc: Ramkumar Ramachandra, Git List, Jonathan Nieder On Fri, Mar 22, 2013 at 07:52:47AM -0700, Junio C Hamano wrote: > Jeff King <peff@peff.net> writes: > > > I think this is OK, and I do not mind if it gets applied. But what I was > > hinting at in my earlier mail was that we might want to do this (I have > > it as a separate patch on top of your 3/6 here, but it would make more > > sense squashed in): > > I would prefer to see a preparatory patch to teach mk_test/mk_empty > to _always_ take the new name (i.e. the result of your patch) and > then do whatever new things on top. I think that is what my patch does (it is meant to come at the point of 3/6, and then the rest would need to be rebased to just use "mk_test" instead of "mk_test_with_name"). > By the way, I am planning to _not_ look at new stuff today. I'd > rather see known recent regressions addressed (and unknown ones > discovered and squashed) before we move forward, and I would > appreciate if regular contributors did the same. Yeah, I have several to look at (the "subdir/" in gitattributes is the biggest one, I think). -Peff ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name() 2013-03-22 14:52 ` Junio C Hamano 2013-03-22 14:59 ` Jeff King @ 2013-03-22 21:14 ` Jonathan Nieder 1 sibling, 0 replies; 29+ messages in thread From: Jonathan Nieder @ 2013-03-22 21:14 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, Ramkumar Ramachandra, Git List Junio C Hamano wrote: > I would prefer to see a preparatory patch to teach mk_test/mk_empty > to _always_ take the new name (i.e. the result of your patch) and > then do whatever new things on top. Yes, that sounds like a good way to go. > By the way, I am planning to _not_ look at new stuff today. I'd > rather see known recent regressions addressed (and unknown ones > discovered and squashed) before we move forward, and I would > appreciate if regular contributors did the same. I've been flushing out my thoughts to avoid forgetting them. ;-) Agreed, though. Thanks. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name() 2013-03-22 14:44 ` Jeff King 2013-03-22 14:52 ` Junio C Hamano @ 2013-03-28 13:03 ` Ramkumar Ramachandra 1 sibling, 0 replies; 29+ messages in thread From: Ramkumar Ramachandra @ 2013-03-28 13:03 UTC (permalink / raw) To: Jeff King; +Cc: Git List, Junio C Hamano, Jonathan Nieder Jeff King wrote: > Subject: [PATCH] t5516: drop implicit arguments from helper functions Thanks a lot for this! I just had to s/ $repo_name/ "$repo_name"/ to fix the quoting. Will post a re-roll soon. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name() 2013-03-22 7:52 ` [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name() Ramkumar Ramachandra 2013-03-22 14:44 ` Jeff King @ 2013-03-22 14:54 ` Junio C Hamano 2013-03-22 14:58 ` Jeff King 1 sibling, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2013-03-22 14:54 UTC (permalink / raw) To: Ramkumar Ramachandra; +Cc: Git List, Jeff King, Jonathan Nieder Ramkumar Ramachandra <artagnon@gmail.com> writes: > mk_empty () { > - rm -fr testrepo && > - mkdir testrepo && > + repo_name="$1" > + test -z "$repo_name" && repo_name=testrepo > + rm -fr $repo_name && > + mkdir $repo_name && Your quoting is sloppy in this entire patch X-<. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name() 2013-03-22 14:54 ` Junio C Hamano @ 2013-03-22 14:58 ` Jeff King 0 siblings, 0 replies; 29+ messages in thread From: Jeff King @ 2013-03-22 14:58 UTC (permalink / raw) To: Junio C Hamano; +Cc: Ramkumar Ramachandra, Git List, Jonathan Nieder On Fri, Mar 22, 2013 at 07:54:06AM -0700, Junio C Hamano wrote: > Ramkumar Ramachandra <artagnon@gmail.com> writes: > > > mk_empty () { > > - rm -fr testrepo && > > - mkdir testrepo && > > + repo_name="$1" > > + test -z "$repo_name" && repo_name=testrepo > > + rm -fr $repo_name && > > + mkdir $repo_name && > > Your quoting is sloppy in this entire patch X-<. I meant to mention that, too. It doesn't matter, as the intent is for it always to be a simple name like "testrepo", but that assumption is not documented anywhere. I suspect the quoting in my patch is sloppy, too. -Peff ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push 2013-03-22 7:52 [PATCH v3 0/6] Support triangular workflows Ramkumar Ramachandra ` (2 preceding siblings ...) 2013-03-22 7:52 ` [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name() Ramkumar Ramachandra @ 2013-03-22 7:52 ` Ramkumar Ramachandra 2013-03-22 21:21 ` Jonathan Nieder 2013-03-22 7:52 ` [PATCH 5/6] remote.c: introduce remote.pushdefault Ramkumar Ramachandra 2013-03-22 7:52 ` [PATCH 6/6] remote.c: introduce branch.<name>.pushremote Ramkumar Ramachandra 5 siblings, 1 reply; 29+ messages in thread From: Ramkumar Ramachandra @ 2013-03-22 7:52 UTC (permalink / raw) To: Git List; +Cc: Junio C Hamano, Jeff King, Jonathan Nieder Currently, do_push() in push.c calls remote_get(), which gets the configured remote for fetching and pushing. Replace this call with a call to pushremote_get() instead, a new function that will return the remote configured specifically for pushing. This function tries to work with the string pushremote_name, before falling back to the codepath of remote_get(). This patch has no visible impact, but serves to enable future patches to introduce configuration variables to set pushremote_name. For example, you can now do the following in handle_config(): if (!strcmp(key, "remote.pushdefault")) git_config_string(&pushremote_name, key, value); Then, pushes will automatically go to the remote specified by remote.pushdefault. Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com> --- builtin/push.c | 2 +- remote.c | 25 +++++++++++++++++++++---- remote.h | 1 + 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/builtin/push.c b/builtin/push.c index 42b129d..d447a80 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -322,7 +322,7 @@ static int push_with_options(struct transport *transport, int flags) static int do_push(const char *repo, int flags) { int i, errs; - struct remote *remote = remote_get(repo); + struct remote *remote = pushremote_get(repo); const char **url; int url_nr; diff --git a/remote.c b/remote.c index 5bd59bb..185ac11 100644 --- a/remote.c +++ b/remote.c @@ -48,6 +48,7 @@ static int branches_nr; static struct branch *current_branch; static const char *default_remote_name; +static const char *pushremote_name; static int explicit_default_remote_name; static struct rewrites rewrites; @@ -670,17 +671,21 @@ static int valid_remote_nick(const char *name) return !strchr(name, '/'); /* no slash */ } -struct remote *remote_get(const char *name) +static struct remote *remote_get_1(const char *name, const char *pushremote_name) { struct remote *ret; int name_given = 0; - read_config(); if (name) name_given = 1; else { - name = default_remote_name; - name_given = explicit_default_remote_name; + if (pushremote_name) { + name = pushremote_name; + name_given = 1; + } else { + name = default_remote_name; + name_given = explicit_default_remote_name; + } } ret = make_remote(name, 0); @@ -699,6 +704,18 @@ struct remote *remote_get(const char *name) return ret; } +struct remote *remote_get(const char *name) +{ + read_config(); + return remote_get_1(name, NULL); +} + +struct remote *pushremote_get(const char *name) +{ + read_config(); + return remote_get_1(name, pushremote_name); +} + int remote_is_configured(const char *name) { int i; diff --git a/remote.h b/remote.h index 251d8fd..99a437f 100644 --- a/remote.h +++ b/remote.h @@ -51,6 +51,7 @@ struct remote { }; struct remote *remote_get(const char *name); +struct remote *pushremote_get(const char *name); int remote_is_configured(const char *name); typedef int each_remote_fn(struct remote *remote, void *priv); -- 1.8.2.62.ga35d936.dirty ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push 2013-03-22 7:52 ` [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push Ramkumar Ramachandra @ 2013-03-22 21:21 ` Jonathan Nieder 2013-03-22 22:13 ` Junio C Hamano 0 siblings, 1 reply; 29+ messages in thread From: Jonathan Nieder @ 2013-03-22 21:21 UTC (permalink / raw) To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano, Jeff King Ramkumar Ramachandra wrote: > This patch has no visible impact, but > serves to enable future patches to introduce configuration variables > to set pushremote_name. For example, you can now do the following in > handle_config(): > > if (!strcmp(key, "remote.pushdefault")) > git_config_string(&pushremote_name, key, value); Thanks. [...] > --- a/builtin/push.c > +++ b/builtin/push.c > @@ -322,7 +322,7 @@ static int push_with_options(struct transport *transport, int flags) > static int do_push(const char *repo, int flags) > { > int i, errs; > - struct remote *remote = remote_get(repo); > + struct remote *remote = pushremote_get(repo); "struct remote" has url and pushurl fields. What do they mean in the context of these two accessors? /me is confused. Is the idea that now I should not use pushurl any more, and that I should use pushremote_get and use url instead? Hope that helps, Jonathan ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push 2013-03-22 21:21 ` Jonathan Nieder @ 2013-03-22 22:13 ` Junio C Hamano 2013-03-22 23:41 ` Jonathan Nieder 2013-03-23 12:57 ` Ramkumar Ramachandra 0 siblings, 2 replies; 29+ messages in thread From: Junio C Hamano @ 2013-03-22 22:13 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Ramkumar Ramachandra, Git List, Jeff King Jonathan Nieder <jrnieder@gmail.com> writes: >> --- a/builtin/push.c >> +++ b/builtin/push.c >> @@ -322,7 +322,7 @@ static int push_with_options(struct transport *transport, int flags) >> static int do_push(const char *repo, int flags) >> { >> int i, errs; >> - struct remote *remote = remote_get(repo); >> + struct remote *remote = pushremote_get(repo); > > "struct remote" has url and pushurl fields. What do they mean in the > context of these two accessors? /me is confused. > > Is the idea that now I should not use pushurl any more, and that I > should use pushremote_get and use url instead? I thought the basic idea from the user-level is: - If you have to use different URL to push to and fetch from the logically same location (e.g. git://k.org/pub/scm/git/git.git used for fetch, k.org:/pub/scm/git/git.git/ used for push), use url for fetch, pushurl for push and you don't have to bother with per-branch pushremote at all. You are logically working with the same remote, perhaps called 'origin'. - If you push to and fetch from logically different repositories, (e.g. fetch from https://github.com/gitster/git, push to github.com:artagnon/git), you may want to call your upstream 'origin' and your publishing repository 'mine'. You set the remote.pushdefault to 'mine', perhaps like: [remote "mine"] url = github.com:artagnon/git (this can also be written with remote.mine.pushurl). By splitting remote_get() used for fetch and pushremote_get() used for push, the latter function can return 'origin' and 'mine' for these two cases, while remote_get() will return 'origin' for both of these cases. At the programming level, you would still ask what the URL to be pushed to to the remote obtained here, and would use pushurl if defined, or url otherwise. Ram, am I following your thoughts correctly? ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push 2013-03-22 22:13 ` Junio C Hamano @ 2013-03-22 23:41 ` Jonathan Nieder 2013-03-23 13:18 ` Ramkumar Ramachandra 2013-03-23 12:57 ` Ramkumar Ramachandra 1 sibling, 1 reply; 29+ messages in thread From: Jonathan Nieder @ 2013-03-22 23:41 UTC (permalink / raw) To: Junio C Hamano; +Cc: Ramkumar Ramachandra, Git List, Jeff King Junio C Hamano wrote: > Jonathan Nieder <jrnieder@gmail.com> writes: >>> - struct remote *remote = remote_get(repo); >>> + struct remote *remote = pushremote_get(repo); >> >> "struct remote" has url and pushurl fields. What do they mean in the >> context of these two accessors? /me is confused. >> >> Is the idea that now I should not use pushurl any more, and that I >> should use pushremote_get and use url instead? [...] > At the programming level, you would still ask what the > URL to be pushed to to the remote obtained here, and would use > pushurl if defined, or url otherwise. Ah, I think I see. It might be more convenient to the caller if pushremote_get returned a remote with url set to the pushurl, but that would prevent sharing the struct with other callers that want that remote for fetching. So instead, the idea is something like remote: support a different default remote for pushing Teach remote_get() to accept an argument FOR_FETCH or FOR_PUSH that determines, when no remote is passed to it, whether to use the default remote for fetching or the default for pushing. The default remote for fetching is stored in the static var "default_remote_name", while the default for pushing, if set, is in "default_push_remote_name". Currently there is never a different default for pushing set but later patches will change that. If remote_get() gained a new required parameter, that would force all call sites to be examined (even any potential call sites added by new patches in flight) and there would no longer be need for the remote_get_1() function. What do you think? Jonathan ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push 2013-03-22 23:41 ` Jonathan Nieder @ 2013-03-23 13:18 ` Ramkumar Ramachandra 0 siblings, 0 replies; 29+ messages in thread From: Ramkumar Ramachandra @ 2013-03-23 13:18 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Junio C Hamano, Git List, Jeff King Jonathan Nieder wrote: > Junio C Hamano wrote: >> Jonathan Nieder <jrnieder@gmail.com> writes: > >>>> - struct remote *remote = remote_get(repo); >>>> + struct remote *remote = pushremote_get(repo); >>> >>> "struct remote" has url and pushurl fields. What do they mean in the >>> context of these two accessors? /me is confused. >>> >>> Is the idea that now I should not use pushurl any more, and that I >>> should use pushremote_get and use url instead? > [...] >> At the programming level, you would still ask what the >> URL to be pushed to to the remote obtained here, and would use >> pushurl if defined, or url otherwise. > > Ah, I think I see. It might be more convenient to the caller if > pushremote_get returned a remote with url set to the pushurl, but > that would prevent sharing the struct with other callers that want > that remote for fetching. We started off with a generic "remote" (for both fetching and pushing), then added .pushurl on top of this remote. Now we've introduced something called a pushremote, a logically distinct remote from "remote"; pushremote_get() is meant to return this logically different remote, falling back to the remote_get() codepath if not present. This is a perfect migration that trivially preserves backward compatibility. > So instead, the idea is something like > > remote: support a different default remote for pushing > > Teach remote_get() to accept an argument FOR_FETCH or FOR_PUSH > that determines, when no remote is passed to it, whether to use > the default remote for fetching or the default for pushing. > > The default remote for fetching is stored in the static var > "default_remote_name", while the default for pushing, if set, > is in "default_push_remote_name". > > Currently there is never a different default for pushing set > but later patches will change that. > > If remote_get() gained a new required parameter, that would force all > call sites to be examined (even any potential call sites added by new > patches in flight) and there would no longer be need for the > remote_get_1() function. I like this, but let's save it for a future patch as it requires some extensive refactoring. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push 2013-03-22 22:13 ` Junio C Hamano 2013-03-22 23:41 ` Jonathan Nieder @ 2013-03-23 12:57 ` Ramkumar Ramachandra 1 sibling, 0 replies; 29+ messages in thread From: Ramkumar Ramachandra @ 2013-03-23 12:57 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jonathan Nieder, Git List, Jeff King Junio C Hamano wrote: > Jonathan Nieder <jrnieder@gmail.com> writes: > >>> --- a/builtin/push.c >>> +++ b/builtin/push.c >>> @@ -322,7 +322,7 @@ static int push_with_options(struct transport *transport, int flags) >>> static int do_push(const char *repo, int flags) >>> { >>> int i, errs; >>> - struct remote *remote = remote_get(repo); >>> + struct remote *remote = pushremote_get(repo); >> >> "struct remote" has url and pushurl fields. What do they mean in the >> context of these two accessors? /me is confused. >> >> Is the idea that now I should not use pushurl any more, and that I >> should use pushremote_get and use url instead? > > I thought the basic idea from the user-level is: > > - If you have to use different URL to push to and fetch from the > logically same location (e.g. git://k.org/pub/scm/git/git.git > used for fetch, k.org:/pub/scm/git/git.git/ used for push), use > url for fetch, pushurl for push and you don't have to bother with > per-branch pushremote at all. You are logically working with the > same remote, perhaps called 'origin'. > > - If you push to and fetch from logically different repositories, > (e.g. fetch from https://github.com/gitster/git, push to > github.com:artagnon/git), you may want to call your upstream > 'origin' and your publishing repository 'mine'. You set the > remote.pushdefault to 'mine', perhaps like: > > [remote "mine"] > url = github.com:artagnon/git > > (this can also be written with remote.mine.pushurl). > > By splitting remote_get() used for fetch and pushremote_get() used > for push, the latter function can return 'origin' and 'mine' for > these two cases, while remote_get() will return 'origin' for both of > these cases. At the programming level, you would still ask what the > URL to be pushed to to the remote obtained here, and would use > pushurl if defined, or url otherwise. > > Ram, am I following your thoughts correctly? Exactly. I couldn't have said it better myself. ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 5/6] remote.c: introduce remote.pushdefault 2013-03-22 7:52 [PATCH v3 0/6] Support triangular workflows Ramkumar Ramachandra ` (3 preceding siblings ...) 2013-03-22 7:52 ` [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push Ramkumar Ramachandra @ 2013-03-22 7:52 ` Ramkumar Ramachandra 2013-03-22 14:56 ` Jeff King 2013-03-22 7:52 ` [PATCH 6/6] remote.c: introduce branch.<name>.pushremote Ramkumar Ramachandra 5 siblings, 1 reply; 29+ messages in thread From: Ramkumar Ramachandra @ 2013-03-22 7:52 UTC (permalink / raw) To: Git List; +Cc: Junio C Hamano, Jeff King, Jonathan Nieder This new configuration variable defines the default remote to push to, and overrides `branch.<name>.remote` for all branches. It is useful in the typical triangular-workflow setup, where the remote you're fetching from is different from the remote you're pushing to. Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com> --- Documentation/config.txt | 13 ++++++++++--- remote.c | 5 +++++ t/t5516-fetch-push.sh | 12 ++++++++++++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index b3023b8..09a8294 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -723,9 +723,12 @@ branch.autosetuprebase:: This option defaults to never. branch.<name>.remote:: - When in branch <name>, it tells 'git fetch' and 'git push' which - remote to fetch from/push to. It defaults to `origin` if no remote is - configured. `origin` is also used if you are not on any branch. + When on branch <name>, it tells 'git fetch' and 'git push' + which remote to fetch from/push to. The remote to push to + may be overridden with `remote.pushdefault` (for all branches). + If no remote is configured, or if you are not on any branch, + it defaults to `origin` for fetching and `remote.pushdefault` + for pushing. branch.<name>.merge:: Defines, together with branch.<name>.remote, the upstream branch @@ -1894,6 +1897,10 @@ receive.updateserverinfo:: If set to true, git-receive-pack will run git-update-server-info after receiving data from git-push and updating refs. +remote.pushdefault:: + The remote to push to by default. Overrides + `branch.<name>.remote` for all branches. + remote.<name>.url:: The URL of a remote repository. See linkgit:git-fetch[1] or linkgit:git-push[1]. diff --git a/remote.c b/remote.c index 185ac11..bdb542c 100644 --- a/remote.c +++ b/remote.c @@ -350,6 +350,11 @@ static int handle_config(const char *key, const char *value, void *cb) const char *subkey; struct remote *remote; struct branch *branch; + if (!prefixcmp(key, "remote.")) { + if (!strcmp(key + 7, "pushdefault")) + if (git_config_string(&pushremote_name, key, value)) + return -1; + } if (!prefixcmp(key, "branch.")) { name = key + 7; subkey = strrchr(name, '.'); diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index a546c2c..63171f1 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -517,6 +517,18 @@ test_expect_success 'push with config remote.*.push = HEAD' ' git config --remove-section remote.there git config --remove-section branch.master +test_expect_success 'push with remote.pushdefault' ' + mk_test_with_name up_repo heads/frotz && + mk_test_with_name down_repo heads/master && + test_config remote.up.url up_repo && + test_config remote.down.url down_repo && + test_config branch.master.remote up && + test_config remote.pushdefault down && + git push && + test_must_fail check_push_result_with_name up_repo $the_commit heads/master && + check_push_result_with_name down_repo $the_commit heads/master +' + test_expect_success 'push with config remote.*.pushurl' ' mk_test heads/master && -- 1.8.2.62.ga35d936.dirty ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 5/6] remote.c: introduce remote.pushdefault 2013-03-22 7:52 ` [PATCH 5/6] remote.c: introduce remote.pushdefault Ramkumar Ramachandra @ 2013-03-22 14:56 ` Jeff King 2013-03-27 9:39 ` Ramkumar Ramachandra 0 siblings, 1 reply; 29+ messages in thread From: Jeff King @ 2013-03-22 14:56 UTC (permalink / raw) To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano, Jonathan Nieder On Fri, Mar 22, 2013 at 01:22:35PM +0530, Ramkumar Ramachandra wrote: > diff --git a/remote.c b/remote.c > index 185ac11..bdb542c 100644 > --- a/remote.c > +++ b/remote.c > @@ -350,6 +350,11 @@ static int handle_config(const char *key, const char *value, void *cb) > const char *subkey; > struct remote *remote; > struct branch *branch; > + if (!prefixcmp(key, "remote.")) { > + if (!strcmp(key + 7, "pushdefault")) > + if (git_config_string(&pushremote_name, key, value)) > + return -1; > + } > if (!prefixcmp(key, "branch.")) { > name = key + 7; > subkey = strrchr(name, '.'); I was going to say "shouldn't we return 0" here, both on successful read of pushdefault, but also just when we have remote.*. But the answer is "yes" to the first one (we know we have handled the key), but "no" to the second, because we end up parsing remote.*.* later. So I think this should at least be: if (!prefixcmp(key, "remote.")) { if (!strcmp(key + 7, "pushdefault")) return git_config_string(&pushremote_name, key, value)); /* do not return; we handle other remote.* below */ } but also possibly just move it with the other remote parsing, like: diff --git a/remote.c b/remote.c index 02e6c4c..d3d740a 100644 --- a/remote.c +++ b/remote.c @@ -388,9 +388,16 @@ static int handle_config(const char *key, const char *value, void *cb) add_instead_of(rewrite, xstrdup(value)); } } + if (prefixcmp(key, "remote.")) return 0; name = key + 7; + + /* Handle any global remote.* variables */ + if (!strcmp(name, "pushdefault")) + return git_config_string(&pushremote_name, key, value); + + /* Now handle any remote.NAME.* variables */ if (*name == '/') { warning("Config remote shorthand cannot begin with '/': %s", name); -Peff ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 5/6] remote.c: introduce remote.pushdefault 2013-03-22 14:56 ` Jeff King @ 2013-03-27 9:39 ` Ramkumar Ramachandra 0 siblings, 0 replies; 29+ messages in thread From: Ramkumar Ramachandra @ 2013-03-27 9:39 UTC (permalink / raw) To: Jeff King; +Cc: Git List, Junio C Hamano, Jonathan Nieder Jeff King wrote: > but also possibly just move it with the other remote parsing, like: > [...] Done. Thanks for the dose of sanity. ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 6/6] remote.c: introduce branch.<name>.pushremote 2013-03-22 7:52 [PATCH v3 0/6] Support triangular workflows Ramkumar Ramachandra ` (4 preceding siblings ...) 2013-03-22 7:52 ` [PATCH 5/6] remote.c: introduce remote.pushdefault Ramkumar Ramachandra @ 2013-03-22 7:52 ` Ramkumar Ramachandra 2013-03-22 17:37 ` Junio C Hamano 5 siblings, 1 reply; 29+ messages in thread From: Ramkumar Ramachandra @ 2013-03-22 7:52 UTC (permalink / raw) To: Git List; +Cc: Junio C Hamano, Jeff King, Jonathan Nieder This new configuration variable overrides `remote.pushdefault` and `branch.<name>.remote` for pushes. In a typical triangular-workflow setup, you would want to set `remote.pushdefault` to specify the remote to push to for all branches, and use this option to override it for a specific branch. Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com> --- Documentation/config.txt | 18 ++++++++++++++---- remote.c | 4 ++++ t/t5516-fetch-push.sh | 15 +++++++++++++++ 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 09a8294..6595cd6 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -726,9 +726,18 @@ branch.<name>.remote:: When on branch <name>, it tells 'git fetch' and 'git push' which remote to fetch from/push to. The remote to push to may be overridden with `remote.pushdefault` (for all branches). - If no remote is configured, or if you are not on any branch, - it defaults to `origin` for fetching and `remote.pushdefault` - for pushing. + The remote to push to, for the current branch, may be further + overridden by `branch.<name>.pushremote`. If no remote is + configured, or if you are not on any branch, it defaults to + `origin` for fetching and `remote.pushdefault` for pushing. + +branch.<name>.pushremote:: + When on branch <name>, it overrides `branch.<name>.remote` + when pushing. It also overrides `remote.pushdefault` when + pushing from branch <name>. In a typical triangular-workflow + setup, you would want to set `remote.pushdefault` to specify + the remote to push to for all branches, and use this option to + override it for a specific branch. branch.<name>.merge:: Defines, together with branch.<name>.remote, the upstream branch @@ -1899,7 +1908,8 @@ receive.updateserverinfo:: remote.pushdefault:: The remote to push to by default. Overrides - `branch.<name>.remote` for all branches. + `branch.<name>.remote` for all branches, and is overridden by + `branch.<name>.pushremote` for specific branches. remote.<name>.url:: The URL of a remote repository. See linkgit:git-fetch[1] or diff --git a/remote.c b/remote.c index bdb542c..0be648d 100644 --- a/remote.c +++ b/remote.c @@ -368,6 +368,10 @@ static int handle_config(const char *key, const char *value, void *cb) default_remote_name = branch->remote_name; explicit_default_remote_name = 1; } + } else if (!strcmp(subkey, ".pushremote")) { + if (branch == current_branch) + if (git_config_string(&pushremote_name, key, value)) + return -1; } else if (!strcmp(subkey, ".merge")) { if (!value) return config_error_nonbool(key); diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 63171f1..3f91874 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -539,6 +539,21 @@ test_expect_success 'push with config remote.*.pushurl' ' check_push_result $the_commit heads/master ' +test_expect_success 'push with config branch.*.pushremote' ' + mk_test_with_name up_repo heads/frotz && + mk_test_with_name side_repo heads/quux && + mk_test_with_name down_repo heads/master && + test_config remote.up.url up_repo && + test_config remote.pushdefault side_repo && + test_config remote.down.url down_repo && + test_config branch.master.remote up && + test_config branch.master.pushremote down && + git push && + test_must_fail check_push_result_with_name up_repo $the_commit heads/master && + test_must_fail check_push_result_with_name side_repo $the_commit heads/master && + check_push_result_with_name down_repo $the_commit heads/master +' + # clean up the cruft left with the previous one git config --remove-section remote.there -- 1.8.2.62.ga35d936.dirty ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 6/6] remote.c: introduce branch.<name>.pushremote 2013-03-22 7:52 ` [PATCH 6/6] remote.c: introduce branch.<name>.pushremote Ramkumar Ramachandra @ 2013-03-22 17:37 ` Junio C Hamano 0 siblings, 0 replies; 29+ messages in thread From: Junio C Hamano @ 2013-03-22 17:37 UTC (permalink / raw) To: Ramkumar Ramachandra; +Cc: Git List, Jeff King, Jonathan Nieder Ramkumar Ramachandra <artagnon@gmail.com> writes: > This new configuration variable overrides `remote.pushdefault` and > `branch.<name>.remote` for pushes. In a typical triangular-workflow > setup, you would want to set `remote.pushdefault` to specify the > remote to push to for all branches, and use this option to override it > for a specific branch. > > Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com> > --- > Documentation/config.txt | 18 ++++++++++++++---- > remote.c | 4 ++++ > t/t5516-fetch-push.sh | 15 +++++++++++++++ > 3 files changed, 33 insertions(+), 4 deletions(-) > > diff --git a/Documentation/config.txt b/Documentation/config.txt > index 09a8294..6595cd6 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -726,9 +726,18 @@ branch.<name>.remote:: > When on branch <name>, it tells 'git fetch' and 'git push' > which remote to fetch from/push to. The remote to push to > may be overridden with `remote.pushdefault` (for all branches). > - If no remote is configured, or if you are not on any branch, > - it defaults to `origin` for fetching and `remote.pushdefault` > - for pushing. > + The remote to push to, for the current branch, may be further > + overridden by `branch.<name>.pushremote`. If no remote is > + configured, or if you are not on any branch, it defaults to > + `origin` for fetching and `remote.pushdefault` for pushing. Nice write-up. It may be easier to read if the new text is in a separate paragraph, though. > +branch.<name>.pushremote:: > + When on branch <name>, it overrides `branch.<name>.remote` > + when pushing. It also overrides `remote.pushdefault` when > + pushing from branch <name>. Perhaps s/when pushing/for pushing/; Or "Specify what remote to push to when on branch <name>, overriding `branch.<name>.remote` and `remote.pushdefault`." > + In a typical triangular-workflow > + setup,... Is there an "atypical triangular-workflow"? Drop "typical" and explain what you mean by triangular, perhaps like When you pull from one place (e.g. your upstream) and push to another place (e.g. your own publishing repository), Then the rest of the text flows more naturally without ever introducing a new lingo "triangular" that is not in glossary. > + ... you would want to set `remote.pushdefault` to specify > + the remote to push to for all branches, and use this option to > + override it for a specific branch. ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 0/6] Re-roll rr/triangle @ 2013-04-02 7:40 Ramkumar Ramachandra 2013-04-02 7:40 ` [PATCH 6/6] remote.c: introduce branch.<name>.pushremote Ramkumar Ramachandra 0 siblings, 1 reply; 29+ messages in thread From: Ramkumar Ramachandra @ 2013-04-02 7:40 UTC (permalink / raw) To: Git List; +Cc: Junio C Hamano, Jeff King, Jonathan Nieder There are two important changes in this round: 1. Rebased on latest master, resolving all conflicts (all in t5516). I ran the testsuite immediately after resolving the conflicts, so everything should be good. 2. Peff's suggestion to avoid using test_must_fail on compound statements. I've changed the meaning of the tests slightly to (approximately) invert the tests. The inter-diff follows. diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 13028a4..3281ff1 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -521,14 +521,14 @@ git config --remove-section remote.there git config --remove-section branch.master test_expect_success 'push with remote.pushdefault' ' - mk_test up_repo heads/frotz && + mk_test up_repo heads/master && mk_test down_repo heads/master && test_config remote.up.url up_repo && test_config remote.down.url down_repo && test_config branch.master.remote up && test_config remote.pushdefault down && git push && - test_must_fail check_push_result up_repo $the_commit heads/master && + check_push_result up_repo $the_first_commit heads/master && check_push_result down_repo $the_commit heads/master ' @@ -543,8 +543,8 @@ test_expect_success 'push with config remote.*.pushurl' ' ' test_expect_success 'push with config branch.*.pushremote' ' - mk_test up_repo heads/frotz && - mk_test side_repo heads/quux && + mk_test up_repo heads/master && + mk_test side_repo heads/master && mk_test down_repo heads/master && test_config remote.up.url up_repo && test_config remote.pushdefault side_repo && @@ -552,8 +552,8 @@ test_expect_success 'push with config branch.*.pushremote' ' test_config branch.master.remote up && test_config branch.master.pushremote down && git push && - test_must_fail check_push_result up_repo $the_commit heads/master && - test_must_fail check_push_result side_repo $the_commit heads/master && + check_push_result up_repo $the_first_commit heads/master && + check_push_result side_repo $the_first_commit heads/master && check_push_result down_repo $the_commit heads/master ' Jeff King (1): t5516 (fetch-push): drop implicit arguments from helper functions Ramkumar Ramachandra (5): remote.c: simplify a bit of code using git_config_string() t5516 (fetch-push): update test description remote.c: introduce a way to have different remotes for fetch/push remote.c: introduce remote.pushdefault remote.c: introduce branch.<name>.pushremote Documentation/config.txt | 24 +++- builtin/push.c | 2 +- remote.c | 41 ++++-- remote.h | 1 + t/t5516-fetch-push.sh | 322 +++++++++++++++++++++++++++-------------------- 5 files changed, 241 insertions(+), 149 deletions(-) -- 1.8.2.363.g901f5bc ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 6/6] remote.c: introduce branch.<name>.pushremote 2013-04-02 7:40 [PATCH 0/6] Re-roll rr/triangle Ramkumar Ramachandra @ 2013-04-02 7:40 ` Ramkumar Ramachandra 0 siblings, 0 replies; 29+ messages in thread From: Ramkumar Ramachandra @ 2013-04-02 7:40 UTC (permalink / raw) To: Git List; +Cc: Junio C Hamano, Jeff King, Jonathan Nieder This new configuration variable overrides `remote.pushdefault` and `branch.<name>.remote` for pushes. When you pull from one place (e.g. your upstream) and push to another place (e.g. your own publishing repository), you would want to set `remote.pushdefault` to specify the remote to push to for all branches, and use this option to override it for a specific branch. Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com> --- Documentation/config.txt | 19 +++++++++++++++---- remote.c | 4 ++++ t/t5516-fetch-push.sh | 15 +++++++++++++++ 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 9c6fd4a..3d750e0 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -730,9 +730,19 @@ branch.<name>.remote:: When on branch <name>, it tells 'git fetch' and 'git push' which remote to fetch from/push to. The remote to push to may be overridden with `remote.pushdefault` (for all branches). - If no remote is configured, or if you are not on any branch, - it defaults to `origin` for fetching and `remote.pushdefault` - for pushing. + The remote to push to, for the current branch, may be further + overridden by `branch.<name>.pushremote`. If no remote is + configured, or if you are not on any branch, it defaults to + `origin` for fetching and `remote.pushdefault` for pushing. + +branch.<name>.pushremote:: + When on branch <name>, it overrides `branch.<name>.remote` for + pushing. It also overrides `remote.pushdefault` for pushing + from branch <name>. When you pull from one place (e.g. your + upstream) and push to another place (e.g. your own publishing + repository), you would want to set `remote.pushdefault` to + specify the remote to push to for all branches, and use this + option to override it for a specific branch. branch.<name>.merge:: Defines, together with branch.<name>.remote, the upstream branch @@ -1903,7 +1913,8 @@ receive.updateserverinfo:: remote.pushdefault:: The remote to push to by default. Overrides - `branch.<name>.remote` for all branches. + `branch.<name>.remote` for all branches, and is overridden by + `branch.<name>.pushremote` for specific branches. remote.<name>.url:: The URL of a remote repository. See linkgit:git-fetch[1] or diff --git a/remote.c b/remote.c index 6337e11..68eb99b 100644 --- a/remote.c +++ b/remote.c @@ -364,6 +364,10 @@ static int handle_config(const char *key, const char *value, void *cb) default_remote_name = branch->remote_name; explicit_default_remote_name = 1; } + } else if (!strcmp(subkey, ".pushremote")) { + if (branch == current_branch) + if (git_config_string(&pushremote_name, key, value)) + return -1; } else if (!strcmp(subkey, ".merge")) { if (!value) return config_error_nonbool(key); diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 797b537..7bf1555 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -519,6 +519,21 @@ test_expect_success 'push with config remote.*.pushurl' ' check_push_result testrepo $the_commit heads/master ' +test_expect_success 'push with config branch.*.pushremote' ' + mk_test up_repo heads/master && + mk_test side_repo heads/master && + mk_test down_repo heads/master && + test_config remote.up.url up_repo && + test_config remote.pushdefault side_repo && + test_config remote.down.url down_repo && + test_config branch.master.remote up && + test_config branch.master.pushremote down && + git push && + check_push_result up_repo $the_first_commit heads/master && + check_push_result side_repo $the_first_commit heads/master && + check_push_result down_repo $the_commit heads/master +' + test_expect_success 'push with dry-run' ' mk_test testrepo heads/master && -- 1.8.2.363.g901f5bc ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v4 0/6] Support triangular workflows @ 2013-03-28 13:26 Ramkumar Ramachandra 2013-03-28 13:26 ` [PATCH 6/6] remote.c: introduce branch.<name>.pushremote Ramkumar Ramachandra 0 siblings, 1 reply; 29+ messages in thread From: Ramkumar Ramachandra @ 2013-03-28 13:26 UTC (permalink / raw) To: Git List; +Cc: Junio C Hamano, Jeff King, Jonathan Nieder Hi, The changes in this round are: 1. Peff submitted a patch to squash into [3/6]. Since his patch essentially reverts mine, I've blamed him for the change. 2. Peff suggested a code movement in [5/6] to make things flow more naturally. 3. Jonathan suggested a better test description in [2/6]. 4. Junio suggested a minor documentation update in [6/6]. My build of git has had this feature for two weeks now (since the first iteration), and I'm very happy with it. Jeff King (1): t5516 (fetch-push): drop implicit arguments from helper functions Ramkumar Ramachandra (5): remote.c: simplify a bit of code using git_config_string() t5516 (fetch-push): update test description remote.c: introduce a way to have different remotes for fetch/push remote.c: introduce remote.pushdefault remote.c: introduce branch.<name>.pushremote Documentation/config.txt | 24 +++- builtin/push.c | 2 +- remote.c | 41 ++++-- remote.h | 1 + t/t5516-fetch-push.sh | 316 +++++++++++++++++++++++++++-------------------- 5 files changed, 238 insertions(+), 146 deletions(-) -- 1.8.2.141.g3797f84 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 6/6] remote.c: introduce branch.<name>.pushremote 2013-03-28 13:26 [PATCH v4 0/6] Support triangular workflows Ramkumar Ramachandra @ 2013-03-28 13:26 ` Ramkumar Ramachandra 0 siblings, 0 replies; 29+ messages in thread From: Ramkumar Ramachandra @ 2013-03-28 13:26 UTC (permalink / raw) To: Git List; +Cc: Junio C Hamano, Jeff King, Jonathan Nieder This new configuration variable overrides `remote.pushdefault` and `branch.<name>.remote` for pushes. When you pull from one place (e.g. your upstream) and push to another place (e.g. your own publishing repository), you would want to set `remote.pushdefault` to specify the remote to push to for all branches, and use this option to override it for a specific branch. Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com> --- Documentation/config.txt | 19 +++++++++++++++---- remote.c | 4 ++++ t/t5516-fetch-push.sh | 15 +++++++++++++++ 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index dd78171..ec579c5 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -730,9 +730,19 @@ branch.<name>.remote:: When on branch <name>, it tells 'git fetch' and 'git push' which remote to fetch from/push to. The remote to push to may be overridden with `remote.pushdefault` (for all branches). - If no remote is configured, or if you are not on any branch, - it defaults to `origin` for fetching and `remote.pushdefault` - for pushing. + The remote to push to, for the current branch, may be further + overridden by `branch.<name>.pushremote`. If no remote is + configured, or if you are not on any branch, it defaults to + `origin` for fetching and `remote.pushdefault` for pushing. + +branch.<name>.pushremote:: + When on branch <name>, it overrides `branch.<name>.remote` for + pushing. It also overrides `remote.pushdefault` for pushing + from branch <name>. When you pull from one place (e.g. your + upstream) and push to another place (e.g. your own publishing + repository), you would want to set `remote.pushdefault` to + specify the remote to push to for all branches, and use this + option to override it for a specific branch. branch.<name>.merge:: Defines, together with branch.<name>.remote, the upstream branch @@ -1903,7 +1913,8 @@ receive.updateserverinfo:: remote.pushdefault:: The remote to push to by default. Overrides - `branch.<name>.remote` for all branches. + `branch.<name>.remote` for all branches, and is overridden by + `branch.<name>.pushremote` for specific branches. remote.<name>.url:: The URL of a remote repository. See linkgit:git-fetch[1] or diff --git a/remote.c b/remote.c index 49c4b8b..e89b1b7 100644 --- a/remote.c +++ b/remote.c @@ -364,6 +364,10 @@ static int handle_config(const char *key, const char *value, void *cb) default_remote_name = branch->remote_name; explicit_default_remote_name = 1; } + } else if (!strcmp(subkey, ".pushremote")) { + if (branch == current_branch) + if (git_config_string(&pushremote_name, key, value)) + return -1; } else if (!strcmp(subkey, ".merge")) { if (!value) return config_error_nonbool(key); diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index ac1ec9d..13028a4 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -542,6 +542,21 @@ test_expect_success 'push with config remote.*.pushurl' ' check_push_result testrepo $the_commit heads/master ' +test_expect_success 'push with config branch.*.pushremote' ' + mk_test up_repo heads/frotz && + mk_test side_repo heads/quux && + mk_test down_repo heads/master && + test_config remote.up.url up_repo && + test_config remote.pushdefault side_repo && + test_config remote.down.url down_repo && + test_config branch.master.remote up && + test_config branch.master.pushremote down && + git push && + test_must_fail check_push_result up_repo $the_commit heads/master && + test_must_fail check_push_result side_repo $the_commit heads/master && + check_push_result down_repo $the_commit heads/master +' + # clean up the cruft left with the previous one git config --remove-section remote.there -- 1.8.2.141.g3797f84 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 0/6] Support triangular workflows @ 2013-03-20 12:44 Ramkumar Ramachandra 2013-03-20 12:45 ` [PATCH 6/6] remote.c: introduce branch.<name>.pushremote Ramkumar Ramachandra 0 siblings, 1 reply; 29+ messages in thread From: Ramkumar Ramachandra @ 2013-03-20 12:44 UTC (permalink / raw) To: Git List; +Cc: Junio C Hamano, Jeff King, Eric Sunshine, Jonathan Nieder This follows-up [1], with three important differences: 1. pushremote_get() and remote_get() share code better. Thanks Jeff. 2. All spelling mistakes have been corrected. Thanks Eric. 3. One new test for each of the new configuration variables. The extra two parts [2/6] and [3/6] preprare the file for introducing tests. However, I've not gone overboard in this preparation; I don't replicate the work done by Jonathan in [2]. Thanks for reading. [1]: http://thread.gmane.org/gmane.comp.version-control.git/218410 [2]: http://thread.gmane.org/gmane.comp.version-control.git/218451/focus=218465 Ramkumar Ramachandra (6): remote.c: simplify a bit of code using git_config_string() t5516 (fetch-push): update test description t5516 (fetch-push): introduce mk_test_with_name() remote.c: introduce a way to have different remotes for fetch/push remote.c: introduce remote.pushdefault remote.c: introduce branch.<name>.pushremote Documentation/config.txt | 23 +++++++++++++++--- builtin/push.c | 2 +- remote.c | 36 +++++++++++++++++++++------ remote.h | 1 + t/t5516-fetch-push.sh | 63 ++++++++++++++++++++++++++++++++++++++++-------- 5 files changed, 104 insertions(+), 21 deletions(-) -- 1.8.2 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 6/6] remote.c: introduce branch.<name>.pushremote 2013-03-20 12:44 [PATCH v2 0/6] Support triangular workflows Ramkumar Ramachandra @ 2013-03-20 12:45 ` Ramkumar Ramachandra 2013-03-20 13:03 ` Tay Ray Chuan 0 siblings, 1 reply; 29+ messages in thread From: Ramkumar Ramachandra @ 2013-03-20 12:45 UTC (permalink / raw) To: Git List; +Cc: Junio C Hamano, Jeff King, Eric Sunshine, Jonathan Nieder This new configuration variable overrides `remote.pushdefault` and `branch.<name>.remote` for pushes. In a typical triangular-workflow setup, you would want to set `remote.pushdefault` to specify the remote to push to for all branches, and use this option to override it for a specific branch. Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com> --- Documentation/config.txt | 18 ++++++++++++++---- remote.c | 3 +++ t/t5516-fetch-push.sh | 15 +++++++++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index e813c33..4b9647a 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -726,9 +726,18 @@ branch.<name>.remote:: When on branch <name>, it tells 'git fetch' and 'git push' which remote to fetch from/push to. The remote to push to may be overridden with `remote.pushdefault` (for all branches). - If no remote is configured, or if you are not on any branch, - it defaults to `origin` for fetching and `remote.pushdefault` - for pushing. + The remote to push to, for the current branch, may be further + overridden by `branch.<name>.pushremote`. If no remote is + configured, or if you are not on any branch, it defaults to + `origin` for fetching and `remote.pushdefault` for pushing. + +branch.<name>.pushremote:: + When on branch <name>, it overrides `branch.<name>.remote` + when pushing. It also overrides `remote.pushdefault` when + pushing from branch <name>. In a typical triangular-workflow + setup, you would want to set `remote.pushdefault` to specify + the remote to push to for all branches, and use this option to + override it for a specific branch. branch.<name>.merge:: Defines, together with branch.<name>.remote, the upstream branch @@ -1899,7 +1908,8 @@ receive.updateserverinfo:: remote.pushdefault:: The remote to push to by default. Overrides - `branch.<name>.remote` for all branches. + `branch.<name>.remote` for all branches, and is overridden by + `branch.<name>.pushremote` for specific branches. remote.<name>.url:: The URL of a remote repository. See linkgit:git-fetch[1] or diff --git a/remote.c b/remote.c index 68056c7..c988168 100644 --- a/remote.c +++ b/remote.c @@ -366,6 +366,9 @@ static int handle_config(const char *key, const char *value, void *cb) default_remote_name = branch->remote_name; explicit_default_remote_name = 1; } + } else if (!strcmp(subkey, ".pushremote")) { + if (branch == current_branch) + git_config_string(&pushremote_name, key, value); } else if (!strcmp(subkey, ".merge")) { if (!value) return config_error_nonbool(key); diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 63171f1..3f91874 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -539,6 +539,21 @@ test_expect_success 'push with config remote.*.pushurl' ' check_push_result $the_commit heads/master ' +test_expect_success 'push with config branch.*.pushremote' ' + mk_test_with_name up_repo heads/frotz && + mk_test_with_name side_repo heads/quux && + mk_test_with_name down_repo heads/master && + test_config remote.up.url up_repo && + test_config remote.pushdefault side_repo && + test_config remote.down.url down_repo && + test_config branch.master.remote up && + test_config branch.master.pushremote down && + git push && + test_must_fail check_push_result_with_name up_repo $the_commit heads/master && + test_must_fail check_push_result_with_name side_repo $the_commit heads/master && + check_push_result_with_name down_repo $the_commit heads/master +' + # clean up the cruft left with the previous one git config --remove-section remote.there -- 1.8.2 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 6/6] remote.c: introduce branch.<name>.pushremote 2013-03-20 12:45 ` [PATCH 6/6] remote.c: introduce branch.<name>.pushremote Ramkumar Ramachandra @ 2013-03-20 13:03 ` Tay Ray Chuan 2013-03-20 13:35 ` Ramkumar Ramachandra 0 siblings, 1 reply; 29+ messages in thread From: Tay Ray Chuan @ 2013-03-20 13:03 UTC (permalink / raw) To: Ramkumar Ramachandra Cc: Git List, Junio C Hamano, Jeff King, Eric Sunshine, Jonathan Nieder On Wed, Mar 20, 2013 at 8:45 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote: > This new configuration variable overrides `remote.pushdefault` and > `branch.<name>.remote` for pushes. In a typical triangular-workflow > setup, you would want to set `remote.pushdefault` to specify the > remote to push to for all branches, and use this option to override it > for a specific branch. > > Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com> > --- > Documentation/config.txt | 18 ++++++++++++++---- > remote.c | 3 +++ > t/t5516-fetch-push.sh | 15 +++++++++++++++ > 3 files changed, 32 insertions(+), 4 deletions(-) Shouldn't this patch be squashed into 5/6 because of... > diff --git a/Documentation/config.txt b/Documentation/config.txt > index e813c33..4b9647a 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -726,9 +726,18 @@ branch.<name>.remote:: > When on branch <name>, it tells 'git fetch' and 'git push' > which remote to fetch from/push to. The remote to push to > may be overridden with `remote.pushdefault` (for all branches). > - If no remote is configured, or if you are not on any branch, > - it defaults to `origin` for fetching and `remote.pushdefault` > - for pushing. > + The remote to push to, for the current branch, may be further > + overridden by `branch.<name>.pushremote`. If no remote is > + configured, or if you are not on any branch, it defaults to > + `origin` for fetching and `remote.pushdefault` for pushing. > + ...this? (Since this description was introduced in 5/6) -- Cheers, Ray Chuan ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 6/6] remote.c: introduce branch.<name>.pushremote 2013-03-20 13:03 ` Tay Ray Chuan @ 2013-03-20 13:35 ` Ramkumar Ramachandra 0 siblings, 0 replies; 29+ messages in thread From: Ramkumar Ramachandra @ 2013-03-20 13:35 UTC (permalink / raw) To: Tay Ray Chuan Cc: Git List, Junio C Hamano, Jeff King, Eric Sunshine, Jonathan Nieder Tay Ray Chuan wrote: > On Wed, Mar 20, 2013 at 8:45 PM, Ramkumar Ramachandra > <artagnon@gmail.com> wrote: >> This new configuration variable overrides `remote.pushdefault` and >> `branch.<name>.remote` for pushes. In a typical triangular-workflow >> setup, you would want to set `remote.pushdefault` to specify the >> remote to push to for all branches, and use this option to override it >> for a specific branch. >> >> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com> >> --- >> Documentation/config.txt | 18 ++++++++++++++---- >> remote.c | 3 +++ >> t/t5516-fetch-push.sh | 15 +++++++++++++++ >> 3 files changed, 32 insertions(+), 4 deletions(-) > > Shouldn't this patch be squashed into 5/6 because of... > >> diff --git a/Documentation/config.txt b/Documentation/config.txt >> index e813c33..4b9647a 100644 >> --- a/Documentation/config.txt >> +++ b/Documentation/config.txt >> @@ -726,9 +726,18 @@ branch.<name>.remote:: >> When on branch <name>, it tells 'git fetch' and 'git push' >> which remote to fetch from/push to. The remote to push to >> may be overridden with `remote.pushdefault` (for all branches). >> - If no remote is configured, or if you are not on any branch, >> - it defaults to `origin` for fetching and `remote.pushdefault` >> - for pushing. >> + The remote to push to, for the current branch, may be further >> + overridden by `branch.<name>.pushremote`. If no remote is >> + configured, or if you are not on any branch, it defaults to >> + `origin` for fetching and `remote.pushdefault` for pushing. >> + > > ...this? (Since this description was introduced in 5/6) Huh? This patch introduces branch.<name>.pushremote: the relevant code and documentation changes. 5/6 introduces remote.pushdefault, which is completely different. ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2013-04-02 7:40 UTC | newest] Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-03-22 7:52 [PATCH v3 0/6] Support triangular workflows Ramkumar Ramachandra 2013-03-22 7:52 ` [PATCH 1/6] remote.c: simplify a bit of code using git_config_string() Ramkumar Ramachandra 2013-03-22 18:26 ` Jonathan Nieder 2013-03-22 7:52 ` [PATCH 2/6] t5516 (fetch-push): update test description Ramkumar Ramachandra 2013-03-22 18:29 ` Jonathan Nieder 2013-03-22 7:52 ` [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name() Ramkumar Ramachandra 2013-03-22 14:44 ` Jeff King 2013-03-22 14:52 ` Junio C Hamano 2013-03-22 14:59 ` Jeff King 2013-03-22 21:14 ` Jonathan Nieder 2013-03-28 13:03 ` Ramkumar Ramachandra 2013-03-22 14:54 ` Junio C Hamano 2013-03-22 14:58 ` Jeff King 2013-03-22 7:52 ` [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push Ramkumar Ramachandra 2013-03-22 21:21 ` Jonathan Nieder 2013-03-22 22:13 ` Junio C Hamano 2013-03-22 23:41 ` Jonathan Nieder 2013-03-23 13:18 ` Ramkumar Ramachandra 2013-03-23 12:57 ` Ramkumar Ramachandra 2013-03-22 7:52 ` [PATCH 5/6] remote.c: introduce remote.pushdefault Ramkumar Ramachandra 2013-03-22 14:56 ` Jeff King 2013-03-27 9:39 ` Ramkumar Ramachandra 2013-03-22 7:52 ` [PATCH 6/6] remote.c: introduce branch.<name>.pushremote Ramkumar Ramachandra 2013-03-22 17:37 ` Junio C Hamano -- strict thread matches above, loose matches on Subject: below -- 2013-04-02 7:40 [PATCH 0/6] Re-roll rr/triangle Ramkumar Ramachandra 2013-04-02 7:40 ` [PATCH 6/6] remote.c: introduce branch.<name>.pushremote Ramkumar Ramachandra 2013-03-28 13:26 [PATCH v4 0/6] Support triangular workflows Ramkumar Ramachandra 2013-03-28 13:26 ` [PATCH 6/6] remote.c: introduce branch.<name>.pushremote Ramkumar Ramachandra 2013-03-20 12:44 [PATCH v2 0/6] Support triangular workflows Ramkumar Ramachandra 2013-03-20 12:45 ` [PATCH 6/6] remote.c: introduce branch.<name>.pushremote Ramkumar Ramachandra 2013-03-20 13:03 ` Tay Ray Chuan 2013-03-20 13:35 ` Ramkumar Ramachandra
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).