{"thread":{"id":"30961","subject":"[PATCH 1/6] Rename remote.c's default_remote_name static variables.","startedAt":"2012-07-05T22:11:11Z","lastAt":"2012-07-06T21:49:11Z","messageCount":18,"participants":["marcnarc@xiplink.com","Junio C Hamano","Phil Hord","Marc Branchaud"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"194673","messageId":"1341526277-17055-1-git-send-email-marcnarc@xiplink.com","threadId":"30961","inReplyTo":null,"subject":"[PATCH 0/6] Default remote","fromName":"","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-07-05T22:11:11Z","receivedAt":"2012-07-05T22:11:11Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"This series is a follow-up to an earlier patch and discussion[1] that\nsuggested adding a remote.default setting.\n\nThe first patch simply renames a couple of variables in remote.c so that\nthe second patch is easier to understand.\n\nThe second patch teaches remote.c to look for remote.default in the config:\n\n - When deciding which remote to use, the code first tries the currently\n   checked-out branch's remote.  If there isn't one it tries remote.default,\n   and if that also fails it falls back to \"origin\".\n\n - The patch also adds a couple of helper functions for other code to use:\n   remote_get_default_name() and remote_count().  remote_get_default_name()\n   returns the value of remote.default, or \"origin\" if it's not configured\n   (this preserves existing behavior).  remote_count() is used in patch\n   four by \"git remote add\".\n\nThe third patch teaches \"git clone\" to set remote.default.\n\nThe fourth patch teaches \"git remote\" about remote.default.  In addition to\nmodifying the existing \"add\" \"rm\" and \"rename\" commands, it also adds a new\n\"git remote default\" command to get/set remote.default.  The advantage of this\nover just using \"git config\" directly is twofold:\n\n - \"git remote default foo\" checks that foo is a configured remote.\n\n - \"git remote default\" (with no parameter) returns \"origin\" if \n   remote.default isn't configured.\n\nNote that this patch changes the way push.default=matching works when the\ncurrently checked-out branch has no remote.  Before \"git push\" would error out\nwith \"No configured push destination\" but now it succeeds with \"Everything\nup-to-date\".  I personally think this is a good thing.\n\nThe fifth patch just adds a test that plain \"git fetch\" respects\nremote.default when on remoteless branch.  I suppose it could be squashed into\npatch four, but it's really more of a side-effect of that work and not\nstrictly required by it.  So I felt patch four would be more understandable\nwithout it.\n\nThe sixth patch changes git-parse-remote.sh:get_default_remote() to use\n\"git remote default\" instead of implementing its own default-finding logic.\nBecause \"git remote default\" returns \"origin\" when no remote.default is\nconfigured, this change preserves the old behavior in existing repos.  The\ntest accompanying this patch essentially tests the same condition that\ninspired the original discussion[1].  (git-submodule is pretty much the only\nthing that uses get_default_remote().)\n\nThis series still needs documentation updates, which I'll do if/when we\nagree on the code changes.\n\n\t\tM.\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/200145\n\nMarc Branchaud (6):\n      Rename remote.c's default_remote_name static variables.\n      Teach remote.c about the remote.default configuration setting.\n      Teach clone to set remote.default.\n      Teach \"git remote\" about remote.default.\n      Test that plain \"git fetch\" uses remote.default when on a detached HEAD.\n      Teach get_default_remote to respect remote.default.\n\n builtin/clone.c            |  2 ++\n builtin/remote.c           | 29 +++++++++++++++++++++\n git-parse-remote.sh        |  5 +---\n remote.c                   | 35 ++++++++++++++++++++-----\n remote.h                   |  2 ++\n t/t5505-remote.sh          | 64 ++++++++++++++++++++++++++++++++++++++++++++++\n t/t5510-fetch.sh           | 17 ++++++++++++\n t/t5512-ls-remote.sh       |  8 +++++-\n t/t5528-push-default.sh    |  4 +--\n t/t5601-clone.sh           | 10 ++++++++\n t/t5702-clone-options.sh   |  7 +++--\n t/t7400-submodule-basic.sh | 21 +++++++++++++++\n 12 files changed, 188 insertions(+), 16 deletions(-)\n"},{"id":"194671","messageId":"1341526277-17055-2-git-send-email-marcnarc@xiplink.com","threadId":"30961","inReplyTo":"1341526277-17055-1-git-send-email-marcnarc@xiplink.com","subject":"[PATCH 1/6] Rename remote.c's default_remote_name static variables.","fromName":"","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-07-05T22:11:12Z","receivedAt":"2012-07-05T22:11:12Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"From: Marc Branchaud <marcnarc@xiplink.com>\n\nThis prepares the code to handle a true remote.default configuration value.\n\ndefault_remote_name --> effective_remote_name\nexplicit_default_remote_name --> explicit_effective_remote_name\n\nSigned-off-by: Marc Branchaud <marcnarc@xiplink.com>\n---\n remote.c | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 6833538..6f371e0 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -47,8 +47,8 @@ static int branches_alloc;\n static int branches_nr;\n \n static struct branch *current_branch;\n-static const char *default_remote_name;\n-static int explicit_default_remote_name;\n+static const char *effective_remote_name;\n+static int explicit_effective_remote_name;\n \n static struct rewrites rewrites;\n static struct rewrites rewrites_push;\n@@ -360,8 +360,8 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t\t\t\treturn config_error_nonbool(key);\n \t\t\tbranch->remote_name = xstrdup(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+\t\t\t\teffective_remote_name = branch->remote_name;\n+\t\t\t\texplicit_effective_remote_name = 1;\n \t\t\t}\n \t\t} else if (!strcmp(subkey, \".merge\")) {\n \t\t\tif (!value)\n@@ -481,9 +481,9 @@ static void read_config(void)\n \tunsigned char sha1[20];\n \tconst char *head_ref;\n \tint flag;\n-\tif (default_remote_name) /* did this already */\n+\tif (effective_remote_name) /* did this already */\n \t\treturn;\n-\tdefault_remote_name = xstrdup(\"origin\");\n+\teffective_remote_name = xstrdup(\"origin\");\n \tcurrent_branch = NULL;\n \thead_ref = resolve_ref_unsafe(\"HEAD\", sha1, 0, &flag);\n \tif (head_ref && (flag & REF_ISSYMREF) &&\n@@ -680,8 +680,8 @@ struct remote *remote_get(const char *name)\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\tname = effective_remote_name;\n+\t\tname_given = explicit_effective_remote_name;\n \t}\n \n \tret = make_remote(name, 0);\n-- \n1.7.11.1\n"},{"id":"194674","messageId":"1341526277-17055-3-git-send-email-marcnarc@xiplink.com","threadId":"30961","inReplyTo":"1341526277-17055-1-git-send-email-marcnarc@xiplink.com","subject":"[PATCH 2/6] Teach remote.c about the remote.default configuration setting.","fromName":"","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-07-05T22:11:13Z","receivedAt":"2012-07-05T22:11:13Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"From: Marc Branchaud <marcnarc@xiplink.com>\n\nThe code now has a default_remote_name and an effective_remote_name:\n - default_remote_name is set by remote.default in the config, or is \"origin\"\n   if remote.default doesn't exist (\"origin\" was the fallback value before\n   this change).\n - effective_remote_name is the name of the remote tracked by the current\n   branch, or is default_remote_name if the current branch doesn't have a\n   remote.\n\nAlso add two new helper functions for other code modules to use:\n - remote_get_default_name() returns default_remote_name.\n - remote_count() returns the number of remotes.\n\nSigned-off-by: Marc Branchaud <marcnarc@xiplink.com>\n---\n remote.c | 27 ++++++++++++++++++++++++---\n remote.h |  2 ++\n 2 files changed, 26 insertions(+), 3 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 6f371e0..ccada0d 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -47,6 +47,7 @@ static int branches_alloc;\n static int branches_nr;\n \n static struct branch *current_branch;\n+static const char *default_remote_name;\n static const char *effective_remote_name;\n static int explicit_effective_remote_name;\n \n@@ -390,6 +391,7 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t}\n \tif (prefixcmp(key,  \"remote.\"))\n \t\treturn 0;\n+\n \tname = key + 7;\n \tif (*name == '/') {\n \t\twarning(\"Config remote shorthand cannot begin with '/': %s\",\n@@ -397,8 +399,12 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \tsubkey = strrchr(name, '.');\n-\tif (!subkey)\n+\tif (!subkey) {\n+\t\t/* Look for remote.default */\n+\t\tif (!strcmp(name, \"default\"))\n+\t\t\tdefault_remote_name = xstrdup(value);\n \t\treturn 0;\n+\t}\n \tremote = make_remote(name, subkey - name);\n \tremote->origin = REMOTE_CONFIG;\n \tif (!strcmp(subkey, \".mirror\"))\n@@ -481,9 +487,8 @@ static void read_config(void)\n \tunsigned char sha1[20];\n \tconst char *head_ref;\n \tint flag;\n-\tif (effective_remote_name) /* did this already */\n+\tif (default_remote_name) /* did this already */\n \t\treturn;\n-\teffective_remote_name = xstrdup(\"origin\");\n \tcurrent_branch = NULL;\n \thead_ref = resolve_ref_unsafe(\"HEAD\", sha1, 0, &flag);\n \tif (head_ref && (flag & REF_ISSYMREF) &&\n@@ -492,6 +497,10 @@ static void read_config(void)\n \t\t\tmake_branch(head_ref + strlen(\"refs/heads/\"), 0);\n \t}\n \tgit_config(handle_config, NULL);\n+\tif (!default_remote_name)\n+\t\tdefault_remote_name = \"origin\";\n+\tif (!effective_remote_name)\n+\t\teffective_remote_name = default_remote_name;\n \talias_all_urls();\n }\n \n@@ -671,6 +680,18 @@ static int valid_remote_nick(const char *name)\n \treturn !strchr(name, '/'); /* no slash */\n }\n \n+const char *remote_get_default_name()\n+{\n+\tread_config();\n+\treturn default_remote_name;\n+}\n+\n+int remote_count()\n+{\n+\tread_config();\n+\treturn remotes_nr;\n+}\n+\n struct remote *remote_get(const char *name)\n {\n \tstruct remote *ret;\ndiff --git a/remote.h b/remote.h\nindex 251d8fd..f9aac87 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -52,6 +52,8 @@ struct remote {\n \n struct remote *remote_get(const char *name);\n int remote_is_configured(const char *name);\n+const char *remote_get_default_name();\n+int remote_count();\n \n typedef int each_remote_fn(struct remote *remote, void *priv);\n int for_each_remote(each_remote_fn fn, void *priv);\n-- \n1.7.11.1\n"},{"id":"194675","messageId":"1341526277-17055-4-git-send-email-marcnarc@xiplink.com","threadId":"30961","inReplyTo":"1341526277-17055-1-git-send-email-marcnarc@xiplink.com","subject":"[PATCH 3/6] Teach clone to set remote.default.","fromName":"","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-07-05T22:11:14Z","receivedAt":"2012-07-05T22:11:14Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"From: Marc Branchaud <marcnarc@xiplink.com>\n\nSigned-off-by: Marc Branchaud <marcnarc@xiplink.com>\n---\n builtin/clone.c          |  2 ++\n t/t5601-clone.sh         | 10 ++++++++++\n t/t5702-clone-options.sh |  7 +++++--\n 3 files changed, 17 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex a4d8d25..b198456 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -770,6 +770,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tgit_config_set(key.buf, repo);\n \tstrbuf_reset(&key);\n \n+\tgit_config_set(\"remote.default\", option_origin);\n+\n \tif (option_reference.nr)\n \t\tsetup_reference();\n \ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex 67869b4..046610d 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -57,6 +57,16 @@ test_expect_success 'clone checks out files' '\n \n '\n \n+test_expect_success 'clone sets remote.default' '\n+\n+\trm -fr dst &&\n+\tgit clone src dst &&\n+\t(\n+\t\tcd dst &&\n+\t\ttest \"$(git config --get remote.default)\" = origin\n+\t)\n+'\n+\n test_expect_success 'clone respects GIT_WORK_TREE' '\n \n \tGIT_WORK_TREE=worktree git clone src bare &&\ndiff --git a/t/t5702-clone-options.sh b/t/t5702-clone-options.sh\nindex 02cb024..9e573dd 100755\n--- a/t/t5702-clone-options.sh\n+++ b/t/t5702-clone-options.sh\n@@ -15,8 +15,11 @@ test_expect_success 'setup' '\n test_expect_success 'clone -o' '\n \n \tgit clone -o foo parent clone-o &&\n-\t(cd clone-o && git rev-parse --verify refs/remotes/foo/master)\n-\n+\t(\n+\t\tcd clone-o &&\n+\t\tgit rev-parse --verify refs/remotes/foo/master &&\n+\t\ttest \"$(git config --get remote.default)\" = foo\n+\t)\n '\n \n test_expect_success 'redirected clone' '\n-- \n1.7.11.1\n"},{"id":"194676","messageId":"1341526277-17055-5-git-send-email-marcnarc@xiplink.com","threadId":"30961","inReplyTo":"1341526277-17055-1-git-send-email-marcnarc@xiplink.com","subject":"[PATCH 4/6] Teach \"git remote\" about remote.default.","fromName":"","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-07-05T22:11:15Z","receivedAt":"2012-07-05T22:11:15Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"From: Marc Branchaud <marcnarc@xiplink.com>\n\nThe \"rename\" and \"rm\" commands now handle the case where the remote being\nchanged is the default remote.\n\nIf the \"add\" command is used to add the repo's first remote, that remote\nbecomes the default remote.\n\nAlso added a \"default\" command to get or set the default remote:\n \"git remote default\" (with no parameter) displays the current default remote.\n \"git remote default <remo>\" checks if <remo> is a configured remote and if\n so changes remote.default to <remo>.\n\n(The test in t5528 had to be changed because now \"git push\" when\npush.default=matching succeeds with \"Everything up-to-date\".  It used to\nfail with \"No configured push destination\".)\n\nSigned-off-by: Marc Branchaud <marcnarc@xiplink.com>\n---\n builtin/remote.c        | 29 ++++++++++++++++++++++\n t/t5505-remote.sh       | 64 +++++++++++++++++++++++++++++++++++++++++++++++++\n t/t5512-ls-remote.sh    |  8 ++++++-\n t/t5528-push-default.sh |  4 ++--\n 4 files changed, 102 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 920262d..ac98765 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -12,6 +12,7 @@ static const char * const builtin_remote_usage[] = {\n \t\"git remote add [-t <branch>] [-m <master>] [-f] [--tags|--no-tags] [--mirror=<fetch|push>] <name> <url>\",\n \t\"git remote rename <old> <new>\",\n \t\"git remote rm <name>\",\n+\t\"git remote default [name]\",\n \t\"git remote set-head <name> (-a | -d | <branch>)\",\n \t\"git remote [-v | --verbose] show [-n] <name>\",\n \t\"git remote prune [-n | --dry-run] <name>\",\n@@ -198,6 +199,9 @@ static int add(int argc, const char **argv)\n \tif (!valid_fetch_refspec(buf2.buf))\n \t\tdie(_(\"'%s' is not a valid remote name\"), name);\n \n+\tif (remote_count() == 1)\n+\t\tgit_config_set(\"remote.default\", name);\n+\n \tstrbuf_addf(&buf, \"remote.%s.url\", name);\n \tif (git_config_set(buf.buf, url))\n \t\treturn 1;\n@@ -656,6 +660,10 @@ static int mv(int argc, const char **argv)\n \t\treturn error(_(\"Could not rename config section '%s' to '%s'\"),\n \t\t\t\tbuf.buf, buf2.buf);\n \n+\tif (!strcmp(oldremote->name, remote_get_default_name())) {\n+\t\tgit_config_set(\"remote.default\", newremote->name);\n+\t}\n+\n \tstrbuf_reset(&buf);\n \tstrbuf_addf(&buf, \"remote.%s.fetch\", rename.new);\n \tif (git_config_set_multivar(buf.buf, NULL, NULL, 1))\n@@ -798,6 +806,10 @@ static int rm(int argc, const char **argv)\n \tif (git_config_rename_section(buf.buf, NULL) < 1)\n \t\treturn error(_(\"Could not remove config section '%s'\"), buf.buf);\n \n+\tif (!strcmp(remote->name, remote_get_default_name())) {\n+\t\tgit_config_set(\"remote.default\", NULL);\n+\t}\n+\n \tread_branches();\n \tfor (i = 0; i < branch_list.nr; i++) {\n \t\tstruct string_list_item *item = branch_list.items + i;\n@@ -845,6 +857,21 @@ static int rm(int argc, const char **argv)\n \treturn result;\n }\n \n+static int deflt(int argc, const char **argv)\n+{\n+\tif (argc < 2)\n+\t\tprintf_ln(\"%s\", remote_get_default_name());\n+\telse {\n+\t\tconst char *name = argv[1];\n+\t\tif (remote_is_configured(name)) {\n+\t\t\tgit_config_set(\"remote.default\", name);\n+\t\t\tprintf_ln(_(\"Default remote set to '%s'.\"), name);\n+\t\t} else\n+\t\t\treturn error(_(\"No remote named '%s'.\"), name);\n+\t}\n+\treturn 0;\n+}\n+\n static void clear_push_info(void *util, const char *string)\n {\n \tstruct push_info *info = util;\n@@ -1582,6 +1609,8 @@ int cmd_remote(int argc, const char **argv, const char *prefix)\n \t\tresult = mv(argc, argv);\n \telse if (!strcmp(argv[0], \"rm\"))\n \t\tresult = rm(argc, argv);\n+\telse if (!strcmp(argv[0], \"default\"))\n+\t\tresult = deflt(argc, argv);\n \telse if (!strcmp(argv[0], \"set-head\"))\n \t\tresult = set_head(argc, argv);\n \telse if (!strcmp(argv[0], \"set-branches\"))\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex e8af615..d9070cd 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -77,6 +77,15 @@ test_expect_success 'add another remote' '\n )\n '\n \n+test_expect_success 'add first remote sets default' '\n+(\n+\ttest_create_repo default-first-remote &&\n+\tcd default-first-remote &&\n+\tgit remote add first /path/to/first &&\n+\ttest \"$(git config --get remote.default)\" = first\n+)\n+'\n+\n test_expect_success 'remote forces tracking branches' '\n (\n \tcd test &&\n@@ -107,6 +116,16 @@ test_expect_success 'remove remote' '\n )\n '\n \n+test_expect_success 'remove default remote' '\n+(\n+\tgit clone one no-default\n+\tcd no-default &&\n+\ttest \"$(git config --get remote.default)\" = origin   # Sanity check\n+\tgit remote rm origin &&\n+\ttest_must_fail git config --get remote.default\n+)\n+'\n+\n test_expect_success 'remove remote protects local branches' '\n (\n \tcd test &&\n@@ -662,6 +681,16 @@ test_expect_success 'rename a remote with name prefix of other remote' '\n \n '\n \n+test_expect_success 'rename the default remote' '\n+\tgit clone one default-rename &&\n+\t(cd default-rename &&\n+\t\ttest \"$(git config --get remote.default)\" = origin   # Sanity?\n+\t\tgit remote rename origin tyrion\n+\t\ttest \"$(git config --get remote.default)\" = tyrion\n+\t)\n+\n+'\n+\n cat > remotes_origin << EOF\n URL: $(pwd)/one\n Push: refs/heads/master:refs/heads/upstream\n@@ -997,4 +1026,39 @@ test_expect_success 'remote set-url --delete baz' '\n \tcmp expect actual\n '\n \n+test_expect_success 'remote default' '\n+(\n+\tgit clone -o up one default\n+\tcd default &&\n+\ttest \"$(git remote default)\" = up\n+)\n+'\n+\n+test_expect_success 'remote default (none)' '\n+(\n+\ttest_create_repo default-none &&\n+\tcd default-none &&\n+\ttest \"$(git remote default)\" = origin\n+)\n+'\n+\n+test_expect_success 'remote default set' '\n+(\n+\ttest_create_repo default-set &&\n+\tcd default-set &&\n+\tgit remote add A /foo-A &&\n+\tgit remote add B /foo-A &&\n+\tgit remote default B &&\n+\ttest \"$(git remote default)\" = B\n+)\n+'\n+\n+test_expect_success 'remote default bad' '\n+(\n+\tgit clone one default-bad &&\n+\tcd default-bad &&\n+\ttest_must_fail git remote default NonExistant\n+)\n+'\n+\n test_done\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex 6764d51..853b045 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -40,7 +40,13 @@ test_expect_success 'ls-remote self' '\n '\n \n test_expect_success 'dies when no remote specified and no default remotes found' '\n-\ttest_must_fail git ls-remote\n+\ttest_create_repo no-remotes &&\n+\t(\n+\t\tcd no-remotes &&\n+\t\ttest_must_fail git ls-remote &&\n+\t\ttest_config remote.default blop &&\n+\t\ttest_must_fail git ls-remote\n+\t)\n '\n \n test_expect_success 'use \"origin\" when no remote specified' '\ndiff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh\nindex 4736da8..6a08998 100755\n--- a/t/t5528-push-default.sh\n+++ b/t/t5528-push-default.sh\n@@ -71,10 +71,10 @@ test_expect_success '\"upstream\" does not push when remotes do not match' '\n \ttest_must_fail git push parent2\n '\n \n-test_expect_success 'push from/to new branch with upstream, matching and simple' '\n+test_expect_success 'push from/to new remoteless branch with upstream, matching and simple' '\n \tgit checkout -b new-branch &&\n \ttest_push_failure simple &&\n-\ttest_push_failure matching &&\n+\ttest_push_success matching master &&\n \ttest_push_failure upstream\n '\n \n-- \n1.7.11.1\n"},{"id":"194677","messageId":"1341526277-17055-6-git-send-email-marcnarc@xiplink.com","threadId":"30961","inReplyTo":"1341526277-17055-1-git-send-email-marcnarc@xiplink.com","subject":"[PATCH 5/6] Test that plain \"git fetch\" uses remote.default when on a detached HEAD.","fromName":"","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-07-05T22:11:16Z","receivedAt":"2012-07-05T22:11:16Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"From: Marc Branchaud <marcnarc@xiplink.com>\n\nSigned-off-by: Marc Branchaud <marcnarc@xiplink.com>\n---\n t/t5510-fetch.sh | 17 +++++++++++++++++\n 1 file changed, 17 insertions(+)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex d7a19a1..8ecd996 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -69,6 +69,23 @@ test_expect_success \"fetch test\" '\n \ttest \"z$mine\" = \"z$his\"\n '\n \n+test_expect_success \"fetch test detached HEAD uses default remote\" '\n+\tcd \"$D\" &&\n+\tgit clone -o foo . default-remote &&\n+\tgit checkout -b detached &&\n+\ttest_commit update1 &&\n+\t(\n+\t\tcd default-remote &&\n+\t\tgit checkout HEAD@{0} &&\n+\t\tgit fetch &&\n+\t\ttest -f .git/refs/remotes/foo/detached &&\n+\t\tmine=`git rev-parse refs/remotes/foo/detached` &&\n+\t\this=`cd .. && git rev-parse refs/heads/detached` &&\n+\t\ttest \"z$mine\" = \"z$his\"\n+\t) &&\n+\tgit checkout master\n+'\n+\n test_expect_success \"fetch test for-merge\" '\n \tcd \"$D\" &&\n \tcd three &&\n-- \n1.7.11.1\n"},{"id":"194672","messageId":"1341526277-17055-7-git-send-email-marcnarc@xiplink.com","threadId":"30961","inReplyTo":"1341526277-17055-1-git-send-email-marcnarc@xiplink.com","subject":"[PATCH 6/6] Teach get_default_remote to respect remote.default.","fromName":"","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-07-05T22:11:17Z","receivedAt":"2012-07-05T22:11:17Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"From: Marc Branchaud <marcnarc@xiplink.com>\n\nUse \"git remote default\" instead of replicating its logic.\n\nThe unit test checks a relative-path submodule because the submodule code\nis (almost) the only thing that uses get_default_remote.\n\nSigned-off-by: Marc Branchaud <marcnarc@xiplink.com>\n---\n git-parse-remote.sh        |  5 +----\n t/t7400-submodule-basic.sh | 21 +++++++++++++++++++++\n 2 files changed, 22 insertions(+), 4 deletions(-)\n\ndiff --git a/git-parse-remote.sh b/git-parse-remote.sh\nindex 484b2e6..49257f0 100644\n--- a/git-parse-remote.sh\n+++ b/git-parse-remote.sh\n@@ -5,10 +5,7 @@\n GIT_DIR=$(git rev-parse -q --git-dir) || :;\n \n get_default_remote () {\n-\tcurr_branch=$(git symbolic-ref -q HEAD)\n-\tcurr_branch=\"${curr_branch#refs/heads/}\"\n-\torigin=$(git config --get \"branch.$curr_branch.remote\")\n-\techo ${origin:-origin}\n+\techo $(git remote default)\n }\n \n get_remote_merge_branch () {\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 81827e6..2ac2ffc 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -507,6 +507,27 @@ test_expect_success 'relative path works with user@host:path' '\n \t)\n '\n \n+test_expect_success 'realtive path works when superproject on detached HEAD' '\n+\t(\n+\t\ttest_create_repo detach &&\n+\t\tcd detach &&\n+\t\ttest_create_repo sub &&\n+\t\t(\n+\t\t\tcd sub &&\n+\t\t\ttest_commit foo\n+\t\t) &&\n+\t\tgit add sub &&\n+\t\tgit commit -m \"added sub\" &&\n+\t\trm -rf sub &&\n+\t\tgit checkout HEAD@{0} &&\n+\t\tgit config -f .gitmodules submodule.sub.path sub &&\n+\t\tgit config -f .gitmodules submodule.sub.url ../subrepo &&\n+\t\tgit remote add awkward /path/to/awkward\n+\t\tgit submodule init sub &&\n+\t\ttest \"$(git config submodule.sub.url)\" = /path/to/subrepo\n+\t)\n+'\n+\n test_expect_success 'moving the superproject does not break submodules' '\n \t(\n \t\tcd addtest &&\n-- \n1.7.11.1\n"},{"id":"194681","messageId":"7v4nplrfe4.fsf@alter.siamese.dyndns.org","threadId":"30961","inReplyTo":"1341526277-17055-3-git-send-email-marcnarc@xiplink.com","subject":"Re: [PATCH 2/6] Teach remote.c about the remote.default configuration setting.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-05T22:50:59Z","receivedAt":"2012-07-05T22:50:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"marcnarc@xiplink.com writes:\n\n> From: Marc Branchaud <marcnarc@xiplink.com>\n>\n> The code now has a default_remote_name and an effective_remote_name:\n>  - default_remote_name is set by remote.default in the config, or is \"origin\"\n>    if remote.default doesn't exist (\"origin\" was the fallback value before\n>    this change).\n\n\n>  - effective_remote_name is the name of the remote tracked by the current\n>    branch, or is default_remote_name if the current branch doesn't have a\n>    remote.\n\nThe explanation of the latter belongs to the previous step, I think.\nI am not sure if \"effective\" is the best name for the concept the\nabove explains, though.\n\n> @@ -390,6 +391,7 @@ static int handle_config(const char *key, const char *value, void *cb)\n>  \t}\n>  \tif (prefixcmp(key,  \"remote.\"))\n>  \t\treturn 0;\n> +\n\nWhy?\n\n>  \tname = key + 7;\n> @@ -671,6 +680,18 @@ static int valid_remote_nick(const char *name)\n>  \treturn !strchr(name, '/'); /* no slash */\n>  }\n>  \n> +const char *remote_get_default_name()\n\nconst char *remote_get_default_name(void)\n\n> +{\n> +\tread_config();\n> +\treturn default_remote_name;\n> +}\n\nHrmph.  I am too lazy to read outside the context of your patch to\nmake sure, but isn't the root cause of the problem that when we try\nto find which remote the current branch is configured to interact\nwith, we grab branch->remote_name (and this is done by calling\ngit_config() to open and read the configuration file once already)\nand if it is empty we default to \"origin\"?  Wouldn't the callback\nfunction that is used for that invocation of git_config() a much\nbetter place to set \"default_remote_name\" variable, instead of\nhaving us to read the entire configuration file one more time only\nto get the value of this variable?\n\n> +int remote_count()\n\nint remote_count(void)\n\n> +{\n> +\tread_config();\n> +\treturn remotes_nr;\n> +}\n\nLikewise.  Especially it is unclear who benefits from the function\nuntil a new caller is introduced.  I would prefer not to see the\naddition of this function in this patch.\n\n>  struct remote *remote_get(const char *name)\n>  {\n>  \tstruct remote *ret;\n> diff --git a/remote.h b/remote.h\n> index 251d8fd..f9aac87 100644\n> --- a/remote.h\n> +++ b/remote.h\n> @@ -52,6 +52,8 @@ struct remote {\n>  \n>  struct remote *remote_get(const char *name);\n>  int remote_is_configured(const char *name);\n> +const char *remote_get_default_name();\n> +int remote_count();\n\nconst char *remote_get_default_name(void);\nint remote_count(void);\n"},{"id":"194682","messageId":"7vzk7dq0qk.fsf@alter.siamese.dyndns.org","threadId":"30961","inReplyTo":"1341526277-17055-4-git-send-email-marcnarc@xiplink.com","subject":"Re: [PATCH 3/6] Teach clone to set remote.default.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-05T22:52:51Z","receivedAt":"2012-07-05T22:52:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"marcnarc@xiplink.com writes:\n\n> From: Marc Branchaud <marcnarc@xiplink.com>\n>\n> Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>\n> ---\n>  builtin/clone.c          |  2 ++\n>  t/t5601-clone.sh         | 10 ++++++++++\n>  t/t5702-clone-options.sh |  7 +++++--\n>  3 files changed, 17 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index a4d8d25..b198456 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -770,6 +770,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n>  \tgit_config_set(key.buf, repo);\n>  \tstrbuf_reset(&key);\n>  \n> +\tgit_config_set(\"remote.default\", option_origin);\n> +\n\nIs this something we would want to do unconditionally?  If so why?\n\nOr is this what we want to do only when the \"--origin name\" option\nis used?\n"},{"id":"194719","messageId":"CABURp0oYfzKrkKOZJrH2hrYMTPbFe_i5mMKQ3HnWQdGZa=oujw@mail.gmail.com","threadId":"30961","inReplyTo":"1341526277-17055-5-git-send-email-marcnarc@xiplink.com","subject":"Re: [PATCH 4/6] Teach \"git remote\" about remote.default.","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2012-07-06T12:51:35Z","receivedAt":"2012-07-06T12:51:35Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Thu, Jul 5, 2012 at 6:11 PM,  <marcnarc@xiplink.com> wrote:\n> From: Marc Branchaud <marcnarc@xiplink.com>\n>\n> The \"rename\" and \"rm\" commands now handle the case where the remote being\n> changed is the default remote.\n\nI think this is the right thing to do.  But I noticed a subtle\nbehavior change that we may wish to consider.\n\nToday I might do this (contrived example):\n\ngit checkout somelocalbranch\ngit push # pushes to \"origin\" by default\ngit remote rename origin origin1\ngit add origin ssh://new-server/foo\ngit push  # pushes to \"origin\" by default\n\nBut after this change, the last command is different.  Now it pushes\nto \"origin1\" because the rename set the remote.default to \"origin1\",\neven though it was previously not set at all.  It did this because the\n\"oldname\" is compared to the \"remote_get_default_name()\", which\nreturns \"origin\" by default.  So the old setting, which did not exist,\nis now \"renamed\" to have an actual value, and the actual value is not\n\"origin\".\n\nOne can easily contrive an alternative example showing that this is a\ngood thing.  As I said, I think it is the right thing to do.\n\nBut it is different, I think.\n\nI doubt many script writers are counting on default settings to carry\nthe day, so they are probably more explicit about how they push.  But\nI didn't see this mentioned in the patch.\n\nPhil\n"},{"id":"194720","messageId":"4FF6F805.20403@xiplink.com","threadId":"30961","inReplyTo":"7v4nplrfe4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/6] Teach remote.c about the remote.default configuration setting.","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-07-06T14:36:53Z","receivedAt":"2012-07-06T14:36:53Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"On 12-07-05 06:50 PM, Junio C Hamano wrote:\n> marcnarc@xiplink.com writes:\n> \n>> From: Marc Branchaud <marcnarc@xiplink.com>\n>>\n>> The code now has a default_remote_name and an effective_remote_name:\n>>  - default_remote_name is set by remote.default in the config, or is \"origin\"\n>>    if remote.default doesn't exist (\"origin\" was the fallback value before\n>>    this change).\n> \n> \n>>  - effective_remote_name is the name of the remote tracked by the current\n>>    branch, or is default_remote_name if the current branch doesn't have a\n>>    remote.\n> \n> The explanation of the latter belongs to the previous step, I think.\n> I am not sure if \"effective\" is the best name for the concept the\n> above explains, though.\n\nWell, the previous commit removes default_remote_name, so the explanation\nwouldn't be valid verbatim.\n\nHow about keeping the above here, and I could add the following to the\nprevious commit's message:\n\n\teffective_remote_name is the remote name that is currently \"in\n\teffect\".  This is the currently checked-out branch's remote, or\n\t\"origin\" if the branch has no remote (or the working tree is a\n\tdetached HEAD).\n\n>> @@ -390,6 +391,7 @@ static int handle_config(const char *key, const char *value, void *cb)\n>>  \t}\n>>  \tif (prefixcmp(key,  \"remote.\"))\n>>  \t\treturn 0;\n>> +\n> \n> Why?\n\nOops, just a newline I neglected to clean up from some earlier hacking.  Sorry.\n\n>>  \tname = key + 7;\n>> @@ -671,6 +680,18 @@ static int valid_remote_nick(const char *name)\n>>  \treturn !strchr(name, '/'); /* no slash */\n>>  }\n>>  \n>> +const char *remote_get_default_name()\n> \n> const char *remote_get_default_name(void)\n\nOK.\n\n>> +{\n>> +\tread_config();\n>> +\treturn default_remote_name;\n>> +}\n> \n> Hrmph.  I am too lazy to read outside the context of your patch to\n> make sure, but isn't the root cause of the problem that when we try\n> to find which remote the current branch is configured to interact\n> with, we grab branch->remote_name (and this is done by calling\n> git_config() to open and read the configuration file once already)\n> and if it is empty we default to \"origin\"?  Wouldn't the callback\n> function that is used for that invocation of git_config() a much\n> better place to set \"default_remote_name\" variable, instead of\n> having us to read the entire configuration file one more time only\n> to get the value of this variable?\n\nThe read_config() function already has logic to avoid re-parsing the entire\nconfig over and over again.  There are many places in remote.c that call\nread_config(), and I thought I was just following that pattern.\n\nAlso, making the code be\n\n\tif (!default_remote_name)\n\t\tread_config();\n\treturn default_remote_name;\n\nwould just replicate the check that read_config() already does.\n\n>> +int remote_count()\n> \n> int remote_count(void)\n\nOK.\n\n>> +{\n>> +\tread_config();\n>> +\treturn remotes_nr;\n>> +}\n> \n> Likewise.\n\nDoing something like\n\n\tif (!remotes_nr)\n\t\tread_config():\n\treturn remotes_nr;\n\nwould be wrong, since having 0 remotes is perfectly fine.  And making the\ncheck here be\n\tif (!default_remote_name)\nseems confusing, and again it duplicates the check read_config() already does.\n\n> Especially it is unclear who benefits from the function\n> until a new caller is introduced.  I would prefer not to see the\n> addition of this function in this patch.\n\nOK, I can move it to the \"git remote\" patch.  The same could be said of\nremote_get_default_name() though.\n\n>>  struct remote *remote_get(const char *name)\n>>  {\n>>  \tstruct remote *ret;\n>> diff --git a/remote.h b/remote.h\n>> index 251d8fd..f9aac87 100644\n>> --- a/remote.h\n>> +++ b/remote.h\n>> @@ -52,6 +52,8 @@ struct remote {\n>>  \n>>  struct remote *remote_get(const char *name);\n>>  int remote_is_configured(const char *name);\n>> +const char *remote_get_default_name();\n>> +int remote_count();\n> \n> const char *remote_get_default_name(void);\n> int remote_count(void);\n\nGot it.\n\nI'll make these changes in a few days, after folks have had a chance to\nreview.  If things settle down I'll re-roll with the documentation updates too.\n\n\t\tM.\n"},{"id":"194721","messageId":"4FF6F811.7000808@xiplink.com","threadId":"30961","inReplyTo":"7vzk7dq0qk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/6] Teach clone to set remote.default.","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-07-06T14:37:05Z","receivedAt":"2012-07-06T14:37:05Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"On 12-07-05 06:52 PM, Junio C Hamano wrote:\n> marcnarc@xiplink.com writes:\n> \n>> From: Marc Branchaud <marcnarc@xiplink.com>\n>>\n>> Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>\n>> ---\n>>  builtin/clone.c          |  2 ++\n>>  t/t5601-clone.sh         | 10 ++++++++++\n>>  t/t5702-clone-options.sh |  7 +++++--\n>>  3 files changed, 17 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/builtin/clone.c b/builtin/clone.c\n>> index a4d8d25..b198456 100644\n>> --- a/builtin/clone.c\n>> +++ b/builtin/clone.c\n>> @@ -770,6 +770,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n>>  \tgit_config_set(key.buf, repo);\n>>  \tstrbuf_reset(&key);\n>>  \n>> +\tgit_config_set(\"remote.default\", option_origin);\n>> +\n> \n> Is this something we would want to do unconditionally?  If so why?\n\nI think so, yes.\n\n> Or is this what we want to do only when the \"--origin name\" option\n> is used?\n\nIf remote.default isn't set, then if someone does\n\t\tgit remote rename origin foo\nthe default remote will still be \"origin\" (modulo the currently-checked-out\nbranch stuff).\n\n\t\tM.\n"},{"id":"194722","messageId":"4FF6F996.8080205@xiplink.com","threadId":"30961","inReplyTo":"CABURp0oYfzKrkKOZJrH2hrYMTPbFe_i5mMKQ3HnWQdGZa=oujw@mail.gmail.com","subject":"Re: [PATCH 4/6] Teach \"git remote\" about remote.default.","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-07-06T14:43:34Z","receivedAt":"2012-07-06T14:43:34Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"On 12-07-06 08:51 AM, Phil Hord wrote:\n> On Thu, Jul 5, 2012 at 6:11 PM,  <marcnarc@xiplink.com> wrote:\n>> From: Marc Branchaud <marcnarc@xiplink.com>\n>>\n>> The \"rename\" and \"rm\" commands now handle the case where the remote being\n>> changed is the default remote.\n> \n> I think this is the right thing to do.  But I noticed a subtle\n> behavior change that we may wish to consider.\n> \n> Today I might do this (contrived example):\n> \n> git checkout somelocalbranch\n> git push # pushes to \"origin\" by default\n> git remote rename origin origin1\n> git add origin ssh://new-server/foo\n> git push  # pushes to \"origin\" by default\n> \n> But after this change, the last command is different.  Now it pushes\n> to \"origin1\" because the rename set the remote.default to \"origin1\",\n> even though it was previously not set at all.  It did this because the\n> \"oldname\" is compared to the \"remote_get_default_name()\", which\n> returns \"origin\" by default.  So the old setting, which did not exist,\n> is now \"renamed\" to have an actual value, and the actual value is not\n> \"origin\".\n> \n> One can easily contrive an alternative example showing that this is a\n> good thing.  As I said, I think it is the right thing to do.\n> \n> But it is different, I think.\n\nYes, I agree it is different.\n\n> I doubt many script writers are counting on default settings to carry\n> the day, so they are probably more explicit about how they push.  But\n> I didn't see this mentioned in the patch.\n\nI think this sort of thing is better suited to the documentation of\nremote.default.  I'm planning to re-roll this series with documentation\nupdates, and I'll include your example.\n\n\t\tM.\n"},{"id":"194749","messageId":"7vhatkofe6.fsf@alter.siamese.dyndns.org","threadId":"30961","inReplyTo":"4FF6F805.20403@xiplink.com","subject":"Re: [PATCH 2/6] Teach remote.c about the remote.default configuration setting.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-06T19:31:29Z","receivedAt":"2012-07-06T19:31:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Marc Branchaud <marcnarc@xiplink.com> writes:\n\n> On 12-07-05 06:50 PM, Junio C Hamano wrote:\n>> \n>>>  - effective_remote_name is the name of the remote tracked by the current\n>>>    branch, or is default_remote_name if the current branch doesn't have a\n>>>    remote.\n>> \n>> The explanation of the latter belongs to the previous step, I think.\n>> I am not sure if \"effective\" is the best name for the concept the\n>> above explains, though.\n>\n> Well, the previous commit removes default_remote_name, so the explanation\n> wouldn't be valid verbatim.\n\nThe previous one introduces \"effective\" (which I still think is not\nthe best word for the semantics you are trying to give to the\nvariable) without explaining what the variable is for and justifying\nwhy \"effective\" is the right word (or at least a better than\n\"default\") for it.  Something like the \"- effective_remote_name is the ...\"\nabove is necessary in its commit log message.\n\n> How about keeping the above here, and I could add the following to the\n> previous commit's message:\n>\n> \teffective_remote_name is the remote name that is currently \"in\n> \teffect\".  This is the currently checked-out branch's remote, or\n> \t\"origin\" if the branch has no remote (or the working tree is a\n> \tdetached HEAD).\n\nYeah, along that line.\n\n> The read_config() function already has logic to avoid re-parsing the entire\n> config over and over again.  There are many places in remote.c that call\n> read_config(), and I thought I was just following that pattern.\n\nOK.\n"},{"id":"194750","messageId":"7vd348of0z.fsf@alter.siamese.dyndns.org","threadId":"30961","inReplyTo":"4FF6F811.7000808@xiplink.com","subject":"Re: [PATCH 3/6] Teach clone to set remote.default.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-06T19:39:24Z","receivedAt":"2012-07-06T19:39:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Marc Branchaud <marcnarc@xiplink.com> writes:\n\n> If remote.default isn't set, then if someone does\n> \t\tgit remote rename origin foo\n> the default remote will still be \"origin\" (modulo the currently-checked-out\n> branch stuff).\n\nWhy?  I thought the proposed semantics was \"if remote.default is\nunset, the default value of 'origin' is used where remote.default\nwould have been used _everywhere_\".  If \"remote rename\" wants to\nupdate the value of remote.default from 'origin' to 'foo' (which may\nor may not be the right thing to do, for which a separate discussion\nseems to exist already), and if it sees that the repository does not\nhave remote.default, shouldn't it still set it to 'foo', just like\nthe case where remote.default exists and set to 'origin'?\n\nYour updated \"remote rename\" must work correctly in a repository\nthat was created long ago, where remote.default was not set to\nanything (and default 'origin' was used) after all.\n\nOr am I missing some subtle issues?\n"},{"id":"194752","messageId":"4FF7433F.606@xiplink.com","threadId":"30961","inReplyTo":"7vhatkofe6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/6] Teach remote.c about the remote.default configuration setting.","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-07-06T19:57:51Z","receivedAt":"2012-07-06T19:57:51Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"On 12-07-06 03:31 PM, Junio C Hamano wrote:\n> Marc Branchaud <marcnarc@xiplink.com> writes:\n> \n>> On 12-07-05 06:50 PM, Junio C Hamano wrote:\n>>>\n>>>>  - effective_remote_name is the name of the remote tracked by the current\n>>>>    branch, or is default_remote_name if the current branch doesn't have a\n>>>>    remote.\n>>>\n>>> The explanation of the latter belongs to the previous step, I think.\n>>> I am not sure if \"effective\" is the best name for the concept the\n>>> above explains, though.\n>>\n>> Well, the previous commit removes default_remote_name, so the explanation\n>> wouldn't be valid verbatim.\n> \n> The previous one introduces \"effective\" (which I still think is not\n> the best word for the semantics you are trying to give to the\n> variable)\n\nI'm open to suggestions.\n\n\t\tM.\n"},{"id":"194758","messageId":"4FF74DD4.1060800@xiplink.com","threadId":"30961","inReplyTo":"7vd348of0z.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/6] Teach clone to set remote.default.","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-07-06T20:43:00Z","receivedAt":"2012-07-06T20:43:00Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"On 12-07-06 03:39 PM, Junio C Hamano wrote:\n> Marc Branchaud <marcnarc@xiplink.com> writes:\n> \n>> If remote.default isn't set, then if someone does\n>> \t\tgit remote rename origin foo\n>> the default remote will still be \"origin\" (modulo the currently-checked-out\n>> branch stuff).\n> \n> Why?\n\nErm, actually, my statement is incorrect.  Doh!\n\n> I thought the proposed semantics was \"if remote.default is\n> unset, the default value of 'origin' is used where remote.default\n> would have been used _everywhere_\".\n\nYes, true.\n\n> If \"remote rename\" wants to\n> update the value of remote.default from 'origin' to 'foo' (which may\n> or may not be the right thing to do, for which a separate discussion\n> seems to exist already),\n\nAre you talking about the sub-thread Phil Hord & I spawned about patch #4?  I\nthink Phil & I are in agreement there that it is the right thing to do.  If\nanyone disagrees please speak up!\n\n> and if it sees that the repository does not\n> have remote.default, shouldn't it still set it to 'foo', just like\n> the case where remote.default exists and set to 'origin'?\n\nThe proposed code actually already does that.  I'll add a unit test for this\ncase.\n\nSo why change \"git clone\" to always set remote.default if the functionality\nremains the same either way?\n\nTo me it makes a more consistent implementation.  Since \"git remote add\" sets\nremote.default if it's adding the first remote to the repository, when clone\nitself adds the first remote it should do the same.\n\nPlus this approach makes \"clone -o\" also work without any special-casing, so\nthe code is cleaner, IMHO.\n\nIf this justification is adequate, I'll add it to the commit message.  It may\nthen make more sense to have this commit come after the \"git remote\" changes\nin the series.\n\n> Your updated \"remote rename\" must work correctly in a repository\n> that was created long ago, where remote.default was not set to\n> anything (and default 'origin' was used) after all.\n> \n> Or am I missing some subtle issues?\n\nI agree with that requirement, and believe the proposed code fulfils it.\n\n\t\tM.\n"},{"id":"194760","messageId":"4FF75D57.7010603@xiplink.com","threadId":"30961","inReplyTo":"4FF74DD4.1060800@xiplink.com","subject":"Re: [PATCH 3/6] Teach clone to set remote.default.","fromName":"Marc Branchaud","fromEmail":"mbranchaud@xiplink.com","sentAt":"2012-07-06T21:49:11Z","receivedAt":"2012-07-06T21:49:11Z","isPatch":true,"sender":{"key":"mbranchaud@xiplink.com","avatar":null},"body":"On 12-07-06 04:43 PM, Marc Branchaud wrote:\n> \n> So why change \"git clone\" to always set remote.default if the functionality\n> remains the same either way?\n> \n> To me it makes a more consistent implementation.  Since \"git remote add\" sets\n> remote.default if it's adding the first remote to the repository, when clone\n> itself adds the first remote it should do the same.\n> \n> Plus this approach makes \"clone -o\" also work without any special-casing, so\n> the code is cleaner, IMHO.\n\nAlso, it means that\n\n\tgit clone /some/repo\n\nand\n\n\tgit clone -o origin /some/repo\n\nproduce exactly the same result.\n\n\t\tM.\n"}]}