{"thread":{"id":"33239","subject":"[PATCH v2 0/6] Support triangular workflows","startedAt":"2013-03-20T12:44:55Z","lastAt":"2013-03-23T12:42:38Z","messageCount":33,"participants":["Ramkumar Ramachandra","Tay Ray Chuan","Jonathan Nieder","Junio C Hamano","Jeff King","Philip Oakley"],"isPatch":true,"patchVersion":2,"patchTotal":6},"messages":[{"id":"211745","messageId":"1363783501-27981-1-git-send-email-artagnon@gmail.com","threadId":"33239","inReplyTo":null,"subject":"[PATCH v2 0/6] Support triangular workflows","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-20T12:44:55Z","receivedAt":"2013-03-20T12:44:55Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"This follows-up [1], with three important differences:\n\n1. pushremote_get() and remote_get() share code better. Thanks Jeff.\n\n2. All spelling mistakes have been corrected. Thanks Eric.\n\n3. One new test for each of the new configuration variables. The\nextra two parts [2/6] and [3/6] preprare the file for introducing\ntests. However, I've not gone overboard in this preparation; I don't\nreplicate the work done by Jonathan in [2].\n\nThanks for reading.\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/218410\n[2]: http://thread.gmane.org/gmane.comp.version-control.git/218451/focus=218465\n\nRamkumar Ramachandra (6):\n remote.c: simplify a bit of code using git_config_string()\n t5516 (fetch-push): update test description\n t5516 (fetch-push): introduce mk_test_with_name()\n remote.c: introduce a way to have different remotes for fetch/push\n remote.c: introduce remote.pushdefault\n remote.c: introduce branch..pushremote\n\n Documentation/config.txt | 23 +++++++++++++++---\n builtin/push.c | 2 +-\n remote.c | 36 +++++++++++++++++++++------\n remote.h | 1 +\n t/t5516-fetch-push.sh | 63 ++++++++++++++++++++++++++++++++++++++++--------\n 5 files changed, 104 insertions(+), 21 deletions(-)\n\n-- \n1.8.2\n"},{"id":"211747","messageId":"1363783501-27981-2-git-send-email-artagnon@gmail.com","threadId":"33239","inReplyTo":"1363783501-27981-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/6] remote.c: simplify a bit of code using git_config_string()","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-20T12:44:56Z","receivedAt":"2013-03-20T12:44:56Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"A small segment where handle_config() parses the branch.remote\nconfiguration variable can be simplified using git_config_string().\n\nSigned-off-by: Ramkumar Ramachandra \n---\n remote.c | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex e53a6eb..45b69d6 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -356,9 +356,7 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t\t\treturn 0;\n \t\tbranch = make_branch(name, subkey - name);\n \t\tif (!strcmp(subkey, \".remote\")) {\n-\t\t\tif (!value)\n-\t\t\t\treturn config_error_nonbool(key);\n-\t\t\tbranch->remote_name = xstrdup(value);\n+\t\t\tgit_config_string(&branch->remote_name, key, value);\n \t\t\tif (branch == current_branch) {\n \t\t\t\tdefault_remote_name = branch->remote_name;\n \t\t\t\texplicit_default_remote_name = 1;\n-- \n1.8.2\n"},{"id":"211746","messageId":"1363783501-27981-3-git-send-email-artagnon@gmail.com","threadId":"33239","inReplyTo":"1363783501-27981-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/6] t5516 (fetch-push): update test description","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-20T12:44:57Z","receivedAt":"2013-03-20T12:44:57Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The file was originally created in bcdb34f (Test wildcard push/fetch,\n2007-06-08), and only contained tests that exercised wildcard\nfunctionality at the time. In subsequent commits, many other tests\nunrelated to wildcards were added but the test description was never\nupdated. Fix this.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n t/t5516-fetch-push.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex c31e5c1..bfeec60 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-test_description='fetching and pushing, with or without wildcard'\n+test_description='fetching and pushing'\n \n . ./test-lib.sh\n \n-- \n1.8.2\n"},{"id":"211749","messageId":"1363783501-27981-4-git-send-email-artagnon@gmail.com","threadId":"33239","inReplyTo":"1363783501-27981-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name()","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-20T12:44:58Z","receivedAt":"2013-03-20T12:44:58Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"mk_test() creates a repository with the constant name \"testrepo\", and\nthis may be limiting for tests that need to create more than one\nrepository for testing. To fix this, create a new mk_test_with_name()\nwhich accepts the repository name as $1. Reimplement mk_test() as a\nspecial case of this function, making sure that no tests need to be\nrewritten. Do the same thing for check_push_result().\n\nSigned-off-by: Ramkumar Ramachandra \n---\n t/t5516-fetch-push.sh | 34 +++++++++++++++++++++++++---------\n 1 file changed, 25 insertions(+), 9 deletions(-)\n\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex bfeec60..a546c2c 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -7,27 +7,32 @@ test_description='fetching and pushing'\n D=`pwd`\n \n mk_empty () {\n-\trm -fr testrepo &&\n-\tmkdir testrepo &&\n+\trepo_name=\"$1\"\n+\ttest -z \"$repo_name\" && repo_name=testrepo\n+\trm -fr $repo_name &&\n+\tmkdir $repo_name &&\n \t(\n-\t\tcd testrepo &&\n+\t\tcd $repo_name &&\n \t\tgit init &&\n \t\tgit config receive.denyCurrentBranch warn &&\n \t\tmv .git/hooks .git/hooks-disabled\n \t)\n }\n \n-mk_test () {\n-\tmk_empty &&\n+mk_test_with_name () {\n+\trepo_name=\"$1\"\n+\tshift\n+\n+\tmk_empty $repo_name &&\n \t(\n \t\tfor ref in \"$@\"\n \t\tdo\n-\t\t\tgit push testrepo $the_first_commit:refs/$ref || {\n+\t\t\tgit push $repo_name $the_first_commit:refs/$ref || {\n \t\t\t\techo \"Oops, push refs/$ref failure\"\n \t\t\t\texit 1\n \t\t\t}\n \t\tdone &&\n-\t\tcd testrepo &&\n+\t\tcd $repo_name &&\n \t\tfor ref in \"$@\"\n \t\tdo\n \t\t\tr=$(git show-ref -s --verify refs/$ref) &&\n@@ -40,6 +45,10 @@ mk_test () {\n \t)\n }\n \n+mk_test () {\n+\tmk_test_with_name testrepo \"$@\"\n+}\n+\n mk_test_with_hooks() {\n \tmk_test \"$@\" &&\n \t(\n@@ -79,9 +88,12 @@ mk_child() {\n \tgit clone testrepo \"$1\"\n }\n \n-check_push_result () {\n+check_push_result_with_name () {\n+\trepo_name=\"$1\"\n+\tshift\n+\n \t(\n-\t\tcd testrepo &&\n+\t\tcd $repo_name &&\n \t\tit=\"$1\" &&\n \t\tshift\n \t\tfor ref in \"$@\"\n@@ -96,6 +108,10 @@ check_push_result () {\n \t)\n }\n \n+check_push_result () {\n+\tcheck_push_result_with_name testrepo \"$@\"\n+}\n+\n test_expect_success setup '\n \n \t>path1 &&\n-- \n1.8.2\n"},{"id":"211748","messageId":"1363783501-27981-5-git-send-email-artagnon@gmail.com","threadId":"33239","inReplyTo":"1363783501-27981-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-20T12:44:59Z","receivedAt":"2013-03-20T12:44:59Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Currently, do_push() in push.c calls remote_get(), which gets the\nconfigured remote for fetching and pushing. Replace this call with a\ncall to pushremote_get() instead, a new function that will return the\nremote configured specifically for pushing. This function tries to\nwork with the string pushremote_name, before falling back to the\ncodepath of remote_get(). This patch has no visible impact, but\nserves to enable future patches to introduce configuration variables\nto set this variable.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n builtin/push.c | 2 +-\n remote.c | 25 +++++++++++++++++++++----\n remote.h | 1 +\n 3 files changed, 23 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 42b129d..d447a80 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -322,7 +322,7 @@ static int push_with_options(struct transport *transport, int flags)\n static int do_push(const char *repo, int flags)\n {\n \tint i, errs;\n-\tstruct remote *remote = remote_get(repo);\n+\tstruct remote *remote = pushremote_get(repo);\n \tconst char **url;\n \tint url_nr;\n \ndiff --git a/remote.c b/remote.c\nindex 45b69d6..cf7a65d 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -48,6 +48,7 @@ static int branches_nr;\n \n static struct branch *current_branch;\n static const char *default_remote_name;\n+static const char *pushremote_name;\n static int explicit_default_remote_name;\n \n static struct rewrites rewrites;\n@@ -669,17 +670,21 @@ static int valid_remote_nick(const char *name)\n \treturn !strchr(name, '/'); /* no slash */\n }\n \n-struct remote *remote_get(const char *name)\n+static struct remote *remote_get_1(const char *name, const char *pushremote_name)\n {\n \tstruct remote *ret;\n \tint name_given = 0;\n \n-\tread_config();\n \tif (name)\n \t\tname_given = 1;\n \telse {\n-\t\tname = default_remote_name;\n-\t\tname_given = explicit_default_remote_name;\n+\t\tif (pushremote_name) {\n+\t\t\tname = pushremote_name;\n+\t\t\tname_given = 1;\n+\t\t} else {\n+\t\t\tname = default_remote_name;\n+\t\t\tname_given = explicit_default_remote_name;\n+\t\t}\n \t}\n \n \tret = make_remote(name, 0);\n@@ -698,6 +703,18 @@ struct remote *remote_get(const char *name)\n \treturn ret;\n }\n \n+struct remote *remote_get(const char *name)\n+{\n+\tread_config();\n+\treturn remote_get_1(name, NULL);\n+}\n+\n+struct remote *pushremote_get(const char *name)\n+{\n+\tread_config();\n+\treturn remote_get_1(name, pushremote_name);\n+}\n+\n int remote_is_configured(const char *name)\n {\n \tint i;\ndiff --git a/remote.h b/remote.h\nindex 251d8fd..99a437f 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -51,6 +51,7 @@ struct remote {\n };\n \n struct remote *remote_get(const char *name);\n+struct remote *pushremote_get(const char *name);\n int remote_is_configured(const char *name);\n \n typedef int each_remote_fn(struct remote *remote, void *priv);\n-- \n1.8.2\n"},{"id":"211751","messageId":"1363783501-27981-6-git-send-email-artagnon@gmail.com","threadId":"33239","inReplyTo":"1363783501-27981-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 5/6] remote.c: introduce remote.pushdefault","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-20T12:45:00Z","receivedAt":"2013-03-20T12:45:00Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"This new configuration variable defines the default remote to push to,\nand overrides `branch..remote` for all branches. It is useful\nin the typical triangular-workflow setup, where the remote you're\nfetching from is different from the remote you're pushing to.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n Documentation/config.txt | 13 ++++++++++---\n remote.c | 4 ++++\n t/t5516-fetch-push.sh | 12 ++++++++++++\n 3 files changed, 26 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex bbba728..e813c33 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -723,9 +723,12 @@ branch.autosetuprebase::\n \tThis option defaults to never.\n \n branch..remote::\n-\tWhen in branch , it tells 'git fetch' and 'git push' which\n-\tremote to fetch from/push to. It defaults to `origin` if no remote is\n-\tconfigured. `origin` is also used if you are not on any branch.\n+\tWhen on branch , it tells 'git fetch' and 'git push'\n+\twhich remote to fetch from/push to. The remote to push to\n+\tmay be overridden with `remote.pushdefault` (for all branches).\n+\tIf no remote is configured, or if you are not on any branch,\n+\tit defaults to `origin` for fetching and `remote.pushdefault`\n+\tfor pushing.\n \n branch..merge::\n \tDefines, together with branch..remote, the upstream branch\n@@ -1894,6 +1897,10 @@ receive.updateserverinfo::\n \tIf set to true, git-receive-pack will run git-update-server-info\n \tafter receiving data from git-push and updating refs.\n \n+remote.pushdefault::\n+\tThe remote to push to by default. Overrides\n+\t`branch..remote` for all branches.\n+\n remote..url::\n \tThe URL of a remote repository. See linkgit:git-fetch[1] or\n \tlinkgit:git-push[1].\ndiff --git a/remote.c b/remote.c\nindex cf7a65d..68056c7 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -350,6 +350,10 @@ static int handle_config(const char *key, const char *value, void *cb)\n \tconst char *subkey;\n \tstruct remote *remote;\n \tstruct branch *branch;\n+\tif (!prefixcmp(key, \"remote.\")) {\n+\t\tif (!strcmp(key + 7, \"pushdefault\"))\n+\t\t\tgit_config_string(&pushremote_name, key, value);\n+\t}\n \tif (!prefixcmp(key, \"branch.\")) {\n \t\tname = key + 7;\n \t\tsubkey = strrchr(name, '.');\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex a546c2c..63171f1 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -517,6 +517,18 @@ test_expect_success 'push with config remote.*.push = HEAD' '\n git config --remove-section remote.there\n git config --remove-section branch.master\n \n+test_expect_success 'push with remote.pushdefault' '\n+\tmk_test_with_name up_repo heads/frotz &&\n+\tmk_test_with_name down_repo heads/master &&\n+\ttest_config remote.up.url up_repo &&\n+\ttest_config remote.down.url down_repo &&\n+\ttest_config branch.master.remote up &&\n+\ttest_config remote.pushdefault down &&\n+\tgit push &&\n+\ttest_must_fail check_push_result_with_name up_repo $the_commit heads/master &&\n+\tcheck_push_result_with_name down_repo $the_commit heads/master\n+'\n+\n test_expect_success 'push with config remote.*.pushurl' '\n \n \tmk_test heads/master &&\n-- \n1.8.2\n"},{"id":"211750","messageId":"1363783501-27981-7-git-send-email-artagnon@gmail.com","threadId":"33239","inReplyTo":"1363783501-27981-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 6/6] remote.c: introduce branch..pushremote","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-20T12:45:01Z","receivedAt":"2013-03-20T12:45:01Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"This new configuration variable overrides `remote.pushdefault` and\n`branch..remote` for pushes. In a typical triangular-workflow\nsetup, you would want to set `remote.pushdefault` to specify the\nremote to push to for all branches, and use this option to override it\nfor a specific branch.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n Documentation/config.txt | 18 ++++++++++++++----\n remote.c | 3 +++\n t/t5516-fetch-push.sh | 15 +++++++++++++++\n 3 files changed, 32 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex e813c33..4b9647a 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -726,9 +726,18 @@ branch..remote::\n \tWhen on branch , it tells 'git fetch' and 'git push'\n \twhich remote to fetch from/push to. The remote to push to\n \tmay be overridden with `remote.pushdefault` (for all branches).\n-\tIf no remote is configured, or if you are not on any branch,\n-\tit defaults to `origin` for fetching and `remote.pushdefault`\n-\tfor pushing.\n+\tThe remote to push to, for the current branch, may be further\n+\toverridden by `branch..pushremote`. If no remote is\n+\tconfigured, or if you are not on any branch, it defaults to\n+\t`origin` for fetching and `remote.pushdefault` for pushing.\n+\n+branch..pushremote::\n+\tWhen on branch , it overrides `branch..remote`\n+\twhen pushing. It also overrides `remote.pushdefault` when\n+\tpushing from branch . In a typical triangular-workflow\n+\tsetup, you would want to set `remote.pushdefault` to specify\n+\tthe remote to push to for all branches, and use this option to\n+\toverride it for a specific branch.\n \n branch..merge::\n \tDefines, together with branch..remote, the upstream branch\n@@ -1899,7 +1908,8 @@ receive.updateserverinfo::\n \n remote.pushdefault::\n \tThe remote to push to by default. Overrides\n-\t`branch..remote` for all branches.\n+\t`branch..remote` for all branches, and is overridden by\n+\t`branch..pushremote` for specific branches.\n \n remote..url::\n \tThe URL of a remote repository. See linkgit:git-fetch[1] or\ndiff --git a/remote.c b/remote.c\nindex 68056c7..c988168 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -366,6 +366,9 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t\t\t\tdefault_remote_name = branch->remote_name;\n \t\t\t\texplicit_default_remote_name = 1;\n \t\t\t}\n+\t\t} else if (!strcmp(subkey, \".pushremote\")) {\n+\t\t\tif (branch == current_branch)\n+\t\t\t\tgit_config_string(&pushremote_name, key, value);\n \t\t} else if (!strcmp(subkey, \".merge\")) {\n \t\t\tif (!value)\n \t\t\t\treturn config_error_nonbool(key);\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 63171f1..3f91874 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -539,6 +539,21 @@ test_expect_success 'push with config remote.*.pushurl' '\n \tcheck_push_result $the_commit heads/master\n '\n \n+test_expect_success 'push with config branch.*.pushremote' '\n+\tmk_test_with_name up_repo heads/frotz &&\n+\tmk_test_with_name side_repo heads/quux &&\n+\tmk_test_with_name down_repo heads/master &&\n+\ttest_config remote.up.url up_repo &&\n+\ttest_config remote.pushdefault side_repo &&\n+\ttest_config remote.down.url down_repo &&\n+\ttest_config branch.master.remote up &&\n+\ttest_config branch.master.pushremote down &&\n+\tgit push &&\n+\ttest_must_fail check_push_result_with_name up_repo $the_commit heads/master &&\n+\ttest_must_fail check_push_result_with_name side_repo $the_commit heads/master &&\n+\tcheck_push_result_with_name down_repo $the_commit heads/master\n+'\n+\n # clean up the cruft left with the previous one\n git config --remove-section remote.there\n \n-- \n1.8.2\n"},{"id":"211752","messageId":"CALUzUxobybPOqsLgEFVOCK2OLOvyqHtAiuyi8wozOSYeWzkhNg@mail.gmail.com","threadId":"33239","inReplyTo":"1363783501-27981-7-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 6/6] remote.c: introduce branch..pushremote","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2013-03-20T13:03:10Z","receivedAt":"2013-03-20T13:03:10Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"On Wed, Mar 20, 2013 at 8:45 PM, Ramkumar Ramachandra\n wrote:\n> This new configuration variable overrides `remote.pushdefault` and\n> `branch..remote` for pushes. In a typical triangular-workflow\n> setup, you would want to set `remote.pushdefault` to specify the\n> remote to push to for all branches, and use this option to override it\n> for a specific branch.\n>\n> Signed-off-by: Ramkumar Ramachandra \n> ---\n> Documentation/config.txt | 18 ++++++++++++++----\n> remote.c | 3 +++\n> t/t5516-fetch-push.sh | 15 +++++++++++++++\n> 3 files changed, 32 insertions(+), 4 deletions(-)\n\nShouldn't this patch be squashed into 5/6 because of...\n\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index e813c33..4b9647a 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -726,9 +726,18 @@ branch..remote::\n> When on branch , it tells 'git fetch' and 'git push'\n> which remote to fetch from/push to. The remote to push to\n> may be overridden with `remote.pushdefault` (for all branches).\n> - If no remote is configured, or if you are not on any branch,\n> - it defaults to `origin` for fetching and `remote.pushdefault`\n> - for pushing.\n> + The remote to push to, for the current branch, may be further\n> + overridden by `branch..pushremote`. If no remote is\n> + configured, or if you are not on any branch, it defaults to\n> + `origin` for fetching and `remote.pushdefault` for pushing.\n> +\n\n...this? (Since this description was introduced in 5/6)\n\n-- \nCheers,\nRay Chuan\n"},{"id":"211753","messageId":"CALUzUxq1gJY_gSQt4iffrVzawx6Qk4=BWnPoCsZqFeHUpkgwCg@mail.gmail.com","threadId":"33239","inReplyTo":"1363783501-27981-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH v2 0/6] Support triangular workflows","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2013-03-20T13:06:32Z","receivedAt":"2013-03-20T13:06:32Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"On Wed, Mar 20, 2013 at 8:44 PM, Ramkumar Ramachandra\n wrote:\n> remote.c: introduce remote.pushdefault\n> remote.c: introduce branch..pushremote\n\nPerhaps we should clarify how this differs from remote.pushurl in the\ndocumentation for it, in git-config and/or git-push. Maybe even\ninclude the design decisions behind it from [1]. :)\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/215702/focus=215717\n\n--\nCheers,\nRay Chuan\n"},{"id":"211754","messageId":"CALkWK0miT-Gtku1VU0qpX6L2VnrwgHnmpYwuXCME55cSGscgTA@mail.gmail.com","threadId":"33239","inReplyTo":"CALUzUxobybPOqsLgEFVOCK2OLOvyqHtAiuyi8wozOSYeWzkhNg@mail.gmail.com","subject":"Re: [PATCH 6/6] remote.c: introduce branch..pushremote","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-20T13:35:32Z","receivedAt":"2013-03-20T13:35:32Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Tay Ray Chuan wrote:\n> On Wed, Mar 20, 2013 at 8:45 PM, Ramkumar Ramachandra\n> wrote:\n>> This new configuration variable overrides `remote.pushdefault` and\n>> `branch..remote` for pushes. In a typical triangular-workflow\n>> setup, you would want to set `remote.pushdefault` to specify the\n>> remote to push to for all branches, and use this option to override it\n>> for a specific branch.\n>>\n>> Signed-off-by: Ramkumar Ramachandra \n>> ---\n>> Documentation/config.txt | 18 ++++++++++++++----\n>> remote.c | 3 +++\n>> t/t5516-fetch-push.sh | 15 +++++++++++++++\n>> 3 files changed, 32 insertions(+), 4 deletions(-)\n>\n> Shouldn't this patch be squashed into 5/6 because of...\n>\n>> diff --git a/Documentation/config.txt b/Documentation/config.txt\n>> index e813c33..4b9647a 100644\n>> --- a/Documentation/config.txt\n>> +++ b/Documentation/config.txt\n>> @@ -726,9 +726,18 @@ branch..remote::\n>> When on branch , it tells 'git fetch' and 'git push'\n>> which remote to fetch from/push to. The remote to push to\n>> may be overridden with `remote.pushdefault` (for all branches).\n>> - If no remote is configured, or if you are not on any branch,\n>> - it defaults to `origin` for fetching and `remote.pushdefault`\n>> - for pushing.\n>> + The remote to push to, for the current branch, may be further\n>> + overridden by `branch..pushremote`. If no remote is\n>> + configured, or if you are not on any branch, it defaults to\n>> + `origin` for fetching and `remote.pushdefault` for pushing.\n>> +\n>\n> ...this? (Since this description was introduced in 5/6)\n\nHuh? This patch introduces branch..pushremote: the relevant\ncode and documentation changes. 5/6 introduces remote.pushdefault,\nwhich is completely different.\n"},{"id":"211780","messageId":"20130320180707.GH3655@google.com","threadId":"33239","inReplyTo":"1363783501-27981-2-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 1/6] remote.c: simplify a bit of code using git_config_string()","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-20T18:07:07Z","receivedAt":"2013-03-20T18:07:07Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -356,9 +356,7 @@ static int handle_config(const char *key, const char *value, void *cb)\n> \t\t\treturn 0;\n> \t\tbranch = make_branch(name, subkey - name);\n> \t\tif (!strcmp(subkey, \".remote\")) {\n> -\t\t\tif (!value)\n> -\t\t\t\treturn config_error_nonbool(key);\n> -\t\t\tbranch->remote_name = xstrdup(value);\n> +\t\t\tgit_config_string(&branch->remote_name, key, value);\n\nShouldn't this say\n\n\t\t\tif (git_config_string(&branch->remote_name, key, value))\n\t\t\t\treturn -1;\n\nor something?\n\nThanks,\nJonathan\n"},{"id":"211781","messageId":"CALkWK0mKKRmVdvvLmrqCPdPQoeYxWaFcuixyonnDM6o8Nt7ppg@mail.gmail.com","threadId":"33239","inReplyTo":"20130320180707.GH3655@google.com","subject":"Re: [PATCH 1/6] remote.c: simplify a bit of code using git_config_string()","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-20T18:12:35Z","receivedAt":"2013-03-20T18:12:35Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Jonathan Nieder wrote:\n> Ramkumar Ramachandra wrote:\n>\n>> --- a/remote.c\n>> +++ b/remote.c\n>> @@ -356,9 +356,7 @@ static int handle_config(const char *key, const char *value, void *cb)\n>> return 0;\n>> branch = make_branch(name, subkey - name);\n>> if (!strcmp(subkey, \".remote\")) {\n>> - if (!value)\n>> - return config_error_nonbool(key);\n>> - branch->remote_name = xstrdup(value);\n>> + git_config_string(&branch->remote_name, key, value);\n>\n> Shouldn't this say\n>\n> if (git_config_string(&branch->remote_name, key, value))\n> return -1;\n>\n> or something?\n\nYes, and so should the instances in [5/6] and [6/6]. Thanks for catching it.\n"},{"id":"211783","messageId":"20130320182213.GI3655@google.com","threadId":"33239","inReplyTo":"1363783501-27981-3-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 2/6] t5516 (fetch-push): update test description","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-20T18:22:13Z","receivedAt":"2013-03-20T18:22:13Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> --- a/t/t5516-fetch-push.sh\n> +++ b/t/t5516-fetch-push.sh\n> @@ -1,6 +1,6 @@\n> #!/bin/sh\n> \n> -test_description='fetching and pushing, with or without wildcard'\n> +test_description='fetching and pushing'\n\nI'm not thrilled with the description before or after. Would it make\nsense to do something like the following?\n\n\ttest_description='Tests of basic fetch/push functionality.\n\t\n\tThese tests create small test repositories and fetch from and\n\tpush to them, testing:\n\n\t * commandline syntax\n\t * refspecs and default refspecs\n\t * fast-forward detection and overriding fast-forward detection\n\t * configuration (insteadOf, pushInsteadOf, [remote \"name\"] push,\n\t etc)\n\t * hooks\n\t * --porcelain output format\n\t * hiderefs\n\t'\n"},{"id":"211785","messageId":"20130320182830.GJ3655@google.com","threadId":"33239","inReplyTo":"1363783501-27981-4-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name()","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-20T18:28:30Z","receivedAt":"2013-03-20T18:28:30Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> mk_test() creates a repository with the constant name \"testrepo\", and\n> this may be limiting for tests that need to create more than one\n> repository for testing. To fix this, create a new mk_test_with_name()\n> which accepts the repository name as $1. Reimplement mk_test() as a\n> special case of this function, making sure that no tests need to be\n> rewritten.\n\nWhy not give mk_test an optional parameter?\n\n\trepo_name=${1:-testrepo}\n\nOh, it is because mk_test already takes arguments naming refs to push.\nI suppose the change description could make this clearer.\n\nWhy not use mk_test and then rename, like so?\n\n\tmk_test ...refs... &&\n\tmv testrepo testrepo-a &&\n\n\tmk_test ...refs... &&\n\tmv testrepo testrepo-b &&\n\t...\n\nI dunno. The helper functions at the top of this test are already\nintimidating, so I guess I am looking for a way to avoid making that\nproblem worse. One way would be to add an opening comment before\nthe function definition explaining how it is meant to be used. See\nt/test-lib-functions.sh for examples, such as test_cmp.\n\nHope that helps,\nJonathan\n"},{"id":"211786","messageId":"20130320183015.GK3655@google.com","threadId":"33239","inReplyTo":"1363783501-27981-5-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-20T18:30:15Z","receivedAt":"2013-03-20T18:30:15Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Currently, do_push() in push.c calls remote_get(), which gets the\n> configured remote for fetching and pushing. Replace this call with a\n> call to pushremote_get() instead, a new function that will return the\n> remote configured specifically for pushing. This function tries to\n> work with the string pushremote_name, before falling back to the\n> codepath of remote_get(). This patch has no visible impact, but\n> serves to enable future patches to introduce configuration variables\n> to set this variable.\n\nThe above description does not make the impact of this change clear to\nme. Could you give a before-and-after example? How will this\ninternal API change make my life easier as a developer?\n\nHope that helps,\nJonathan\n"},{"id":"211788","messageId":"20130320183255.GL3655@google.com","threadId":"33239","inReplyTo":"1363783501-27981-6-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 5/6] remote.c: introduce remote.pushdefault","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-20T18:32:55Z","receivedAt":"2013-03-20T18:32:55Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> This new configuration variable defines the default remote to push to,\n> and overrides `branch..remote` for all branches.\n\nMicronit: I think this would be easier to explain if it came after\npatch 6, since then you could say \"In other words, it is a default for\nbranch..pushremote for all branches\" and readers would not have\nto wonder \"Why does the more generic configuration override the more\nspecific one?\".\n\nMy two cents,\nJonathan\n"},{"id":"211789","messageId":"CALkWK0nu5oMw-+uh8mLRUOyAFoETUpKZfYuO2LLJAgF0+XHpRA@mail.gmail.com","threadId":"33239","inReplyTo":"20130320182213.GI3655@google.com","subject":"Re: [PATCH 2/6] t5516 (fetch-push): update test description","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-20T18:33:51Z","receivedAt":"2013-03-20T18:33:51Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Jonathan Nieder wrote:\n> Ramkumar Ramachandra wrote:\n>\n>> --- a/t/t5516-fetch-push.sh\n>> +++ b/t/t5516-fetch-push.sh\n>> @@ -1,6 +1,6 @@\n>> #!/bin/sh\n>>\n>> -test_description='fetching and pushing, with or without wildcard'\n>> +test_description='fetching and pushing'\n>\n> I'm not thrilled with the description before or after. Would it make\n> sense to do something like the following?\n>\n> test_description='Tests of basic fetch/push functionality.\n>\n> These tests create small test repositories and fetch from and\n> push to them, testing:\n>\n> * commandline syntax\n> * refspecs and default refspecs\n> * fast-forward detection and overriding fast-forward detection\n> * configuration (insteadOf, pushInsteadOf, [remote \"name\"] push,\n> etc)\n> * hooks\n> * --porcelain output format\n> * hiderefs\n> '\n\nNo. When I want to add a test for branch..pushremote, I grep\nfor branch.*.pushurl, and open files with sensible names; I'm not\ngoing to open up the file and read a long description of what tests it\nalready contains. The filename and test headlines are sufficient.\nOur test suite is bad enough as it is (inconsistent style, missing &&,\nfalse positives)- I'm against adding to the maintenance burden.\n"},{"id":"211791","messageId":"20130320183538.GM3655@google.com","threadId":"33239","inReplyTo":"CALkWK0nu5oMw-+uh8mLRUOyAFoETUpKZfYuO2LLJAgF0+XHpRA@mail.gmail.com","subject":"Re: [PATCH 2/6] t5516 (fetch-push): update test description","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-20T18:35:38Z","receivedAt":"2013-03-20T18:35:38Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> When I want to add a test for branch..pushremote, I grep\n> for branch.*.pushurl, and open files with sensible names; I'm not\n> going to open up the file and read a long description of what tests it\n> already contains.\n\nHuh? The test_description is output for \"./t5516-* --help\" and is\nsupposed to help people hacking on the test to understand its setup\nand its purpose.\n"},{"id":"211792","messageId":"CALkWK0mCVb6N76QU+U3iTO_gmU4PmhrTOgU53DAMd5x1bCQEtA@mail.gmail.com","threadId":"33239","inReplyTo":"20130320182830.GJ3655@google.com","subject":"Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name()","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-20T18:38:54Z","receivedAt":"2013-03-20T18:38:54Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Jonathan Nieder wrote:\n> Ramkumar Ramachandra wrote:\n>\n>> mk_test() creates a repository with the constant name \"testrepo\", and\n>> this may be limiting for tests that need to create more than one\n>> repository for testing. To fix this, create a new mk_test_with_name()\n>> which accepts the repository name as $1. Reimplement mk_test() as a\n>> special case of this function, making sure that no tests need to be\n>> rewritten.\n>\n> Why not give mk_test an optional parameter?\n>\n> repo_name=${1:-testrepo}\n>\n> Oh, it is because mk_test already takes arguments naming refs to push.\n> I suppose the change description could make this clearer.\n\nIsn't it obvious?\n\n> Why not use mk_test and then rename, like so?\n>\n> mk_test ...refs... &&\n> mv testrepo testrepo-a &&\n>\n> mk_test ...refs... &&\n> mv testrepo testrepo-b &&\n> ...\n\nNo. This is ugly. mk_test() should not hardcode \"testrepo\".\n\n> I dunno. The helper functions at the top of this test are already\n> intimidating, so I guess I am looking for a way to avoid making that\n> problem worse. One way would be to add an opening comment before\n> the function definition explaining how it is meant to be used. See\n> t/test-lib-functions.sh for examples, such as test_cmp.\n\nMy patch does not make the situation worse in any way: it just adds\none line that passes $1 as a parameter to existing code. Yes, the\nfunctions and tests can be improved greatly, but I refrained from\ndoing so because of your series. We can save it for later.\n"},{"id":"211795","messageId":"20130320184157.GO3655@google.com","threadId":"33239","inReplyTo":"CALkWK0mCVb6N76QU+U3iTO_gmU4PmhrTOgU53DAMd5x1bCQEtA@mail.gmail.com","subject":"Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name()","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-20T18:41:57Z","receivedAt":"2013-03-20T18:41:57Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n> Jonathan Nieder wrote:\n\n>> I dunno. The helper functions at the top of this test are already\n>> intimidating, so I guess I am looking for a way to avoid making that\n>> problem worse.\n[...]\n> My patch does not make the situation worse in any way:\n\nUm, yes it does. It adds another function to learn to an already\nintimidating list.\n\nHoping that clarifies,\nJonathan\n"},{"id":"211800","messageId":"7vk3p1ewcn.fsf@alter.siamese.dyndns.org","threadId":"33239","inReplyTo":"20130320183255.GL3655@google.com","subject":"Re: [PATCH 5/6] remote.c: introduce remote.pushdefault","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-20T18:53:44Z","receivedAt":"2013-03-20T18:53:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Ramkumar Ramachandra wrote:\n>\n>> This new configuration variable defines the default remote to push to,\n>> and overrides `branch..remote` for all branches.\n>\n> Micronit: I think this would be easier to explain if it came after\n> patch 6, since then you could say \"In other words, it is a default for\n> branch..pushremote for all branches\" and readers would not have\n> to wonder \"Why does the more generic configuration override the more\n> specific one?\".\n>\n> My two cents,\n> Jonathan\n\nThanks for all good comments (not only to this one but to others in\nthe series). I agree with all of them.\n"},{"id":"211802","messageId":"20130320185844.GB30165@sigill.intra.peff.net","threadId":"33239","inReplyTo":"20130320184157.GO3655@google.com","subject":"Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-20T18:58:45Z","receivedAt":"2013-03-20T18:58:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 20, 2013 at 11:41:57AM -0700, Jonathan Nieder wrote:\n\n> Ramkumar Ramachandra wrote:\n> > Jonathan Nieder wrote:\n> \n> >> I dunno. The helper functions at the top of this test are already\n> >> intimidating, so I guess I am looking for a way to avoid making that\n> >> problem worse.\n> [...]\n> > My patch does not make the situation worse in any way:\n> \n> Um, yes it does. It adds another function to learn to an already\n> intimidating list.\n\nPersonally I do not find the set of helper functions intimidating. I\ntend to read the tests in a top-down manner: a test is interesting\n(usually because it fails), and then I want to see what it is doing, so\nI look at any functions it calls, and so forth.\n\nWhat I usually find _much_ harder to debug is when there is hidden state\nleftover from other tests. So even though it is longer to write, I would\nmuch rather see:\n\n test_expect_success 'check that frob only affects foo' '\n set_state_of foo &&\n set_state_of bar &&\n git frob &&\n check_state_of foo &&\n check_state_of bar\n '\n\nthan for the test to assume the state of \"foo\" or \"bar\" prior to the\ntest. And I think helper functions can help make writing those sorts of\npre-conditions more reasonable (and without looking too hard, I think\nt5516 does an OK job of that).\n\nRelated to that is when the helper functions operate on hidden state. In\nthis instance, we have tests that do things like:\n\n mk_empty &&\n git push testrepo refs/heads/master:refs/remotes/origin/master\n\nand as a reader I say \"wait, what's in testrepo?\". I can follow mk_empty\nand see that it hardcodes testrepo, but it is even better if the\nfunction and its arguments are named in a way that I don't have to. So\neven though it is more typing, I would argue that:\n\n mk_empty testrepo &&\n git push testrepo ...\n\nis better, because the test script is more readable as a unit.\n\nNone of this is that huge a deal to me (and yet I seem to have written a\npage about it :) ), but I figure while we are bikeshedding about test\nstyle...\n\n-Peff\n"},{"id":"211804","messageId":"7vfvzpevwf.fsf@alter.siamese.dyndns.org","threadId":"33239","inReplyTo":"1363783501-27981-5-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-20T19:03:28Z","receivedAt":"2013-03-20T19:03:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> \tif (name)\n> \t\tname_given = 1;\n> \telse {\n> -\t\tname = default_remote_name;\n> -\t\tname_given = explicit_default_remote_name;\n> +\t\tif (pushremote_name) {\n> +\t\t\tname = pushremote_name;\n> +\t\t\tname_given = 1;\n> +\t\t} else {\n> +\t\t\tname = default_remote_name;\n> +\t\t\tname_given = explicit_default_remote_name;\n> +\t\t}\n> \t}\n\nThe code to read branch.$name.remote configuration flips\nexplicit_default_remote_name to one when it is used to set the\ndefault_remote_name, and that controls the value of name_given in\nthis codepath. At this point in the series, you do not have a\ncorresponding branch.$name.pushremote, but your [6/6] does not seem\nto do the same.\n\nWhy isn't it necessary to add explicit_default_pushremote_name and\ndo the same here in patch [6/6]?\n"},{"id":"211810","messageId":"CALkWK0=29fu92A8_XQCmW86U2y-ktNAOy4qMmJAhyYHo8FgX1Q@mail.gmail.com","threadId":"33239","inReplyTo":"7vfvzpevwf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-20T19:43:58Z","receivedAt":"2013-03-20T19:43:58Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> Ramkumar Ramachandra writes:\n>\n>> if (name)\n>> name_given = 1;\n>> else {\n>> - name = default_remote_name;\n>> - name_given = explicit_default_remote_name;\n>> + if (pushremote_name) {\n>> + name = pushremote_name;\n>> + name_given = 1;\n>> + } else {\n>> + name = default_remote_name;\n>> + name_given = explicit_default_remote_name;\n>> + }\n>> }\n>\n> The code to read branch.$name.remote configuration flips\n> explicit_default_remote_name to one when it is used to set the\n> default_remote_name, and that controls the value of name_given in\n> this codepath. At this point in the series, you do not have a\n> corresponding branch.$name.pushremote, but your [6/6] does not seem\n> to do the same.\n>\n> Why isn't it necessary to add explicit_default_pushremote_name and\n> do the same here in patch [6/6]?\n\nSorry, I'm still trying to understand your comment. Okay, yes:\nbranch.$name.remote does flip explicit_default_remote_name, because we\nneed to know if the default remote name was explicitly given. Wait,\nhow is explicit_default_remote_name used to set default_remote_name?\nDon't you mean name_given? It controls name_give, yes. At this point\nI don't have .pushremote, yes: I'm setting up for [5/6] and [6/6]. My\n[6/6] doesn't seem to do the \"same\"? The same thing as .remote? Are\nyou asking why .pushremote doesn't flip explicit_default_remote_name\nlike .remote does? Because .pushremote can only ever be specified\nexplicitly: otherwise, it falls back to the .remote logic.\n\nOkay, next paragraph. Why isn't it necessary to add\nexplicit_default_pushremote_name? Like I said, .pushremote can only\never be specified explicitly. There is no implicit fallback (like\n\"origin\"): it just falls back to the .remote codepath, if not\nexplicitly specified. In other words, it's just a small override on\nthe .remote codepath.\n"},{"id":"211812","messageId":"CALkWK0=vrtO6Mjaaj-bWZgd=8VOd+O6Q7Vebr=mkiXLEBBeYsg@mail.gmail.com","threadId":"33239","inReplyTo":"20130320183255.GL3655@google.com","subject":"Re: [PATCH 5/6] remote.c: introduce remote.pushdefault","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-20T19:46:40Z","receivedAt":"2013-03-20T19:46:40Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Jonathan Nieder wrote:\n> Ramkumar Ramachandra wrote:\n>\n>> This new configuration variable defines the default remote to push to,\n>> and overrides `branch..remote` for all branches.\n>\n> Micronit: I think this would be easier to explain if it came after\n> patch 6, since then you could say \"In other words, it is a default for\n> branch..pushremote for all branches\" and readers would not have\n> to wonder \"Why does the more generic configuration override the more\n> specific one?\".\n\nI'm sorry, but this is not worth a rebase.\n"},{"id":"211813","messageId":"7v7gl1etu4.fsf@alter.siamese.dyndns.org","threadId":"33239","inReplyTo":"CALkWK0=29fu92A8_XQCmW86U2y-ktNAOy4qMmJAhyYHo8FgX1Q@mail.gmail.com","subject":"Re: [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-20T19:48:03Z","receivedAt":"2013-03-20T19:48:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> ... There is no implicit fallback (like\n> \"origin\"): it just falls back to the .remote codepath, if not\n> explicitly specified.\n\nThat one sentence is enough to explain the apparent asymmetry, which\nbothered me.\n"},{"id":"211815","messageId":"7v38vpetmp.fsf@alter.siamese.dyndns.org","threadId":"33239","inReplyTo":"20130320185844.GB30165@sigill.intra.peff.net","subject":"Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-20T19:52:30Z","receivedAt":"2013-03-20T19:52:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> ... even though it is more typing, I would argue that:\n>\n> mk_empty testrepo &&\n> git push testrepo ...\n>\n> is better, because the test script is more readable as a unit.\n>\n> None of this is that huge a deal to me (and yet I seem to have written a\n> page about it :) ), but I figure while we are bikeshedding about test\n> style...\n\n;-) But the above is a good point.\n"},{"id":"211819","messageId":"20130320200010.GR3655@google.com","threadId":"33239","inReplyTo":"20130320185844.GB30165@sigill.intra.peff.net","subject":"Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name()","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-20T20:00:11Z","receivedAt":"2013-03-20T20:00:11Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> I\n> tend to read the tests in a top-down manner: a test is interesting\n> (usually because it fails), and then I want to see what it is doing, so\n> I look at any functions it calls, and so forth.\n>\n> What I usually find _much_ harder to debug is when there is hidden state\n> leftover from other tests.\n\nThanks for articulating this. I agree that keeping track of state is\nthe hardest part of working with git's tests.\n\nTo clarify my earlier comment, I was reading the test script from the\npoint of view of someone who wants to add an additional test, rather\nthan someone debugging an existing one. That person has a difficult\ntask: she needs to understand\n\n * What do the existing tests in the script do? This information\n is important in deciding whether the new test will be redundant.\n\n * How do I work with the particular dialect used in the script,\n including helpers? A new test should fit in with the style of its\n surroundings to avoid contributing to an unmaintainable mess.\n\n * What is the intended scope of the test script? Does my new test\n belong in this script?\n\n * What is the logical progression of the script? What story does this\n script tell? Where should I insert my test to maintain a logical\n ordering?\n\n * What state that later tests rely on do I need to avoid clobbering?\n\nGenerally the best way to help such a person is to make the script\nvery simple. In particular, using standard helpers instead of custom\nfunctions when appropriate and documenting helpers used to give new\nreaders a quick introduction to the dialect can help a lot.\n"},{"id":"211832","messageId":"26A2BE0824FE4C999592D392FCBEF988@PhilipOakley","threadId":"33239","inReplyTo":"1363783501-27981-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH v2 0/6] Support triangular workflows","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2013-03-20T22:59:19Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Ramkumar Ramachandra\" \nSent: Wednesday, March 20, 2013 12:44 PM\n> This follows-up [1], with three important differences:\n>\n> 1. pushremote_get() and remote_get() share code better. Thanks Jeff.\n>\n> 2. All spelling mistakes have been corrected. Thanks Eric.\n>\n> 3. One new test for each of the new configuration variables. The\n> extra two parts [2/6] and [3/6] preprare the file for introducing\n> tests. However, I've not gone overboard in this preparation; I don't\n> replicate the work done by Jonathan in [2].\n>\n> Thanks for reading.\n>\n> [1]: http://thread.gmane.org/gmane.comp.version-control.git/218410\n> [2]: \n> http://thread.gmane.org/gmane.comp.version-control.git/218451/focus=218465\n>\n> Ramkumar Ramachandra (6):\n> remote.c: simplify a bit of code using git_config_string()\n> t5516 (fetch-push): update test description\n> t5516 (fetch-push): introduce mk_test_with_name()\n> remote.c: introduce a way to have different remotes for fetch/push\n> remote.c: introduce remote.pushdefault\n> remote.c: introduce branch..pushremote\n>\n> Documentation/config.txt | 23 +++++++++++++++---\n\nShouldn't Documentation/gitworkflows.txt also be updated with the \ntriangular workflow and its configuration?\n\n> builtin/push.c | 2 +-\n> remote.c | 36 +++++++++++++++++++++------\n> remote.h | 1 +\n> t/t5516-fetch-push.sh | 63 \n> ++++++++++++++++++++++++++++++++++++++++--------\n> 5 files changed, 104 insertions(+), 21 deletions(-)\n>\n> -- \n> 1.8.2\n>\n"},{"id":"211914","messageId":"CALkWK0=Mca1YiT0HVVrLrzmN2hGWuMJZPvNKNzb=0H7TxXuYwQ@mail.gmail.com","threadId":"33239","inReplyTo":"26A2BE0824FE4C999592D392FCBEF988@PhilipOakley","subject":"Re: [PATCH v2 0/6] Support triangular workflows","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-22T07:41:30Z","receivedAt":"2013-03-22T07:41:30Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Philip Oakley wrote:\n> From: \"Ramkumar Ramachandra\" \n> Sent: Wednesday, March 20, 2013 12:44 PM\n>\n>> This follows-up [1], with three important differences:\n>>\n>> 1. pushremote_get() and remote_get() share code better. Thanks Jeff.\n>>\n>> 2. All spelling mistakes have been corrected. Thanks Eric.\n>>\n>> 3. One new test for each of the new configuration variables. The\n>> extra two parts [2/6] and [3/6] preprare the file for introducing\n>> tests. However, I've not gone overboard in this preparation; I don't\n>> replicate the work done by Jonathan in [2].\n>>\n>> Thanks for reading.\n>>\n>> [1]: http://thread.gmane.org/gmane.comp.version-control.git/218410\n>> [2]:\n>> http://thread.gmane.org/gmane.comp.version-control.git/218451/focus=218465\n>>\n>> Ramkumar Ramachandra (6):\n>> remote.c: simplify a bit of code using git_config_string()\n>> t5516 (fetch-push): update test description\n>> t5516 (fetch-push): introduce mk_test_with_name()\n>> remote.c: introduce a way to have different remotes for fetch/push\n>> remote.c: introduce remote.pushdefault\n>> remote.c: introduce branch..pushremote\n>>\n>> Documentation/config.txt | 23 +++++++++++++++---\n>\n>\n> Shouldn't Documentation/gitworkflows.txt also be updated with the triangular\n> workflow and its configuration?\n\nCurrently, Documentation/gitworkflows.txt makes no effort to explain\nhow to set up configuration variables for centralized or distributed\nworkflows. I don't see how I could patch the existing document to\ninclude this workflow, without changing the entire structure of the\ndocument (left as an exercise for future patches).\n"},{"id":"211915","messageId":"CALkWK0mj-DqGogJwjudLPbNz1Kz_MNAziEWjCu_m_POEWGZ3-A@mail.gmail.com","threadId":"33239","inReplyTo":"CALUzUxq1gJY_gSQt4iffrVzawx6Qk4=BWnPoCsZqFeHUpkgwCg@mail.gmail.com","subject":"Re: [PATCH v2 0/6] Support triangular workflows","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-22T07:44:29Z","receivedAt":"2013-03-22T07:44:29Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Tay Ray Chuan wrote:\n> On Wed, Mar 20, 2013 at 8:44 PM, Ramkumar Ramachandra\n> wrote:\n>> remote.c: introduce remote.pushdefault\n>> remote.c: introduce branch..pushremote\n>\n> Perhaps we should clarify how this differs from remote.pushurl in the\n> documentation for it, in git-config and/or git-push. Maybe even\n> include the design decisions behind it from [1]. :)\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/215702/focus=215717\n\nActually, it's quite obvious when you think about it: remote.pushurl\ndoesn't allow remote tracking branches, and this is a very serious\nlimitation. If anything, the documentation for remote.pushurl should\nbe updated to explain that it has a very narrow usecase (will do in a\nfuture patch).\n"},{"id":"211950","messageId":"7v7gkz8nyf.fsf@alter.siamese.dyndns.org","threadId":"33239","inReplyTo":"26A2BE0824FE4C999592D392FCBEF988@PhilipOakley","subject":"Re: [PATCH v2 0/6] Support triangular workflows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-22T15:16:08Z","receivedAt":"2013-03-22T15:16:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" writes:\n\n> Shouldn't Documentation/gitworkflows.txt also be updated with the\n> triangular workflow and its configuration?\n\nWhat is missing from gitworkflows documentation is actually a\nnon-triangular workflow, where people pull from and push into the\nsame central repository.\n\nThe \"merge workflow\" part in the \"distributed workflows\" section\nteaches:\n\n * fetching others' work from and merging it to yours;\n * publishing your work to ; and\n * advertising your work.\n\nand it is written for the triangular workflow.\n\nTwo triangles are involved. The maintainer may pull from you but\npushes to her own, which is one triangle, and you pull from the\nmaintainer and push to your own, which is another.\n\nAs to your suggestion, I do think it is reasonable to clarify these\ntriangles with an illustration, and to even add descriptions for\nshort-cut configurations as a side note.\n"},{"id":"212048","messageId":"CALkWK0=fOxuWvXKL4fQ9cd0Rs2S71_9GB=eP8PXbwk_+Ke8wtQ@mail.gmail.com","threadId":"33239","inReplyTo":"7v7gkz8nyf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 0/6] Support triangular workflows","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-23T12:42:38Z","receivedAt":"2013-03-23T12:42:38Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> \"Philip Oakley\" writes:\n>\n>> Shouldn't Documentation/gitworkflows.txt also be updated with the\n>> triangular workflow and its configuration?\n>\n> What is missing from gitworkflows documentation is actually a\n> non-triangular workflow, where people pull from and push into the\n> same central repository.\n>\n> The \"merge workflow\" part in the \"distributed workflows\" section\n> teaches:\n>\n> * fetching others' work from and merging it to yours;\n> * publishing your work to ; and\n> * advertising your work.\n>\n> and it is written for the triangular workflow.\n>\n> Two triangles are involved. The maintainer may pull from you but\n> pushes to her own, which is one triangle, and you pull from the\n> maintainer and push to your own, which is another.\n>\n> As to your suggestion, I do think it is reasonable to clarify these\n> triangles with an illustration, and to even add descriptions for\n> short-cut configurations as a side note.\n\nI think the gitworkflows document should be rewritten with focus on\nsetting up remotes and configuration variables for specific workflows.\n Once these are set up, pushing/ pulling (git pull is currently\nbroken, although I'm working on fixing it) should Just Work. To push\nto a different remote/ refspec, the user can read the manpage of git\npush/ git pull. A workflow is about setting up good defaults that\nwork 90% of the time with no additional effort.\n"}]}