{"thread":{"id":"49524","subject":"[PATCH 0/2] add an advice on unqualified push","startedAt":"2018-10-10T10:41:59Z","lastAt":"2018-11-14T07:00:56Z","messageCount":53,"participants":["Ævar Arnfjörð Bjarmason","Jeff King","Junio C Hamano","Stefan Beller"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"359997","messageId":"20181010104145.25610-1-avarab@gmail.com","threadId":"49524","inReplyTo":null,"subject":"[PATCH 0/2] add an advice on unqualified push","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-10T10:41:43Z","receivedAt":"2018-10-10T10:41:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Years ago I accidentally deleted the \"master\" branch at work (due to\ngit push origin $emptyvar:master), and while I could tell from the\nreflogs what SHA-1 I needed on the other side ran into the fairly\ncryptic error message, certainly to me when the adrenaline is flowing\nand you've just ruined something in production.\n\nSo this series makes that error message better, and suggests to the\nuser how they can fix the situation, first I needed to mark some\nstrings in remote.c for i18n.\n\nIt would be better if we could just give the user a full command to\ncopy/paste, i.e. what the ran but with refs/{heads,tags}/,\nbut between passing the remote name down, and handling any push\noptions I think it's better for now just to suggest the refspec.\n\nÆvar Arnfjörð Bjarmason (2):\n i18n: remote.c: mark error(...) messages for translation\n push: add an advice on unqualified push\n\n Documentation/config.txt | 7 ++++\n advice.c | 2 +\n advice.h | 1 +\n remote.c | 86 +++++++++++++++++++++++++++++++---------\n t/t5505-remote.sh | 25 ++++++++++++\n 5 files changed, 102 insertions(+), 19 deletions(-)\n\n-- \n2.19.1.390.gf3a00b506f\n\n"},{"id":"359998","messageId":"20181010104145.25610-2-avarab@gmail.com","threadId":"49524","inReplyTo":"20181010104145.25610-1-avarab@gmail.com","subject":"[PATCH 1/2] i18n: remote.c: mark error(...) messages for translation","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-10T10:41:44Z","receivedAt":"2018-10-10T10:42:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Mark up the error(...) messages in remote.c for translation. The likes\nof \"unable to push to unqualified destination\" are relatively big\nparts of the UI, i.e. error messages shown when \"git push\" fails.\n\nI don't think any of these are plumbing, an the entire test suite\npasses when running the tests with GIT_GETTEXT_POISON=1 (after\nbuilding with GETTEXT_POISON).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n remote.c | 34 +++++++++++++++++-----------------\n 1 file changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 682f2a01f9..cc5553acc2 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -406,7 +406,7 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t\tif (!remote->receivepack)\n \t\t\tremote->receivepack = v;\n \t\telse\n-\t\t\terror(\"more than one receivepack given, using the first\");\n+\t\t\terror(_(\"more than one receivepack given, using the first\"));\n \t} else if (!strcmp(subkey, \"uploadpack\")) {\n \t\tconst char *v;\n \t\tif (git_config_string(&v, key, value))\n@@ -414,7 +414,7 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t\tif (!remote->uploadpack)\n \t\t\tremote->uploadpack = v;\n \t\telse\n-\t\t\terror(\"more than one uploadpack given, using the first\");\n+\t\t\terror(_(\"more than one uploadpack given, using the first\"));\n \t} else if (!strcmp(subkey, \"tagopt\")) {\n \t\tif (!strcmp(value, \"--no-tags\"))\n \t\t\tremote->fetch_tags = -1;\n@@ -707,7 +707,7 @@ static void query_refspecs_multiple(struct refspec *rs,\n \tint find_src = !query->src;\n \n \tif (find_src && !query->dst)\n-\t\terror(\"query_refspecs_multiple: need either src or dst\");\n+\t\terror(_(\"query_refspecs_multiple: need either src or dst\"));\n \n \tfor (i = 0; i < rs->nr; i++) {\n \t\tstruct refspec_item *refspec = &rs->items[i];\n@@ -735,7 +735,7 @@ int query_refspecs(struct refspec *rs, struct refspec_item *query)\n \tchar **result = find_src ? &query->src : &query->dst;\n \n \tif (find_src && !query->dst)\n-\t\treturn error(\"query_refspecs: need either src or dst\");\n+\t\treturn error(_(\"query_refspecs: need either src or dst\"));\n \n \tfor (i = 0; i < rs->nr; i++) {\n \t\tstruct refspec_item *refspec = &rs->items[i];\n@@ -995,12 +995,12 @@ static int match_explicit_lhs(struct ref *src,\n \t\t * way to delete 'other' ref at the remote end.\n \t\t */\n \t\tif (try_explicit_object_name(rs->src, match) < 0)\n-\t\t\treturn error(\"src refspec %s does not match any.\", rs->src);\n+\t\t\treturn error(_(\"src refspec %s does not match any.\"), rs->src);\n \t\tif (allocated_match)\n \t\t\t*allocated_match = 1;\n \t\treturn 0;\n \tdefault:\n-\t\treturn error(\"src refspec %s matches more than one.\", rs->src);\n+\t\treturn error(_(\"src refspec %s matches more than one.\"), rs->src);\n \t}\n }\n \n@@ -1041,30 +1041,30 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \t\tif (starts_with(dst_value, \"refs/\"))\n \t\t\tmatched_dst = make_linked_ref(dst_value, dst_tail);\n \t\telse if (is_null_oid(&matched_src->new_oid))\n-\t\t\terror(\"unable to delete '%s': remote ref does not exist\",\n+\t\t\terror(_(\"unable to delete '%s': remote ref does not exist\"),\n \t\t\t dst_value);\n \t\telse if ((dst_guess = guess_ref(dst_value, matched_src))) {\n \t\t\tmatched_dst = make_linked_ref(dst_guess, dst_tail);\n \t\t\tfree(dst_guess);\n \t\t} else\n-\t\t\terror(\"unable to push to unqualified destination: %s\\n\"\n-\t\t\t \"The destination refspec neither matches an \"\n-\t\t\t \"existing ref on the remote nor\\n\"\n-\t\t\t \"begins with refs/, and we are unable to \"\n-\t\t\t \"guess a prefix based on the source ref.\",\n+\t\t\terror(_(\"unable to push to unqualified destination: %s\\n\"\n+\t\t\t\t\"The destination refspec neither matches an \"\n+\t\t\t\t\"existing ref on the remote nor\\n\"\n+\t\t\t\t\"begins with refs/, and we are unable to \"\n+\t\t\t\t\"guess a prefix based on the source ref.\"),\n \t\t\t dst_value);\n \t\tbreak;\n \tdefault:\n \t\tmatched_dst = NULL;\n-\t\terror(\"dst refspec %s matches more than one.\",\n+\t\terror(_(\"dst refspec %s matches more than one.\"),\n \t\t dst_value);\n \t\tbreak;\n \t}\n \tif (!matched_dst)\n \t\treturn -1;\n \tif (matched_dst->peer_ref)\n-\t\treturn error(\"dst ref %s receives from more than one src.\",\n-\t\t matched_dst->name);\n+\t\treturn error(_(\"dst ref %s receives from more than one src.\"),\n+\t\t\t matched_dst->name);\n \telse {\n \t\tmatched_dst->peer_ref = allocated_src ?\n \t\t\t\t\tmatched_src :\n@@ -1763,7 +1763,7 @@ int get_fetch_map(const struct ref *remote_refs,\n \t\t\tif (!starts_with((*rmp)->peer_ref->name, \"refs/\") ||\n \t\t\t check_refname_format((*rmp)->peer_ref->name, 0)) {\n \t\t\t\tstruct ref *ignore = *rmp;\n-\t\t\t\terror(\"* Ignoring funny ref '%s' locally\",\n+\t\t\t\terror(_(\"* Ignoring funny ref '%s' locally\"),\n \t\t\t\t (*rmp)->peer_ref->name);\n \t\t\t\t*rmp = (*rmp)->next;\n \t\t\t\tfree(ignore->peer_ref);\n@@ -2131,7 +2131,7 @@ static int parse_push_cas_option(struct push_cas_option *cas, const char *arg, i\n \telse if (!colon[1])\n \t\toidclr(&entry->expect);\n \telse if (get_oid(colon + 1, &entry->expect))\n-\t\treturn error(\"cannot parse expected object name '%s'\", colon + 1);\n+\t\treturn error(_(\"cannot parse expected object name '%s'\"), colon + 1);\n \treturn 0;\n }\n \n-- \n2.19.1.390.gf3a00b506f\n\n"},{"id":"359999","messageId":"20181010104145.25610-3-avarab@gmail.com","threadId":"49524","inReplyTo":"20181010104145.25610-1-avarab@gmail.com","subject":"[PATCH 2/2] push: add an advice on unqualified push","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-10T10:41:45Z","receivedAt":"2018-10-10T10:42:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Improve the error message added in f8aae12034 (\"push: allow\nunqualified dest refspecs to DWIM\", 2008-04-23), which before this\nchange looks like this:\n\n $ git push avar v2.19.0^{commit}:newbranch -n\n error: unable to push to unqualified destination: newbranch\n The destination refspec neither matches an existing ref on the remote nor\n begins with refs/, and we are unable to guess a prefix based on the source ref.\n error: failed to push some refs to 'git@github.com:avar/git.git'\n\nThis message needed to be read very carefully to spot how to fix the\nerror, i.e. to push to refs/heads/newbranch, and it didn't use the\nadvice system (since initial addition of the error predated it).\n\nFix both of those, now the message will look like this instead:\n\n $ ./git-push avar v2.19.0^{commit}:newbranch -n\n error: unable to push to unqualified destination: newbranch\n hint: The destination refspec neither matches an existing\n hint: ref on the remote nor begins with refs/, and we are\n hint: unable to guess a prefix based on the source ref.\n hint:\n hint: The part of the refspec is a commit object.\n hint: Did you mean to create a new branch by pushing to\n hint: 'v2.19.0^{commit}:refs/heads/newbranch'?\n error: failed to push some refs to 'git@github.com:avar/git.git'\n\nWhen trying to push a tag, tree or a blob we suggest that perhaps the\nuser meant to push them to refs/tags/ instead.\n\nThe if/else duplication for all of OBJ_{COMMIT,TAG,TREE,BLOB} is\nunfortunate, but is required to correctly mark the messages for\ntranslation.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/config.txt | 7 +++++\n advice.c | 2 ++\n advice.h | 1 +\n remote.c | 62 +++++++++++++++++++++++++++++++++++-----\n t/t5505-remote.sh | 25 ++++++++++++++++\n 5 files changed, 90 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 1546833213..fd455e2739 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -320,6 +320,13 @@ advice.*::\n \t\ttries to overwrite a remote ref that points at an\n \t\tobject that is not a commit-ish, or make the remote\n \t\tref point at an object that is not a commit-ish.\n+\tpushAmbigiousRefName::\n+\t\tShown when linkgit:git-push[1] gives up trying to\n+\t\tguess based on the source and destination refs what\n+\t\tremote ref namespace the source belongs in, but where\n+\t\twe can still suggest that the user push to either\n+\t\trefs/heads/* or refs/tags/* based on the type of the\n+\t\tsource object.\n \tstatusHints::\n \t\tShow directions on how to proceed from the current\n \t\tstate in the output of linkgit:git-status[1], in\ndiff --git a/advice.c b/advice.c\nindex 3561cd64e9..84e9d0168d 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -9,6 +9,7 @@ int advice_push_non_ff_matching = 1;\n int advice_push_already_exists = 1;\n int advice_push_fetch_first = 1;\n int advice_push_needs_force = 1;\n+int advice_push_ambiguous_ref_name = 1;\n int advice_status_hints = 1;\n int advice_status_u_option = 1;\n int advice_commit_before_merge = 1;\n@@ -62,6 +63,7 @@ static struct {\n \t{ \"pushAlreadyExists\", &advice_push_already_exists },\n \t{ \"pushFetchFirst\", &advice_push_fetch_first },\n \t{ \"pushNeedsForce\", &advice_push_needs_force },\n+\t{ \"pushAmbigiousRefName\", &advice_push_ambiguous_ref_name },\n \t{ \"statusHints\", &advice_status_hints },\n \t{ \"statusUoption\", &advice_status_u_option },\n \t{ \"commitBeforeMerge\", &advice_commit_before_merge },\ndiff --git a/advice.h b/advice.h\nindex ab24df0fd0..d2445cab8b 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -9,6 +9,7 @@ extern int advice_push_non_ff_matching;\n extern int advice_push_already_exists;\n extern int advice_push_fetch_first;\n extern int advice_push_needs_force;\n+extern int advice_push_ambiguous_ref_name;\n extern int advice_status_hints;\n extern int advice_status_u_option;\n extern int advice_commit_before_merge;\ndiff --git a/remote.c b/remote.c\nindex cc5553acc2..78fa2d9aff 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -13,6 +13,7 @@\n #include \"mergesort.h\"\n #include \"argv-array.h\"\n #include \"commit-reach.h\"\n+#include \"advice.h\"\n \n enum map_direction { FROM_SRC, FROM_DST };\n \n@@ -1046,13 +1047,60 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \t\telse if ((dst_guess = guess_ref(dst_value, matched_src))) {\n \t\t\tmatched_dst = make_linked_ref(dst_guess, dst_tail);\n \t\t\tfree(dst_guess);\n-\t\t} else\n-\t\t\terror(_(\"unable to push to unqualified destination: %s\\n\"\n-\t\t\t\t\"The destination refspec neither matches an \"\n-\t\t\t\t\"existing ref on the remote nor\\n\"\n-\t\t\t\t\"begins with refs/, and we are unable to \"\n-\t\t\t\t\"guess a prefix based on the source ref.\"),\n-\t\t\t dst_value);\n+\t\t} else {\n+\t\t\tstruct object_id oid;\n+\t\t\tenum object_type type;\n+\n+\t\t\terror(\"unable to push to unqualified destination: %s\", dst_value);\n+\t\t\tif (!advice_push_ambiguous_ref_name)\n+\t\t\t\tbreak;\n+\t\t\tif (get_oid(matched_src->name, &oid))\n+\t\t\t\tBUG(\"'%s' is not a valid object, \"\n+\t\t\t\t \"match_explicit_lhs() should catch this!\",\n+\t\t\t\t matched_src->name);\n+\t\t\ttype = oid_object_info(the_repository, &oid, NULL);\n+\t\t\tif (type == OBJ_COMMIT) {\n+\n+\t\t\t\tadvise(_(\"The destination refspec neither matches an existing\\n\"\n+\t\t\t\t\t \"ref on the remote nor begins with refs/, and we are\\n\"\n+\t\t\t\t\t \"unable to guess a prefix based on the source ref.\\n\"\n+\t\t\t\t\t \"\\n\"\n+\t\t\t\t\t \"The part of the refspec is a commit object.\\n\"\n+\t\t\t\t\t \"Did you mean to create a new branch by pushing to\\n\"\n+\t\t\t\t\t \"'%s:refs/heads/%s'?\"),\n+\t\t\t\t matched_src->name, dst_value);\n+\t\t\t} else if (type == OBJ_TAG) {\n+\t\t\t\tadvise(_(\"The destination refspec neither matches an existing\\n\"\n+\t\t\t\t\t \"ref on the remote nor begins with refs/, and we are\\n\"\n+\t\t\t\t\t \"unable to guess a prefix based on the source ref.\\n\"\n+\t\t\t\t\t \"\\n\"\n+\t\t\t\t\t \"The part of the refspec is a tag object.\\n\"\n+\t\t\t\t\t \"Did you mean to create a new tag by pushing to\\n\"\n+\t\t\t\t\t \"'%s:refs/tags/%s'?\"),\n+\t\t\t\t matched_src->name, dst_value);\n+\t\t\t} else if (type == OBJ_TREE) {\n+\t\t\t\tadvise(_(\"The destination refspec neither matches an existing\\n\"\n+\t\t\t\t\t \"ref on the remote nor begins with refs/, and we are\\n\"\n+\t\t\t\t\t \"unable to guess a prefix based on the source ref.\\n\"\n+\t\t\t\t\t \"\\n\"\n+\t\t\t\t\t \"The part of the refspec is a tree object.\\n\"\n+\t\t\t\t\t \"Did you mean to tag a new tree by pushing to\\n\"\n+\t\t\t\t\t \"'%s:refs/tags/%s'?\"),\n+\t\t\t\t matched_src->name, dst_value);\n+\t\t\t} else if (type == OBJ_BLOB) {\n+\t\t\t\tadvise(_(\"The destination refspec neither matches an existing\\n\"\n+\t\t\t\t\t \"ref on the remote nor begins with refs/, and we are\\n\"\n+\t\t\t\t\t \"unable to guess a prefix based on the source ref.\\n\"\n+\t\t\t\t\t \"\\n\"\n+\t\t\t\t\t \"The part of the refspec is a blob object.\\n\"\n+\t\t\t\t\t \"Did you mean to tag a new blob by pushing to\\n\"\n+\t\t\t\t\t \"'%s:refs/tags/%s'?\"),\n+\t\t\t\t matched_src->name, dst_value);\n+\t\t\t} else {\n+\t\t\t\tBUG(\"'%s' should be commit/tag/tree/blob, is '%d'\",\n+\t\t\t\t matched_src->name, type);\n+\t\t\t}\n+\t\t}\n \t\tbreak;\n \tdefault:\n \t\tmatched_dst = NULL;\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex d2a2cdd453..1eabf06aa4 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -1222,4 +1222,29 @@ test_expect_success 'add remote matching the \"insteadOf\" URL' '\n \tgit remote add backup xyz@example.com\n '\n \n+test_expect_success 'unqualified refspec DWIM and advice' '\n+\ttest_when_finished \"(cd test && git tag -d some-tag)\" &&\n+\t(\n+\t\tcd test &&\n+\t\tgit tag -a -m \"Some tag\" some-tag master &&\n+\t\tfor type in commit tag tree blob\n+\t\tdo\n+\t\t\tif test \"$type\" = \"blob\"\n+\t\t\tthen\n+\t\t\t\toid=$(git rev-parse some-tag:file)\n+\t\t\telse\n+\t\t\t\toid=$(git rev-parse some-tag^{$type})\n+\t\t\tfi &&\n+\t\t\ttest_must_fail git push origin $oid:dst -n 2>err &&\n+\t\t\ttest_i18ngrep \"error: unable to push\" err &&\n+\t\t\ttest_i18ngrep \"hint: Did you mean\" err &&\n+\t\t\ttest_must_fail git -c advice.pushAmbigiousRefName=false \\\n+\t\t\t\tpush origin $oid:dst -n 2>err &&\n+\t\t\ttest_i18ngrep \"error: unable to push\" err &&\n+\t\t\ttest_i18ngrep ! \"hint: Did you mean\" err\n+\t\tdone\n+\t)\n+'\n+\n+\n test_done\n-- \n2.19.1.390.gf3a00b506f\n\n"},{"id":"360068","messageId":"20181010205505.GB12949@sigill.intra.peff.net","threadId":"49524","inReplyTo":"20181010104145.25610-3-avarab@gmail.com","subject":"Re: [PATCH 2/2] push: add an advice on unqualified push","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-10T20:55:06Z","receivedAt":"2018-10-10T20:55:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 10, 2018 at 10:41:45AM +0000, Ævar Arnfjörð Bjarmason wrote:\n\n> Improve the error message added in f8aae12034 (\"push: allow\n> unqualified dest refspecs to DWIM\", 2008-04-23), which before this\n> change looks like this:\n> \n> $ git push avar v2.19.0^{commit}:newbranch -n\n> error: unable to push to unqualified destination: newbranch\n> The destination refspec neither matches an existing ref on the remote nor\n> begins with refs/, and we are unable to guess a prefix based on the source ref.\n> error: failed to push some refs to 'git@github.com:avar/git.git'\n\nThanks for looking into this. Despite being largely responsible for that\nmessage myself, I always cringe when I see it because it's so opaque.\n\n> This message needed to be read very carefully to spot how to fix the\n> error, i.e. to push to refs/heads/newbranch, and it didn't use the\n> advice system (since initial addition of the error predated it).\n> \n> Fix both of those, now the message will look like this instead:\n> \n> $ ./git-push avar v2.19.0^{commit}:newbranch -n\n> error: unable to push to unqualified destination: newbranch\n> hint: The destination refspec neither matches an existing\n> hint: ref on the remote nor begins with refs/, and we are\n> hint: unable to guess a prefix based on the source ref.\n> hint:\n> hint: The part of the refspec is a commit object.\n> hint: Did you mean to create a new branch by pushing to\n> hint: 'v2.19.0^{commit}:refs/heads/newbranch'?\n> error: failed to push some refs to 'git@github.com:avar/git.git'\n> \n> When trying to push a tag, tree or a blob we suggest that perhaps the\n> user meant to push them to refs/tags/ instead.\n\nThis is much better, and I love the customized behavior based on the\nobject type.\n\nI wonder if we could reword the first paragraph to be a little less\nconfusing, and spell out what we tried already. E.g., something like:\n\n The destination you provided is not a full refname (i.e., starting\n with \"ref\"). Git tried to guess what you meant by:\n\n - looking for a matching branch or tag on the remote side\n\n - looking at the refname of the local source\n\n but neither worked.\n\n The part of the refspec is a commit object.\n Did you mean...\n\nI'm not sure about saying \"branch or tag\" in the first bullet. It's\nfriendlier to most users, but less technically correct (if you said\n\"notes/foo\", I believe we'd match an existing \"refs/notes/foo\", because\nit's really just using the normal lookup rules).\n\nAlso, as an aside, I wonder if we should allow \"heads/foo\" to work as\n\"refs/heads/foo\" (even when no such ref already exists). But that is\ntotally orthogonal to changing the message.\n\n> The if/else duplication for all of OBJ_{COMMIT,TAG,TREE,BLOB} is\n> unfortunate, but is required to correctly mark the messages for\n> translation.\n\nI think it would probably be OK to put the first paragraph in its own\nvariable. I know we try to avoid translation lego, but I'd think\nparagraphs are separate units. Or are you worried about how to get them\ninto the same advise() call? I don't know that we need to, but we could\nalso plug one into the other with a \"%s\" (and leave a translator note).\n\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 1546833213..fd455e2739 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -320,6 +320,13 @@ advice.*::\n> \t\ttries to overwrite a remote ref that points at an\n> \t\tobject that is not a commit-ish, or make the remote\n> \t\tref point at an object that is not a commit-ish.\n> +\tpushAmbigiousRefName::\n> +\t\tShown when linkgit:git-push[1] gives up trying to\n> +\t\tguess based on the source and destination refs what\n> +\t\tremote ref namespace the source belongs in, but where\n> +\t\twe can still suggest that the user push to either\n> +\t\trefs/heads/* or refs/tags/* based on the type of the\n> +\t\tsource object.\n\nI guess you could argue that this is \"ambiguous\", but usually we'd use\nthat term to mean \"there were two branches that matched on the other\nside\". But here it's actually \"no branches matched\" (actually, it makes\nme wonder what we'd do pushing \"foo\" when that name is ambiguous on the\nother side).\n\nSo I wonder if this ought to be pushUnqualifiedRefname or something.\n\n> @@ -1046,13 +1047,60 @@ static int match_explicit(struct ref *src, struct ref *dst,\n> \t\telse if ((dst_guess = guess_ref(dst_value, matched_src))) {\n> \t\t\tmatched_dst = make_linked_ref(dst_guess, dst_tail);\n> \t\t\tfree(dst_guess);\n> -\t\t} else\n> -\t\t\terror(_(\"unable to push to unqualified destination: %s\\n\"\n> -\t\t\t\t\"The destination refspec neither matches an \"\n> -\t\t\t\t\"existing ref on the remote nor\\n\"\n> -\t\t\t\t\"begins with refs/, and we are unable to \"\n> -\t\t\t\t\"guess a prefix based on the source ref.\"),\n> -\t\t\t dst_value);\n> +\t\t} else {\n> +\t\t\tstruct object_id oid;\n> +\t\t\tenum object_type type;\n> +\n> +\t\t\terror(\"unable to push to unqualified destination: %s\", dst_value);\n> +\t\t\tif (!advice_push_ambiguous_ref_name)\n> +\t\t\t\tbreak;\n\nThis break feels funny, because it controls flow much larger than this\nif/else. It does the right thing now, but it must remain in sync with\nwhat comes at the end of that long string of advise() messages.\n\nCan we just do it as:\n\n if (advice_push_ambiguous_ref_name) {\n\tstruct object_id oid;\n\tenum object_type type;\n\n\tif (get_oid(...))\n\t etc...\n }\n\ninstead? That pushes your indentation one level in, but I think the\nwhole conditional body might be cleaner in a helper function anyway.\n\n-Peff\n"},{"id":"360069","messageId":"20181010205542.GC12949@sigill.intra.peff.net","threadId":"49524","inReplyTo":"20181010104145.25610-2-avarab@gmail.com","subject":"Re: [PATCH 1/2] i18n: remote.c: mark error(...) messages for translation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-10T20:55:42Z","receivedAt":"2018-10-10T20:55:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 10, 2018 at 10:41:44AM +0000, Ævar Arnfjörð Bjarmason wrote:\n\n> Mark up the error(...) messages in remote.c for translation. The likes\n> of \"unable to push to unqualified destination\" are relatively big\n> parts of the UI, i.e. error messages shown when \"git push\" fails.\n> \n> I don't think any of these are plumbing, an the entire test suite\n> passes when running the tests with GIT_GETTEXT_POISON=1 (after\n> building with GETTEXT_POISON).\n\nSo obviously the second patch is much more interesting, and I focused\nmost of my comments there. ;)\n\nBut this one seems like an obvious improvement.\n\n-Peff\n"},{"id":"360075","messageId":"87r2gxebsi.fsf@evledraar.gmail.com","threadId":"49524","inReplyTo":"20181010205505.GB12949@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] push: add an advice on unqualified push","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-10T21:23:25Z","receivedAt":"2018-10-10T21:23:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 10 2018, Jeff King wrote:\n\n> On Wed, Oct 10, 2018 at 10:41:45AM +0000, Ævar Arnfjörð Bjarmason wrote:\n>\n>> Improve the error message added in f8aae12034 (\"push: allow\n>> unqualified dest refspecs to DWIM\", 2008-04-23), which before this\n>> change looks like this:\n>>\n>> $ git push avar v2.19.0^{commit}:newbranch -n\n>> error: unable to push to unqualified destination: newbranch\n>> The destination refspec neither matches an existing ref on the remote nor\n>> begins with refs/, and we are unable to guess a prefix based on the source ref.\n>> error: failed to push some refs to 'git@github.com:avar/git.git'\n>\n> Thanks for looking into this. Despite being largely responsible for that\n> message myself, I always cringe when I see it because it's so opaque.\n>\n>> This message needed to be read very carefully to spot how to fix the\n>> error, i.e. to push to refs/heads/newbranch, and it didn't use the\n>> advice system (since initial addition of the error predated it).\n>>\n>> Fix both of those, now the message will look like this instead:\n>>\n>> $ ./git-push avar v2.19.0^{commit}:newbranch -n\n>> error: unable to push to unqualified destination: newbranch\n>> hint: The destination refspec neither matches an existing\n>> hint: ref on the remote nor begins with refs/, and we are\n>> hint: unable to guess a prefix based on the source ref.\n>> hint:\n>> hint: The part of the refspec is a commit object.\n>> hint: Did you mean to create a new branch by pushing to\n>> hint: 'v2.19.0^{commit}:refs/heads/newbranch'?\n>> error: failed to push some refs to 'git@github.com:avar/git.git'\n>>\n>> When trying to push a tag, tree or a blob we suggest that perhaps the\n>> user meant to push them to refs/tags/ instead.\n>\n> This is much better, and I love the customized behavior based on the\n> object type.\n>\n> I wonder if we could reword the first paragraph to be a little less\n> confusing, and spell out what we tried already. E.g., something like:\n>\n> The destination you provided is not a full refname (i.e., starting\n> with \"ref\"). Git tried to guess what you meant by:\n>\n> - looking for a matching branch or tag on the remote side\n>\n> - looking at the refname of the local source\n>\n> but neither worked.\n>\n> The part of the refspec is a commit object.\n> Did you mean...\n\nYeah that makes sense. I was trying to avoid touching the existing\nwording to make this more surgical, but you came up with it, and since\nyou don't like it I'll just change that too.\n\n> I'm not sure about saying \"branch or tag\" in the first bullet. It's\n> friendlier to most users, but less technically correct (if you said\n> \"notes/foo\", I believe we'd match an existing \"refs/notes/foo\", because\n> it's really just using the normal lookup rules).\n>\n> Also, as an aside, I wonder if we should allow \"heads/foo\" to work as\n> \"refs/heads/foo\" (even when no such ref already exists). But that is\n> totally orthogonal to changing the message.\n>\n>> The if/else duplication for all of OBJ_{COMMIT,TAG,TREE,BLOB} is\n>> unfortunate, but is required to correctly mark the messages for\n>> translation.\n>\n> I think it would probably be OK to put the first paragraph in its own\n> variable. I know we try to avoid translation lego, but I'd think\n> paragraphs are separate units. Or are you worried about how to get them\n> into the same advise() call? I don't know that we need to, but we could\n> also plug one into the other with a \"%s\" (and leave a translator note).\n\nTo be honest from being on the code side of a much bigger i18n effort\n(the MediaWiki/WikiMedia software) back in the early days of my career I\njust do this sort of thing reflexively, because from experience when I\nstarted trying to simplify stuff by making assumptions I was wrong every\ntime.\n\nAlthough in that case there were >100+ languages, so maybe we can get\naway with this.\n\nIn this case one red flag I see is that we make a reference to \"the\nsource ref\" in the first paragraph, and then in the second we'll either\ntalk about \"commit\", \"tag\" or \"blob\" etc. Now imagine a language where\nthose words have different genders, and where even secondary references\nto those things (\"the source ref\") spill over and need to be changed\ntoo.\n\nYou also get languages where a message that stretches multiple\nparagraphs flows more naturally if the wording is re-arranged, even\nbetween paragraphs. This is why document translation systems generally\nsplit things by sections at best (not paragraphs), or just by whole\ndocuments.\n\n>> diff --git a/Documentation/config.txt b/Documentation/config.txt\n>> index 1546833213..fd455e2739 100644\n>> --- a/Documentation/config.txt\n>> +++ b/Documentation/config.txt\n>> @@ -320,6 +320,13 @@ advice.*::\n>> \t\ttries to overwrite a remote ref that points at an\n>> \t\tobject that is not a commit-ish, or make the remote\n>> \t\tref point at an object that is not a commit-ish.\n>> +\tpushAmbigiousRefName::\n>> +\t\tShown when linkgit:git-push[1] gives up trying to\n>> +\t\tguess based on the source and destination refs what\n>> +\t\tremote ref namespace the source belongs in, but where\n>> +\t\twe can still suggest that the user push to either\n>> +\t\trefs/heads/* or refs/tags/* based on the type of the\n>> +\t\tsource object.\n>\n> I guess you could argue that this is \"ambiguous\", but usually we'd use\n> that term to mean \"there were two branches that matched on the other\n> side\". But here it's actually \"no branches matched\" (actually, it makes\n> me wonder what we'd do pushing \"foo\" when that name is ambiguous on the\n> other side).\n>\n> So I wonder if this ought to be pushUnqualifiedRefname or something.\n\nYeah that sounds better. Will change it.\n\n>> @@ -1046,13 +1047,60 @@ static int match_explicit(struct ref *src, struct ref *dst,\n>> \t\telse if ((dst_guess = guess_ref(dst_value, matched_src))) {\n>> \t\t\tmatched_dst = make_linked_ref(dst_guess, dst_tail);\n>> \t\t\tfree(dst_guess);\n>> -\t\t} else\n>> -\t\t\terror(_(\"unable to push to unqualified destination: %s\\n\"\n>> -\t\t\t\t\"The destination refspec neither matches an \"\n>> -\t\t\t\t\"existing ref on the remote nor\\n\"\n>> -\t\t\t\t\"begins with refs/, and we are unable to \"\n>> -\t\t\t\t\"guess a prefix based on the source ref.\"),\n>> -\t\t\t dst_value);\n>> +\t\t} else {\n>> +\t\t\tstruct object_id oid;\n>> +\t\t\tenum object_type type;\n>> +\n>> +\t\t\terror(\"unable to push to unqualified destination: %s\", dst_value);\n>> +\t\t\tif (!advice_push_ambiguous_ref_name)\n>> +\t\t\t\tbreak;\n>\n> This break feels funny, because it controls flow much larger than this\n> if/else. It does the right thing now, but it must remain in sync with\n> what comes at the end of that long string of advise() messages.\n>\n> Can we just do it as:\n>\n> if (advice_push_ambiguous_ref_name) {\n> \tstruct object_id oid;\n> \tenum object_type type;\n>\n> \tif (get_oid(...))\n> \t etc...\n> }\n>\n> instead? That pushes your indentation one level in, but I think the\n> whole conditional body might be cleaner in a helper function anyway.\n\nI started out with that and found myself really constrained by the 72\nchar ceiling which I'm already smashing through with these ~95 character\nlines (but at least it's under 100!). But sure, we can do with 8 more.\n"},{"id":"360090","messageId":"xmqqbm81lb7c.fsf@gitster-ct.c.googlers.com","threadId":"49524","inReplyTo":"20181010205505.GB12949@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] push: add an advice on unqualified push","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-10T21:54:15Z","receivedAt":"2018-10-10T21:54:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> Fix both of those, now the message will look like this instead:\n>> \n>> $ ./git-push avar v2.19.0^{commit}:newbranch -n\n>> error: unable to push to unqualified destination: newbranch\n>> hint: The destination refspec neither matches an existing\n>> hint: ref on the remote nor begins with refs/, and we are\n>> hint: unable to guess a prefix based on the source ref.\n>> hint:\n>> hint: The part of the refspec is a commit object.\n>> hint: Did you mean to create a new branch by pushing to\n>> hint: 'v2.19.0^{commit}:refs/heads/newbranch'?\n>> error: failed to push some refs to 'git@github.com:avar/git.git'\n>> \n>> When trying to push a tag, tree or a blob we suggest that perhaps the\n>> user meant to push them to refs/tags/ instead.\n>\n> This is much better, and I love the customized behavior based on the\n> object type.\n>\n> I wonder if we could reword the first paragraph to be a little less\n> confusing, and spell out what we tried already. E.g., something like:\n>\n> The destination you provided is not a full refname (i.e., starting\n> with \"ref\"). Git tried to guess what you meant by:\n\ns|ref|refs/|; I fully agree that \"unqualified destination\" was a\npoor way to communicate the failure to those who would likely hit\nthis error path, because somebody who can ell what's qualified and\nwhat's not would not be triggering the error in the first place.\n\n> - looking for a matching branch or tag on the remote side\n>\n> - looking at the refname of the local source\n>\n> but neither worked.\n>\n> The part of the refspec is a commit object.\n> Did you mean...\n\nLooks great.\n\n> I'm not sure about saying \"branch or tag\" in the first bullet. It's\n> friendlier to most users, but less technically correct (if you said\n> \"notes/foo\", I believe we'd match an existing \"refs/notes/foo\", because\n> it's really just using the normal lookup rules).\n\nAn alternative may be \"looking for a ref that matches %s on the\nremote side\". I am no longer a total newbie, so I cannot tell how\nwell that message would help one to connect notes/foo one just typed\nwith refs/notes/foo that potentially exists on the remote side.\n\n> Also, as an aside, I wonder if we should allow \"heads/foo\" to work as\n> \"refs/heads/foo\" (even when no such ref already exists). But that is\n> totally orthogonal to changing the message.\n\nI am neutral on this point but agree that it is better done outside\nthis patch.\n"},{"id":"360116","messageId":"20181011001641.GB13853@sigill.intra.peff.net","threadId":"49524","inReplyTo":"87r2gxebsi.fsf@evledraar.gmail.com","subject":"Re: [PATCH 2/2] push: add an advice on unqualified push","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-11T00:16:41Z","receivedAt":"2018-10-11T00:16:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 10, 2018 at 11:23:25PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> > I wonder if we could reword the first paragraph to be a little less\n> > confusing, and spell out what we tried already. E.g., something like:\n> >\n> > The destination you provided is not a full refname (i.e., starting\n> > with \"ref\"). Git tried to guess what you meant by:\n> >\n> > - looking for a matching branch or tag on the remote side\n> >\n> > - looking at the refname of the local source\n> >\n> > but neither worked.\n> >\n> > The part of the refspec is a commit object.\n> > Did you mean...\n> \n> Yeah that makes sense. I was trying to avoid touching the existing\n> wording to make this more surgical, but you came up with it, and since\n> you don't like it I'll just change that too.\n\nI certainly know the feeling of trying to avoid wording bikeshed\ndiscussions. But in this instance, please feel free to aggressively\nrewrite that old message. ;)\n\nWhat I wrote above was off-the-cuff, and I also do not mind if you use\nit as a starting point to make improvements (or take it wholesale if you\nreally like it).\n\n> > I think it would probably be OK to put the first paragraph in its own\n> > variable. I know we try to avoid translation lego, but I'd think\n> > paragraphs are separate units. Or are you worried about how to get them\n> > into the same advise() call? I don't know that we need to, but we could\n> > also plug one into the other with a \"%s\" (and leave a translator note).\n> \n> To be honest from being on the code side of a much bigger i18n effort\n> (the MediaWiki/WikiMedia software) back in the early days of my career I\n> just do this sort of thing reflexively, because from experience when I\n> started trying to simplify stuff by making assumptions I was wrong every\n> time.\n> [...]\n\nOK, I'm happy to defer to your judgement here. I have very little\ntranslation experience myself.\n\n> > Can we just do it as:\n> >\n> > if (advice_push_ambiguous_ref_name) {\n> > \tstruct object_id oid;\n> > \tenum object_type type;\n> >\n> > \tif (get_oid(...))\n> > \t etc...\n> > }\n> >\n> > instead? That pushes your indentation one level in, but I think the\n> > whole conditional body might be cleaner in a helper function anyway.\n> \n> I started out with that and found myself really constrained by the 72\n> char ceiling which I'm already smashing through with these ~95 character\n> lines (but at least it's under 100!). But sure, we can do with 8 more.\n\nThat's why I suggested the helper function. :) I'm also not opposed to\npulling messages out to static file-level variables, even if they're\nonly used once. Sometimes it's nice to have them left-aligned (or close\nto it) to see how they'll actually look in a terminal.\n\n-Peff\n"},{"id":"360117","messageId":"20181011001910.GC13853@sigill.intra.peff.net","threadId":"49524","inReplyTo":"xmqqbm81lb7c.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/2] push: add an advice on unqualified push","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-11T00:19:11Z","receivedAt":"2018-10-11T00:19:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 11, 2018 at 06:54:15AM +0900, Junio C Hamano wrote:\n\n> > I'm not sure about saying \"branch or tag\" in the first bullet. It's\n> > friendlier to most users, but less technically correct (if you said\n> > \"notes/foo\", I believe we'd match an existing \"refs/notes/foo\", because\n> > it's really just using the normal lookup rules).\n> \n> An alternative may be \"looking for a ref that matches %s on the\n> remote side\". I am no longer a total newbie, so I cannot tell how\n> well that message would help one to connect notes/foo one just typed\n> with refs/notes/foo that potentially exists on the remote side.\n\nYeah. Really, it would be nice to imply that it somehow does the same\nDWIM lookup that we do for local refs. But I didn't know how to say\nthat. Possibly we could refer to the documentation, but it's buried in\ngit-rev-parse.\n\n> > Also, as an aside, I wonder if we should allow \"heads/foo\" to work as\n> > \"refs/heads/foo\" (even when no such ref already exists). But that is\n> > totally orthogonal to changing the message.\n> \n> I am neutral on this point but agree that it is better done outside\n> this patch.\n\nYeah, definitely. I would almost call it a leftover bit, but I think the\nsubtlety is not in the code, but in whether it is a good thing to be\ndoing (i.e., too many false positives).\n\n-Peff\n"},{"id":"360241","messageId":"xmqq5zy8f6gr.fsf@gitster-ct.c.googlers.com","threadId":"49524","inReplyTo":"87r2gxebsi.fsf@evledraar.gmail.com","subject":"Re: [PATCH 2/2] push: add an advice on unqualified push","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-11T22:45:24Z","receivedAt":"2018-10-11T22:45:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Wed, Oct 10 2018, Jeff King wrote:\n>\n>> This is much better, and I love the customized behavior based on the\n>> object type.\n>>\n>> I wonder if we could reword the first paragraph to be a little less\n>> confusing, and spell out what we tried already. E.g., something like:\n>> ...\n>\n> Yeah that makes sense. I was trying to avoid touching the existing\n> wording to make this more surgical, but you came up with it, and since\n> you don't like it I'll just change that too.\n\nOK, for now I'll mark these two patches \"read\" in my inbox and\nforget about them, expecting that a reroll of 2/2 with improved\nmessages would appear not in too distant future.\n\nThanks, both.\n"},{"id":"361628","messageId":"87r2gclnjc.fsf@evledraar.gmail.com","threadId":"49524","inReplyTo":"xmqq5zy8f6gr.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/2] push: add an advice on unqualified push","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-26T15:45:27Z","receivedAt":"2018-10-26T15:45:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Oct 11 2018, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> On Wed, Oct 10 2018, Jeff King wrote:\n>>\n>>> This is much better, and I love the customized behavior based on the\n>>> object type.\n>>>\n>>> I wonder if we could reword the first paragraph to be a little less\n>>> confusing, and spell out what we tried already. E.g., something like:\n>>> ...\n>>\n>> Yeah that makes sense. I was trying to avoid touching the existing\n>> wording to make this more surgical, but you came up with it, and since\n>> you don't like it I'll just change that too.\n>\n> OK, for now I'll mark these two patches \"read\" in my inbox and\n> forget about them, expecting that a reroll of 2/2 with improved\n> messages would appear not in too distant future.\n\nI was going to submit an update to this, as an additional improvement\ncan anyone think of a reason not to always infer that we'd like a new\nbranch if the LHS of the refspec starts with refs/remotes/* ?\n\n u git (push-advice-on-unqualified-src-2 $>) $ ./git-push avar refs/remotes/origin/master:newbranch -n\n To github.com:avar/git.git\n * [new branch] origin/master -> newbranch\n u git (push-advice-on-unqualified-src-2 $>) $ git diff\n diff --git a/remote.c b/remote.c\n index 5b679df02d..949a9bd079 100644\n --- a/remote.c\n +++ b/remote.c\n @@ -969,7 +969,8 @@ static char *guess_ref(const char *name, struct ref *peer)\n if (!r)\n return NULL;\n\n - if (starts_with(r, \"refs/heads/\"))\n + if (starts_with(r, \"refs/heads/\") ||\n + starts_with(r, \"refs/remotes/\"))\n strbuf_addstr(&buf, \"refs/heads/\");\n else if (starts_with(r, \"refs/tags/\"))\n strbuf_addstr(&buf, \"refs/tags/\");\n\nMaybe we need to be really paranoid here and also check if it's a\n\"commit\", i.e. you could setup a refspec like:\n\n fetch = +refs/tags/*:refs/remotes/origin-tags/*\n"},{"id":"361636","messageId":"20181026192734.9609-1-avarab@gmail.com","threadId":"49524","inReplyTo":"xmqq5zy8f6gr.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v2 0/7] fixes for unqualified push","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-26T19:27:27Z","receivedAt":"2018-10-26T19:27:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This has grown to a 7-part series for v2 (from 2 patches). This\naddresse all the feedback for v1 and then some.\n\nÆvar Arnfjörð Bjarmason (7):\n remote.c: add braces in anticipation of a follow-up change\n i18n: remote.c: mark error(...) messages for translation\n push: improve the error shown on unqualified push\n push: move unqualified refname error into a function\n push: add an advice on unqualified push\n push: test that doesn't DWYM if is unqualified\n push: add DWYM support for \"git push refs/remotes/...:\"\n\n Documentation/config.txt | 7 +++\n advice.c | 2 +\n advice.h | 1 +\n remote.c | 124 +++++++++++++++++++++++++++++++--------\n t/t5505-remote.sh | 57 ++++++++++++++++++\n 5 files changed, 166 insertions(+), 25 deletions(-)\n\n-- \n2.19.1.759.g500967bb5e\n\n"},{"id":"361637","messageId":"20181026192734.9609-2-avarab@gmail.com","threadId":"49524","inReplyTo":"xmqq5zy8f6gr.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v2 1/7] remote.c: add braces in anticipation of a follow-up change","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-26T19:27:28Z","receivedAt":"2018-10-26T19:27:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The CodingGuidelines say \"When there are multiple arms to a\nconditional and some of them require braces, enclose even a single\nline block in braces for consistency.\". Fix the code in\nmatch_explicit() to conform.\n\nWhile I'm at it change the if/else if/else in guess_ref() to use\nbraces. This is not currently needed, but a follow-up change will add\na new multi-line condition to that logic.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n remote.c | 16 +++++++++-------\n 1 file changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 81f4f01b00..18cae48daa 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -968,12 +968,13 @@ static char *guess_ref(const char *name, struct ref *peer)\n \tif (!r)\n \t\treturn NULL;\n \n-\tif (starts_with(r, \"refs/heads/\"))\n+\tif (starts_with(r, \"refs/heads/\")) {\n \t\tstrbuf_addstr(&buf, \"refs/heads/\");\n-\telse if (starts_with(r, \"refs/tags/\"))\n+\t} else if (starts_with(r, \"refs/tags/\")) {\n \t\tstrbuf_addstr(&buf, \"refs/tags/\");\n-\telse\n+\t} else {\n \t\treturn NULL;\n+\t}\n \n \tstrbuf_addstr(&buf, name);\n \treturn strbuf_detach(&buf, NULL);\n@@ -1038,21 +1039,22 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \tcase 1:\n \t\tbreak;\n \tcase 0:\n-\t\tif (starts_with(dst_value, \"refs/\"))\n+\t\tif (starts_with(dst_value, \"refs/\")) {\n \t\t\tmatched_dst = make_linked_ref(dst_value, dst_tail);\n-\t\telse if (is_null_oid(&matched_src->new_oid))\n+\t\t} else if (is_null_oid(&matched_src->new_oid)) {\n \t\t\terror(\"unable to delete '%s': remote ref does not exist\",\n \t\t\t dst_value);\n-\t\telse if ((dst_guess = guess_ref(dst_value, matched_src))) {\n+\t\t} else if ((dst_guess = guess_ref(dst_value, matched_src))) {\n \t\t\tmatched_dst = make_linked_ref(dst_guess, dst_tail);\n \t\t\tfree(dst_guess);\n-\t\t} else\n+\t\t} else {\n \t\t\terror(\"unable to push to unqualified destination: %s\\n\"\n \t\t\t \"The destination refspec neither matches an \"\n \t\t\t \"existing ref on the remote nor\\n\"\n \t\t\t \"begins with refs/, and we are unable to \"\n \t\t\t \"guess a prefix based on the source ref.\",\n \t\t\t dst_value);\n+\t\t}\n \t\tbreak;\n \tdefault:\n \t\tmatched_dst = NULL;\n-- \n2.19.1.759.g500967bb5e\n\n"},{"id":"361638","messageId":"20181026192734.9609-3-avarab@gmail.com","threadId":"49524","inReplyTo":"xmqq5zy8f6gr.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v2 2/7] i18n: remote.c: mark error(...) messages for translation","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-26T19:27:29Z","receivedAt":"2018-10-26T19:27:51Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Mark up the error(...) messages in remote.c for translation. The likes\nof \"unable to push to unqualified destination\" are relatively big\nparts of the UI, i.e. error messages shown when \"git push\" fails.\n\nI don't think any of these are plumbing, an the entire test suite\npasses when running the tests with GIT_GETTEXT_POISON=1 (after\nbuilding with GETTEXT_POISON).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n remote.c | 35 ++++++++++++++++++-----------------\n 1 file changed, 18 insertions(+), 17 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 18cae48daa..5cb3d00bfb 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -406,7 +406,7 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t\tif (!remote->receivepack)\n \t\t\tremote->receivepack = v;\n \t\telse\n-\t\t\terror(\"more than one receivepack given, using the first\");\n+\t\t\terror(_(\"more than one receivepack given, using the first\"));\n \t} else if (!strcmp(subkey, \"uploadpack\")) {\n \t\tconst char *v;\n \t\tif (git_config_string(&v, key, value))\n@@ -414,7 +414,7 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t\tif (!remote->uploadpack)\n \t\t\tremote->uploadpack = v;\n \t\telse\n-\t\t\terror(\"more than one uploadpack given, using the first\");\n+\t\t\terror(_(\"more than one uploadpack given, using the first\"));\n \t} else if (!strcmp(subkey, \"tagopt\")) {\n \t\tif (!strcmp(value, \"--no-tags\"))\n \t\t\tremote->fetch_tags = -1;\n@@ -707,7 +707,7 @@ static void query_refspecs_multiple(struct refspec *rs,\n \tint find_src = !query->src;\n \n \tif (find_src && !query->dst)\n-\t\terror(\"query_refspecs_multiple: need either src or dst\");\n+\t\terror(_(\"query_refspecs_multiple: need either src or dst\"));\n \n \tfor (i = 0; i < rs->nr; i++) {\n \t\tstruct refspec_item *refspec = &rs->items[i];\n@@ -735,7 +735,7 @@ int query_refspecs(struct refspec *rs, struct refspec_item *query)\n \tchar **result = find_src ? &query->src : &query->dst;\n \n \tif (find_src && !query->dst)\n-\t\treturn error(\"query_refspecs: need either src or dst\");\n+\t\treturn error(_(\"query_refspecs: need either src or dst\"));\n \n \tfor (i = 0; i < rs->nr; i++) {\n \t\tstruct refspec_item *refspec = &rs->items[i];\n@@ -996,12 +996,12 @@ static int match_explicit_lhs(struct ref *src,\n \t\t * way to delete 'other' ref at the remote end.\n \t\t */\n \t\tif (try_explicit_object_name(rs->src, match) < 0)\n-\t\t\treturn error(\"src refspec %s does not match any.\", rs->src);\n+\t\t\treturn error(_(\"src refspec %s does not match any.\"), rs->src);\n \t\tif (allocated_match)\n \t\t\t*allocated_match = 1;\n \t\treturn 0;\n \tdefault:\n-\t\treturn error(\"src refspec %s matches more than one.\", rs->src);\n+\t\treturn error(_(\"src refspec %s matches more than one.\"), rs->src);\n \t}\n }\n \n@@ -1041,32 +1041,33 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \tcase 0:\n \t\tif (starts_with(dst_value, \"refs/\")) {\n \t\t\tmatched_dst = make_linked_ref(dst_value, dst_tail);\n+\n \t\t} else if (is_null_oid(&matched_src->new_oid)) {\n-\t\t\terror(\"unable to delete '%s': remote ref does not exist\",\n+\t\t\terror(_(\"unable to delete '%s': remote ref does not exist\"),\n \t\t\t dst_value);\n \t\t} else if ((dst_guess = guess_ref(dst_value, matched_src))) {\n \t\t\tmatched_dst = make_linked_ref(dst_guess, dst_tail);\n \t\t\tfree(dst_guess);\n \t\t} else {\n-\t\t\terror(\"unable to push to unqualified destination: %s\\n\"\n-\t\t\t \"The destination refspec neither matches an \"\n-\t\t\t \"existing ref on the remote nor\\n\"\n-\t\t\t \"begins with refs/, and we are unable to \"\n-\t\t\t \"guess a prefix based on the source ref.\",\n+\t\t\terror(_(\"unable to push to unqualified destination: %s\\n\"\n+\t\t\t\t\"The destination refspec neither matches an \"\n+\t\t\t\t\"existing ref on the remote nor\\n\"\n+\t\t\t\t\"begins with refs/, and we are unable to \"\n+\t\t\t\t\"guess a prefix based on the source ref.\"),\n \t\t\t dst_value);\n \t\t}\n \t\tbreak;\n \tdefault:\n \t\tmatched_dst = NULL;\n-\t\terror(\"dst refspec %s matches more than one.\",\n+\t\terror(_(\"dst refspec %s matches more than one.\"),\n \t\t dst_value);\n \t\tbreak;\n \t}\n \tif (!matched_dst)\n \t\treturn -1;\n \tif (matched_dst->peer_ref)\n-\t\treturn error(\"dst ref %s receives from more than one src.\",\n-\t\t matched_dst->name);\n+\t\treturn error(_(\"dst ref %s receives from more than one src.\"),\n+\t\t\t matched_dst->name);\n \telse {\n \t\tmatched_dst->peer_ref = allocated_src ?\n \t\t\t\t\tmatched_src :\n@@ -1765,7 +1766,7 @@ int get_fetch_map(const struct ref *remote_refs,\n \t\t\tif (!starts_with((*rmp)->peer_ref->name, \"refs/\") ||\n \t\t\t check_refname_format((*rmp)->peer_ref->name, 0)) {\n \t\t\t\tstruct ref *ignore = *rmp;\n-\t\t\t\terror(\"* Ignoring funny ref '%s' locally\",\n+\t\t\t\terror(_(\"* Ignoring funny ref '%s' locally\"),\n \t\t\t\t (*rmp)->peer_ref->name);\n \t\t\t\t*rmp = (*rmp)->next;\n \t\t\t\tfree(ignore->peer_ref);\n@@ -2133,7 +2134,7 @@ static int parse_push_cas_option(struct push_cas_option *cas, const char *arg, i\n \telse if (!colon[1])\n \t\toidclr(&entry->expect);\n \telse if (get_oid(colon + 1, &entry->expect))\n-\t\treturn error(\"cannot parse expected object name '%s'\", colon + 1);\n+\t\treturn error(_(\"cannot parse expected object name '%s'\"), colon + 1);\n \treturn 0;\n }\n \n-- \n2.19.1.759.g500967bb5e\n\n"},{"id":"361639","messageId":"20181026192734.9609-4-avarab@gmail.com","threadId":"49524","inReplyTo":"xmqq5zy8f6gr.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v2 3/7] push: improve the error shown on unqualified push","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-26T19:27:30Z","receivedAt":"2018-10-26T19:27:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Improve the error message added in f8aae12034 (\"push: allow\nunqualified dest refspecs to DWIM\", 2008-04-23), which before this\nchange looks like this:\n\n $ git push avar v2.19.0^{commit}:newbranch -n\n error: unable to push to unqualified destination: newbranch\n The destination refspec neither matches an existing ref on the remote nor\n begins with refs/, and we are unable to guess a prefix based on the source ref.\n error: failed to push some refs to 'git@github.com:avar/git.git'\n\nThis message needed to be read very carefully to spot how to fix the\nerror, i.e. to push to refs/heads/newbranch. Now the message will look\nlike this instead:\n\n $ ./git-push avar v2.19.0^{commit}:newbranch -n\n error: The destination you provided is not a full refname (i.e.,\n starting with \"refs/\"). We tried to guess what you meant by:\n\n - Looking for a ref that matches 'newbranch' on the remote side.\n - Checking if the being pushed ('v2.19.0^{commit}')\n is a ref in \"refs/{heads,tags}/\". If so we add a\n corresponding refs/{heads,tags}/ prefix on the remote side.\n\n Neither worked, so we gave up. You must fully-qualify the ref.\n error: failed to push some refs to 'git@github.com:avar/git.git'\n\nThis improvement is the result of on-list discussion in [1] and [2],\nas well as my own fixes / reformatting / phrasing on top.\n\nThe suggestion by Jeff on-list was to make that second bullet point\n\"Looking at the refname of the local source.\". The version being added\nhere is more verbose, but also more accurate. saying \"local source\"\ncould refer to any ref in the local refstore, including something in\nrefs/remotes/*. A later change will teach guess_ref() to infer a ref\ntype from remote-tracking refs, so let's not confuse the two.\n\nWhile I'm at it, add a \"TRANSLATORS\" comment since the message has\ngotten more complex and it's not as clear what the two %s's refer to.\n\n1. https://public-inbox.org/git/20181010205505.GB12949@sigill.intra.peff.net/\n2. https://public-inbox.org/git/xmqqbm81lb7c.fsf@gitster-ct.c.googlers.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n remote.c | 22 ++++++++++++++++------\n 1 file changed, 16 insertions(+), 6 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 5cb3d00bfb..f4b438ff74 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1049,12 +1049,22 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \t\t\tmatched_dst = make_linked_ref(dst_guess, dst_tail);\n \t\t\tfree(dst_guess);\n \t\t} else {\n-\t\t\terror(_(\"unable to push to unqualified destination: %s\\n\"\n-\t\t\t\t\"The destination refspec neither matches an \"\n-\t\t\t\t\"existing ref on the remote nor\\n\"\n-\t\t\t\t\"begins with refs/, and we are unable to \"\n-\t\t\t\t\"guess a prefix based on the source ref.\"),\n-\t\t\t dst_value);\n+\t\t\t/*\n+\t\t\t * TRANSLATORS: \"matches '%s'%\" is the \n+\t\t\t * part of \"git push :\"\n+\t\t\t * push, and \"being pushed ('%s')\" is the\n+\t\t\t * .\n+\t\t\t */\n+\t\t\terror(_(\"The destination you provided is not a full refname (i.e.,\\n\"\n+\t\t\t\t\"starting with \\\"refs/\\\"). We tried to guess what you meant by:\\n\"\n+\t\t\t\t\"\\n\"\n+\t\t\t\t\"- Looking for a ref that matches '%s' on the remote side.\\n\"\n+\t\t\t\t\"- Checking if the being pushed ('%s')\\n\"\n+\t\t\t\t\" is a ref in \\\"refs/{heads,tags}/\\\". If so we add a corresponding\\n\"\n+\t\t\t\t\" refs/{heads,tags}/ prefix on the remote side.\\n\"\n+\t\t\t\t\"\\n\"\n+\t\t\t\t\"Neither worked, so we gave up. You must fully-qualify the ref.\"),\n+\t\t\t dst_value, matched_src->name);\n \t\t}\n \t\tbreak;\n \tdefault:\n-- \n2.19.1.759.g500967bb5e\n\n"},{"id":"361640","messageId":"20181026192734.9609-6-avarab@gmail.com","threadId":"49524","inReplyTo":"xmqq5zy8f6gr.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v2 5/7] push: add an advice on unqualified push","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-26T19:27:32Z","receivedAt":"2018-10-26T19:27:58Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add an advice to the recently improved error message added in\nf8aae12034 (\"push: allow unqualified dest refspecs to DWIM\",\n2008-04-23).\n\nNow with advice.pushUnqualifiedRefName=true (on by default) we show a\nhint about how to proceed:\n\n $ ./git-push avar v2.19.0^{commit}:newbranch -n\n error: The destination you provided is not a full refname (i.e.,\n starting with \"refs/\"). We tried to guess what you meant by:\n\n - Looking for a ref that matches newbranch on the remote side.\n - Looking at the refname of the local source.\n\n Neither worked, so we gave up. You must fully-qualify the ref.\n hint: The part of the refspec is a commit object.\n hint: Did you mean to create a new branch by pushing to\n hint: 'v2.19.0^{commit}:refs/heads/newbranch'?\n error: failed to push some refs to 'git@github.com:avar/git.git'\n\nWhen trying to push a tag, tree or a blob we suggest that perhaps the\nuser meant to push them to refs/tags/ instead.\n\nThe if/else duplication for all of OBJ_{COMMIT,TAG,TREE,BLOB} is\nunfortunate, but is required to correctly mark the messages for\ntranslation. See the discussion in\n<87r2gxebsi.fsf@evledraar.gmail.com> about that.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/config.txt | 7 +++++++\n advice.c | 2 ++\n advice.h | 1 +\n remote.c | 37 +++++++++++++++++++++++++++++++++++++\n t/t5505-remote.sh | 25 +++++++++++++++++++++++++\n 5 files changed, 72 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 552827935a..8ca465702e 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -320,6 +320,13 @@ advice.*::\n \t\ttries to overwrite a remote ref that points at an\n \t\tobject that is not a commit-ish, or make the remote\n \t\tref point at an object that is not a commit-ish.\n+\tpushUnqualifiedRefname::\n+\t\tShown when linkgit:git-push[1] gives up trying to\n+\t\tguess based on the source and destination refs what\n+\t\tremote ref namespace the source belongs in, but where\n+\t\twe can still suggest that the user push to either\n+\t\trefs/heads/* or refs/tags/* based on the type of the\n+\t\tsource object.\n \tstatusHints::\n \t\tShow directions on how to proceed from the current\n \t\tstate in the output of linkgit:git-status[1], in\ndiff --git a/advice.c b/advice.c\nindex 3561cd64e9..3089a4ca65 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -9,6 +9,7 @@ int advice_push_non_ff_matching = 1;\n int advice_push_already_exists = 1;\n int advice_push_fetch_first = 1;\n int advice_push_needs_force = 1;\n+int advice_push_unqualified_ref_name = 1;\n int advice_status_hints = 1;\n int advice_status_u_option = 1;\n int advice_commit_before_merge = 1;\n@@ -62,6 +63,7 @@ static struct {\n \t{ \"pushAlreadyExists\", &advice_push_already_exists },\n \t{ \"pushFetchFirst\", &advice_push_fetch_first },\n \t{ \"pushNeedsForce\", &advice_push_needs_force },\n+\t{ \"pushUnqualifiedRefName\", &advice_push_unqualified_ref_name },\n \t{ \"statusHints\", &advice_status_hints },\n \t{ \"statusUoption\", &advice_status_u_option },\n \t{ \"commitBeforeMerge\", &advice_commit_before_merge },\ndiff --git a/advice.h b/advice.h\nindex ab24df0fd0..9a2f8b5226 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -9,6 +9,7 @@ extern int advice_push_non_ff_matching;\n extern int advice_push_already_exists;\n extern int advice_push_fetch_first;\n extern int advice_push_needs_force;\n+extern int advice_push_unqualified_ref_name;\n extern int advice_status_hints;\n extern int advice_status_u_option;\n extern int advice_commit_before_merge;\ndiff --git a/remote.c b/remote.c\nindex c7a0b9c46f..93f802509d 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -13,6 +13,7 @@\n #include \"mergesort.h\"\n #include \"argv-array.h\"\n #include \"commit-reach.h\"\n+#include \"advice.h\"\n \n enum map_direction { FROM_SRC, FROM_DST };\n \n@@ -1008,6 +1009,9 @@ static int match_explicit_lhs(struct ref *src,\n static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\t\t\t\t\t const char *matched_src_name)\n {\n+\tstruct object_id oid;\n+\tenum object_type type;\n+\n \t/*\n \t * TRANSLATORS: \"matches '%s'%\" is the part of \"git push\n \t * :\" push, and \"being pushed ('%s')\" is\n@@ -1023,6 +1027,39 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\t\"\\n\"\n \t\t\"Neither worked, so we gave up. You must fully-qualify the ref.\"),\n \t dst_value, matched_src_name);\n+\n+\tif (!advice_push_unqualified_ref_name)\n+\t\treturn;\n+\n+\tif (get_oid(matched_src_name, &oid))\n+\t\tBUG(\"'%s' is not a valid object, \"\n+\t\t \"match_explicit_lhs() should catch this!\",\n+\t\t matched_src_name);\n+\ttype = oid_object_info(the_repository, &oid, NULL);\n+\tif (type == OBJ_COMMIT) {\n+\t\tadvise(_(\"The part of the refspec is a commit object.\\n\"\n+\t\t\t \"Did you mean to create a new branch by pushing to\\n\"\n+\t\t\t \"'%s:refs/heads/%s'?\"),\n+\t\t matched_src_name, dst_value);\n+\t} else if (type == OBJ_TAG) {\n+\t\tadvise(_(\"The part of the refspec is a tag object.\\n\"\n+\t\t\t \"Did you mean to create a new tag by pushing to\\n\"\n+\t\t\t \"'%s:refs/tags/%s'?\"),\n+\t\t matched_src_name, dst_value);\n+\t} else if (type == OBJ_TREE) {\n+\t\tadvise(_(\"The part of the refspec is a tree object.\\n\"\n+\t\t\t \"Did you mean to tag a new tree by pushing to\\n\"\n+\t\t\t \"'%s:refs/tags/%s'?\"),\n+\t\t matched_src_name, dst_value);\n+\t} else if (type == OBJ_BLOB) {\n+\t\tadvise(_(\"The part of the refspec is a blob object.\\n\"\n+\t\t\t \"Did you mean to tag a new blob by pushing to\\n\"\n+\t\t\t \"'%s:refs/tags/%s'?\"),\n+\t\t matched_src_name, dst_value);\n+\t} else {\n+\t\tBUG(\"'%s' should be commit/tag/tree/blob, is '%d'\",\n+\t\t matched_src_name, type);\n+\t}\n }\n \n static int match_explicit(struct ref *src, struct ref *dst,\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex d2a2cdd453..2e58721f98 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -1222,4 +1222,29 @@ test_expect_success 'add remote matching the \"insteadOf\" URL' '\n \tgit remote add backup xyz@example.com\n '\n \n+test_expect_success 'unqualified refspec DWIM and advice' '\n+\ttest_when_finished \"(cd test && git tag -d some-tag)\" &&\n+\t(\n+\t\tcd test &&\n+\t\tgit tag -a -m \"Some tag\" some-tag master &&\n+\t\tfor type in commit tag tree blob\n+\t\tdo\n+\t\t\tif test \"$type\" = \"blob\"\n+\t\t\tthen\n+\t\t\t\toid=$(git rev-parse some-tag:file)\n+\t\t\telse\n+\t\t\t\toid=$(git rev-parse some-tag^{$type})\n+\t\t\tfi &&\n+\t\t\ttest_must_fail git push origin $oid:dst 2>err &&\n+\t\t\ttest_i18ngrep \"error: The destination you\" err &&\n+\t\t\ttest_i18ngrep \"hint: Did you mean\" err &&\n+\t\t\ttest_must_fail git -c advice.pushUnqualifiedRefName=false \\\n+\t\t\t\tpush origin $oid:dst 2>err &&\n+\t\t\ttest_i18ngrep \"error: The destination you\" err &&\n+\t\t\ttest_i18ngrep ! \"hint: Did you mean\" err\n+\t\tdone\n+\t)\n+'\n+\n+\n test_done\n-- \n2.19.1.759.g500967bb5e\n\n"},{"id":"361641","messageId":"20181026192734.9609-7-avarab@gmail.com","threadId":"49524","inReplyTo":"xmqq5zy8f6gr.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v2 6/7] push: test that doesn't DWYM if is unqualified","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-26T19:27:33Z","receivedAt":"2018-10-26T19:27:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a test asserting that \"git push origin :\" where is\na branch, tag, tree or blob in refs/remotes/* doesn't DWYM when \nis unqualified. This has never worked, but there's been no test for\nthis behavior.\n\nSee f88395ac23 (\"Renaming push.\", 2005-08-03), bb9fca80ce (\"git-push:\nUpdate description of refspecs and add examples\", 2007-06-09) and\nf8aae12034 (\"push: allow unqualified dest refspecs to DWIM\",\n2008-04-23) which are most relevant commits that have changed or\ndocumented the behavior of this feature in the past.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t5505-remote.sh | 28 ++++++++++++++++++++++++++++\n 1 file changed, 28 insertions(+)\n\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 2e58721f98..979a13b415 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -1246,5 +1246,33 @@ test_expect_success 'unqualified refspec DWIM and advice' '\n \t)\n '\n \n+test_expect_success 'refs/remotes/* refspec and unqualified DWIM and advice' '\n+\t(\n+\t\tcd two &&\n+\t\tgit tag -a -m \"Some tag\" some-tag master &&\n+\t\tgit update-ref refs/trees/my-head-tree HEAD^{tree} &&\n+\t\tgit update-ref refs/blobs/my-file-blob HEAD:file\n+\t) &&\n+\t(\n+\t\tcd test &&\n+\t\tgit config --add remote.two.fetch \"+refs/tags/*:refs/remotes/two-tags/*\" &&\n+\t\tgit config --add remote.two.fetch \"+refs/trees/*:refs/remotes/two-trees/*\" &&\n+\t\tgit config --add remote.two.fetch \"+refs/blobs/*:refs/remotes/two-blobs/*\" &&\n+\t\tgit fetch --no-tags two &&\n+\n+\t\ttest_must_fail git push origin refs/remotes/two/another:dst 2>err &&\n+\t\ttest_i18ngrep \"error: The destination you\" err &&\n+\n+\t\ttest_must_fail git push origin refs/remotes/two-tags/some-tag:dst-tag 2>err &&\n+\t\ttest_i18ngrep \"error: The destination you\" err &&\n+\n+\t\ttest_must_fail git push origin refs/remotes/two-trees/my-head-tree:dst-tree 2>err &&\n+\t\ttest_i18ngrep \"error: The destination you\" err &&\n+\n+\t\ttest_must_fail git push origin refs/remotes/two-blobs/my-file-blob:dst-blob 2>err &&\n+\t\ttest_i18ngrep \"error: The destination you\" err\n+\t)\n+'\n+\n \n test_done\n-- \n2.19.1.759.g500967bb5e\n\n"},{"id":"361642","messageId":"20181026192734.9609-8-avarab@gmail.com","threadId":"49524","inReplyTo":"xmqq5zy8f6gr.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v2 7/7] push: add DWYM support for \"git push refs/remotes/...:\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-26T19:27:34Z","receivedAt":"2018-10-26T19:28:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add DWYM support for pushing a ref in refs/remotes/* when the \nref is unqualified, e.g.:\n\n git push origin refs/remotes/origin/master:upstream-master\n\nBefore this we wouldn't know what do do with\nrefs/remotes/origin/master, now we'll look it up and discover that\nit's a commit (or tag) and add a refs/{heads,tags}/ prefix to as\nappropriate.\n\nI'm bending over backwards and assuming that someone might have hacked\nin remote tracking tags (see [1] for a discussion of how that can be\ndone), but punting on any tree or blob objects found under\nrefs/remotes/*.\n\nThis is the first use of the %N$ style of printf format in\nthe *.[ch] files in our codebase, but it's supported by POSIX[2] and\nthere's existing uses for it in po/*.po files, so hopefully it won't\ncause any trouble. It's more obvious for translators than to have a\n3rd argument to the function identical to the 2nd, by re-using the 2nd\nit's clear that we're continuing to talk about the part of the\nrefspec.\n\n1. https://public-inbox.org/git/87zi1jxjqn.fsf@evledraar.gmail.com/\n2. http://pubs.opengroup.org/onlinepubs/7908799/xsh/fprintf.html\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n remote.c | 20 +++++++++++++++++++-\n t/t5505-remote.sh | 14 +++++++++-----\n 2 files changed, 28 insertions(+), 6 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 93f802509d..c243e3d89e 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -973,6 +973,21 @@ static char *guess_ref(const char *name, struct ref *peer)\n \t\tstrbuf_addstr(&buf, \"refs/heads/\");\n \t} else if (starts_with(r, \"refs/tags/\")) {\n \t\tstrbuf_addstr(&buf, \"refs/tags/\");\n+\t} else if (starts_with(r, \"refs/remotes/\")) {\n+\t\tstruct object_id oid;\n+\t\tenum object_type type;\n+\n+\t\tif (get_oid(peer->name, &oid))\n+\t\t\tBUG(\"'%s' is not a valid object, \"\n+\t\t\t \"match_explicit_lhs() should catch this!\",\n+\t\t\t peer->name);\n+\t\ttype = oid_object_info(the_repository, &oid, NULL);\n+\t\tif (type == OBJ_COMMIT)\n+\t\t\tstrbuf_addstr(&buf, \"refs/heads/\");\n+\t\telse if (type == OBJ_TAG)\n+\t\t\tstrbuf_addstr(&buf, \"refs/tags/\");\n+\t\telse\n+\t\t\treturn NULL;\n \t} else {\n \t\treturn NULL;\n \t}\n@@ -1024,8 +1039,11 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\t\"- Checking if the being pushed ('%s')\\n\"\n \t\t\" is a ref in \\\"refs/{heads,tags}/\\\". If so we add a corresponding\\n\"\n \t\t\" refs/{heads,tags}/ prefix on the remote side.\\n\"\n+\t\t\"- Checking if the being pushed ('%2$s')\\n\"\n+\t\t\" is a commit or tag in \\\"refs/remotes/*\\\". Then we infer a\\n\"\n+\t\t\" corresponding refs/{heads,tags} on the remote side.\\n\"\n \t\t\"\\n\"\n-\t\t\"Neither worked, so we gave up. You must fully-qualify the ref.\"),\n+\t\t\"None of those worked, so we gave up. You must fully-qualify the ref.\"),\n \t dst_value, matched_src_name);\n \n \tif (!advice_push_unqualified_ref_name)\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 979a13b415..a6337b50e4 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -1260,11 +1260,15 @@ test_expect_success 'refs/remotes/* refspec and unqualified DWIM and\n \t\tgit config --add remote.two.fetch \"+refs/blobs/*:refs/remotes/two-blobs/*\" &&\n \t\tgit fetch --no-tags two &&\n \n-\t\ttest_must_fail git push origin refs/remotes/two/another:dst 2>err &&\n-\t\ttest_i18ngrep \"error: The destination you\" err &&\n-\n-\t\ttest_must_fail git push origin refs/remotes/two-tags/some-tag:dst-tag 2>err &&\n-\t\ttest_i18ngrep \"error: The destination you\" err &&\n+\t\techo commit >expected &&\n+\t\tgit push origin refs/remotes/two/another:dst &&\n+\t\tgit -C ../one cat-file -t refs/heads/dst >actual &&\n+\t\ttest_cmp expected actual &&\n+\n+\t\techo tag >expected &&\n+\t\tgit push origin refs/remotes/two-tags/some-tag:dst-tag &&\n+\t\tgit -C ../one cat-file -t refs/tags/dst-tag >actual &&\n+\t\ttest_cmp expected actual &&\n \n \t\ttest_must_fail git push origin refs/remotes/two-trees/my-head-tree:dst-tree 2>err &&\n \t\ttest_i18ngrep \"error: The destination you\" err &&\n-- \n2.19.1.759.g500967bb5e\n\n"},{"id":"361643","messageId":"20181026192734.9609-5-avarab@gmail.com","threadId":"49524","inReplyTo":"xmqq5zy8f6gr.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v2 4/7] push: move unqualified refname error into a function","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-26T19:27:31Z","receivedAt":"2018-10-26T19:28:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"A follow-up change will extend this error message with the advice\nfacility. Doing so would make the indentation too deeply nested for\ncomfort. So let's split this into a helper function.\n\nThere's no changes to the wording here. Just code moving &\nre-indentation, and re-flowing the \"TRANSLATORS\" comment.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n remote.c | 38 ++++++++++++++++++++++----------------\n 1 file changed, 22 insertions(+), 16 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex f4b438ff74..c7a0b9c46f 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1005,6 +1005,26 @@ static int match_explicit_lhs(struct ref *src,\n \t}\n }\n \n+static void show_push_unqualified_ref_name_error(const char *dst_value,\n+\t\t\t\t\t\t const char *matched_src_name)\n+{\n+\t/*\n+\t * TRANSLATORS: \"matches '%s'%\" is the part of \"git push\n+\t * :\" push, and \"being pushed ('%s')\" is\n+\t * the .\n+\t */\n+\terror(_(\"The destination you provided is not a full refname (i.e.,\\n\"\n+\t\t\"starting with \\\"refs/\\\"). We tried to guess what you meant by:\\n\"\n+\t\t\"\\n\"\n+\t\t\"- Looking for a ref that matches '%s' on the remote side.\\n\"\n+\t\t\"- Checking if the being pushed ('%s')\\n\"\n+\t\t\" is a ref in \\\"refs/{heads,tags}/\\\". If so we add a corresponding\\n\"\n+\t\t\" refs/{heads,tags}/ prefix on the remote side.\\n\"\n+\t\t\"\\n\"\n+\t\t\"Neither worked, so we gave up. You must fully-qualify the ref.\"),\n+\t dst_value, matched_src_name);\n+}\n+\n static int match_explicit(struct ref *src, struct ref *dst,\n \t\t\t struct ref ***dst_tail,\n \t\t\t struct refspec_item *rs)\n@@ -1049,22 +1069,8 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \t\t\tmatched_dst = make_linked_ref(dst_guess, dst_tail);\n \t\t\tfree(dst_guess);\n \t\t} else {\n-\t\t\t/*\n-\t\t\t * TRANSLATORS: \"matches '%s'%\" is the \n-\t\t\t * part of \"git push :\"\n-\t\t\t * push, and \"being pushed ('%s')\" is the\n-\t\t\t * .\n-\t\t\t */\n-\t\t\terror(_(\"The destination you provided is not a full refname (i.e.,\\n\"\n-\t\t\t\t\"starting with \\\"refs/\\\"). We tried to guess what you meant by:\\n\"\n-\t\t\t\t\"\\n\"\n-\t\t\t\t\"- Looking for a ref that matches '%s' on the remote side.\\n\"\n-\t\t\t\t\"- Checking if the being pushed ('%s')\\n\"\n-\t\t\t\t\" is a ref in \\\"refs/{heads,tags}/\\\". If so we add a corresponding\\n\"\n-\t\t\t\t\" refs/{heads,tags}/ prefix on the remote side.\\n\"\n-\t\t\t\t\"\\n\"\n-\t\t\t\t\"Neither worked, so we gave up. You must fully-qualify the ref.\"),\n-\t\t\t dst_value, matched_src->name);\n+\t\t\tshow_push_unqualified_ref_name_error(dst_value,\n+\t\t\t\t\t\t\t matched_src->name);\n \t\t}\n \t\tbreak;\n \tdefault:\n-- \n2.19.1.759.g500967bb5e\n\n"},{"id":"361649","messageId":"CAGZ79kYTHx6vz_YTCLBTBnoxPEV0sCa+Q46OmMVxuY09Dm1Skw@mail.gmail.com","threadId":"49524","inReplyTo":"20181026192734.9609-2-avarab@gmail.com","subject":"Re: [PATCH v2 1/7] remote.c: add braces in anticipation of a follow-up change","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-26T21:05:11Z","receivedAt":"2018-10-26T21:05:26Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Oct 26, 2018 at 12:27 PM Ævar Arnfjörð Bjarmason\n wrote:\n>\n> The CodingGuidelines say \"When there are multiple arms to a\n> conditional and some of them require braces, enclose even a single\n> line block in braces for consistency.\". Fix the code in\n> match_explicit() to conform.\n\nA tangent from a bystander:\n\nThis sounds like a lovely transformation that we'd want to\napply to the whole tree (Who wouldn't want to conform to\nour self-imposed coding guidelines?),\nso I tried coming up with a coccinelle patch to do that,\nbut I did not manage to produce such a patch, yet.\nSo far I am at\n\n@@\nexpression E1;\nstatement S1, S2;\n@@\n {<...\n if (E1) {\n S1;\n-} else\n+} else {\n S2;\n+}\n ...>}\n\nbut this doesn't transform the simple test program that I wrote.\n\n> While I'm at it change the if/else if/else in guess_ref() to use\n> braces. This is not currently needed, but a follow-up change will add\n> a new multi-line condition to that logic.\n\nThe patch looks good.\n\nStefan\n"},{"id":"361651","messageId":"20181026230741.23321-1-avarab@gmail.com","threadId":"49524","inReplyTo":"20181026192734.9609-1-avarab@gmail.com","subject":"[PATCH v3 0/8] fixes for unqualified push","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-26T23:07:33Z","receivedAt":"2018-10-26T23:07:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"After sending out v2 I noticed I didn't update the examples in a\ncouple of the commit messages, and figured I'd finish this up by\nadding a patch to document how this works in the \"git-push\"\nmanpage. This behavior has never been properly documented. range-diff\nwith v2:\n \n 1: ca8eb6dc28 = 1: ca8eb6dc28 remote.c: add braces in anticipation of a follow-up change\n 2: b0e15b6ff1 = 2: b0e15b6ff1 i18n: remote.c: mark error(...) messages for translation\n 3: 052fc5860e = 3: 052fc5860e push: improve the error shown on unqualified push\n 4: e6aa2e360f = 4: e6aa2e360f push: move unqualified refname error into a function\n 5: 57840952b2 ! 5: dcf566e16e push: add an advice on unqualified push\n @@ -13,8 +13,10 @@\n error: The destination you provided is not a full refname (i.e.,\n starting with \"refs/\"). We tried to guess what you meant by:\n \n - - Looking for a ref that matches newbranch on the remote side.\n - - Looking at the refname of the local source.\n + - Looking for a ref that matches 'newbranch' on the remote side.\n + - Checking if the being pushed ('v2.19.0^{commit}')\n + is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n + refs/{heads,tags}/ prefix on the remote side.\n \n Neither worked, so we gave up. You must fully-qualify the ref.\n hint: The part of the refspec is a commit object.\n 6: a2d98855cc = 6: 92ff753437 push: test that doesn't DWYM if is unqualified\n 7: 4e1953da82 ! 7: 58eeb0f3f3 push: add DWYM support for \"git push refs/remotes/...:\"\n @@ -3,22 +3,44 @@\n push: add DWYM support for \"git push refs/remotes/...:\"\n \n Add DWYM support for pushing a ref in refs/remotes/* when the \n - ref is unqualified, e.g.:\n + ref is unqualified. Now instead of erroring out we support e.g.:\n \n - git push origin refs/remotes/origin/master:upstream-master\n + $ ./git-push avar refs/remotes/origin/master:upstream-master -n\n + To github.com:avar/git.git\n + * [new branch] origin/master -> upstream-master\n \n Before this we wouldn't know what do do with\n refs/remotes/origin/master, now we'll look it up and discover that\n it's a commit (or tag) and add a refs/{heads,tags}/ prefix to as\n appropriate.\n \n + The error message emitted when we still don't know what to do has been\n + amended to note that this is something we tried:\n +\n + $ ./git-push avar v2.19.0^{commit}:newbranch -n\n + error: The destination you provided is not a full refname (i.e.,\n + starting with \"refs/\"). We tried to guess what you meant by:\n +\n + - Looking for a ref that matches 'newbranch' on the remote side.\n + - Checking if the being pushed ('v2.19.0^{commit}')\n + is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n + refs/{heads,tags}/ prefix on the remote side.\n + - Checking if the being pushed ('v2.19.0^{commit}')\n + is a commit or tag in \"refs/remotes/*\". Then we infer a\n + corresponding refs/{heads,tags} on the remote side.\n +\n + None of those worked, so we gave up. You must fully-qualify the ref.\n + hint: The part of the refspec is a commit object.\n + hint: Did you mean to create a new branch by pushing to\n + hint: 'v2.19.0^{commit}:refs/heads/newbranch'?\n +\n I'm bending over backwards and assuming that someone might have hacked\n in remote tracking tags (see [1] for a discussion of how that can be\n done), but punting on any tree or blob objects found under\n refs/remotes/*.\n \n This is the first use of the %N$ style of printf format in\n - the *.[ch] files in our codebase, but it's supported by POSIX[2] and\n + the *.[ch] files in our codebase. It's supported by POSIX[2] and\n there's existing uses for it in po/*.po files, so hopefully it won't\n cause any trouble. It's more obvious for translators than to have a\n 3rd argument to the function identical to the 2nd, by re-using the 2nd\n -: ---------- > 8: bc171b0312 push doc: document the DWYM behavior pushing to unqualified \n\nÆvar Arnfjörð Bjarmason (8):\n remote.c: add braces in anticipation of a follow-up change\n i18n: remote.c: mark error(...) messages for translation\n push: improve the error shown on unqualified push\n push: move unqualified refname error into a function\n push: add an advice on unqualified push\n push: test that doesn't DWYM if is unqualified\n push: add DWYM support for \"git push refs/remotes/...:\"\n push doc: document the DWYM behavior pushing to unqualified \n\n Documentation/config.txt | 7 +++\n Documentation/git-push.txt | 27 ++++++++\n advice.c | 2 +\n advice.h | 1 +\n remote.c | 124 +++++++++++++++++++++++++++++--------\n t/t5505-remote.sh | 57 +++++++++++++++++\n 6 files changed, 193 insertions(+), 25 deletions(-)\n\n-- \n2.19.1.759.g500967bb5e\n\n"},{"id":"361652","messageId":"20181026230741.23321-2-avarab@gmail.com","threadId":"49524","inReplyTo":"20181026192734.9609-1-avarab@gmail.com","subject":"[PATCH v3 1/8] remote.c: add braces in anticipation of a follow-up change","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-26T23:07:34Z","receivedAt":"2018-10-26T23:07:55Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The CodingGuidelines say \"When there are multiple arms to a\nconditional and some of them require braces, enclose even a single\nline block in braces for consistency.\". Fix the code in\nmatch_explicit() to conform.\n\nWhile I'm at it change the if/else if/else in guess_ref() to use\nbraces. This is not currently needed, but a follow-up change will add\na new multi-line condition to that logic.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n remote.c | 16 +++++++++-------\n 1 file changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 81f4f01b00..18cae48daa 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -968,12 +968,13 @@ static char *guess_ref(const char *name, struct ref *peer)\n \tif (!r)\n \t\treturn NULL;\n \n-\tif (starts_with(r, \"refs/heads/\"))\n+\tif (starts_with(r, \"refs/heads/\")) {\n \t\tstrbuf_addstr(&buf, \"refs/heads/\");\n-\telse if (starts_with(r, \"refs/tags/\"))\n+\t} else if (starts_with(r, \"refs/tags/\")) {\n \t\tstrbuf_addstr(&buf, \"refs/tags/\");\n-\telse\n+\t} else {\n \t\treturn NULL;\n+\t}\n \n \tstrbuf_addstr(&buf, name);\n \treturn strbuf_detach(&buf, NULL);\n@@ -1038,21 +1039,22 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \tcase 1:\n \t\tbreak;\n \tcase 0:\n-\t\tif (starts_with(dst_value, \"refs/\"))\n+\t\tif (starts_with(dst_value, \"refs/\")) {\n \t\t\tmatched_dst = make_linked_ref(dst_value, dst_tail);\n-\t\telse if (is_null_oid(&matched_src->new_oid))\n+\t\t} else if (is_null_oid(&matched_src->new_oid)) {\n \t\t\terror(\"unable to delete '%s': remote ref does not exist\",\n \t\t\t dst_value);\n-\t\telse if ((dst_guess = guess_ref(dst_value, matched_src))) {\n+\t\t} else if ((dst_guess = guess_ref(dst_value, matched_src))) {\n \t\t\tmatched_dst = make_linked_ref(dst_guess, dst_tail);\n \t\t\tfree(dst_guess);\n-\t\t} else\n+\t\t} else {\n \t\t\terror(\"unable to push to unqualified destination: %s\\n\"\n \t\t\t \"The destination refspec neither matches an \"\n \t\t\t \"existing ref on the remote nor\\n\"\n \t\t\t \"begins with refs/, and we are unable to \"\n \t\t\t \"guess a prefix based on the source ref.\",\n \t\t\t dst_value);\n+\t\t}\n \t\tbreak;\n \tdefault:\n \t\tmatched_dst = NULL;\n-- \n2.19.1.759.g500967bb5e\n\n"},{"id":"361653","messageId":"20181026230741.23321-3-avarab@gmail.com","threadId":"49524","inReplyTo":"20181026192734.9609-1-avarab@gmail.com","subject":"[PATCH v3 2/8] i18n: remote.c: mark error(...) messages for translation","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-26T23:07:35Z","receivedAt":"2018-10-26T23:07:57Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Mark up the error(...) messages in remote.c for translation. The likes\nof \"unable to push to unqualified destination\" are relatively big\nparts of the UI, i.e. error messages shown when \"git push\" fails.\n\nI don't think any of these are plumbing, an the entire test suite\npasses when running the tests with GIT_GETTEXT_POISON=1 (after\nbuilding with GETTEXT_POISON).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n remote.c | 35 ++++++++++++++++++-----------------\n 1 file changed, 18 insertions(+), 17 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 18cae48daa..5cb3d00bfb 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -406,7 +406,7 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t\tif (!remote->receivepack)\n \t\t\tremote->receivepack = v;\n \t\telse\n-\t\t\terror(\"more than one receivepack given, using the first\");\n+\t\t\terror(_(\"more than one receivepack given, using the first\"));\n \t} else if (!strcmp(subkey, \"uploadpack\")) {\n \t\tconst char *v;\n \t\tif (git_config_string(&v, key, value))\n@@ -414,7 +414,7 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t\tif (!remote->uploadpack)\n \t\t\tremote->uploadpack = v;\n \t\telse\n-\t\t\terror(\"more than one uploadpack given, using the first\");\n+\t\t\terror(_(\"more than one uploadpack given, using the first\"));\n \t} else if (!strcmp(subkey, \"tagopt\")) {\n \t\tif (!strcmp(value, \"--no-tags\"))\n \t\t\tremote->fetch_tags = -1;\n@@ -707,7 +707,7 @@ static void query_refspecs_multiple(struct refspec *rs,\n \tint find_src = !query->src;\n \n \tif (find_src && !query->dst)\n-\t\terror(\"query_refspecs_multiple: need either src or dst\");\n+\t\terror(_(\"query_refspecs_multiple: need either src or dst\"));\n \n \tfor (i = 0; i < rs->nr; i++) {\n \t\tstruct refspec_item *refspec = &rs->items[i];\n@@ -735,7 +735,7 @@ int query_refspecs(struct refspec *rs, struct refspec_item *query)\n \tchar **result = find_src ? &query->src : &query->dst;\n \n \tif (find_src && !query->dst)\n-\t\treturn error(\"query_refspecs: need either src or dst\");\n+\t\treturn error(_(\"query_refspecs: need either src or dst\"));\n \n \tfor (i = 0; i < rs->nr; i++) {\n \t\tstruct refspec_item *refspec = &rs->items[i];\n@@ -996,12 +996,12 @@ static int match_explicit_lhs(struct ref *src,\n \t\t * way to delete 'other' ref at the remote end.\n \t\t */\n \t\tif (try_explicit_object_name(rs->src, match) < 0)\n-\t\t\treturn error(\"src refspec %s does not match any.\", rs->src);\n+\t\t\treturn error(_(\"src refspec %s does not match any.\"), rs->src);\n \t\tif (allocated_match)\n \t\t\t*allocated_match = 1;\n \t\treturn 0;\n \tdefault:\n-\t\treturn error(\"src refspec %s matches more than one.\", rs->src);\n+\t\treturn error(_(\"src refspec %s matches more than one.\"), rs->src);\n \t}\n }\n \n@@ -1041,32 +1041,33 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \tcase 0:\n \t\tif (starts_with(dst_value, \"refs/\")) {\n \t\t\tmatched_dst = make_linked_ref(dst_value, dst_tail);\n+\n \t\t} else if (is_null_oid(&matched_src->new_oid)) {\n-\t\t\terror(\"unable to delete '%s': remote ref does not exist\",\n+\t\t\terror(_(\"unable to delete '%s': remote ref does not exist\"),\n \t\t\t dst_value);\n \t\t} else if ((dst_guess = guess_ref(dst_value, matched_src))) {\n \t\t\tmatched_dst = make_linked_ref(dst_guess, dst_tail);\n \t\t\tfree(dst_guess);\n \t\t} else {\n-\t\t\terror(\"unable to push to unqualified destination: %s\\n\"\n-\t\t\t \"The destination refspec neither matches an \"\n-\t\t\t \"existing ref on the remote nor\\n\"\n-\t\t\t \"begins with refs/, and we are unable to \"\n-\t\t\t \"guess a prefix based on the source ref.\",\n+\t\t\terror(_(\"unable to push to unqualified destination: %s\\n\"\n+\t\t\t\t\"The destination refspec neither matches an \"\n+\t\t\t\t\"existing ref on the remote nor\\n\"\n+\t\t\t\t\"begins with refs/, and we are unable to \"\n+\t\t\t\t\"guess a prefix based on the source ref.\"),\n \t\t\t dst_value);\n \t\t}\n \t\tbreak;\n \tdefault:\n \t\tmatched_dst = NULL;\n-\t\terror(\"dst refspec %s matches more than one.\",\n+\t\terror(_(\"dst refspec %s matches more than one.\"),\n \t\t dst_value);\n \t\tbreak;\n \t}\n \tif (!matched_dst)\n \t\treturn -1;\n \tif (matched_dst->peer_ref)\n-\t\treturn error(\"dst ref %s receives from more than one src.\",\n-\t\t matched_dst->name);\n+\t\treturn error(_(\"dst ref %s receives from more than one src.\"),\n+\t\t\t matched_dst->name);\n \telse {\n \t\tmatched_dst->peer_ref = allocated_src ?\n \t\t\t\t\tmatched_src :\n@@ -1765,7 +1766,7 @@ int get_fetch_map(const struct ref *remote_refs,\n \t\t\tif (!starts_with((*rmp)->peer_ref->name, \"refs/\") ||\n \t\t\t check_refname_format((*rmp)->peer_ref->name, 0)) {\n \t\t\t\tstruct ref *ignore = *rmp;\n-\t\t\t\terror(\"* Ignoring funny ref '%s' locally\",\n+\t\t\t\terror(_(\"* Ignoring funny ref '%s' locally\"),\n \t\t\t\t (*rmp)->peer_ref->name);\n \t\t\t\t*rmp = (*rmp)->next;\n \t\t\t\tfree(ignore->peer_ref);\n@@ -2133,7 +2134,7 @@ static int parse_push_cas_option(struct push_cas_option *cas, const char *arg, i\n \telse if (!colon[1])\n \t\toidclr(&entry->expect);\n \telse if (get_oid(colon + 1, &entry->expect))\n-\t\treturn error(\"cannot parse expected object name '%s'\", colon + 1);\n+\t\treturn error(_(\"cannot parse expected object name '%s'\"), colon + 1);\n \treturn 0;\n }\n \n-- \n2.19.1.759.g500967bb5e\n\n"},{"id":"361654","messageId":"20181026230741.23321-4-avarab@gmail.com","threadId":"49524","inReplyTo":"20181026192734.9609-1-avarab@gmail.com","subject":"[PATCH v3 3/8] push: improve the error shown on unqualified push","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-26T23:07:36Z","receivedAt":"2018-10-26T23:07:58Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Improve the error message added in f8aae12034 (\"push: allow\nunqualified dest refspecs to DWIM\", 2008-04-23), which before this\nchange looks like this:\n\n $ git push avar v2.19.0^{commit}:newbranch -n\n error: unable to push to unqualified destination: newbranch\n The destination refspec neither matches an existing ref on the remote nor\n begins with refs/, and we are unable to guess a prefix based on the source ref.\n error: failed to push some refs to 'git@github.com:avar/git.git'\n\nThis message needed to be read very carefully to spot how to fix the\nerror, i.e. to push to refs/heads/newbranch. Now the message will look\nlike this instead:\n\n $ ./git-push avar v2.19.0^{commit}:newbranch -n\n error: The destination you provided is not a full refname (i.e.,\n starting with \"refs/\"). We tried to guess what you meant by:\n\n - Looking for a ref that matches 'newbranch' on the remote side.\n - Checking if the being pushed ('v2.19.0^{commit}')\n is a ref in \"refs/{heads,tags}/\". If so we add a\n corresponding refs/{heads,tags}/ prefix on the remote side.\n\n Neither worked, so we gave up. You must fully-qualify the ref.\n error: failed to push some refs to 'git@github.com:avar/git.git'\n\nThis improvement is the result of on-list discussion in [1] and [2],\nas well as my own fixes / reformatting / phrasing on top.\n\nThe suggestion by Jeff on-list was to make that second bullet point\n\"Looking at the refname of the local source.\". The version being added\nhere is more verbose, but also more accurate. saying \"local source\"\ncould refer to any ref in the local refstore, including something in\nrefs/remotes/*. A later change will teach guess_ref() to infer a ref\ntype from remote-tracking refs, so let's not confuse the two.\n\nWhile I'm at it, add a \"TRANSLATORS\" comment since the message has\ngotten more complex and it's not as clear what the two %s's refer to.\n\n1. https://public-inbox.org/git/20181010205505.GB12949@sigill.intra.peff.net/\n2. https://public-inbox.org/git/xmqqbm81lb7c.fsf@gitster-ct.c.googlers.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n remote.c | 22 ++++++++++++++++------\n 1 file changed, 16 insertions(+), 6 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 5cb3d00bfb..f4b438ff74 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1049,12 +1049,22 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \t\t\tmatched_dst = make_linked_ref(dst_guess, dst_tail);\n \t\t\tfree(dst_guess);\n \t\t} else {\n-\t\t\terror(_(\"unable to push to unqualified destination: %s\\n\"\n-\t\t\t\t\"The destination refspec neither matches an \"\n-\t\t\t\t\"existing ref on the remote nor\\n\"\n-\t\t\t\t\"begins with refs/, and we are unable to \"\n-\t\t\t\t\"guess a prefix based on the source ref.\"),\n-\t\t\t dst_value);\n+\t\t\t/*\n+\t\t\t * TRANSLATORS: \"matches '%s'%\" is the \n+\t\t\t * part of \"git push :\"\n+\t\t\t * push, and \"being pushed ('%s')\" is the\n+\t\t\t * .\n+\t\t\t */\n+\t\t\terror(_(\"The destination you provided is not a full refname (i.e.,\\n\"\n+\t\t\t\t\"starting with \\\"refs/\\\"). We tried to guess what you meant by:\\n\"\n+\t\t\t\t\"\\n\"\n+\t\t\t\t\"- Looking for a ref that matches '%s' on the remote side.\\n\"\n+\t\t\t\t\"- Checking if the being pushed ('%s')\\n\"\n+\t\t\t\t\" is a ref in \\\"refs/{heads,tags}/\\\". If so we add a corresponding\\n\"\n+\t\t\t\t\" refs/{heads,tags}/ prefix on the remote side.\\n\"\n+\t\t\t\t\"\\n\"\n+\t\t\t\t\"Neither worked, so we gave up. You must fully-qualify the ref.\"),\n+\t\t\t dst_value, matched_src->name);\n \t\t}\n \t\tbreak;\n \tdefault:\n-- \n2.19.1.759.g500967bb5e\n\n"},{"id":"361655","messageId":"20181026230741.23321-5-avarab@gmail.com","threadId":"49524","inReplyTo":"20181026192734.9609-1-avarab@gmail.com","subject":"[PATCH v3 4/8] push: move unqualified refname error into a function","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-26T23:07:37Z","receivedAt":"2018-10-26T23:07:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"A follow-up change will extend this error message with the advice\nfacility. Doing so would make the indentation too deeply nested for\ncomfort. So let's split this into a helper function.\n\nThere's no changes to the wording here. Just code moving &\nre-indentation, and re-flowing the \"TRANSLATORS\" comment.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n remote.c | 38 ++++++++++++++++++++++----------------\n 1 file changed, 22 insertions(+), 16 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex f4b438ff74..c7a0b9c46f 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1005,6 +1005,26 @@ static int match_explicit_lhs(struct ref *src,\n \t}\n }\n \n+static void show_push_unqualified_ref_name_error(const char *dst_value,\n+\t\t\t\t\t\t const char *matched_src_name)\n+{\n+\t/*\n+\t * TRANSLATORS: \"matches '%s'%\" is the part of \"git push\n+\t * :\" push, and \"being pushed ('%s')\" is\n+\t * the .\n+\t */\n+\terror(_(\"The destination you provided is not a full refname (i.e.,\\n\"\n+\t\t\"starting with \\\"refs/\\\"). We tried to guess what you meant by:\\n\"\n+\t\t\"\\n\"\n+\t\t\"- Looking for a ref that matches '%s' on the remote side.\\n\"\n+\t\t\"- Checking if the being pushed ('%s')\\n\"\n+\t\t\" is a ref in \\\"refs/{heads,tags}/\\\". If so we add a corresponding\\n\"\n+\t\t\" refs/{heads,tags}/ prefix on the remote side.\\n\"\n+\t\t\"\\n\"\n+\t\t\"Neither worked, so we gave up. You must fully-qualify the ref.\"),\n+\t dst_value, matched_src_name);\n+}\n+\n static int match_explicit(struct ref *src, struct ref *dst,\n \t\t\t struct ref ***dst_tail,\n \t\t\t struct refspec_item *rs)\n@@ -1049,22 +1069,8 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \t\t\tmatched_dst = make_linked_ref(dst_guess, dst_tail);\n \t\t\tfree(dst_guess);\n \t\t} else {\n-\t\t\t/*\n-\t\t\t * TRANSLATORS: \"matches '%s'%\" is the \n-\t\t\t * part of \"git push :\"\n-\t\t\t * push, and \"being pushed ('%s')\" is the\n-\t\t\t * .\n-\t\t\t */\n-\t\t\terror(_(\"The destination you provided is not a full refname (i.e.,\\n\"\n-\t\t\t\t\"starting with \\\"refs/\\\"). We tried to guess what you meant by:\\n\"\n-\t\t\t\t\"\\n\"\n-\t\t\t\t\"- Looking for a ref that matches '%s' on the remote side.\\n\"\n-\t\t\t\t\"- Checking if the being pushed ('%s')\\n\"\n-\t\t\t\t\" is a ref in \\\"refs/{heads,tags}/\\\". If so we add a corresponding\\n\"\n-\t\t\t\t\" refs/{heads,tags}/ prefix on the remote side.\\n\"\n-\t\t\t\t\"\\n\"\n-\t\t\t\t\"Neither worked, so we gave up. You must fully-qualify the ref.\"),\n-\t\t\t dst_value, matched_src->name);\n+\t\t\tshow_push_unqualified_ref_name_error(dst_value,\n+\t\t\t\t\t\t\t matched_src->name);\n \t\t}\n \t\tbreak;\n \tdefault:\n-- \n2.19.1.759.g500967bb5e\n\n"},{"id":"361656","messageId":"20181026230741.23321-7-avarab@gmail.com","threadId":"49524","inReplyTo":"20181026192734.9609-1-avarab@gmail.com","subject":"[PATCH v3 6/8] push: test that doesn't DWYM if is unqualified","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-26T23:07:39Z","receivedAt":"2018-10-26T23:08:02Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a test asserting that \"git push origin :\" where is\na branch, tag, tree or blob in refs/remotes/* doesn't DWYM when \nis unqualified. This has never worked, but there's been no test for\nthis behavior.\n\nSee f88395ac23 (\"Renaming push.\", 2005-08-03), bb9fca80ce (\"git-push:\nUpdate description of refspecs and add examples\", 2007-06-09) and\nf8aae12034 (\"push: allow unqualified dest refspecs to DWIM\",\n2008-04-23) which are most relevant commits that have changed or\ndocumented the behavior of this feature in the past.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t5505-remote.sh | 28 ++++++++++++++++++++++++++++\n 1 file changed, 28 insertions(+)\n\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 2e58721f98..979a13b415 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -1246,5 +1246,33 @@ test_expect_success 'unqualified refspec DWIM and advice' '\n \t)\n '\n \n+test_expect_success 'refs/remotes/* refspec and unqualified DWIM and advice' '\n+\t(\n+\t\tcd two &&\n+\t\tgit tag -a -m \"Some tag\" some-tag master &&\n+\t\tgit update-ref refs/trees/my-head-tree HEAD^{tree} &&\n+\t\tgit update-ref refs/blobs/my-file-blob HEAD:file\n+\t) &&\n+\t(\n+\t\tcd test &&\n+\t\tgit config --add remote.two.fetch \"+refs/tags/*:refs/remotes/two-tags/*\" &&\n+\t\tgit config --add remote.two.fetch \"+refs/trees/*:refs/remotes/two-trees/*\" &&\n+\t\tgit config --add remote.two.fetch \"+refs/blobs/*:refs/remotes/two-blobs/*\" &&\n+\t\tgit fetch --no-tags two &&\n+\n+\t\ttest_must_fail git push origin refs/remotes/two/another:dst 2>err &&\n+\t\ttest_i18ngrep \"error: The destination you\" err &&\n+\n+\t\ttest_must_fail git push origin refs/remotes/two-tags/some-tag:dst-tag 2>err &&\n+\t\ttest_i18ngrep \"error: The destination you\" err &&\n+\n+\t\ttest_must_fail git push origin refs/remotes/two-trees/my-head-tree:dst-tree 2>err &&\n+\t\ttest_i18ngrep \"error: The destination you\" err &&\n+\n+\t\ttest_must_fail git push origin refs/remotes/two-blobs/my-file-blob:dst-blob 2>err &&\n+\t\ttest_i18ngrep \"error: The destination you\" err\n+\t)\n+'\n+\n \n test_done\n-- \n2.19.1.759.g500967bb5e\n\n"},{"id":"361657","messageId":"20181026230741.23321-6-avarab@gmail.com","threadId":"49524","inReplyTo":"20181026192734.9609-1-avarab@gmail.com","subject":"[PATCH v3 5/8] push: add an advice on unqualified push","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-26T23:07:38Z","receivedAt":"2018-10-26T23:08:02Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add an advice to the recently improved error message added in\nf8aae12034 (\"push: allow unqualified dest refspecs to DWIM\",\n2008-04-23).\n\nNow with advice.pushUnqualifiedRefName=true (on by default) we show a\nhint about how to proceed:\n\n $ ./git-push avar v2.19.0^{commit}:newbranch -n\n error: The destination you provided is not a full refname (i.e.,\n starting with \"refs/\"). We tried to guess what you meant by:\n\n - Looking for a ref that matches 'newbranch' on the remote side.\n - Checking if the being pushed ('v2.19.0^{commit}')\n is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n refs/{heads,tags}/ prefix on the remote side.\n\n Neither worked, so we gave up. You must fully-qualify the ref.\n hint: The part of the refspec is a commit object.\n hint: Did you mean to create a new branch by pushing to\n hint: 'v2.19.0^{commit}:refs/heads/newbranch'?\n error: failed to push some refs to 'git@github.com:avar/git.git'\n\nWhen trying to push a tag, tree or a blob we suggest that perhaps the\nuser meant to push them to refs/tags/ instead.\n\nThe if/else duplication for all of OBJ_{COMMIT,TAG,TREE,BLOB} is\nunfortunate, but is required to correctly mark the messages for\ntranslation. See the discussion in\n<87r2gxebsi.fsf@evledraar.gmail.com> about that.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/config.txt | 7 +++++++\n advice.c | 2 ++\n advice.h | 1 +\n remote.c | 37 +++++++++++++++++++++++++++++++++++++\n t/t5505-remote.sh | 25 +++++++++++++++++++++++++\n 5 files changed, 72 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 552827935a..8ca465702e 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -320,6 +320,13 @@ advice.*::\n \t\ttries to overwrite a remote ref that points at an\n \t\tobject that is not a commit-ish, or make the remote\n \t\tref point at an object that is not a commit-ish.\n+\tpushUnqualifiedRefname::\n+\t\tShown when linkgit:git-push[1] gives up trying to\n+\t\tguess based on the source and destination refs what\n+\t\tremote ref namespace the source belongs in, but where\n+\t\twe can still suggest that the user push to either\n+\t\trefs/heads/* or refs/tags/* based on the type of the\n+\t\tsource object.\n \tstatusHints::\n \t\tShow directions on how to proceed from the current\n \t\tstate in the output of linkgit:git-status[1], in\ndiff --git a/advice.c b/advice.c\nindex 3561cd64e9..3089a4ca65 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -9,6 +9,7 @@ int advice_push_non_ff_matching = 1;\n int advice_push_already_exists = 1;\n int advice_push_fetch_first = 1;\n int advice_push_needs_force = 1;\n+int advice_push_unqualified_ref_name = 1;\n int advice_status_hints = 1;\n int advice_status_u_option = 1;\n int advice_commit_before_merge = 1;\n@@ -62,6 +63,7 @@ static struct {\n \t{ \"pushAlreadyExists\", &advice_push_already_exists },\n \t{ \"pushFetchFirst\", &advice_push_fetch_first },\n \t{ \"pushNeedsForce\", &advice_push_needs_force },\n+\t{ \"pushUnqualifiedRefName\", &advice_push_unqualified_ref_name },\n \t{ \"statusHints\", &advice_status_hints },\n \t{ \"statusUoption\", &advice_status_u_option },\n \t{ \"commitBeforeMerge\", &advice_commit_before_merge },\ndiff --git a/advice.h b/advice.h\nindex ab24df0fd0..9a2f8b5226 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -9,6 +9,7 @@ extern int advice_push_non_ff_matching;\n extern int advice_push_already_exists;\n extern int advice_push_fetch_first;\n extern int advice_push_needs_force;\n+extern int advice_push_unqualified_ref_name;\n extern int advice_status_hints;\n extern int advice_status_u_option;\n extern int advice_commit_before_merge;\ndiff --git a/remote.c b/remote.c\nindex c7a0b9c46f..93f802509d 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -13,6 +13,7 @@\n #include \"mergesort.h\"\n #include \"argv-array.h\"\n #include \"commit-reach.h\"\n+#include \"advice.h\"\n \n enum map_direction { FROM_SRC, FROM_DST };\n \n@@ -1008,6 +1009,9 @@ static int match_explicit_lhs(struct ref *src,\n static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\t\t\t\t\t const char *matched_src_name)\n {\n+\tstruct object_id oid;\n+\tenum object_type type;\n+\n \t/*\n \t * TRANSLATORS: \"matches '%s'%\" is the part of \"git push\n \t * :\" push, and \"being pushed ('%s')\" is\n@@ -1023,6 +1027,39 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\t\"\\n\"\n \t\t\"Neither worked, so we gave up. You must fully-qualify the ref.\"),\n \t dst_value, matched_src_name);\n+\n+\tif (!advice_push_unqualified_ref_name)\n+\t\treturn;\n+\n+\tif (get_oid(matched_src_name, &oid))\n+\t\tBUG(\"'%s' is not a valid object, \"\n+\t\t \"match_explicit_lhs() should catch this!\",\n+\t\t matched_src_name);\n+\ttype = oid_object_info(the_repository, &oid, NULL);\n+\tif (type == OBJ_COMMIT) {\n+\t\tadvise(_(\"The part of the refspec is a commit object.\\n\"\n+\t\t\t \"Did you mean to create a new branch by pushing to\\n\"\n+\t\t\t \"'%s:refs/heads/%s'?\"),\n+\t\t matched_src_name, dst_value);\n+\t} else if (type == OBJ_TAG) {\n+\t\tadvise(_(\"The part of the refspec is a tag object.\\n\"\n+\t\t\t \"Did you mean to create a new tag by pushing to\\n\"\n+\t\t\t \"'%s:refs/tags/%s'?\"),\n+\t\t matched_src_name, dst_value);\n+\t} else if (type == OBJ_TREE) {\n+\t\tadvise(_(\"The part of the refspec is a tree object.\\n\"\n+\t\t\t \"Did you mean to tag a new tree by pushing to\\n\"\n+\t\t\t \"'%s:refs/tags/%s'?\"),\n+\t\t matched_src_name, dst_value);\n+\t} else if (type == OBJ_BLOB) {\n+\t\tadvise(_(\"The part of the refspec is a blob object.\\n\"\n+\t\t\t \"Did you mean to tag a new blob by pushing to\\n\"\n+\t\t\t \"'%s:refs/tags/%s'?\"),\n+\t\t matched_src_name, dst_value);\n+\t} else {\n+\t\tBUG(\"'%s' should be commit/tag/tree/blob, is '%d'\",\n+\t\t matched_src_name, type);\n+\t}\n }\n \n static int match_explicit(struct ref *src, struct ref *dst,\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex d2a2cdd453..2e58721f98 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -1222,4 +1222,29 @@ test_expect_success 'add remote matching the \"insteadOf\" URL' '\n \tgit remote add backup xyz@example.com\n '\n \n+test_expect_success 'unqualified refspec DWIM and advice' '\n+\ttest_when_finished \"(cd test && git tag -d some-tag)\" &&\n+\t(\n+\t\tcd test &&\n+\t\tgit tag -a -m \"Some tag\" some-tag master &&\n+\t\tfor type in commit tag tree blob\n+\t\tdo\n+\t\t\tif test \"$type\" = \"blob\"\n+\t\t\tthen\n+\t\t\t\toid=$(git rev-parse some-tag:file)\n+\t\t\telse\n+\t\t\t\toid=$(git rev-parse some-tag^{$type})\n+\t\t\tfi &&\n+\t\t\ttest_must_fail git push origin $oid:dst 2>err &&\n+\t\t\ttest_i18ngrep \"error: The destination you\" err &&\n+\t\t\ttest_i18ngrep \"hint: Did you mean\" err &&\n+\t\t\ttest_must_fail git -c advice.pushUnqualifiedRefName=false \\\n+\t\t\t\tpush origin $oid:dst 2>err &&\n+\t\t\ttest_i18ngrep \"error: The destination you\" err &&\n+\t\t\ttest_i18ngrep ! \"hint: Did you mean\" err\n+\t\tdone\n+\t)\n+'\n+\n+\n test_done\n-- \n2.19.1.759.g500967bb5e\n\n"},{"id":"361658","messageId":"20181026230741.23321-8-avarab@gmail.com","threadId":"49524","inReplyTo":"20181026192734.9609-1-avarab@gmail.com","subject":"[PATCH v3 7/8] push: add DWYM support for \"git push refs/remotes/...:\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-26T23:07:40Z","receivedAt":"2018-10-26T23:08:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add DWYM support for pushing a ref in refs/remotes/* when the \nref is unqualified. Now instead of erroring out we support e.g.:\n\n $ ./git-push avar refs/remotes/origin/master:upstream-master -n\n To github.com:avar/git.git\n * [new branch] origin/master -> upstream-master\n\nBefore this we wouldn't know what do do with\nrefs/remotes/origin/master, now we'll look it up and discover that\nit's a commit (or tag) and add a refs/{heads,tags}/ prefix to as\nappropriate.\n\nThe error message emitted when we still don't know what to do has been\namended to note that this is something we tried:\n\n $ ./git-push avar v2.19.0^{commit}:newbranch -n\n error: The destination you provided is not a full refname (i.e.,\n starting with \"refs/\"). We tried to guess what you meant by:\n\n - Looking for a ref that matches 'newbranch' on the remote side.\n - Checking if the being pushed ('v2.19.0^{commit}')\n is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n refs/{heads,tags}/ prefix on the remote side.\n - Checking if the being pushed ('v2.19.0^{commit}')\n is a commit or tag in \"refs/remotes/*\". Then we infer a\n corresponding refs/{heads,tags} on the remote side.\n\n None of those worked, so we gave up. You must fully-qualify the ref.\n hint: The part of the refspec is a commit object.\n hint: Did you mean to create a new branch by pushing to\n hint: 'v2.19.0^{commit}:refs/heads/newbranch'?\n\nI'm bending over backwards and assuming that someone might have hacked\nin remote tracking tags (see [1] for a discussion of how that can be\ndone), but punting on any tree or blob objects found under\nrefs/remotes/*.\n\nThis is the first use of the %N$ style of printf format in\nthe *.[ch] files in our codebase. It's supported by POSIX[2] and\nthere's existing uses for it in po/*.po files, so hopefully it won't\ncause any trouble. It's more obvious for translators than to have a\n3rd argument to the function identical to the 2nd, by re-using the 2nd\nit's clear that we're continuing to talk about the part of the\nrefspec.\n\n1. https://public-inbox.org/git/87zi1jxjqn.fsf@evledraar.gmail.com/\n2. http://pubs.opengroup.org/onlinepubs/7908799/xsh/fprintf.html\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n remote.c | 20 +++++++++++++++++++-\n t/t5505-remote.sh | 14 +++++++++-----\n 2 files changed, 28 insertions(+), 6 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 93f802509d..c243e3d89e 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -973,6 +973,21 @@ static char *guess_ref(const char *name, struct ref *peer)\n \t\tstrbuf_addstr(&buf, \"refs/heads/\");\n \t} else if (starts_with(r, \"refs/tags/\")) {\n \t\tstrbuf_addstr(&buf, \"refs/tags/\");\n+\t} else if (starts_with(r, \"refs/remotes/\")) {\n+\t\tstruct object_id oid;\n+\t\tenum object_type type;\n+\n+\t\tif (get_oid(peer->name, &oid))\n+\t\t\tBUG(\"'%s' is not a valid object, \"\n+\t\t\t \"match_explicit_lhs() should catch this!\",\n+\t\t\t peer->name);\n+\t\ttype = oid_object_info(the_repository, &oid, NULL);\n+\t\tif (type == OBJ_COMMIT)\n+\t\t\tstrbuf_addstr(&buf, \"refs/heads/\");\n+\t\telse if (type == OBJ_TAG)\n+\t\t\tstrbuf_addstr(&buf, \"refs/tags/\");\n+\t\telse\n+\t\t\treturn NULL;\n \t} else {\n \t\treturn NULL;\n \t}\n@@ -1024,8 +1039,11 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\t\"- Checking if the being pushed ('%s')\\n\"\n \t\t\" is a ref in \\\"refs/{heads,tags}/\\\". If so we add a corresponding\\n\"\n \t\t\" refs/{heads,tags}/ prefix on the remote side.\\n\"\n+\t\t\"- Checking if the being pushed ('%2$s')\\n\"\n+\t\t\" is a commit or tag in \\\"refs/remotes/*\\\". Then we infer a\\n\"\n+\t\t\" corresponding refs/{heads,tags} on the remote side.\\n\"\n \t\t\"\\n\"\n-\t\t\"Neither worked, so we gave up. You must fully-qualify the ref.\"),\n+\t\t\"None of those worked, so we gave up. You must fully-qualify the ref.\"),\n \t dst_value, matched_src_name);\n \n \tif (!advice_push_unqualified_ref_name)\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 979a13b415..a6337b50e4 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -1260,11 +1260,15 @@ test_expect_success 'refs/remotes/* refspec and unqualified DWIM and\n \t\tgit config --add remote.two.fetch \"+refs/blobs/*:refs/remotes/two-blobs/*\" &&\n \t\tgit fetch --no-tags two &&\n \n-\t\ttest_must_fail git push origin refs/remotes/two/another:dst 2>err &&\n-\t\ttest_i18ngrep \"error: The destination you\" err &&\n-\n-\t\ttest_must_fail git push origin refs/remotes/two-tags/some-tag:dst-tag 2>err &&\n-\t\ttest_i18ngrep \"error: The destination you\" err &&\n+\t\techo commit >expected &&\n+\t\tgit push origin refs/remotes/two/another:dst &&\n+\t\tgit -C ../one cat-file -t refs/heads/dst >actual &&\n+\t\ttest_cmp expected actual &&\n+\n+\t\techo tag >expected &&\n+\t\tgit push origin refs/remotes/two-tags/some-tag:dst-tag &&\n+\t\tgit -C ../one cat-file -t refs/tags/dst-tag >actual &&\n+\t\ttest_cmp expected actual &&\n \n \t\ttest_must_fail git push origin refs/remotes/two-trees/my-head-tree:dst-tree 2>err &&\n \t\ttest_i18ngrep \"error: The destination you\" err &&\n-- \n2.19.1.759.g500967bb5e\n\n"},{"id":"361659","messageId":"20181026230741.23321-9-avarab@gmail.com","threadId":"49524","inReplyTo":"20181026192734.9609-1-avarab@gmail.com","subject":"[PATCH v3 8/8] push doc: document the DWYM behavior pushing to unqualified ","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-26T23:07:41Z","receivedAt":"2018-10-26T23:08:06Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Document the DWYM behavior that kicks in when pushing to an\nunqualified reference.\n\nThis behavior was added in f88395ac23 (\"Renaming push.\", 2005-08-03)\nand f8aae12034 (\"push: allow unqualified dest refspecs to DWIM\",\n2008-04-23), and somewhat documented in bb9fca80ce (\"git-push: Update\ndescription of refspecs and add examples\", 2007-06-09), but has never\nbeen fully documented.\n\nThe closest we got to having documented it was the description in the\ncommit message for f8aae12034, which I've borrowed from in writing\nthis documentation.\n\nLet's also refer to this new documentation from the existing\ndocumentation we had (added in bb9fca80ce).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/git-push.txt | 27 +++++++++++++++++++++++++++\n 1 file changed, 27 insertions(+)\n\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex a5fc54aeab..fb95c2e395 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -73,6 +73,30 @@ be omitted--such a push will update a ref that `` normally updates\n without any `` on the command line. Otherwise, missing\n `:` means to update the same ref as the ``.\n +\n+If doesn't start with `refs/` (e.g. `refs/heads/master`) we will\n+try to infer where in `refs/*` on the destination it\n+belongs based on the the type of being pushed and whether \n+is ambiguous.\n++\n+--\n+* If unambiguously refers to a ref on the remote,\n+ then push to that ref.\n+\n+* If resolves to a ref starting with refs/heads/ or refs/tags/,\n+ then prepend that to .\n+\n+* If starts with refs/remotes/ check if that reference refers to\n+ a commit or tag, then refs/heads/ or refs/tags/ to as\n+ appropriate.\n+\n+* Other ambiguity resolutions might be added in the future, but for\n+ now any other cases will error out with an error indicating what we\n+ tried, and depending on the `advice.pushUnqualifiedRefname`\n+ configuration (see linkgit:git-config[1]) suggest what refs/\n+ namespace you may have wanted to push to.\n+\n+--\n++\n The object referenced by is used to update the reference\n on the remote side. Whether this is allowed depends on where in\n `refs/*` the reference lives as described in detail below, in\n@@ -591,6 +615,9 @@ the ones in the examples below) can be configured as the default for\n \t`refs/remotes/satellite/master`) in the `mothership` repository;\n \tdo the same for `dev` and `satellite/dev`.\n +\n+See the section describing `...` above for a discussion of\n+the matching semantics.\n++\n This is to emulate `git fetch` run on the `mothership` using `git\n push` that is run in the opposite direction in order to integrate\n the work done on `satellite`, and is often necessary when you can\n-- \n2.19.1.759.g500967bb5e\n\n"},{"id":"361856","messageId":"xmqq5zxla7nd.fsf@gitster-ct.c.googlers.com","threadId":"49524","inReplyTo":"87r2gclnjc.fsf@evledraar.gmail.com","subject":"Re: [PATCH 2/2] push: add an advice on unqualified push","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-29T01:00:54Z","receivedAt":"2018-10-29T01:00:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> I was going to submit an update to this, as an additional improvement\n> can anyone think of a reason not to always infer that we'd like a new\n> branch if the LHS of the refspec starts with refs/remotes/* ?\n\nDepends on what purpose the remote you are pushing into serves. My\ninstinct tells me that it may be more likely to be emulating the\ncase where the remote, which is hosted on a box on which for some\nreason it is cumbersome for you to get an interactive shell prompt,\ndid the same fetch as your local repository and stored the same\nvalue in its remote-tracking branch than creating a local branch. I\ndo not say it is entirely unlikely that the push wants to create a\nlocal branch there, though. It can be a way to \"reprint\" what\nsomebody else published as their local branch, which you copied to\nyour remote-tracking branches, to the destination of your push. I\njust felt that it is less likely.\n\nTo put it another way, I would think both of these two have at most\nthe same probability that the push wants to go to a local branch:\n\n\tgit push refs/remotes/foo:foo\n\tgit push :foo\n\nand I would further say that the former is less likely than the\nlatter that it wants to create a local branch, because it is more\nplausible that it wants to create a similar remote-tracking branch\nthere.\n"},{"id":"361870","messageId":"xmqqy3ah5qtm.fsf@gitster-ct.c.googlers.com","threadId":"49524","inReplyTo":"xmqq5zxla7nd.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/2] push: add an advice on unqualified push","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-29T04:17:57Z","receivedAt":"2018-10-29T04:21:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> To put it another way, I would think both of these two have at most\n> the same probability that the push wants to go to a local branch:\n>\n> \tgit push refs/remotes/foo:foo\n> \tgit push :foo\n>\n> and I would further say that the former is less likely than the\n> latter that it wants to create a local branch, because it is more\n> plausible that it wants to create a similar remote-tracking branch\n> there.\n\nThis needs clarification. I do not mean \"it is more plausible that\nit wants remote-tracking rather than local\". What I meant was that\nbetween the two cases, pushing refs/remotes/foo:foo is more likely a\nsign that the user wants to create a local branch than pushing other\nrandom sha1 expression, like 40eaf9377fe649:foo, is.\n\n"},{"id":"361872","messageId":"xmqq8t2h5opm.fsf@gitster-ct.c.googlers.com","threadId":"49524","inReplyTo":"20181026230741.23321-4-avarab@gmail.com","subject":"Re: [PATCH v3 3/8] push: improve the error shown on unqualified push","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-29T05:03:33Z","receivedAt":"2018-10-29T05:05:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Improve the error message added in f8aae12034 (\"push: allow\n> unqualified dest refspecs to DWIM\", 2008-04-23), which before this\n> change looks like this:\n>\n> $ git push avar v2.19.0^{commit}:newbranch -n\n> error: unable to push to unqualified destination: newbranch\n> The destination refspec neither matches an existing ref on the remote nor\n> begins with refs/, and we are unable to guess a prefix based on the source ref.\n> error: failed to push some refs to 'git@github.com:avar/git.git'\n>\n> This message needed to be read very carefully to spot how to fix the\n> error, i.e. to push to refs/heads/newbranch. Now the message will look\n> like this instead:\n>\n> $ ./git-push avar v2.19.0^{commit}:newbranch -n\n> error: The destination you provided is not a full refname (i.e.,\n> starting with \"refs/\"). We tried to guess what you meant by:\n>\n> - Looking for a ref that matches 'newbranch' on the remote side.\n> - Checking if the being pushed ('v2.19.0^{commit}')\n> is a ref in \"refs/{heads,tags}/\". If so we add a\n> corresponding refs/{heads,tags}/ prefix on the remote side.\n\nIf so, we would have added ..., but we couldn't, because was\nnot such a local ref.\n\nBut is that a useful/actionable piece of information? The user said\nv2.19.0^{commit} because there was no such local ref the user could\nhave used instead to allow the DWIM to work on the destination side.\n\nPerhaps it may be a good thing to remember for the next time, but it\ndoes not help the user at all while redoing this failed push.\n\n> Neither worked, so we gave up. You must fully-qualify the ref.\n> error: failed to push some refs to 'git@github.com:avar/git.git'\n\nI am not sure if this is an improvement, quite honestly. \n\nThe only part that directly matters to the end user and is is more\nunderstandable than the original is \"You must fully qualify the ref\"\n(by the way, that dash is probably not what you want). As I already\nsaid, \"if this were local ref we can guess the location, it would\nhave worked better\" is not relevant to the end user, so it is a\nbetter use of the extra bytes to explain what it is to \"fully\"\nqualify the ref, than telling what would have helped us make a\nbetter guess. Perhaps something along the lines of...\n\n\t'newbranch' does not match any existing ref on the remote\n\tside. Please fully specify the destination refname starting\n\tfrom \"refs/\" (e.g. \"v2.19.0^{commit}:refs/heads/newbranch\"\n\tfor creating a \"newbranch\" branch).\n\n"},{"id":"361873","messageId":"xmqqva5l49np.fsf@gitster-ct.c.googlers.com","threadId":"49524","inReplyTo":"20181026230741.23321-6-avarab@gmail.com","subject":"Re: [PATCH v3 5/8] push: add an advice on unqualified push","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-29T05:14:02Z","receivedAt":"2018-10-29T05:14:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> +\tif (!advice_push_unqualified_ref_name)\n> +\t\treturn;\n> +\n> +\tif (get_oid(matched_src_name, &oid))\n> +\t\tBUG(\"'%s' is not a valid object, \"\n> +\t\t \"match_explicit_lhs() should catch this!\",\n> +\t\t matched_src_name);\n> +\ttype = oid_object_info(the_repository, &oid, NULL);\n> +\tif (type == OBJ_COMMIT) {\n> +\t\tadvise(_(\"The part of the refspec is a commit object.\\n\"\n> +\t\t\t \"Did you mean to create a new branch by pushing to\\n\"\n> +\t\t\t \"'%s:refs/heads/%s'?\"),\n> +\t\t matched_src_name, dst_value);\n\nGood, except that \"git push $there notes/commits^0:newnotes\" may not\nwant to become a branch and neither may \"git push $there stash:wip\",\nI think it is a reasonable piece of advice we'd give by default.\n\nI do not think it is worth the effort of inspecting the tree of the\ncommit object to special case notes and stash ;-)\n\n> +\t} else if (type == OBJ_TAG) {\n> +\t\tadvise(_(\"The part of the refspec is a tag object.\\n\"\n> +\t\t\t \"Did you mean to create a new tag by pushing to\\n\"\n> +\t\t\t \"'%s:refs/tags/%s'?\"),\n> +\t\t matched_src_name, dst_value);\n\nGood.\n\n> +\t} else if (type == OBJ_TREE) {\n> +\t\tadvise(_(\"The part of the refspec is a tree object.\\n\"\n> +\t\t\t \"Did you mean to tag a new tree by pushing to\\n\"\n> +\t\t\t \"'%s:refs/tags/%s'?\"),\n> +\t\t matched_src_name, dst_value);\n> +\t} else if (type == OBJ_BLOB) {\n> +\t\tadvise(_(\"The part of the refspec is a blob object.\\n\"\n> +\t\t\t \"Did you mean to tag a new blob by pushing to\\n\"\n> +\t\t\t \"'%s:refs/tags/%s'?\"),\n> +\t\t matched_src_name, dst_value);\n\nThese two are questionable, but assuming that heads and tags are the\nonly two hiearchies people would push into, they are acceptable\nchoices.\n\n> +\t} else {\n> +\t\tBUG(\"'%s' should be commit/tag/tree/blob, is '%d'\",\n> +\t\t matched_src_name, type);\n> +\t}\n> }\n> \n> static int match_explicit(struct ref *src, struct ref *dst,\n> diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\n> index d2a2cdd453..2e58721f98 100755\n> --- a/t/t5505-remote.sh\n> +++ b/t/t5505-remote.sh\n> @@ -1222,4 +1222,29 @@ test_expect_success 'add remote matching the \"insteadOf\" URL' '\n> \tgit remote add backup xyz@example.com\n> '\n> \n> +test_expect_success 'unqualified refspec DWIM and advice' '\n> +\ttest_when_finished \"(cd test && git tag -d some-tag)\" &&\n> +\t(\n> +\t\tcd test &&\n> +\t\tgit tag -a -m \"Some tag\" some-tag master &&\n> +\t\tfor type in commit tag tree blob\n> +\t\tdo\n> +\t\t\tif test \"$type\" = \"blob\"\n> +\t\t\tthen\n> +\t\t\t\toid=$(git rev-parse some-tag:file)\n> +\t\t\telse\n> +\t\t\t\toid=$(git rev-parse some-tag^{$type})\n> +\t\t\tfi &&\n> +\t\t\ttest_must_fail git push origin $oid:dst 2>err &&\n> +\t\t\ttest_i18ngrep \"error: The destination you\" err &&\n> +\t\t\ttest_i18ngrep \"hint: Did you mean\" err &&\n> +\t\t\ttest_must_fail git -c advice.pushUnqualifiedRefName=false \\\n> +\t\t\t\tpush origin $oid:dst 2>err &&\n> +\t\t\ttest_i18ngrep \"error: The destination you\" err &&\n> +\t\t\ttest_i18ngrep ! \"hint: Did you mean\" err\n\nAny failure in the &&-chain (or the last grep) would not terminate\nthe for loop, so for the purpose of determining the success of this\ntest_expect_success, the last \"blob\" iteration is the only thing\nthat matters.\n\nWhich is probably not what you want. If testing all of these is\nimportant, then you can do this:\n\n\t(\n\t\texit_with=true &&\n\t\tfor type in ...\n\t\tdo\n\t\t\t... many ... &&\n\t\t\t... many ... &&\n\t\t\t... tests ... ||\n\t\t\texit_with=false\n\t\tdone\n\t\t$exit_with\n\t)\n\nor just say \"|| exit\" and leave the loop (and the subprocess running\nit) early on the first failure.\n\n> +\t\tdone\n> +\t)\n> +'\n> +\n> +\n> test_done\n"},{"id":"361874","messageId":"xmqqr2g949fa.fsf@gitster-ct.c.googlers.com","threadId":"49524","inReplyTo":"20181026230741.23321-7-avarab@gmail.com","subject":"Re: [PATCH v3 6/8] push: test that doesn't DWYM if is unqualified","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-29T05:19:05Z","receivedAt":"2018-10-29T05:19:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Add a test asserting that \"git push origin :\" where is\n> a branch, tag, tree or blob in refs/remotes/* doesn't DWYM when \n> is unqualified. This has never worked, but there's been no test for\n> this behavior.\n\n\"has never worked\" sounded as if there is a breakage, but that is\nnot what meant here. We didn't DWIM overly agressively (which would\nhave led us to possibly push into a wrong place) and correctly\nrejected the push instead, right?\n\n> +test_expect_success 'refs/remotes/* refspec and unqualified DWIM and advice' '\n> +\t(\n> +\t\tcd two &&\n> +\t\tgit tag -a -m \"Some tag\" some-tag master &&\n> +\t\tgit update-ref refs/trees/my-head-tree HEAD^{tree} &&\n> +\t\tgit update-ref refs/blobs/my-file-blob HEAD:file\n> +\t) &&\n> +\t(\n> +\t\tcd test &&\n> +\t\tgit config --add remote.two.fetch \"+refs/tags/*:refs/remotes/two-tags/*\" &&\n> +\t\tgit config --add remote.two.fetch \"+refs/trees/*:refs/remotes/two-trees/*\" &&\n> +\t\tgit config --add remote.two.fetch \"+refs/blobs/*:refs/remotes/two-blobs/*\" &&\n> +\t\tgit fetch --no-tags two &&\n> +\n> +\t\ttest_must_fail git push origin refs/remotes/two/another:dst 2>err &&\n> +\t\ttest_i18ngrep \"error: The destination you\" err &&\n> +\n> +\t\ttest_must_fail git push origin refs/remotes/two-tags/some-tag:dst-tag 2>err &&\n\nThis made me go \"Huh? some-tag is one tag; what is the other tag in\ntwo-tags/ hierarchy?\" I think you meant by \"two-tags\" a hierarchy\nto store tags taken from the remote \"two\"; calling it \"tags-from-two\"\nmay have avoided such a confusion.\n\n> +\t\ttest_i18ngrep \"error: The destination you\" err &&\n> +\n> +\t\ttest_must_fail git push origin refs/remotes/two-trees/my-head-tree:dst-tree 2>err &&\n> +\t\ttest_i18ngrep \"error: The destination you\" err &&\n> +\n> +\t\ttest_must_fail git push origin refs/remotes/two-blobs/my-file-blob:dst-blob 2>err &&\n> +\t\ttest_i18ngrep \"error: The destination you\" err\n> +\t)\n> +'\n> +\n> \n> test_done\n"},{"id":"361875","messageId":"xmqqlg6h4964.fsf@gitster-ct.c.googlers.com","threadId":"49524","inReplyTo":"20181026230741.23321-8-avarab@gmail.com","subject":"Re: [PATCH v3 7/8] push: add DWYM support for \"git push refs/remotes/...:\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-29T05:24:35Z","receivedAt":"2018-10-29T05:24:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Add DWYM support for pushing a ref in refs/remotes/* when the \n\nI think most people call it do-what-*I*-mean, not do-what-you-mean.\n\n> ref is unqualified. Now instead of erroring out we support e.g.:\n>\n> $ ./git-push avar refs/remotes/origin/master:upstream-master -n\n> To github.com:avar/git.git\n> * [new branch] origin/master -> upstream-master\n>\n> Before this we wouldn't know what do do with\n> refs/remotes/origin/master, now we'll look it up and discover that\n> it's a commit (or tag) and add a refs/{heads,tags}/ prefix to as\n> appropriate.\n\nI am not sure if this is a good change, as I already hinted earlier.\nIf I were doing \"git push\" to any of my publishing places, I would\nbe irritated if \"refs/remotes/ko/master:something\" created a local\n\"something\" branch at the desitnation, instead of erroring out.\n\nOn the other hand, I do not think I mind all that much if a src that\nis a tag object to automatically go to refs/tags/ (having a tag\nobject in refs/remotes/** is rare enough to matter in the first\nplace).\n"},{"id":"361881","messageId":"xmqq7ei144gn.fsf@gitster-ct.c.googlers.com","threadId":"49524","inReplyTo":"20181026230741.23321-8-avarab@gmail.com","subject":"Re: [PATCH v3 7/8] push: add DWYM support for \"git push refs/remotes/...:\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-29T07:06:16Z","receivedAt":"2018-10-29T07:06:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> This is the first use of the %N$ style of printf format in\n> the *.[ch] files in our codebase. It's supported by POSIX[2] and\n> there's existing uses for it in po/*.po files,...\n\nFor now, I'll eject this from 'pu', as I had spent way too much time\ntrying to make it and other topics work there.\n\n CC remote.o\nremote.c: In function 'show_push_unqualified_ref_name_error':\nremote.c:1035:2: error: $ operand number used after format without operand number [-Werror=format=]\n error(_(\"The destination you provided is not a full refname (i.e.,\\n\"\n ^~~~~\ncc1: all warnings being treated as errors\nMakefile:2323: recipe for target 'remote.o' failed\nmake: *** [remote.o] Error 1\n"},{"id":"361883","messageId":"xmqqy3ah2nic.fsf@gitster-ct.c.googlers.com","threadId":"49524","inReplyTo":"xmqq7ei144gn.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 7/8] push: add DWYM support for \"git push refs/remotes/...:\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-29T07:57:47Z","receivedAt":"2018-10-29T07:57:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> This is the first use of the %N$ style of printf format in\n>> the *.[ch] files in our codebase. It's supported by POSIX[2] and\n>> there's existing uses for it in po/*.po files,...\n>\n> For now, I'll eject this from 'pu', as I had spent way too much time\n> trying to make it and other topics work there.\n>\n> CC remote.o\n> remote.c: In function 'show_push_unqualified_ref_name_error':\n> remote.c:1035:2: error: $ operand number used after format without operand number [-Werror=format=]\n> error(_(\"The destination you provided is not a full refname (i.e.,\\n\"\n> ^~~~~\n> cc1: all warnings being treated as errors\n> Makefile:2323: recipe for target 'remote.o' failed\n> make: *** [remote.o] Error 1\n\nI'll redo 'pu' with the following fix-up on top.\n\n remote.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex c243e3d89e..1927c4b376 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1035,8 +1035,8 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n \terror(_(\"The destination you provided is not a full refname (i.e.,\\n\"\n \t\t\"starting with \\\"refs/\\\"). We tried to guess what you meant by:\\n\"\n \t\t\"\\n\"\n-\t\t\"- Looking for a ref that matches '%s' on the remote side.\\n\"\n-\t\t\"- Checking if the being pushed ('%s')\\n\"\n+\t\t\"- Looking for a ref that matches '%1$s' on the remote side.\\n\"\n+\t\t\"- Checking if the being pushed ('%2$s')\\n\"\n \t\t\" is a ref in \\\"refs/{heads,tags}/\\\". If so we add a corresponding\\n\"\n \t\t\" refs/{heads,tags}/ prefix on the remote side.\\n\"\n \t\t\"- Checking if the being pushed ('%2$s')\\n\"\n-- \n2.19.1-593-gc670b1f876\n\n"},{"id":"361884","messageId":"87k1m1kwk0.fsf@evledraar.gmail.com","threadId":"49524","inReplyTo":"xmqq7ei144gn.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 7/8] push: add DWYM support for \"git push refs/remotes/...:\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-29T08:05:03Z","receivedAt":"2018-10-29T08:05:15Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Oct 29 2018, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> This is the first use of the %N$ style of printf format in\n>> the *.[ch] files in our codebase. It's supported by POSIX[2] and\n>> there's existing uses for it in po/*.po files,...\n>\n> For now, I'll eject this from 'pu', as I had spent way too much time\n> trying to make it and other topics work there.\n\nI was compiling with DEVELOPER=1 but as it turns out:\n\n CFLAGS=\"-O0\" DEVELOPER=1\n\nWasn't doing what I thought, i.e. we just take 'CFLAGS' from the\ncommand-line and don't add any of the DEVELOPER #leftoverbits to\nit. Will fix this and other issues raised.\n\n> CC remote.o\n> remote.c: In function 'show_push_unqualified_ref_name_error':\n> remote.c:1035:2: error: $ operand number used after format without operand number [-Werror=format=]\n> error(_(\"The destination you provided is not a full refname (i.e.,\\n\"\n> ^~~~~\n> cc1: all warnings being treated as errors\n> Makefile:2323: recipe for target 'remote.o' failed\n> make: *** [remote.o] Error 1\n\nWill fix this and other issues raised. FWIW clang gives a much better\nerror about the actual issue:\n\n remote.c:1042:46: error: cannot mix positional and non-positional arguments in format string [-Werror,-Wformat]\n \"- Checking if the being pushed ('%2$s')\\n\"\n\nI.e. this on top fixes it:\n\n - \"- Looking for a ref that matches '%s' on the remote side.\\n\"\n - \"- Checking if the being pushed ('%s')\\n\"\n + \"- Looking for a ref that matches '%1$s' on the remote side.\\n\"\n + \"- Checking if the being pushed ('%2$s')\\n\"\n\nMaybe this whole thing isn't worth it and I should just do:\n\n @@ -1042 +1042 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n - \"- Checking if the being pushed ('%2$s')\\n\"\n + \"- Checking if the being pushed ('%s')\\n\"\n @@ -1047 +1047 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n - dst_value, matched_src_name);\n + dst_value, matched_src_name, matched_src_name);\n\nBut I'm leaning on the side of keeping it for the self-documentation\naspect of \"this is a repeated parameter\". Your objections to this whole\nthing being a stupid idea non-withstanding.\n"},{"id":"361885","messageId":"87in1lkw54.fsf@evledraar.gmail.com","threadId":"49524","inReplyTo":"xmqqlg6h4964.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 7/8] push: add DWYM support for \"git push refs/remotes/...:\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-29T08:13:59Z","receivedAt":"2018-10-29T08:14:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Oct 29 2018, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Add DWYM support for pushing a ref in refs/remotes/* when the \n>\n> I think most people call it do-what-*I*-mean, not do-what-you-mean.\n\nFWIW I picked this up from the perl list where both are used depending\non context. I.e. DW[IY]M depending on if the sentence is one where you'd\ndescribe things from a first or second-person perspective \"I implemented\nthis to DWIM, and it'll DWYM if you invoke it as...\". Also\n\"dwimerry\"[1].\n\n>> ref is unqualified. Now instead of erroring out we support e.g.:\n>>\n>> $ ./git-push avar refs/remotes/origin/master:upstream-master -n\n>> To github.com:avar/git.git\n>> * [new branch] origin/master -> upstream-master\n>>\n>> Before this we wouldn't know what do do with\n>> refs/remotes/origin/master, now we'll look it up and discover that\n>> it's a commit (or tag) and add a refs/{heads,tags}/ prefix to as\n>> appropriate.\n>\n> I am not sure if this is a good change, as I already hinted earlier.\n> If I were doing \"git push\" to any of my publishing places, I would\n> be irritated if \"refs/remotes/ko/master:something\" created a local\n> \"something\" branch at the desitnation, instead of erroring out.\n>\n> On the other hand, I do not think I mind all that much if a src that\n> is a tag object to automatically go to refs/tags/ (having a tag\n> object in refs/remotes/** is rare enough to matter in the first\n> place).\n\nYeah maybe this is going too far. I don't think so, but happy to me\nchallenged on that point :)\n\nI don't think so because the only reason I've ever needed this is\nbecause I deleted some branch accidentally and am using a push from\n\"remotes/*\" to bring it back. I.e. I'll always want branch-for-branch,\nnot to push that as a tag.\n\nOf course it isn't actually a \"branch\", but just a commit object\n(depending on the refspec configuration), so we can't quite make that\nhard assumption.\n\nBut I figured screwing with how refs/remotes/* works by manually adding\nnew refspecs is such an advanced feature that the people doing it are\nprobably all here on-list, and would be the sort of users advanced\nenough to either know the semantics or try this with -n.\n\nWhereas the vast majority of users won't ever screw with it, and if they\never push from remotes/* to another remote almost definitely want a new\nbranch under the new name.\n\n1. https://www.urbandictionary.com/define.php?term=DWYM\n"},{"id":"362147","messageId":"xmqqy3adtopw.fsf@gitster-ct.c.googlers.com","threadId":"49524","inReplyTo":"87in1lkw54.fsf@evledraar.gmail.com","subject":"Re: [PATCH v3 7/8] push: add DWYM support for \"git push refs/remotes/...:\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-01T04:18:35Z","receivedAt":"2018-11-01T04:18:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n>> On the other hand, I do not think I mind all that much if a src that\n>> is a tag object to automatically go to refs/tags/ (having a tag\n>> object in refs/remotes/** is rare enough to matter in the first\n>> place).\n>\n> Yeah maybe this is going too far. I don't think so, but happy to me\n> challenged on that point :)\n>\n> I don't think so because the only reason I've ever needed this is\n> because I deleted some branch accidentally and am using a push from\n> \"remotes/*\" to bring it back. I.e. I'll always want branch-for-branch,\n> not to push that as a tag.\n\nOh, I didn't consider pushing it out as a tag, but now you bring it\nup, I think that it also would make sense in a workflow to tell your\ncolleages to look at (sort of like how people use pastebin---\"look\nhere, this commit has the kind of change I have in mind in this\ndiscussion\") some random commit and the commit happens to be sitting\nat a tip of a remote-trackig branch. Instead of pushing it out as a\nbranch or a remote-tracking branch, which has strong connotations of\ninviting others to build on top, pushing it out as a tag would make\nmore sense in that context.\n\nAnd as I mentioned already, I think it would equally likely, if not\nmore likely, for people like me to push remotes/** out as a\nremote-tracking branch (rather than a local branch) of the\nrepository I'm pushing into.\n\nSo I tend to agree that this is going too far. If the original\nmotivating example was not an ingredient of everyday workflow, but\nwas an one-off \"recovery\", I'd rather see people forced to be more\ncareful by requiring \"push origin/frotz:refs/heads/frotz\" rather\nthan incorrectly DWIDNM \"push origin/frotz:frotz\" and ending up with\ncreating refs/tags/frotz or refs/remotes/origin/frotz, which also\nare plausible choices depending on what the user is trying to\nrecover from, which the sending end would not know (the side on\nwhich the accidental loss of a ref happened earlier is on the remote\nrepository that would be receiving this push, and it _might_ know).\n\nAs to the entirety of the series,\n\n - I do not think this step 7, and its documentation in step 8, are\n particularly a good idea, in their current shape. Pushing tag\n objects to refs/tags/ is probably a good idea, but pushing a\n commit as local branch heads are necessarily not.\n\n - Step 6 is probably a good documentation on the cases in which we\n make and do not make guess on the unqualified push destination.\n\n - Step 5 and earlier looked like good changes.\n\nIf we were to salvage some parts of step 7 (and step 8), we'd\nprobably need fb7c2268 (\"SQUASH???\", 2018-10-29) to number all the\nplaceholders in the printf format string.\n"},{"id":"362254","messageId":"20181102064751.GJ31216@sigill.intra.peff.net","threadId":"49524","inReplyTo":"xmqqva5l49np.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 5/8] push: add an advice on unqualified push","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-02T06:47:52Z","receivedAt":"2018-11-02T06:47:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 29, 2018 at 02:14:02PM +0900, Junio C Hamano wrote:\n\n> Any failure in the &&-chain (or the last grep) would not terminate\n> the for loop, so for the purpose of determining the success of this\n> test_expect_success, the last \"blob\" iteration is the only thing\n> that matters.\n> \n> Which is probably not what you want. If testing all of these is\n> important, then you can do this:\n> \n> \t(\n> \t\texit_with=true &&\n> \t\tfor type in ...\n> \t\tdo\n> \t\t\t... many ... &&\n> \t\t\t... many ... &&\n> \t\t\t... tests ... ||\n> \t\t\texit_with=false\n> \t\tdone\n> \t\t$exit_with\n> \t)\n> \n> or just say \"|| exit\" and leave the loop (and the subprocess running\n> it) early on the first failure.\n\nYou can skip the subshell and just \"|| return 1\" from the chain inside\nthe loop. We run the test snippets inside an extra layer of\nfunction-call exactly to allow that.\n\n-Peff\n"},{"id":"362255","messageId":"20181102065218.GK31216@sigill.intra.peff.net","threadId":"49524","inReplyTo":"20181026230741.23321-1-avarab@gmail.com","subject":"Re: [PATCH v3 0/8] fixes for unqualified push","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-02T06:52:18Z","receivedAt":"2018-11-02T06:52:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 26, 2018 at 11:07:33PM +0000, Ævar Arnfjörð Bjarmason wrote:\n\n> After sending out v2 I noticed I didn't update the examples in a\n> couple of the commit messages, and figured I'd finish this up by\n> adding a patch to document how this works in the \"git-push\"\n> manpage. This behavior has never been properly documented. range-diff\n> with v2:\n\nI read over these patches and the comments from Junio. Overall, I like\nthe direction of the first half 5, and tend to agree with the review\nthat the \"refs/remotes\" thing is going too far.\n\nI didn't dig down into reviewing each line, since it looked like Junio\nalready did, and there's probably a re-roll coming.\n\n-Peff\n"},{"id":"362481","messageId":"87k1lr7obd.fsf@evledraar.gmail.com","threadId":"49524","inReplyTo":"xmqqy3adtopw.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 7/8] push: add DWYM support for \"git push refs/remotes/...:\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-11-05T11:31:50Z","receivedAt":"2018-11-05T11:31:56Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nI'll re-roll this. Hopefully sooner than later. I'll leave out the later\npart of this series as it's more controversial and we can discuss that\nlater on its own. Meanwhile just some replies to this (while I\nremember):\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>>> On the other hand, I do not think I mind all that much if a src that\n>>> is a tag object to automatically go to refs/tags/ (having a tag\n>>> object in refs/remotes/** is rare enough to matter in the first\n>>> place).\n>>\n>> Yeah maybe this is going too far. I don't think so, but happy to me\n>> challenged on that point :)\n>>\n>> I don't think so because the only reason I've ever needed this is\n>> because I deleted some branch accidentally and am using a push from\n>> \"remotes/*\" to bring it back. I.e. I'll always want branch-for-branch,\n>> not to push that as a tag.\n>\n> Oh, I didn't consider pushing it out as a tag, but now you bring it\n> up, I think that it also would make sense in a workflow to tell your\n> colleages to look at (sort of like how people use pastebin---\"look\n> here, this commit has the kind of change I have in mind in this\n> discussion\") some random commit and the commit happens to be sitting\n> at a tip of a remote-trackig branch. Instead of pushing it out as a\n> branch or a remote-tracking branch, which has strong connotations of\n> inviting others to build on top, pushing it out as a tag would make\n> more sense in that context.\n>\n> And as I mentioned already, I think it would equally likely, if not\n> more likely, for people like me to push remotes/** out as a\n> remote-tracking branch (rather than a local branch) of the\n> repository I'm pushing into.\n>\n> So I tend to agree that this is going too far. If the original\n> motivating example was not an ingredient of everyday workflow, but\n> was an one-off \"recovery\", I'd rather see people forced to be more\n> careful by requiring \"push origin/frotz:refs/heads/frotz\" rather\n> than incorrectly DWIDNM \"push origin/frotz:frotz\" and ending up with\n> creating refs/tags/frotz or refs/remotes/origin/frotz, which also\n> are plausible choices depending on what the user is trying to\n> recover from, which the sending end would not know (the side on\n> which the accidental loss of a ref happened earlier is on the remote\n> repository that would be receiving this push, and it _might_ know).\n\nYeah this example was bad, but now since I've written it I've become\nmore convinced that it's the right way to go, just that I didn't explain\nit well.\n\nE.g. just now I did:\n\n git push avar avar/some-branch:some-branch-wip\n git push avar HEAD -f # I was on 'some-branch'\n\nBecause I'd regretted taking the \"some-branch\" name with some WIP code,\nbut didn't want to lose the work, so I wanted to swap.\n\nIt's also arbitrary and contrary to the distributed nature of git that\nwe're treating branches from other repos differently than the ones from\nours.\n\nAfter all sometimes \"other\" is just the repo on my laptop or server. I\nshouldn't need to jump through hoops to re-push stuff from my \"other\"\nrepo anymore than from the local repo.\n\nYes refs/remotes/* isn't guaranteed to be \"other repo's branches\" in the\nsame way our local refs/heads/* is, and this series bends over backwards\nto check if someone's (ab)used it for that, but I think for the purposes\nof the default UI we should treat it as \"branches from other remotes\" if\nwe find commit objects there.\n\nSo I think the *only* thing we should be checking for the purposes of\nthis DWYM feature is what local type of object (branch or tag) we find\non the LHS of the refspec. And if we're sure of its type push to\nrefs/{heads,tags}/* as appropriate.\n\nI don't think it makes any sense as you suggest to move away from \"guess\nbased on existing type\" to breaking the dwimmery because we found the\nbranch in some other place. The user's pushing \"newref\" from an existing\nbranch, let's just assume the new thing is a branch, whether the source\nis local or not.\n\n> As to the entirety of the series,\n>\n> - I do not think this step 7, and its documentation in step 8, are\n> particularly a good idea, in their current shape. Pushing tag\n> objects to refs/tags/ is probably a good idea, but pushing a\n> commit as local branch heads are necessarily not.\n>\n> - Step 6 is probably a good documentation on the cases in which we\n> make and do not make guess on the unqualified push destination.\n>\n> - Step 5 and earlier looked like good changes.\n>\n> If we were to salvage some parts of step 7 (and step 8), we'd\n> probably need fb7c2268 (\"SQUASH???\", 2018-10-29) to number all the\n> placeholders in the printf format string.\n\nThanks. As noted will try to re-roll this soon.\n"},{"id":"362486","messageId":"xmqq4lcviu6l.fsf@gitster-ct.c.googlers.com","threadId":"49524","inReplyTo":"87k1lr7obd.fsf@evledraar.gmail.com","subject":"Re: [PATCH v3 7/8] push: add DWYM support for \"git push refs/remotes/...:\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-05T12:29:38Z","receivedAt":"2018-11-05T12:29:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> After all sometimes \"other\" is just the repo on my laptop or server. I\n> shouldn't need to jump through hoops to re-push stuff from my \"other\"\n> repo anymore than from the local repo.\n>\n> Yes refs/remotes/* isn't guaranteed to be \"other repo's branches\" in the\n> same way our local refs/heads/* is, and this series bends over backwards\n> to check if someone's (ab)used it for that, but I think for the purposes\n> of the default UI we should treat it as \"branches from other remotes\" if\n> we find commit objects there.\n\nI do not think there is *any* disagreement between us that\nrefs/remotes/* is where somebody else's branches are stored.\n\nThe fundamental difference between us is what this \"push\" is doing.\n\nYou are limiting yourself to a single use case where you push to\npublish to the remote repository, as if the work on somebody else's\ncommit were exactly like your own, hence you believe the push should\ngo to refs/heads/* of the receiving repository.\n\nI am also allowing an additional use case I often have to use, where\nI emulate a fetch done at the receiving repository by pushing into it.\nI have refs/remotes/* by fetching somebody else's branches locally,\nand instead of doing the same fetch over there by logging into it\nand doing \"git fetch\" interactively, I push refs/remotes/* I happen\nto have locally into refs/remotes/* there.\n\nAnd I happen to think both are equally valid workflows, and there is\nno strong reason to think everybody would intuitively expect a push\nof my local refs/remotes/* will go to refs/heads/* of the receiving\nrepository like you do. I'd rather want to make sure we do not make\nwrong guesses and force the user to \"recover\".\n\n> So I think the *only* thing we should be checking for the purposes of\n> this DWYM feature is what local type of object (branch or tag) we find\n> on the LHS of the refspec. And if we're sure of its type push to\n> refs/{heads,tags}/* as appropriate.\n\nThat is insufficient as long as you do not consider refs/remotes/*\nas one of the equally valid possibilities that sits next to refs/heads\nand res/tags/.\n\n"},{"id":"363204","messageId":"20181113195245.14296-1-avarab@gmail.com","threadId":"49524","inReplyTo":"20181026230741.23321-1-avarab@gmail.com","subject":"[PATCH v4 0/7] fixes for unqualified push","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-11-13T19:52:38Z","receivedAt":"2018-11-13T19:52:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"I've finally re-rolled this. There wasn't consensus for the change to\nadd DWYM behavior to refs/remotes/*, so I've dropped it, 7/7 is still\nthere testing the current behavior of what we do with that now, since\nwe didn't have any tests for that.\n\nThis should address all feedback on v3, except I haven't reworded the\n\"this is what we do, even though you didn't do that now\" message Junio\nwasn't happy with in\nhttps://public-inbox.org/git/xmqq8t2h5opm.fsf@gitster-ct.c.googlers.com/\n\nI agree that the wording he was critiquing doesn't make sense at that\npoint in the series, but I think if you look over to 5/7 the final\nstate makes more sense. I.e. our message is of the form:\n\n Error, you tried to do X. Sometimes we DWYM, but only in cases:\n\n - A\n - B\n hint: Did you mean to do this thing that'll give you DWYM behavior\n like what we'd do for \"B\"?\n\nI think in that case it makes sense to mention \"B\" in the message to\ncall out what exactly it is we DWYM on, why we didn't do it now, and\nwhat you should do if that was what you meant. Not mentioning B would\nbe more confusing.\n\nBut maybe there's still disagreement on that and we can work out the\ndetails of the wording, but now that a bunch of bugs have been fixed &\nthe controversial part ejected that'll be easier.\n\n1: ca8eb6dc28 = 1: a6b6a5bba5 remote.c: add braces in anticipation of a follow-up change\n2: b0e15b6ff1 = 2: 3335fcedc8 i18n: remote.c: mark error(...) messages for translation\n3: 052fc5860e ! 3: 18a5a685e7 push: improve the error shown on unqualified push\n @@ -22,10 +22,10 @@\n \n - Looking for a ref that matches 'newbranch' on the remote side.\n - Checking if the being pushed ('v2.19.0^{commit}')\n - is a ref in \"refs/{heads,tags}/\". If so we add a\n - corresponding refs/{heads,tags}/ prefix on the remote side.\n + is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n + refs/{heads,tags}/ prefix on the remote side.\n \n - Neither worked, so we gave up. You must fully-qualify the ref.\n + Neither worked, so we gave up. You must fully qualify the ref.\n error: failed to push some refs to 'git@github.com:avar/git.git'\n \n This improvement is the result of on-list discussion in [1] and [2],\n @@ -73,7 +73,7 @@\n +\t\t\t\t\" is a ref in \\\"refs/{heads,tags}/\\\". If so we add a corresponding\\n\"\n +\t\t\t\t\" refs/{heads,tags}/ prefix on the remote side.\\n\"\n +\t\t\t\t\"\\n\"\n -+\t\t\t\t\"Neither worked, so we gave up. You must fully-qualify the ref.\"),\n ++\t\t\t\t\"Neither worked, so we gave up. You must fully qualify the ref.\"),\n +\t\t\t dst_value, matched_src->name);\n \t\t}\n \t\tbreak;\n4: e6aa2e360f ! 4: a10d286cf6 push: move unqualified refname error into a function\n @@ -34,7 +34,7 @@\n +\t\t\" is a ref in \\\"refs/{heads,tags}/\\\". If so we add a corresponding\\n\"\n +\t\t\" refs/{heads,tags}/ prefix on the remote side.\\n\"\n +\t\t\"\\n\"\n -+\t\t\"Neither worked, so we gave up. You must fully-qualify the ref.\"),\n ++\t\t\"Neither worked, so we gave up. You must fully qualify the ref.\"),\n +\t dst_value, matched_src_name);\n +}\n +\n @@ -59,7 +59,7 @@\n -\t\t\t\t\" is a ref in \\\"refs/{heads,tags}/\\\". If so we add a corresponding\\n\"\n -\t\t\t\t\" refs/{heads,tags}/ prefix on the remote side.\\n\"\n -\t\t\t\t\"\\n\"\n --\t\t\t\t\"Neither worked, so we gave up. You must fully-qualify the ref.\"),\n +-\t\t\t\t\"Neither worked, so we gave up. You must fully qualify the ref.\"),\n -\t\t\t dst_value, matched_src->name);\n +\t\t\tshow_push_unqualified_ref_name_error(dst_value,\n +\t\t\t\t\t\t\t matched_src->name);\n5: dcf566e16e ! 5: 6f34c6f753 push: add an advice on unqualified push\n @@ -18,7 +18,7 @@\n is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n refs/{heads,tags}/ prefix on the remote side.\n \n - Neither worked, so we gave up. You must fully-qualify the ref.\n + Neither worked, so we gave up. You must fully qualify the ref.\n hint: The part of the refspec is a commit object.\n hint: Did you mean to create a new branch by pushing to\n hint: 'v2.19.0^{commit}:refs/heads/newbranch'?\n @@ -34,9 +34,9 @@\n \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n - diff --git a/Documentation/config.txt b/Documentation/config.txt\n - --- a/Documentation/config.txt\n - +++ b/Documentation/config.txt\n + diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\n + --- a/Documentation/config/advice.txt\n + +++ b/Documentation/config/advice.txt\n @@\n \t\ttries to overwrite a remote ref that points at an\n \t\tobject that is not a commit-ish, or make the remote\n @@ -107,7 +107,7 @@\n \t * :\" push, and \"being pushed ('%s')\" is\n @@\n \t\t\"\\n\"\n - \t\t\"Neither worked, so we gave up. You must fully-qualify the ref.\"),\n + \t\t\"Neither worked, so we gave up. You must fully qualify the ref.\"),\n \t dst_value, matched_src_name);\n +\n +\tif (!advice_push_unqualified_ref_name)\n @@ -158,6 +158,7 @@\n +\t(\n +\t\tcd test &&\n +\t\tgit tag -a -m \"Some tag\" some-tag master &&\n ++\t\texit_with=true &&\n +\t\tfor type in commit tag tree blob\n +\t\tdo\n +\t\t\tif test \"$type\" = \"blob\"\n @@ -172,8 +173,10 @@\n +\t\t\ttest_must_fail git -c advice.pushUnqualifiedRefName=false \\\n +\t\t\t\tpush origin $oid:dst 2>err &&\n +\t\t\ttest_i18ngrep \"error: The destination you\" err &&\n -+\t\t\ttest_i18ngrep ! \"hint: Did you mean\" err\n -+\t\tdone\n ++\t\t\ttest_i18ngrep ! \"hint: Did you mean\" err ||\n ++\t\t\texit_with=false\n ++\t\tdone &&\n ++\t\t$exit_with\n +\t)\n +'\n +\n6: 92ff753437 ! 6: 86cb0bbd95 push: test that doesn't DWYM if is unqualified\n @@ -4,14 +4,22 @@\n \n Add a test asserting that \"git push origin :\" where is\n a branch, tag, tree or blob in refs/remotes/* doesn't DWYM when \n - is unqualified. This has never worked, but there's been no test for\n - this behavior.\n + is unqualified. This has never been the case, but there haven't been\n + any tests for this behavior.\n \n See f88395ac23 (\"Renaming push.\", 2005-08-03), bb9fca80ce (\"git-push:\n Update description of refspecs and add examples\", 2007-06-09) and\n f8aae12034 (\"push: allow unqualified dest refspecs to DWIM\",\n 2008-04-23) which are most relevant commits that have changed or\n - documented the behavior of this feature in the past.\n + documented the behavior of the DWYM feature in the past.\n +\n + These tests were originally meant to lead up to a patch that made\n + refs/remotes/* on the LHS imply refs/heads/* on the RHS, see [1]. That\n + patch proved controversial and may not ever land in git.git, but we\n + should have the tests that remind us what the current behavior is in\n + case it's ever changed.\n +\n + 1. https://public-inbox.org/git/20181026230741.23321-8-avarab@gmail.com/\n \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n @@ -25,30 +33,29 @@\n +test_expect_success 'refs/remotes/* refspec and unqualified DWIM and advice' '\n +\t(\n +\t\tcd two &&\n -+\t\tgit tag -a -m \"Some tag\" some-tag master &&\n ++\t\tgit tag -a -m \"Some tag\" my-tag master &&\n +\t\tgit update-ref refs/trees/my-head-tree HEAD^{tree} &&\n +\t\tgit update-ref refs/blobs/my-file-blob HEAD:file\n +\t) &&\n +\t(\n +\t\tcd test &&\n -+\t\tgit config --add remote.two.fetch \"+refs/tags/*:refs/remotes/two-tags/*\" &&\n -+\t\tgit config --add remote.two.fetch \"+refs/trees/*:refs/remotes/two-trees/*\" &&\n -+\t\tgit config --add remote.two.fetch \"+refs/blobs/*:refs/remotes/two-blobs/*\" &&\n ++\t\tgit config --add remote.two.fetch \"+refs/tags/*:refs/remotes/tags-from-two/*\" &&\n ++\t\tgit config --add remote.two.fetch \"+refs/trees/*:refs/remotes/trees-from-two/*\" &&\n ++\t\tgit config --add remote.two.fetch \"+refs/blobs/*:refs/remotes/blobs-from-two/*\" &&\n +\t\tgit fetch --no-tags two &&\n +\n +\t\ttest_must_fail git push origin refs/remotes/two/another:dst 2>err &&\n +\t\ttest_i18ngrep \"error: The destination you\" err &&\n +\n -+\t\ttest_must_fail git push origin refs/remotes/two-tags/some-tag:dst-tag 2>err &&\n ++\t\ttest_must_fail git push origin refs/remotes/tags-from-two/my-tag:dst-tag 2>err &&\n +\t\ttest_i18ngrep \"error: The destination you\" err &&\n +\n -+\t\ttest_must_fail git push origin refs/remotes/two-trees/my-head-tree:dst-tree 2>err &&\n ++\t\ttest_must_fail git push origin refs/remotes/trees-from-two/my-head-tree:dst-tree 2>err &&\n +\t\ttest_i18ngrep \"error: The destination you\" err &&\n +\n -+\t\ttest_must_fail git push origin refs/remotes/two-blobs/my-file-blob:dst-blob 2>err &&\n ++\t\ttest_must_fail git push origin refs/remotes/blobs-from-two/my-file-blob:dst-blob 2>err &&\n +\t\ttest_i18ngrep \"error: The destination you\" err\n +\t)\n +'\n -+\n \n test_done\n7: 58eeb0f3f3 < -: ---------- push: add DWYM support for \"git push refs/remotes/...:\"\n8: bc171b0312 ! 7: d5e800cb87 push doc: document the DWYM behavior pushing to unqualified \n @@ -39,10 +39,6 @@\n +* If resolves to a ref starting with refs/heads/ or refs/tags/,\n + then prepend that to .\n +\n -+* If starts with refs/remotes/ check if that reference refers to\n -+ a commit or tag, then refs/heads/ or refs/tags/ to as\n -+ appropriate.\n -+\n +* Other ambiguity resolutions might be added in the future, but for\n + now any other cases will error out with an error indicating what we\n + tried, and depending on the `advice.pushUnqualifiedRefname`\n\nÆvar Arnfjörð Bjarmason (7):\n remote.c: add braces in anticipation of a follow-up change\n i18n: remote.c: mark error(...) messages for translation\n push: improve the error shown on unqualified push\n push: move unqualified refname error into a function\n push: add an advice on unqualified push\n push: test that doesn't DWYM if is unqualified\n push doc: document the DWYM behavior pushing to unqualified \n\n Documentation/config/advice.txt | 7 +++\n Documentation/git-push.txt | 23 +++++++\n advice.c | 2 +\n advice.h | 1 +\n remote.c | 106 ++++++++++++++++++++++++--------\n t/t5505-remote.sh | 55 +++++++++++++++++\n 6 files changed, 169 insertions(+), 25 deletions(-)\n\n-- \n2.19.1.1182.g4ecb1133ce\n\n"},{"id":"363205","messageId":"20181113195245.14296-2-avarab@gmail.com","threadId":"49524","inReplyTo":"20181026230741.23321-1-avarab@gmail.com","subject":"[PATCH v4 1/7] remote.c: add braces in anticipation of a follow-up change","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-11-13T19:52:39Z","receivedAt":"2018-11-13T19:53:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The CodingGuidelines say \"When there are multiple arms to a\nconditional and some of them require braces, enclose even a single\nline block in braces for consistency.\". Fix the code in\nmatch_explicit() to conform.\n\nWhile I'm at it change the if/else if/else in guess_ref() to use\nbraces. This is not currently needed, but a follow-up change will add\na new multi-line condition to that logic.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n remote.c | 16 +++++++++-------\n 1 file changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex b850f2feb3..695b379a44 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -968,12 +968,13 @@ static char *guess_ref(const char *name, struct ref *peer)\n \tif (!r)\n \t\treturn NULL;\n \n-\tif (starts_with(r, \"refs/heads/\"))\n+\tif (starts_with(r, \"refs/heads/\")) {\n \t\tstrbuf_addstr(&buf, \"refs/heads/\");\n-\telse if (starts_with(r, \"refs/tags/\"))\n+\t} else if (starts_with(r, \"refs/tags/\")) {\n \t\tstrbuf_addstr(&buf, \"refs/tags/\");\n-\telse\n+\t} else {\n \t\treturn NULL;\n+\t}\n \n \tstrbuf_addstr(&buf, name);\n \treturn strbuf_detach(&buf, NULL);\n@@ -1038,21 +1039,22 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \tcase 1:\n \t\tbreak;\n \tcase 0:\n-\t\tif (starts_with(dst_value, \"refs/\"))\n+\t\tif (starts_with(dst_value, \"refs/\")) {\n \t\t\tmatched_dst = make_linked_ref(dst_value, dst_tail);\n-\t\telse if (is_null_oid(&matched_src->new_oid))\n+\t\t} else if (is_null_oid(&matched_src->new_oid)) {\n \t\t\terror(\"unable to delete '%s': remote ref does not exist\",\n \t\t\t dst_value);\n-\t\telse if ((dst_guess = guess_ref(dst_value, matched_src))) {\n+\t\t} else if ((dst_guess = guess_ref(dst_value, matched_src))) {\n \t\t\tmatched_dst = make_linked_ref(dst_guess, dst_tail);\n \t\t\tfree(dst_guess);\n-\t\t} else\n+\t\t} else {\n \t\t\terror(\"unable to push to unqualified destination: %s\\n\"\n \t\t\t \"The destination refspec neither matches an \"\n \t\t\t \"existing ref on the remote nor\\n\"\n \t\t\t \"begins with refs/, and we are unable to \"\n \t\t\t \"guess a prefix based on the source ref.\",\n \t\t\t dst_value);\n+\t\t}\n \t\tbreak;\n \tdefault:\n \t\tmatched_dst = NULL;\n-- \n2.19.1.1182.g4ecb1133ce\n\n"},{"id":"363206","messageId":"20181113195245.14296-3-avarab@gmail.com","threadId":"49524","inReplyTo":"20181026230741.23321-1-avarab@gmail.com","subject":"[PATCH v4 2/7] i18n: remote.c: mark error(...) messages for translation","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-11-13T19:52:40Z","receivedAt":"2018-11-13T19:53:02Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Mark up the error(...) messages in remote.c for translation. The likes\nof \"unable to push to unqualified destination\" are relatively big\nparts of the UI, i.e. error messages shown when \"git push\" fails.\n\nI don't think any of these are plumbing, an the entire test suite\npasses when running the tests with GIT_GETTEXT_POISON=1 (after\nbuilding with GETTEXT_POISON).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n remote.c | 35 ++++++++++++++++++-----------------\n 1 file changed, 18 insertions(+), 17 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 695b379a44..35fe8a178f 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -406,7 +406,7 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t\tif (!remote->receivepack)\n \t\t\tremote->receivepack = v;\n \t\telse\n-\t\t\terror(\"more than one receivepack given, using the first\");\n+\t\t\terror(_(\"more than one receivepack given, using the first\"));\n \t} else if (!strcmp(subkey, \"uploadpack\")) {\n \t\tconst char *v;\n \t\tif (git_config_string(&v, key, value))\n@@ -414,7 +414,7 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t\tif (!remote->uploadpack)\n \t\t\tremote->uploadpack = v;\n \t\telse\n-\t\t\terror(\"more than one uploadpack given, using the first\");\n+\t\t\terror(_(\"more than one uploadpack given, using the first\"));\n \t} else if (!strcmp(subkey, \"tagopt\")) {\n \t\tif (!strcmp(value, \"--no-tags\"))\n \t\t\tremote->fetch_tags = -1;\n@@ -707,7 +707,7 @@ static void query_refspecs_multiple(struct refspec *rs,\n \tint find_src = !query->src;\n \n \tif (find_src && !query->dst)\n-\t\terror(\"query_refspecs_multiple: need either src or dst\");\n+\t\terror(_(\"query_refspecs_multiple: need either src or dst\"));\n \n \tfor (i = 0; i < rs->nr; i++) {\n \t\tstruct refspec_item *refspec = &rs->items[i];\n@@ -735,7 +735,7 @@ int query_refspecs(struct refspec *rs, struct refspec_item *query)\n \tchar **result = find_src ? &query->src : &query->dst;\n \n \tif (find_src && !query->dst)\n-\t\treturn error(\"query_refspecs: need either src or dst\");\n+\t\treturn error(_(\"query_refspecs: need either src or dst\"));\n \n \tfor (i = 0; i < rs->nr; i++) {\n \t\tstruct refspec_item *refspec = &rs->items[i];\n@@ -996,12 +996,12 @@ static int match_explicit_lhs(struct ref *src,\n \t\t * way to delete 'other' ref at the remote end.\n \t\t */\n \t\tif (try_explicit_object_name(rs->src, match) < 0)\n-\t\t\treturn error(\"src refspec %s does not match any.\", rs->src);\n+\t\t\treturn error(_(\"src refspec %s does not match any.\"), rs->src);\n \t\tif (allocated_match)\n \t\t\t*allocated_match = 1;\n \t\treturn 0;\n \tdefault:\n-\t\treturn error(\"src refspec %s matches more than one.\", rs->src);\n+\t\treturn error(_(\"src refspec %s matches more than one.\"), rs->src);\n \t}\n }\n \n@@ -1041,32 +1041,33 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \tcase 0:\n \t\tif (starts_with(dst_value, \"refs/\")) {\n \t\t\tmatched_dst = make_linked_ref(dst_value, dst_tail);\n+\n \t\t} else if (is_null_oid(&matched_src->new_oid)) {\n-\t\t\terror(\"unable to delete '%s': remote ref does not exist\",\n+\t\t\terror(_(\"unable to delete '%s': remote ref does not exist\"),\n \t\t\t dst_value);\n \t\t} else if ((dst_guess = guess_ref(dst_value, matched_src))) {\n \t\t\tmatched_dst = make_linked_ref(dst_guess, dst_tail);\n \t\t\tfree(dst_guess);\n \t\t} else {\n-\t\t\terror(\"unable to push to unqualified destination: %s\\n\"\n-\t\t\t \"The destination refspec neither matches an \"\n-\t\t\t \"existing ref on the remote nor\\n\"\n-\t\t\t \"begins with refs/, and we are unable to \"\n-\t\t\t \"guess a prefix based on the source ref.\",\n+\t\t\terror(_(\"unable to push to unqualified destination: %s\\n\"\n+\t\t\t\t\"The destination refspec neither matches an \"\n+\t\t\t\t\"existing ref on the remote nor\\n\"\n+\t\t\t\t\"begins with refs/, and we are unable to \"\n+\t\t\t\t\"guess a prefix based on the source ref.\"),\n \t\t\t dst_value);\n \t\t}\n \t\tbreak;\n \tdefault:\n \t\tmatched_dst = NULL;\n-\t\terror(\"dst refspec %s matches more than one.\",\n+\t\terror(_(\"dst refspec %s matches more than one.\"),\n \t\t dst_value);\n \t\tbreak;\n \t}\n \tif (!matched_dst)\n \t\treturn -1;\n \tif (matched_dst->peer_ref)\n-\t\treturn error(\"dst ref %s receives from more than one src.\",\n-\t\t matched_dst->name);\n+\t\treturn error(_(\"dst ref %s receives from more than one src.\"),\n+\t\t\t matched_dst->name);\n \telse {\n \t\tmatched_dst->peer_ref = allocated_src ?\n \t\t\t\t\tmatched_src :\n@@ -1797,7 +1798,7 @@ int get_fetch_map(const struct ref *remote_refs,\n \t\t\tif (!starts_with((*rmp)->peer_ref->name, \"refs/\") ||\n \t\t\t check_refname_format((*rmp)->peer_ref->name, 0)) {\n \t\t\t\tstruct ref *ignore = *rmp;\n-\t\t\t\terror(\"* Ignoring funny ref '%s' locally\",\n+\t\t\t\terror(_(\"* Ignoring funny ref '%s' locally\"),\n \t\t\t\t (*rmp)->peer_ref->name);\n \t\t\t\t*rmp = (*rmp)->next;\n \t\t\t\tfree(ignore->peer_ref);\n@@ -2165,7 +2166,7 @@ static int parse_push_cas_option(struct push_cas_option *cas, const char *arg, i\n \telse if (!colon[1])\n \t\toidclr(&entry->expect);\n \telse if (get_oid(colon + 1, &entry->expect))\n-\t\treturn error(\"cannot parse expected object name '%s'\", colon + 1);\n+\t\treturn error(_(\"cannot parse expected object name '%s'\"), colon + 1);\n \treturn 0;\n }\n \n-- \n2.19.1.1182.g4ecb1133ce\n\n"},{"id":"363207","messageId":"20181113195245.14296-4-avarab@gmail.com","threadId":"49524","inReplyTo":"20181026230741.23321-1-avarab@gmail.com","subject":"[PATCH v4 3/7] push: improve the error shown on unqualified push","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-11-13T19:52:41Z","receivedAt":"2018-11-13T19:53:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Improve the error message added in f8aae12034 (\"push: allow\nunqualified dest refspecs to DWIM\", 2008-04-23), which before this\nchange looks like this:\n\n $ git push avar v2.19.0^{commit}:newbranch -n\n error: unable to push to unqualified destination: newbranch\n The destination refspec neither matches an existing ref on the remote nor\n begins with refs/, and we are unable to guess a prefix based on the source ref.\n error: failed to push some refs to 'git@github.com:avar/git.git'\n\nThis message needed to be read very carefully to spot how to fix the\nerror, i.e. to push to refs/heads/newbranch. Now the message will look\nlike this instead:\n\n $ ./git-push avar v2.19.0^{commit}:newbranch -n\n error: The destination you provided is not a full refname (i.e.,\n starting with \"refs/\"). We tried to guess what you meant by:\n\n - Looking for a ref that matches 'newbranch' on the remote side.\n - Checking if the being pushed ('v2.19.0^{commit}')\n is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n refs/{heads,tags}/ prefix on the remote side.\n\n Neither worked, so we gave up. You must fully qualify the ref.\n error: failed to push some refs to 'git@github.com:avar/git.git'\n\nThis improvement is the result of on-list discussion in [1] and [2],\nas well as my own fixes / reformatting / phrasing on top.\n\nThe suggestion by Jeff on-list was to make that second bullet point\n\"Looking at the refname of the local source.\". The version being added\nhere is more verbose, but also more accurate. saying \"local source\"\ncould refer to any ref in the local refstore, including something in\nrefs/remotes/*. A later change will teach guess_ref() to infer a ref\ntype from remote-tracking refs, so let's not confuse the two.\n\nWhile I'm at it, add a \"TRANSLATORS\" comment since the message has\ngotten more complex and it's not as clear what the two %s's refer to.\n\n1. https://public-inbox.org/git/20181010205505.GB12949@sigill.intra.peff.net/\n2. https://public-inbox.org/git/xmqqbm81lb7c.fsf@gitster-ct.c.googlers.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n remote.c | 22 ++++++++++++++++------\n 1 file changed, 16 insertions(+), 6 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 35fe8a178f..15da4019c3 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1049,12 +1049,22 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \t\t\tmatched_dst = make_linked_ref(dst_guess, dst_tail);\n \t\t\tfree(dst_guess);\n \t\t} else {\n-\t\t\terror(_(\"unable to push to unqualified destination: %s\\n\"\n-\t\t\t\t\"The destination refspec neither matches an \"\n-\t\t\t\t\"existing ref on the remote nor\\n\"\n-\t\t\t\t\"begins with refs/, and we are unable to \"\n-\t\t\t\t\"guess a prefix based on the source ref.\"),\n-\t\t\t dst_value);\n+\t\t\t/*\n+\t\t\t * TRANSLATORS: \"matches '%s'%\" is the \n+\t\t\t * part of \"git push :\"\n+\t\t\t * push, and \"being pushed ('%s')\" is the\n+\t\t\t * .\n+\t\t\t */\n+\t\t\terror(_(\"The destination you provided is not a full refname (i.e.,\\n\"\n+\t\t\t\t\"starting with \\\"refs/\\\"). We tried to guess what you meant by:\\n\"\n+\t\t\t\t\"\\n\"\n+\t\t\t\t\"- Looking for a ref that matches '%s' on the remote side.\\n\"\n+\t\t\t\t\"- Checking if the being pushed ('%s')\\n\"\n+\t\t\t\t\" is a ref in \\\"refs/{heads,tags}/\\\". If so we add a corresponding\\n\"\n+\t\t\t\t\" refs/{heads,tags}/ prefix on the remote side.\\n\"\n+\t\t\t\t\"\\n\"\n+\t\t\t\t\"Neither worked, so we gave up. You must fully qualify the ref.\"),\n+\t\t\t dst_value, matched_src->name);\n \t\t}\n \t\tbreak;\n \tdefault:\n-- \n2.19.1.1182.g4ecb1133ce\n\n"},{"id":"363208","messageId":"20181113195245.14296-5-avarab@gmail.com","threadId":"49524","inReplyTo":"20181026230741.23321-1-avarab@gmail.com","subject":"[PATCH v4 4/7] push: move unqualified refname error into a function","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-11-13T19:52:42Z","receivedAt":"2018-11-13T19:53:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"A follow-up change will extend this error message with the advice\nfacility. Doing so would make the indentation too deeply nested for\ncomfort. So let's split this into a helper function.\n\nThere's no changes to the wording here. Just code moving &\nre-indentation, and re-flowing the \"TRANSLATORS\" comment.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n remote.c | 38 ++++++++++++++++++++++----------------\n 1 file changed, 22 insertions(+), 16 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 15da4019c3..ba8abf4d32 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1005,6 +1005,26 @@ static int match_explicit_lhs(struct ref *src,\n \t}\n }\n \n+static void show_push_unqualified_ref_name_error(const char *dst_value,\n+\t\t\t\t\t\t const char *matched_src_name)\n+{\n+\t/*\n+\t * TRANSLATORS: \"matches '%s'%\" is the part of \"git push\n+\t * :\" push, and \"being pushed ('%s')\" is\n+\t * the .\n+\t */\n+\terror(_(\"The destination you provided is not a full refname (i.e.,\\n\"\n+\t\t\"starting with \\\"refs/\\\"). We tried to guess what you meant by:\\n\"\n+\t\t\"\\n\"\n+\t\t\"- Looking for a ref that matches '%s' on the remote side.\\n\"\n+\t\t\"- Checking if the being pushed ('%s')\\n\"\n+\t\t\" is a ref in \\\"refs/{heads,tags}/\\\". If so we add a corresponding\\n\"\n+\t\t\" refs/{heads,tags}/ prefix on the remote side.\\n\"\n+\t\t\"\\n\"\n+\t\t\"Neither worked, so we gave up. You must fully qualify the ref.\"),\n+\t dst_value, matched_src_name);\n+}\n+\n static int match_explicit(struct ref *src, struct ref *dst,\n \t\t\t struct ref ***dst_tail,\n \t\t\t struct refspec_item *rs)\n@@ -1049,22 +1069,8 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \t\t\tmatched_dst = make_linked_ref(dst_guess, dst_tail);\n \t\t\tfree(dst_guess);\n \t\t} else {\n-\t\t\t/*\n-\t\t\t * TRANSLATORS: \"matches '%s'%\" is the \n-\t\t\t * part of \"git push :\"\n-\t\t\t * push, and \"being pushed ('%s')\" is the\n-\t\t\t * .\n-\t\t\t */\n-\t\t\terror(_(\"The destination you provided is not a full refname (i.e.,\\n\"\n-\t\t\t\t\"starting with \\\"refs/\\\"). We tried to guess what you meant by:\\n\"\n-\t\t\t\t\"\\n\"\n-\t\t\t\t\"- Looking for a ref that matches '%s' on the remote side.\\n\"\n-\t\t\t\t\"- Checking if the being pushed ('%s')\\n\"\n-\t\t\t\t\" is a ref in \\\"refs/{heads,tags}/\\\". If so we add a corresponding\\n\"\n-\t\t\t\t\" refs/{heads,tags}/ prefix on the remote side.\\n\"\n-\t\t\t\t\"\\n\"\n-\t\t\t\t\"Neither worked, so we gave up. You must fully qualify the ref.\"),\n-\t\t\t dst_value, matched_src->name);\n+\t\t\tshow_push_unqualified_ref_name_error(dst_value,\n+\t\t\t\t\t\t\t matched_src->name);\n \t\t}\n \t\tbreak;\n \tdefault:\n-- \n2.19.1.1182.g4ecb1133ce\n\n"},{"id":"363209","messageId":"20181113195245.14296-6-avarab@gmail.com","threadId":"49524","inReplyTo":"20181026230741.23321-1-avarab@gmail.com","subject":"[PATCH v4 5/7] push: add an advice on unqualified push","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-11-13T19:52:43Z","receivedAt":"2018-11-13T19:53:07Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add an advice to the recently improved error message added in\nf8aae12034 (\"push: allow unqualified dest refspecs to DWIM\",\n2008-04-23).\n\nNow with advice.pushUnqualifiedRefName=true (on by default) we show a\nhint about how to proceed:\n\n $ ./git-push avar v2.19.0^{commit}:newbranch -n\n error: The destination you provided is not a full refname (i.e.,\n starting with \"refs/\"). We tried to guess what you meant by:\n\n - Looking for a ref that matches 'newbranch' on the remote side.\n - Checking if the being pushed ('v2.19.0^{commit}')\n is a ref in \"refs/{heads,tags}/\". If so we add a corresponding\n refs/{heads,tags}/ prefix on the remote side.\n\n Neither worked, so we gave up. You must fully qualify the ref.\n hint: The part of the refspec is a commit object.\n hint: Did you mean to create a new branch by pushing to\n hint: 'v2.19.0^{commit}:refs/heads/newbranch'?\n error: failed to push some refs to 'git@github.com:avar/git.git'\n\nWhen trying to push a tag, tree or a blob we suggest that perhaps the\nuser meant to push them to refs/tags/ instead.\n\nThe if/else duplication for all of OBJ_{COMMIT,TAG,TREE,BLOB} is\nunfortunate, but is required to correctly mark the messages for\ntranslation. See the discussion in\n<87r2gxebsi.fsf@evledraar.gmail.com> about that.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/config/advice.txt | 7 +++++++\n advice.c | 2 ++\n advice.h | 1 +\n remote.c | 37 +++++++++++++++++++++++++++++++++\n t/t5505-remote.sh | 28 +++++++++++++++++++++++++\n 5 files changed, 75 insertions(+)\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex 57fcd4c862..88620429ea 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -30,6 +30,13 @@ advice.*::\n \t\ttries to overwrite a remote ref that points at an\n \t\tobject that is not a commit-ish, or make the remote\n \t\tref point at an object that is not a commit-ish.\n+\tpushUnqualifiedRefname::\n+\t\tShown when linkgit:git-push[1] gives up trying to\n+\t\tguess based on the source and destination refs what\n+\t\tremote ref namespace the source belongs in, but where\n+\t\twe can still suggest that the user push to either\n+\t\trefs/heads/* or refs/tags/* based on the type of the\n+\t\tsource object.\n \tstatusHints::\n \t\tShow directions on how to proceed from the current\n \t\tstate in the output of linkgit:git-status[1], in\ndiff --git a/advice.c b/advice.c\nindex 5f35656409..567209aa79 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -9,6 +9,7 @@ int advice_push_non_ff_matching = 1;\n int advice_push_already_exists = 1;\n int advice_push_fetch_first = 1;\n int advice_push_needs_force = 1;\n+int advice_push_unqualified_ref_name = 1;\n int advice_status_hints = 1;\n int advice_status_u_option = 1;\n int advice_commit_before_merge = 1;\n@@ -63,6 +64,7 @@ static struct {\n \t{ \"pushAlreadyExists\", &advice_push_already_exists },\n \t{ \"pushFetchFirst\", &advice_push_fetch_first },\n \t{ \"pushNeedsForce\", &advice_push_needs_force },\n+\t{ \"pushUnqualifiedRefName\", &advice_push_unqualified_ref_name },\n \t{ \"statusHints\", &advice_status_hints },\n \t{ \"statusUoption\", &advice_status_u_option },\n \t{ \"commitBeforeMerge\", &advice_commit_before_merge },\ndiff --git a/advice.h b/advice.h\nindex 696bf0e7d2..f875f8cd8d 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -9,6 +9,7 @@ extern int advice_push_non_ff_matching;\n extern int advice_push_already_exists;\n extern int advice_push_fetch_first;\n extern int advice_push_needs_force;\n+extern int advice_push_unqualified_ref_name;\n extern int advice_status_hints;\n extern int advice_status_u_option;\n extern int advice_commit_before_merge;\ndiff --git a/remote.c b/remote.c\nindex ba8abf4d32..f7477b8eb6 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -13,6 +13,7 @@\n #include \"mergesort.h\"\n #include \"argv-array.h\"\n #include \"commit-reach.h\"\n+#include \"advice.h\"\n \n enum map_direction { FROM_SRC, FROM_DST };\n \n@@ -1008,6 +1009,9 @@ static int match_explicit_lhs(struct ref *src,\n static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\t\t\t\t\t const char *matched_src_name)\n {\n+\tstruct object_id oid;\n+\tenum object_type type;\n+\n \t/*\n \t * TRANSLATORS: \"matches '%s'%\" is the part of \"git push\n \t * :\" push, and \"being pushed ('%s')\" is\n@@ -1023,6 +1027,39 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,\n \t\t\"\\n\"\n \t\t\"Neither worked, so we gave up. You must fully qualify the ref.\"),\n \t dst_value, matched_src_name);\n+\n+\tif (!advice_push_unqualified_ref_name)\n+\t\treturn;\n+\n+\tif (get_oid(matched_src_name, &oid))\n+\t\tBUG(\"'%s' is not a valid object, \"\n+\t\t \"match_explicit_lhs() should catch this!\",\n+\t\t matched_src_name);\n+\ttype = oid_object_info(the_repository, &oid, NULL);\n+\tif (type == OBJ_COMMIT) {\n+\t\tadvise(_(\"The part of the refspec is a commit object.\\n\"\n+\t\t\t \"Did you mean to create a new branch by pushing to\\n\"\n+\t\t\t \"'%s:refs/heads/%s'?\"),\n+\t\t matched_src_name, dst_value);\n+\t} else if (type == OBJ_TAG) {\n+\t\tadvise(_(\"The part of the refspec is a tag object.\\n\"\n+\t\t\t \"Did you mean to create a new tag by pushing to\\n\"\n+\t\t\t \"'%s:refs/tags/%s'?\"),\n+\t\t matched_src_name, dst_value);\n+\t} else if (type == OBJ_TREE) {\n+\t\tadvise(_(\"The part of the refspec is a tree object.\\n\"\n+\t\t\t \"Did you mean to tag a new tree by pushing to\\n\"\n+\t\t\t \"'%s:refs/tags/%s'?\"),\n+\t\t matched_src_name, dst_value);\n+\t} else if (type == OBJ_BLOB) {\n+\t\tadvise(_(\"The part of the refspec is a blob object.\\n\"\n+\t\t\t \"Did you mean to tag a new blob by pushing to\\n\"\n+\t\t\t \"'%s:refs/tags/%s'?\"),\n+\t\t matched_src_name, dst_value);\n+\t} else {\n+\t\tBUG(\"'%s' should be commit/tag/tree/blob, is '%d'\",\n+\t\t matched_src_name, type);\n+\t}\n }\n \n static int match_explicit(struct ref *src, struct ref *dst,\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex d2a2cdd453..f069cbcc24 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -1222,4 +1222,32 @@ test_expect_success 'add remote matching the \"insteadOf\" URL' '\n \tgit remote add backup xyz@example.com\n '\n \n+test_expect_success 'unqualified refspec DWIM and advice' '\n+\ttest_when_finished \"(cd test && git tag -d some-tag)\" &&\n+\t(\n+\t\tcd test &&\n+\t\tgit tag -a -m \"Some tag\" some-tag master &&\n+\t\texit_with=true &&\n+\t\tfor type in commit tag tree blob\n+\t\tdo\n+\t\t\tif test \"$type\" = \"blob\"\n+\t\t\tthen\n+\t\t\t\toid=$(git rev-parse some-tag:file)\n+\t\t\telse\n+\t\t\t\toid=$(git rev-parse some-tag^{$type})\n+\t\t\tfi &&\n+\t\t\ttest_must_fail git push origin $oid:dst 2>err &&\n+\t\t\ttest_i18ngrep \"error: The destination you\" err &&\n+\t\t\ttest_i18ngrep \"hint: Did you mean\" err &&\n+\t\t\ttest_must_fail git -c advice.pushUnqualifiedRefName=false \\\n+\t\t\t\tpush origin $oid:dst 2>err &&\n+\t\t\ttest_i18ngrep \"error: The destination you\" err &&\n+\t\t\ttest_i18ngrep ! \"hint: Did you mean\" err ||\n+\t\t\texit_with=false\n+\t\tdone &&\n+\t\t$exit_with\n+\t)\n+'\n+\n+\n test_done\n-- \n2.19.1.1182.g4ecb1133ce\n\n"},{"id":"363210","messageId":"20181113195245.14296-7-avarab@gmail.com","threadId":"49524","inReplyTo":"20181026230741.23321-1-avarab@gmail.com","subject":"[PATCH v4 6/7] push: test that doesn't DWYM if is unqualified","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-11-13T19:52:44Z","receivedAt":"2018-11-13T19:53:08Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a test asserting that \"git push origin :\" where is\na branch, tag, tree or blob in refs/remotes/* doesn't DWYM when \nis unqualified. This has never been the case, but there haven't been\nany tests for this behavior.\n\nSee f88395ac23 (\"Renaming push.\", 2005-08-03), bb9fca80ce (\"git-push:\nUpdate description of refspecs and add examples\", 2007-06-09) and\nf8aae12034 (\"push: allow unqualified dest refspecs to DWIM\",\n2008-04-23) which are most relevant commits that have changed or\ndocumented the behavior of the DWYM feature in the past.\n\nThese tests were originally meant to lead up to a patch that made\nrefs/remotes/* on the LHS imply refs/heads/* on the RHS, see [1]. That\npatch proved controversial and may not ever land in git.git, but we\nshould have the tests that remind us what the current behavior is in\ncase it's ever changed.\n\n1. https://public-inbox.org/git/20181026230741.23321-8-avarab@gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t5505-remote.sh | 27 +++++++++++++++++++++++++++\n 1 file changed, 27 insertions(+)\n\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex f069cbcc24..883b32efa0 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -1249,5 +1249,32 @@ test_expect_success 'unqualified refspec DWIM and advice' '\n \t)\n '\n \n+test_expect_success 'refs/remotes/* refspec and unqualified DWIM and advice' '\n+\t(\n+\t\tcd two &&\n+\t\tgit tag -a -m \"Some tag\" my-tag master &&\n+\t\tgit update-ref refs/trees/my-head-tree HEAD^{tree} &&\n+\t\tgit update-ref refs/blobs/my-file-blob HEAD:file\n+\t) &&\n+\t(\n+\t\tcd test &&\n+\t\tgit config --add remote.two.fetch \"+refs/tags/*:refs/remotes/tags-from-two/*\" &&\n+\t\tgit config --add remote.two.fetch \"+refs/trees/*:refs/remotes/trees-from-two/*\" &&\n+\t\tgit config --add remote.two.fetch \"+refs/blobs/*:refs/remotes/blobs-from-two/*\" &&\n+\t\tgit fetch --no-tags two &&\n+\n+\t\ttest_must_fail git push origin refs/remotes/two/another:dst 2>err &&\n+\t\ttest_i18ngrep \"error: The destination you\" err &&\n+\n+\t\ttest_must_fail git push origin refs/remotes/tags-from-two/my-tag:dst-tag 2>err &&\n+\t\ttest_i18ngrep \"error: The destination you\" err &&\n+\n+\t\ttest_must_fail git push origin refs/remotes/trees-from-two/my-head-tree:dst-tree 2>err &&\n+\t\ttest_i18ngrep \"error: The destination you\" err &&\n+\n+\t\ttest_must_fail git push origin refs/remotes/blobs-from-two/my-file-blob:dst-blob 2>err &&\n+\t\ttest_i18ngrep \"error: The destination you\" err\n+\t)\n+'\n \n test_done\n-- \n2.19.1.1182.g4ecb1133ce\n\n"},{"id":"363211","messageId":"20181113195245.14296-8-avarab@gmail.com","threadId":"49524","inReplyTo":"20181026230741.23321-1-avarab@gmail.com","subject":"[PATCH v4 7/7] push doc: document the DWYM behavior pushing to unqualified ","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-11-13T19:52:45Z","receivedAt":"2018-11-13T19:53:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Document the DWYM behavior that kicks in when pushing to an\nunqualified reference.\n\nThis behavior was added in f88395ac23 (\"Renaming push.\", 2005-08-03)\nand f8aae12034 (\"push: allow unqualified dest refspecs to DWIM\",\n2008-04-23), and somewhat documented in bb9fca80ce (\"git-push: Update\ndescription of refspecs and add examples\", 2007-06-09), but has never\nbeen fully documented.\n\nThe closest we got to having documented it was the description in the\ncommit message for f8aae12034, which I've borrowed from in writing\nthis documentation.\n\nLet's also refer to this new documentation from the existing\ndocumentation we had (added in bb9fca80ce).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/git-push.txt | 23 +++++++++++++++++++++++\n 1 file changed, 23 insertions(+)\n\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex a5fc54aeab..6a8a0d958b 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -73,6 +73,26 @@ be omitted--such a push will update a ref that `` normally updates\n without any `` on the command line. Otherwise, missing\n `:` means to update the same ref as the ``.\n +\n+If doesn't start with `refs/` (e.g. `refs/heads/master`) we will\n+try to infer where in `refs/*` on the destination it\n+belongs based on the the type of being pushed and whether \n+is ambiguous.\n++\n+--\n+* If unambiguously refers to a ref on the remote,\n+ then push to that ref.\n+\n+* If resolves to a ref starting with refs/heads/ or refs/tags/,\n+ then prepend that to .\n+\n+* Other ambiguity resolutions might be added in the future, but for\n+ now any other cases will error out with an error indicating what we\n+ tried, and depending on the `advice.pushUnqualifiedRefname`\n+ configuration (see linkgit:git-config[1]) suggest what refs/\n+ namespace you may have wanted to push to.\n+\n+--\n++\n The object referenced by is used to update the reference\n on the remote side. Whether this is allowed depends on where in\n `refs/*` the reference lives as described in detail below, in\n@@ -591,6 +611,9 @@ the ones in the examples below) can be configured as the default for\n \t`refs/remotes/satellite/master`) in the `mothership` repository;\n \tdo the same for `dev` and `satellite/dev`.\n +\n+See the section describing `...` above for a discussion of\n+the matching semantics.\n++\n This is to emulate `git fetch` run on the `mothership` using `git\n push` that is run in the opposite direction in order to integrate\n the work done on `satellite`, and is often necessary when you can\n-- \n2.19.1.1182.g4ecb1133ce\n\n"},{"id":"363324","messageId":"xmqqk1lgnnxc.fsf@gitster-ct.c.googlers.com","threadId":"49524","inReplyTo":"20181113195245.14296-1-avarab@gmail.com","subject":"Re: [PATCH v4 0/7] fixes for unqualified push","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-14T07:00:47Z","receivedAt":"2018-11-14T07:00:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks for an update; will replace.\n"}]}