{"thread":{"id":"32780","subject":"[PATCH v3 6/8] upload-pack: optionally allow fetching from the tips of hidden refs","startedAt":"2013-01-30T18:45:34Z","lastAt":"2014-03-18T14:36:01Z","messageCount":54,"participants":["Junio C Hamano","Michael Haggerty","Jonathan Nieder","Jeff King","Duy Nguyen","Ævar Arnfjörð Bjarmason","Jed Brown","Shawn Pearce"],"isPatch":true,"patchVersion":3,"patchTotal":8},"messages":[{"id":"208297","messageId":"1359571542-19852-1-git-send-email-gitster@pobox.com","threadId":"32780","inReplyTo":null,"subject":"[PATCH v3 0/8] Hiding refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-30T18:45:34Z","receivedAt":"2013-01-30T18:45:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The third round.\n\n - Multi-valued variable transfer.hiderefs lists prefixes of ref\n hierarchies to be hidden from the requests coming over the\n network.\n\n - A configuration optionally allows uploadpack to accept fetch\n requests for an object at the tip of a hidden ref.\n\nElsewhere, we discussed \"delaying ref advertisement\" (aka \"expand\nrefs\"), but it is an orthogonal feature and this \"hiding refs\ncompletely from advertisement\" series does not attempt to address.\n\nPatch #2 (simplify request validation), #4 (clarify the codeflow),\nand #5 (use struct ref) are new. The are all long overdue clean-ups\nfor these codepaths.\n\nThe last patch is an illustration why it wouldn't make sense to\noptionally allow pushing into hidden refs, and not meant to be part\nof the series proper.\n\nFor those who missed it, earlier rounds are at:\n\n http://thread.gmane.org/gmane.comp.version-control.git/213951\n http://thread.gmane.org/gmane.comp.version-control.git/214888\n\nJunio C Hamano (8):\n upload-pack: share more code\n upload-pack: simplify request validation\n upload/receive-pack: allow hiding ref hierarchies\n parse_fetch_refspec(): clarify the codeflow a bit\n fetch: use struct ref to represent refs to be fetched\n upload-pack: optionally allow fetching from the tips of hidden refs\n fetch: fetch objects by their exact SHA-1 object names\n WIP: receive.allowupdatestohidden\n\n Documentation/config.txt | 23 +++++++++++\n builtin/fetch-pack.c | 40 +++++++++++++++----\n builtin/receive-pack.c | 31 +++++++++++++++\n cache.h | 3 +-\n fetch-pack.c | 101 ++++++++++++++++++++++++++++++++---------------\n fetch-pack.h | 11 +++---\n refs.c | 41 +++++++++++++++++++\n refs.h | 3 ++\n remote.c | 41 ++++++++++---------\n remote.h | 1 +\n t/t5512-ls-remote.sh | 9 +++++\n t/t5516-fetch-push.sh | 82 ++++++++++++++++++++++++++++++++++++++\n transport.c | 9 +----\n upload-pack.c | 86 ++++++++++++++++++++++++----------------\n 14 files changed, 374 insertions(+), 107 deletions(-)\n\n-- \n1.8.1.2.589.ga9b91ac\n"},{"id":"208292","messageId":"1359571542-19852-2-git-send-email-gitster@pobox.com","threadId":"32780","inReplyTo":"1359571542-19852-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 1/8] upload-pack: share more code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-30T18:45:35Z","receivedAt":"2013-01-30T18:45:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We mark the objects pointed at our refs with \"OUR_REF\" flag in two\nfunctions (mark_our_ref() and send_ref()), but we can just use the\nformer as a helper for the latter.\n\nUpdate the way mark_our_ref() prepares in-core object to use\nlookup_unknown_object() to delay reading the actual object data,\njust like we did in 435c833 (upload-pack: use peel_ref for ref\nadvertisements, 2012-10-04).\n\nSigned-off-by: Junio C Hamano \n---\n upload-pack.c | 31 ++++++++++++++-----------------\n 1 file changed, 14 insertions(+), 17 deletions(-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 95d8313..3dd220d 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -722,15 +722,28 @@ static void receive_needs(void)\n \tfree(shallows.objects);\n }\n \n+static int mark_our_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)\n+{\n+\tstruct object *o = lookup_unknown_object(sha1);\n+\tif (!o)\n+\t\tdie(\"git upload-pack: cannot find object %s:\", sha1_to_hex(sha1));\n+\tif (!(o->flags & OUR_REF)) {\n+\t\to->flags |= OUR_REF;\n+\t\tnr_our_refs++;\n+\t}\n+\treturn 0;\n+}\n+\n static int send_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)\n {\n \tstatic const char *capabilities = \"multi_ack thin-pack side-band\"\n \t\t\" side-band-64k ofs-delta shallow no-progress\"\n \t\t\" include-tag multi_ack_detailed\";\n-\tstruct object *o = lookup_unknown_object(sha1);\n \tconst char *refname_nons = strip_namespace(refname);\n \tunsigned char peeled[20];\n \n+\tmark_our_ref(refname, sha1, flag, cb_data);\n+\n \tif (capabilities)\n \t\tpacket_write(1, \"%s %s%c%s%s agent=%s\\n\",\n \t\t\t sha1_to_hex(sha1), refname_nons,\n@@ -740,27 +753,11 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n \telse\n \t\tpacket_write(1, \"%s %s\\n\", sha1_to_hex(sha1), refname_nons);\n \tcapabilities = NULL;\n-\tif (!(o->flags & OUR_REF)) {\n-\t\to->flags |= OUR_REF;\n-\t\tnr_our_refs++;\n-\t}\n \tif (!peel_ref(refname, peeled))\n \t\tpacket_write(1, \"%s %s^{}\\n\", sha1_to_hex(peeled), refname_nons);\n \treturn 0;\n }\n \n-static int mark_our_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)\n-{\n-\tstruct object *o = parse_object(sha1);\n-\tif (!o)\n-\t\tdie(\"git upload-pack: cannot find object %s:\", sha1_to_hex(sha1));\n-\tif (!(o->flags & OUR_REF)) {\n-\t\to->flags |= OUR_REF;\n-\t\tnr_our_refs++;\n-\t}\n-\treturn 0;\n-}\n-\n static void upload_pack(void)\n {\n \tif (advertise_refs || !stateless_rpc) {\n-- \n1.8.1.2.589.ga9b91ac\n"},{"id":"208293","messageId":"1359571542-19852-3-git-send-email-gitster@pobox.com","threadId":"32780","inReplyTo":"1359571542-19852-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 2/8] upload-pack: simplify request validation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-30T18:45:36Z","receivedAt":"2013-01-30T18:45:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Long time ago, we used to punt on a large (read: asking for more\nthan 256 refs) fetch request and instead sent a full pack, because\nwe couldn't fit many refs on the command line of rev-list we run\ninternally to enumerate the objects to be sent. To fix this,\n565ebbf (upload-pack: tighten request validation., 2005-10-24),\nadded a check to count the number of refs in the request and matched\nwith the number of refs we advertised, and changed the invocation of\nrev-list to pass \"--all\" to it, still keeping us under the command\nline argument limit.\n\nHowever, these days we feed the list of objects requested and the\nlist of objects the other end is known to have via standard input,\nso there is no longer a valid reason to special case a full clone\nrequest. Remove the code associated with \"create_full_pack\" to\nsimplify the logic.\n\nSigned-off-by: Junio C Hamano \n---\n upload-pack.c | 28 +++++++++++-----------------\n 1 file changed, 11 insertions(+), 17 deletions(-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 3dd220d..3a26a7b 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -28,7 +28,7 @@ static const char upload_pack_usage[] = \"git upload-pack [--strict] [--timeout=<\n \n static unsigned long oldest_have;\n \n-static int multi_ack, nr_our_refs;\n+static int multi_ack;\n static int no_done;\n static int use_thin_pack, use_ofs_delta, use_include_tag;\n static int no_progress, daemon_mode;\n@@ -139,7 +139,6 @@ static void create_pack_file(void)\n {\n \tstruct async rev_list;\n \tstruct child_process pack_objects;\n-\tint create_full_pack = (nr_our_refs == want_obj.nr && !have_obj.nr);\n \tchar data[8193], progress[128];\n \tchar abort_msg[] = \"aborting due to possible repository \"\n \t\t\"corruption on the remote side.\";\n@@ -151,9 +150,7 @@ static void create_pack_file(void)\n \targv[arg++] = \"pack-objects\";\n \tif (!shallow_nr) {\n \t\targv[arg++] = \"--revs\";\n-\t\tif (create_full_pack)\n-\t\t\targv[arg++] = \"--all\";\n-\t\telse if (use_thin_pack)\n+\t\tif (use_thin_pack)\n \t\t\targv[arg++] = \"--thin\";\n \t}\n \n@@ -185,15 +182,15 @@ static void create_pack_file(void)\n \t}\n \telse {\n \t\tFILE *pipe_fd = xfdopen(pack_objects.in, \"w\");\n-\t\tif (!create_full_pack) {\n-\t\t\tint i;\n-\t\t\tfor (i = 0; i < want_obj.nr; i++)\n-\t\t\t\tfprintf(pipe_fd, \"%s\\n\", sha1_to_hex(want_obj.objects[i].item->sha1));\n-\t\t\tfprintf(pipe_fd, \"--not\\n\");\n-\t\t\tfor (i = 0; i < have_obj.nr; i++)\n-\t\t\t\tfprintf(pipe_fd, \"%s\\n\", sha1_to_hex(have_obj.objects[i].item->sha1));\n-\t\t}\n+\t\tint i;\n \n+\t\tfor (i = 0; i < want_obj.nr; i++)\n+\t\t\tfprintf(pipe_fd, \"%s\\n\",\n+\t\t\t\tsha1_to_hex(want_obj.objects[i].item->sha1));\n+\t\tfprintf(pipe_fd, \"--not\\n\");\n+\t\tfor (i = 0; i < have_obj.nr; i++)\n+\t\t\tfprintf(pipe_fd, \"%s\\n\",\n+\t\t\t\tsha1_to_hex(have_obj.objects[i].item->sha1));\n \t\tfprintf(pipe_fd, \"\\n\");\n \t\tfflush(pipe_fd);\n \t\tfclose(pipe_fd);\n@@ -727,10 +724,7 @@ static int mark_our_ref(const char *refname, const unsigned char *sha1, int flag\n \tstruct object *o = lookup_unknown_object(sha1);\n \tif (!o)\n \t\tdie(\"git upload-pack: cannot find object %s:\", sha1_to_hex(sha1));\n-\tif (!(o->flags & OUR_REF)) {\n-\t\to->flags |= OUR_REF;\n-\t\tnr_our_refs++;\n-\t}\n+\to->flags |= OUR_REF;\n \treturn 0;\n }\n \n-- \n1.8.1.2.589.ga9b91ac\n"},{"id":"208295","messageId":"1359571542-19852-4-git-send-email-gitster@pobox.com","threadId":"32780","inReplyTo":"1359571542-19852-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 3/8] upload/receive-pack: allow hiding ref hierarchies","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-30T18:45:37Z","receivedAt":"2013-01-30T18:45:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Teach upload-pack and receive-pack to omit some refs from their\ninitial advertisements by paying attention to the transfer.hiderefs\nmulti-valued configuration variable. Any ref that is under the\nhierarchies listed on the value of this variable is excluded from\nresponses to requests made by \"ls-remote\", \"fetch\", \"clone\", \"push\",\netc.\n\nA typical use case may be\n\n\t[transfer]\n\t\thiderefs = refs/pull\n\nto hide the refs that are internally used by the hosting site and\nshould not be exposed over the network.\n\nBecause these hidden refs do not count as OUR_REF, an attempt to\nfetch objects at the tip of them will be rejected, and because these\nrefs do not get advertised, \"git push :\" will not see local branches\nthat have the same name as them as \"matching\" ones to be sent.\n\nAn attempt to update/delete these hidden refs with an explicit\nrefspec, e.g. \"git push origin :refs/pull/11/head\", is rejected.\n\nThis is not a new restriction. To the pusher, it would appear that\nthere is no such ref, so its push request will conclude with \"Now\nthat I sent you all the data, it is time for you to update the refs.\nI saw that the ref did not exist when I started pushing, and I want\nthe result to point at this commit\". The receiving end will apply\nthe compare-and-swap rule to this request and rejects the push with\n\"Well, your update request conflicts with somebody else; I see there\nis such a ref.\", which is the right thing to do. Otherwise a push to\na hidden ref will always be \"the last one wins\", which is not a good\ndefault.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/config.txt | 11 +++++++++++\n builtin/receive-pack.c | 24 ++++++++++++++++++++++++\n refs.c | 41 +++++++++++++++++++++++++++++++++++++++++\n refs.h | 3 +++\n t/t5512-ls-remote.sh | 9 +++++++++\n t/t5516-fetch-push.sh | 24 ++++++++++++++++++++++++\n upload-pack.c | 14 +++++++++++++-\n 7 files changed, 125 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex ef45c99..f57c802 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2057,6 +2057,17 @@ transfer.fsckObjects::\n \tnot set, the value of this variable is used instead.\n \tDefaults to false.\n \n+transfer.hiderefs::\n+\tString(s) `upload-pack` and `receive-pack` use to decide\n+\twhich refs to omit from their initial advertisement. Use\n+\tmore than one transfer.hiderefs configuration variables to\n+\tspecify multiple prefix strings. A ref that are under the\n+\thierarchies listed on the value of this variable is excluded,\n+\tand is hidden from `git ls-remote`, `git fetch`, `git push :`,\n+\tetc. An attempt to update or delete a hidden ref by `git push`\n+\tis rejected, and an attempt to fetch a hidden ref by `git fetch`\n+\twill fail.\n+\n transfer.unpackLimit::\n \tWhen `fetch.unpackLimit` or `receive.unpackLimit` are\n \tnot set, the value of this variable is used instead.\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex ff781fe..a8248d9 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -59,6 +59,11 @@ static enum deny_action parse_deny_action(const char *var, const char *value)\n \n static int receive_pack_config(const char *var, const char *value, void *cb)\n {\n+\tint status = parse_hide_refs_config(var, value, cb);\n+\n+\tif (status)\n+\t\treturn status;\n+\n \tif (strcmp(var, \"receive.denydeletes\") == 0) {\n \t\tdeny_deletes = git_config_bool(var, value);\n \t\treturn 0;\n@@ -119,6 +124,9 @@ static int receive_pack_config(const char *var, const char *value, void *cb)\n \n static void show_ref(const char *path, const unsigned char *sha1)\n {\n+\tif (ref_is_hidden(path))\n+\t\treturn;\n+\n \tif (sent_capabilities)\n \t\tpacket_write(1, \"%s %s\\n\", sha1_to_hex(sha1), path);\n \telse\n@@ -688,6 +696,20 @@ static int iterate_receive_command_list(void *cb_data, unsigned char sha1[20])\n \treturn -1; /* end of list */\n }\n \n+static void reject_updates_to_hidden(struct command *commands)\n+{\n+\tstruct command *cmd;\n+\n+\tfor (cmd = commands; cmd; cmd = cmd->next) {\n+\t\tif (cmd->error_string || !ref_is_hidden(cmd->ref_name))\n+\t\t\tcontinue;\n+\t\tif (is_null_sha1(cmd->new_sha1))\n+\t\t\tcmd->error_string = \"deny deleting a hidden ref\";\n+\t\telse\n+\t\t\tcmd->error_string = \"deny updating a hidden ref\";\n+\t}\n+}\n+\n static void execute_commands(struct command *commands, const char *unpacker_error)\n {\n \tstruct command *cmd;\n@@ -704,6 +726,8 @@ static void execute_commands(struct command *commands, const char *unpacker_erro\n \t\t\t\t 0, &cmd))\n \t\tset_connectivity_errors(commands);\n \n+\treject_updates_to_hidden(commands);\n+\n \tif (run_receive_hook(commands, pre_receive_hook, 0)) {\n \t\tfor (cmd = commands; cmd; cmd = cmd->next) {\n \t\t\tif (!cmd->error_string)\ndiff --git a/refs.c b/refs.c\nindex 541fec2..e3574ca 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3,6 +3,7 @@\n #include \"object.h\"\n #include \"tag.h\"\n #include \"dir.h\"\n+#include \"string-list.h\"\n \n /*\n * Make sure \"ref\" is something reasonable to have under \".git/refs/\";\n@@ -2556,3 +2557,43 @@ char *shorten_unambiguous_ref(const char *refname, int strict)\n \tfree(short_name);\n \treturn xstrdup(refname);\n }\n+\n+static struct string_list *hide_refs;\n+\n+int parse_hide_refs_config(const char *var, const char *value, void *unused)\n+{\n+\tif (!strcmp(\"transfer.hiderefs\", var)) {\n+\t\tchar *ref;\n+\t\tint len;\n+\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\tref = xstrdup(value);\n+\t\tlen = strlen(ref);\n+\t\twhile (len && ref[len - 1] == '/')\n+\t\t\tref[--len] = '\\0';\n+\t\tif (!hide_refs) {\n+\t\t\thide_refs = xcalloc(1, sizeof(*hide_refs));\n+\t\t\thide_refs->strdup_strings = 1;\n+\t\t}\n+\t\tstring_list_append(hide_refs, ref);\n+\t}\n+\treturn 0;\n+}\n+\n+int ref_is_hidden(const char *refname)\n+{\n+\tstruct string_list_item *item;\n+\n+\tif (!hide_refs)\n+\t\treturn 0;\n+\tfor_each_string_list_item(item, hide_refs) {\n+\t\tint len;\n+\t\tif (prefixcmp(refname, item->string))\n+\t\t\tcontinue;\n+\t\tlen = strlen(item->string);\n+\t\tif (!refname[len] || refname[len] == '/')\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\ndiff --git a/refs.h b/refs.h\nindex d6c2fe2..50b233f 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -147,4 +147,7 @@ int update_ref(const char *action, const char *refname,\n \t\tconst unsigned char *sha1, const unsigned char *oldval,\n \t\tint flags, enum action_on_err onerr);\n \n+extern int parse_hide_refs_config(const char *var, const char *value, void *);\n+extern int ref_is_hidden(const char *);\n+\n #endif /* REFS_H */\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex d16e5d3..d0702ed 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -126,4 +126,13 @@ test_expect_success 'Report match with --exit-code' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'Hide some refs' '\n+\ttest_config transfer.hiderefs refs/tags &&\n+\tgit ls-remote . >actual &&\n+\ttest_unconfig transfer.hiderefs &&\n+\tgit ls-remote . |\n+\tsed -e \"/\trefs\\/tags\\//d\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 6009372..852efb6 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1037,4 +1037,28 @@ test_expect_success 'push --prune refspec' '\n \t! check_push_result $the_first_commit tmp/foo tmp/bar\n '\n \n+test_expect_success 'push to update a hidden ref' '\n+\tmk_test heads/master hidden/one hidden/two hidden/three &&\n+\t(\n+\t\tcd testrepo &&\n+\t\tgit config transfer.hiderefs refs/hidden\n+\t) &&\n+\n+\t# push to unhidden ref succeeds normally\n+\tgit push testrepo master:refs/heads/master &&\n+\tcheck_push_result $the_commit heads/master &&\n+\n+\t# push to update a hidden ref should fail\n+\ttest_must_fail git push testrepo master:refs/hidden/one &&\n+\tcheck_push_result $the_first_commit hidden/one &&\n+\n+\t# push to delete a hidden ref should fail\n+\ttest_must_fail git push testrepo :refs/hidden/two &&\n+\tcheck_push_result $the_first_commit hidden/two &&\n+\n+\t# idempotent push to update a hidden ref should fail\n+\ttest_must_fail git push testrepo $the_first_commit:refs/hidden/three &&\n+\tcheck_push_result $the_first_commit hidden/three\n+'\n+\n test_done\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 3a26a7b..6b10843 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -12,6 +12,7 @@\n #include \"run-command.h\"\n #include \"sigchain.h\"\n #include \"version.h\"\n+#include \"string-list.h\"\n \n static const char upload_pack_usage[] = \"git upload-pack [--strict] [--timeout=] \";\n \n@@ -719,9 +720,13 @@ static void receive_needs(void)\n \tfree(shallows.objects);\n }\n \n+/* return non-zero if the ref is hidden, otherwise 0 */\n static int mark_our_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)\n {\n \tstruct object *o = lookup_unknown_object(sha1);\n+\n+\tif (ref_is_hidden(refname))\n+\t\treturn 1;\n \tif (!o)\n \t\tdie(\"git upload-pack: cannot find object %s:\", sha1_to_hex(sha1));\n \to->flags |= OUR_REF;\n@@ -736,7 +741,8 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n \tconst char *refname_nons = strip_namespace(refname);\n \tunsigned char peeled[20];\n \n-\tmark_our_ref(refname, sha1, flag, cb_data);\n+\tif (mark_our_ref(refname, sha1, flag, cb_data))\n+\t\treturn 0;\n \n \tif (capabilities)\n \t\tpacket_write(1, \"%s %s%c%s%s agent=%s\\n\",\n@@ -773,6 +779,11 @@ static void upload_pack(void)\n \t}\n }\n \n+static int upload_pack_config(const char *var, const char *value, void *unused)\n+{\n+\treturn parse_hide_refs_config(var, value, unused);\n+}\n+\n int main(int argc, char **argv)\n {\n \tchar *dir;\n@@ -824,6 +835,7 @@ int main(int argc, char **argv)\n \t\tdie(\"'%s' does not appear to be a git repository\", dir);\n \tif (is_repository_shallow())\n \t\tdie(\"attempt to fetch/clone from a shallow repository\");\n+\tgit_config(upload_pack_config, NULL);\n \tif (getenv(\"GIT_DEBUG_SEND_PACK\"))\n \t\tdebug_fd = atoi(getenv(\"GIT_DEBUG_SEND_PACK\"));\n \tupload_pack();\n-- \n1.8.1.2.589.ga9b91ac\n"},{"id":"208294","messageId":"1359571542-19852-5-git-send-email-gitster@pobox.com","threadId":"32780","inReplyTo":"1359571542-19852-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 4/8] parse_fetch_refspec(): clarify the codeflow a bit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-30T18:45:38Z","receivedAt":"2013-01-30T18:45:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Most parts of the cascaded if/else if/... checked an allowable\ncondition but some checked forbidden conditions. This makes adding\nnew allowable conditions unnecessarily inconvenient.\n\nSigned-off-by: Junio C Hamano \n---\n remote.c | 29 ++++++++++++-----------------\n 1 file changed, 12 insertions(+), 17 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 4b1153f..1b7828d 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -538,7 +538,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \n \t\t/*\n \t\t * Before going on, special case \":\" (or \"+:\") as a refspec\n-\t\t * for matching refs.\n+\t\t * for pushing matching refs.\n \t\t */\n \t\tif (!fetch && rhs == lhs && rhs[1] == '\\0') {\n \t\t\trs[i].matching = 1;\n@@ -565,26 +565,21 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \t\tflags = REFNAME_ALLOW_ONELEVEL | (is_glob ? REFNAME_REFSPEC_PATTERN : 0);\n \n \t\tif (fetch) {\n-\t\t\t/*\n-\t\t\t * LHS\n-\t\t\t * - empty is allowed; it means HEAD.\n-\t\t\t * - otherwise it must be a valid looking ref.\n-\t\t\t */\n+\t\t\t/* LHS */\n \t\t\tif (!*rs[i].src)\n-\t\t\t\t; /* empty is ok */\n-\t\t\telse if (check_refname_format(rs[i].src, flags))\n+\t\t\t\t; /* empty is ok; it means \"HEAD\" */\n+\t\t\telse if (!check_refname_format(rs[i].src, flags))\n+\t\t\t\t; /* valid looking ref is ok */\n+\t\t\telse\n \t\t\t\tgoto invalid;\n-\t\t\t/*\n-\t\t\t * RHS\n-\t\t\t * - missing is ok, and is same as empty.\n-\t\t\t * - empty is ok; it means not to store.\n-\t\t\t * - otherwise it must be a valid looking ref.\n-\t\t\t */\n+\t\t\t/* RHS */\n \t\t\tif (!rs[i].dst)\n-\t\t\t\t; /* ok */\n+\t\t\t\t; /* missing is ok; it is the same as empty */\n \t\t\telse if (!*rs[i].dst)\n-\t\t\t\t; /* ok */\n-\t\t\telse if (check_refname_format(rs[i].dst, flags))\n+\t\t\t\t; /* empty is ok; it means \"do not store\" */\n+\t\t\telse if (!check_refname_format(rs[i].dst, flags))\n+\t\t\t\t; /* valid looking ref is ok */\n+\t\t\telse\n \t\t\t\tgoto invalid;\n \t\t} else {\n \t\t\t/*\n-- \n1.8.1.2.589.ga9b91ac\n"},{"id":"208296","messageId":"1359571542-19852-6-git-send-email-gitster@pobox.com","threadId":"32780","inReplyTo":"1359571542-19852-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 5/8] fetch: use struct ref to represent refs to be fetched","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-30T18:45:39Z","receivedAt":"2013-01-30T18:45:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Even though \"git fetch\" has full infrastructure to parse refspecs to\nbe fetched and match them against the list of refs to come up with\nthe final list of refs to be fetched, the list of refs that are\nrequested to be fetched were internally converted to a plain list of\nstrings at the transport layer and then passed to the underlying\nfetch-pack driver.\n\nStop this conversion and instead pass around an array of refs.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/fetch-pack.c | 40 ++++++++++++++++++++------\n cache.h | 3 +-\n fetch-pack.c | 79 +++++++++++++++++++++++++++++++---------------------\n fetch-pack.h | 11 ++++----\n transport.c | 9 ++----\n 5 files changed, 89 insertions(+), 53 deletions(-)\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 940ae35..cc6bf8f 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -7,12 +7,31 @@ static const char fetch_pack_usage[] =\n \"[--include-tag] [--upload-pack=] [--depth=] \"\n \"[--no-progress] [-v] [:] [...]\";\n \n+static void add_sought_entry_mem(struct ref ***sought, int *nr, int *alloc,\n+\t\t\t\t const char *name, int namelen)\n+{\n+\tstruct ref *ref = xcalloc(1, sizeof(*ref) + namelen + 1);\n+\n+\tmemcpy(ref->name, name, namelen);\n+\tref->name[namelen] = '\\0';\n+\t(*nr)++;\n+\tALLOC_GROW(*sought, *nr, *alloc);\n+\t(*sought)[*nr - 1] = ref;\n+}\n+\n+static void add_sought_entry(struct ref ***sought, int *nr, int *alloc,\n+\t\t\t const char *string)\n+{\n+\tadd_sought_entry_mem(sought, nr, alloc, string, strlen(string));\n+}\n+\n int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n {\n \tint i, ret;\n \tstruct ref *ref = NULL;\n \tconst char *dest = NULL;\n-\tstruct string_list sought = STRING_LIST_INIT_DUP;\n+\tstruct ref **sought;\n+\tint nr_sought = 0, alloc_sought = 0;\n \tint fd[2];\n \tchar *pack_lockfile = NULL;\n \tchar **pack_lockfile_ptr = NULL;\n@@ -94,7 +113,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \t * refs from the standard input:\n \t */\n \tfor (; i < argc; i++)\n-\t\tstring_list_append(&sought, xstrdup(argv[i]));\n+\t\tadd_sought_entry(&sought, &nr_sought, &alloc_sought, argv[i]);\n \tif (args.stdin_refs) {\n \t\tif (args.stateless_rpc) {\n \t\t\t/* in stateless RPC mode we use pkt-line to read\n@@ -107,14 +126,14 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tbreak;\n \t\t\t\tif (line[n-1] == '\\n')\n \t\t\t\t\tn--;\n-\t\t\t\tstring_list_append(&sought, xmemdupz(line, n));\n+\t\t\t\tadd_sought_entry_mem(&sought, &nr_sought, &alloc_sought, line, n);\n \t\t\t}\n \t\t}\n \t\telse {\n \t\t\t/* read from stdin one ref per line, until EOF */\n \t\t\tstruct strbuf line = STRBUF_INIT;\n \t\t\twhile (strbuf_getline(&line, stdin, '\\n') != EOF)\n-\t\t\t\tstring_list_append(&sought, strbuf_detach(&line, NULL));\n+\t\t\t\tadd_sought_entry(&sought, &nr_sought, &alloc_sought, line.buf);\n \t\t\tstrbuf_release(&line);\n \t\t}\n \t}\n@@ -131,7 +150,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \tget_remote_heads(fd[0], &ref, 0, NULL);\n \n \tref = fetch_pack(&args, fd, conn, ref, dest,\n-\t\t\t &sought, pack_lockfile_ptr);\n+\t\t\t sought, nr_sought, pack_lockfile_ptr);\n \tif (pack_lockfile) {\n \t\tprintf(\"lock %s\\n\", pack_lockfile);\n \t\tfflush(stdout);\n@@ -141,7 +160,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \tif (finish_connect(conn))\n \t\treturn 1;\n \n-\tret = !ref || sought.nr;\n+\tret = !ref;\n \n \t/*\n \t * If the heads to pull were given, we should have consumed\n@@ -149,8 +168,13 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \t * remote no-such-ref' would silently succeed without issuing\n \t * an error.\n \t */\n-\tfor (i = 0; i < sought.nr; i++)\n-\t\terror(\"no such remote ref %s\", sought.items[i].string);\n+\tfor (i = 0; i < nr_sought; i++) {\n+\t\tif (!sought[i] || sought[i]->matched)\n+\t\t\tcontinue;\n+\t\terror(\"no such remote ref %s\", sought[i]->name);\n+\t\tret = 1;\n+\t}\n+\n \twhile (ref) {\n \t\tprintf(\"%s %s\\n\",\n \t\t sha1_to_hex(ref->old_sha1), ref->name);\ndiff --git a/cache.h b/cache.h\nindex c257953..03b3285 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1013,7 +1013,8 @@ struct ref {\n \t\tnonfastforward:1,\n \t\tnot_forwardable:1,\n \t\tupdate:1,\n-\t\tdeletion:1;\n+\t\tdeletion:1,\n+\t\tmatched:1;\n \tenum {\n \t\tREF_STATUS_NONE = 0,\n \t\tREF_STATUS_OK,\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex f0acdf7..915c0b7 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -520,47 +520,37 @@ static void mark_recent_complete_commits(struct fetch_pack_args *args,\n \t}\n }\n \n-static int non_matching_ref(struct string_list_item *item, void *unused)\n-{\n-\tif (item->util) {\n-\t\titem->util = NULL;\n-\t\treturn 0;\n-\t}\n-\telse\n-\t\treturn 1;\n-}\n-\n static void filter_refs(struct fetch_pack_args *args,\n-\t\t\tstruct ref **refs, struct string_list *sought)\n+\t\t\tstruct ref **refs,\n+\t\t\tstruct ref **sought, int nr_sought)\n {\n \tstruct ref *newlist = NULL;\n \tstruct ref **newtail = &newlist;\n \tstruct ref *ref, *next;\n-\tint sought_pos;\n+\tint i;\n \n-\tsought_pos = 0;\n+\ti = 0;\n \tfor (ref = *refs; ref; ref = next) {\n \t\tint keep = 0;\n \t\tnext = ref->next;\n+\n \t\tif (!memcmp(ref->name, \"refs/\", 5) &&\n \t\t check_refname_format(ref->name + 5, 0))\n \t\t\t; /* trash */\n \t\telse {\n-\t\t\twhile (sought_pos < sought->nr) {\n-\t\t\t\tint cmp = strcmp(ref->name, sought->items[sought_pos].string);\n+\t\t\twhile (i < nr_sought) {\n+\t\t\t\tint cmp = strcmp(ref->name, sought[i]->name);\n \t\t\t\tif (cmp < 0)\n \t\t\t\t\tbreak; /* definitely do not have it */\n \t\t\t\telse if (cmp == 0) {\n \t\t\t\t\tkeep = 1; /* definitely have it */\n-\t\t\t\t\tsought->items[sought_pos++].util = \"matched\";\n-\t\t\t\t\tbreak;\n+\t\t\t\t\tsought[i]->matched = 1;\n \t\t\t\t}\n-\t\t\t\telse\n-\t\t\t\t\tsought_pos++; /* might have it; keep looking */\n+\t\t\t\ti++;\n \t\t\t}\n \t\t}\n \n-\t\tif (! keep && args->fetch_all &&\n+\t\tif (!keep && args->fetch_all &&\n \t\t (!args->depth || prefixcmp(ref->name, \"refs/tags/\")))\n \t\t\tkeep = 1;\n \n@@ -573,7 +563,6 @@ static void filter_refs(struct fetch_pack_args *args,\n \t\t}\n \t}\n \n-\tfilter_string_list(sought, 0, non_matching_ref, NULL);\n \t*refs = newlist;\n }\n \n@@ -583,7 +572,8 @@ static void mark_alternate_complete(const struct ref *ref, void *unused)\n }\n \n static int everything_local(struct fetch_pack_args *args,\n-\t\t\t struct ref **refs, struct string_list *sought)\n+\t\t\t struct ref **refs,\n+\t\t\t struct ref **sought, int nr_sought)\n {\n \tstruct ref *ref;\n \tint retval;\n@@ -634,7 +624,7 @@ static int everything_local(struct fetch_pack_args *args,\n \t\t}\n \t}\n \n-\tfilter_refs(args, refs, sought);\n+\tfilter_refs(args, refs, sought, nr_sought);\n \n \tfor (retval = 1, ref = *refs; ref ; ref = ref->next) {\n \t\tconst unsigned char *remote = ref->old_sha1;\n@@ -764,10 +754,17 @@ static int get_pack(struct fetch_pack_args *args,\n \treturn 0;\n }\n \n+static int cmp_ref_by_name(const void *a_, const void *b_)\n+{\n+\tconst struct ref *a = *((const struct ref **)a_);\n+\tconst struct ref *b = *((const struct ref **)b_);\n+\treturn strcmp(a->name, b->name);\n+}\n+\n static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n \t\t\t\t int fd[2],\n \t\t\t\t const struct ref *orig_ref,\n-\t\t\t\t struct string_list *sought,\n+\t\t\t\t struct ref **sought, int nr_sought,\n \t\t\t\t char **pack_lockfile)\n {\n \tstruct ref *ref = copy_ref_list(orig_ref);\n@@ -776,6 +773,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n \tint agent_len;\n \n \tsort_ref_list(&ref, ref_compare_name);\n+\tqsort(sought, nr_sought, sizeof(*sought), cmp_ref_by_name);\n \n \tif (is_repository_shallow() && !server_supports(\"shallow\"))\n \t\tdie(\"Server does not support shallow clients\");\n@@ -824,7 +822,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n \t\t\t\tagent_len, agent_feature);\n \t}\n \n-\tif (everything_local(args, &ref, sought)) {\n+\tif (everything_local(args, &ref, sought, nr_sought)) {\n \t\tpacket_flush(fd[1]);\n \t\tgoto all_done;\n \t}\n@@ -887,11 +885,32 @@ static void fetch_pack_setup(void)\n \tdid_setup = 1;\n }\n \n+static int remove_duplicates_in_refs(struct ref **ref, int nr)\n+{\n+\tstruct string_list names = STRING_LIST_INIT_NODUP;\n+\tint src, dst;\n+\n+\tfor (src = dst = 0; src < nr; src++) {\n+\t\tstruct string_list_item *item;\n+\t\titem = string_list_insert(&names, ref[src]->name);\n+\t\tif (item->util)\n+\t\t\tcontinue; /* already have it */\n+\t\titem->util = ref[src];\n+\t\tif (src != dst)\n+\t\t\tref[dst] = ref[src];\n+\t\tdst++;\n+\t}\n+\tfor (src = dst; src < nr; src++)\n+\t\tref[src] = NULL;\n+\tstring_list_clear(&names, 0);\n+\treturn dst;\n+}\n+\n struct ref *fetch_pack(struct fetch_pack_args *args,\n \t\t int fd[], struct child_process *conn,\n \t\t const struct ref *ref,\n \t\t const char *dest,\n-\t\t struct string_list *sought,\n+\t\t struct ref **sought, int nr_sought,\n \t\t char **pack_lockfile)\n {\n \tstruct stat st;\n@@ -903,16 +922,14 @@ struct ref *fetch_pack(struct fetch_pack_args *args,\n \t\t\tst.st_mtime = 0;\n \t}\n \n-\tif (sought->nr) {\n-\t\tsort_string_list(sought);\n-\t\tstring_list_remove_duplicates(sought, 0);\n-\t}\n+\tif (nr_sought)\n+\t\tnr_sought = remove_duplicates_in_refs(sought, nr_sought);\n \n \tif (!ref) {\n \t\tpacket_flush(fd[1]);\n \t\tdie(\"no matching remote head\");\n \t}\n-\tref_cpy = do_fetch_pack(args, fd, ref, sought, pack_lockfile);\n+\tref_cpy = do_fetch_pack(args, fd, ref, sought, nr_sought, pack_lockfile);\n \n \tif (args->depth > 0) {\n \t\tstatic struct lock_file lock;\ndiff --git a/fetch-pack.h b/fetch-pack.h\nindex cb14871..dc5266c 100644\n--- a/fetch-pack.h\n+++ b/fetch-pack.h\n@@ -20,17 +20,16 @@ struct fetch_pack_args {\n };\n \n /*\n- * sought contains the full names of remote references that should be\n- * updated from. On return, the names that were found on the remote\n- * will have been removed from the list. The util members of the\n- * string_list_items are used internally; they must be NULL on entry\n- * (and will be NULL on exit).\n+ * sought represents remote references that should be updated from.\n+ * On return, the names that were found on the remote will have been\n+ * marked as such.\n */\n struct ref *fetch_pack(struct fetch_pack_args *args,\n \t\t int fd[], struct child_process *conn,\n \t\t const struct ref *ref,\n \t\t const char *dest,\n-\t\t struct string_list *sought,\n+\t\t struct ref **sought,\n+\t\t int nr_sought,\n \t\t char **pack_lockfile);\n \n #endif\ndiff --git a/transport.c b/transport.c\nindex 2673d27..64ce651 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -518,11 +518,9 @@ static int fetch_refs_via_pack(struct transport *transport,\n \t\t\t int nr_heads, struct ref **to_fetch)\n {\n \tstruct git_transport_data *data = transport->data;\n-\tstruct string_list sought = STRING_LIST_INIT_DUP;\n \tconst struct ref *refs;\n \tchar *dest = xstrdup(transport->url);\n \tstruct fetch_pack_args args;\n-\tint i;\n \tstruct ref *refs_tmp = NULL;\n \n \tmemset(&args, 0, sizeof(args));\n@@ -536,9 +534,6 @@ static int fetch_refs_via_pack(struct transport *transport,\n \targs.no_progress = !transport->progress;\n \targs.depth = data->options.depth;\n \n-\tfor (i = 0; i < nr_heads; i++)\n-\t\tstring_list_append(&sought, to_fetch[i]->name);\n-\n \tif (!data->got_remote_heads) {\n \t\tconnect_setup(transport, 0, 0);\n \t\tget_remote_heads(data->fd[0], &refs_tmp, 0, NULL);\n@@ -547,7 +542,8 @@ static int fetch_refs_via_pack(struct transport *transport,\n \n \trefs = fetch_pack(&args, data->fd, data->conn,\n \t\t\t refs_tmp ? refs_tmp : transport->remote_refs,\n-\t\t\t dest, &sought, &transport->pack_lockfile);\n+\t\t\t dest, to_fetch, nr_heads,\n+\t\t\t &transport->pack_lockfile);\n \tclose(data->fd[0]);\n \tclose(data->fd[1]);\n \tif (finish_connect(data->conn))\n@@ -557,7 +553,6 @@ static int fetch_refs_via_pack(struct transport *transport,\n \n \tfree_refs(refs_tmp);\n \n-\tstring_list_clear(&sought, 0);\n \tfree(dest);\n \treturn (refs ? 0 : -1);\n }\n-- \n1.8.1.2.589.ga9b91ac\n"},{"id":"208290","messageId":"1359571542-19852-7-git-send-email-gitster@pobox.com","threadId":"32780","inReplyTo":"1359571542-19852-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 6/8] upload-pack: optionally allow fetching from the tips of hidden refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-30T18:45:40Z","receivedAt":"2013-01-30T18:45:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With uploadpack.allowtipsha1inwant configuration option set, future\nversions of \"git fetch\" that allow an exact object name (likely to\nhave been obtained out of band) on the LHS of the fetch refspec can\nmake a request with a \"want\" line that names an object that may not\nhave been advertised due to transfer.hiderefs configuration.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/config.txt | 8 +++++++-\n upload-pack.c | 25 +++++++++++++++++++------\n 2 files changed, 26 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex f57c802..2dce021 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2066,13 +2066,19 @@ transfer.hiderefs::\n \tand is hidden from `git ls-remote`, `git fetch`, `git push :`,\n \tetc. An attempt to update or delete a hidden ref by `git push`\n \tis rejected, and an attempt to fetch a hidden ref by `git fetch`\n-\twill fail.\n+\twill fail. See also `uploadpack.allowtipsha1inwant`.\n \n transfer.unpackLimit::\n \tWhen `fetch.unpackLimit` or `receive.unpackLimit` are\n \tnot set, the value of this variable is used instead.\n \tThe default value is 100.\n \n+uploadpack.allowtipsha1inwant::\n+\tWhen `transfer.hiderefs` is in effect, allow `upload-pack`\n+\tto accept a fetch request that asks for an object at the tip\n+\tof a hidden ref (by default, such a request is rejected).\n+\tsee also `transfer.hiderefs`.\n+\n url..insteadOf::\n \tAny URL that starts with this value will be rewritten to\n \tstart, instead, with . In cases where some site serves a\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 6b10843..37977e2 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -26,6 +26,7 @@ static const char upload_pack_usage[] = \"git upload-pack [--strict] [--timeout=<\n #define SHALLOW\t\t(1u << 16)\n #define NOT_SHALLOW\t(1u << 17)\n #define CLIENT_SHALLOW\t(1u << 18)\n+#define HIDDEN_REF\t(1u << 19)\n \n static unsigned long oldest_have;\n \n@@ -33,6 +34,7 @@ static int multi_ack;\n static int no_done;\n static int use_thin_pack, use_ofs_delta, use_include_tag;\n static int no_progress, daemon_mode;\n+static int allow_tip_sha1_in_want;\n static int shallow_nr;\n static struct object_array have_obj;\n static struct object_array want_obj;\n@@ -487,6 +489,12 @@ static int get_common_commits(void)\n \t}\n }\n \n+static int is_our_ref(struct object *o)\n+{\n+\treturn o->flags &\n+\t\t((allow_tip_sha1_in_want ? HIDDEN_REF : 0) | OUR_REF);\n+}\n+\n static void check_non_tip(void)\n {\n \tstatic const char *argv[] = {\n@@ -523,7 +531,7 @@ static void check_non_tip(void)\n \t\to = get_indexed_object(--i);\n \t\tif (!o)\n \t\t\tcontinue;\n-\t\tif (!(o->flags & OUR_REF))\n+\t\tif (!is_our_ref(o))\n \t\t\tcontinue;\n \t\tmemcpy(namebuf + 1, sha1_to_hex(o->sha1), 40);\n \t\tif (write_in_full(cmd.in, namebuf, 42) < 0)\n@@ -532,7 +540,7 @@ static void check_non_tip(void)\n \tnamebuf[40] = '\\n';\n \tfor (i = 0; i < want_obj.nr; i++) {\n \t\to = want_obj.objects[i].item;\n-\t\tif (o->flags & OUR_REF)\n+\t\tif (is_our_ref(o))\n \t\t\tcontinue;\n \t\tmemcpy(namebuf, sha1_to_hex(o->sha1), 40);\n \t\tif (write_in_full(cmd.in, namebuf, 41) < 0)\n@@ -566,7 +574,7 @@ error:\n \t/* Pick one of them (we know there at least is one) */\n \tfor (i = 0; i < want_obj.nr; i++) {\n \t\to = want_obj.objects[i].item;\n-\t\tif (!(o->flags & OUR_REF))\n+\t\tif (!is_our_ref(o))\n \t\t\tdie(\"git upload-pack: not our ref %s\",\n \t\t\t sha1_to_hex(o->sha1));\n \t}\n@@ -646,7 +654,7 @@ static void receive_needs(void)\n \t\t\t sha1_to_hex(sha1_buf));\n \t\tif (!(o->flags & WANTED)) {\n \t\t\to->flags |= WANTED;\n-\t\t\tif (!(o->flags & OUR_REF))\n+\t\t\tif (!is_our_ref(o))\n \t\t\t\thas_non_tip = 1;\n \t\t\tadd_object_array(o, NULL, &want_obj);\n \t\t}\n@@ -725,8 +733,10 @@ static int mark_our_ref(const char *refname, const unsigned char *sha1, int flag\n {\n \tstruct object *o = lookup_unknown_object(sha1);\n \n-\tif (ref_is_hidden(refname))\n+\tif (ref_is_hidden(refname)) {\n+\t\to->flags |= HIDDEN_REF;\n \t\treturn 1;\n+\t}\n \tif (!o)\n \t\tdie(\"git upload-pack: cannot find object %s:\", sha1_to_hex(sha1));\n \to->flags |= OUR_REF;\n@@ -745,9 +755,10 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n \t\treturn 0;\n \n \tif (capabilities)\n-\t\tpacket_write(1, \"%s %s%c%s%s agent=%s\\n\",\n+\t\tpacket_write(1, \"%s %s%c%s%s%s agent=%s\\n\",\n \t\t\t sha1_to_hex(sha1), refname_nons,\n \t\t\t 0, capabilities,\n+\t\t\t allow_tip_sha1_in_want ? \" allow-tip-sha1-in-want\" : \"\",\n \t\t\t stateless_rpc ? \" no-done\" : \"\",\n \t\t\t git_user_agent_sanitized());\n \telse\n@@ -781,6 +792,8 @@ static void upload_pack(void)\n \n static int upload_pack_config(const char *var, const char *value, void *unused)\n {\n+\tif (!strcmp(\"uploadpack.allowtipsha1inwant\", var))\n+\t\tallow_tip_sha1_in_want = git_config_bool(var, value);\n \treturn parse_hide_refs_config(var, value, unused);\n }\n \n-- \n1.8.1.2.589.ga9b91ac\n"},{"id":"208298","messageId":"1359571542-19852-8-git-send-email-gitster@pobox.com","threadId":"32780","inReplyTo":"1359571542-19852-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 7/8] fetch: fetch objects by their exact SHA-1 object names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-30T18:45:41Z","receivedAt":"2013-01-30T18:45:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Teach \"git fetch\" to accept an exact SHA-1 object name the user may\nobtain out of band on the LHS of a pathspec, and send it on a \"want\"\nmessage when the server side advertises the allow-tip-sha1-in-want\ncapability.\n\nSigned-off-by: Junio C Hamano \n---\n fetch-pack.c | 22 +++++++++++++++++++++-\n remote.c | 12 +++++++++++-\n remote.h | 1 +\n t/t5516-fetch-push.sh | 34 ++++++++++++++++++++++++++++++++++\n 4 files changed, 67 insertions(+), 2 deletions(-)\n\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 915c0b7..70db646 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -36,7 +36,7 @@ static int marked;\n #define MAX_IN_VAIN 256\n \n static struct commit_list *rev_list;\n-static int non_common_revs, multi_ack, use_sideband;\n+static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;\n \n static void rev_list_push(struct commit *commit, int mark)\n {\n@@ -563,6 +563,21 @@ static void filter_refs(struct fetch_pack_args *args,\n \t\t}\n \t}\n \n+\t/* Append unmatched requests to the list */\n+\tif (allow_tip_sha1_in_want) {\n+\t\tfor (i = 0; i < nr_sought; i++) {\n+\t\t\tref = sought[i];\n+\t\t\tif (ref->matched)\n+\t\t\t\tcontinue;\n+\t\t\tif (get_sha1_hex(ref->name, ref->old_sha1))\n+\t\t\t\tcontinue;\n+\n+\t\t\tref->matched = 1;\n+\t\t\t*newtail = ref;\n+\t\t\tref->next = NULL;\n+\t\t\tnewtail = &ref->next;\n+\t\t}\n+\t}\n \t*refs = newlist;\n }\n \n@@ -803,6 +818,11 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n \t\t\tfprintf(stderr, \"Server supports side-band\\n\");\n \t\tuse_sideband = 1;\n \t}\n+\tif (server_supports(\"allow-tip-sha1-in-want\")) {\n+\t\tif (args->verbose)\n+\t\t\tfprintf(stderr, \"Server supports allow-tip-sha1-in-want\\n\");\n+\t\tallow_tip_sha1_in_want = 1;\n+\t}\n \tif (!server_supports(\"thin-pack\"))\n \t\targs->use_thin_pack = 0;\n \tif (!server_supports(\"no-progress\"))\ndiff --git a/remote.c b/remote.c\nindex 1b7828d..1118d05 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -15,6 +15,7 @@ static struct refspec s_tag_refspec = {\n \t0,\n \t1,\n \t0,\n+\t0,\n \t\"refs/tags/*\",\n \t\"refs/tags/*\"\n };\n@@ -565,9 +566,13 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \t\tflags = REFNAME_ALLOW_ONELEVEL | (is_glob ? REFNAME_REFSPEC_PATTERN : 0);\n \n \t\tif (fetch) {\n+\t\t\tunsigned char unused[40];\n+\n \t\t\t/* LHS */\n \t\t\tif (!*rs[i].src)\n \t\t\t\t; /* empty is ok; it means \"HEAD\" */\n+\t\t\telse if (llen == 40 && !get_sha1_hex(rs[i].src, unused))\n+\t\t\t\trs[i].exact_sha1 = 1; /* ok */\n \t\t\telse if (!check_refname_format(rs[i].src, flags))\n \t\t\t\t; /* valid looking ref is ok */\n \t\t\telse\n@@ -1495,7 +1500,12 @@ int get_fetch_map(const struct ref *remote_refs,\n \t} else {\n \t\tconst char *name = refspec->src[0] ? refspec->src : \"HEAD\";\n \n-\t\tref_map = get_remote_ref(remote_refs, name);\n+\t\tif (refspec->exact_sha1) {\n+\t\t\tref_map = alloc_ref(name);\n+\t\t\tget_sha1_hex(name, ref_map->old_sha1);\n+\t\t} else {\n+\t\t\tref_map = get_remote_ref(remote_refs, name);\n+\t\t}\n \t\tif (!missing_ok && !ref_map)\n \t\t\tdie(\"Couldn't find remote ref %s\", name);\n \t\tif (ref_map) {\ndiff --git a/remote.h b/remote.h\nindex 251d8fd..f7b08f1 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -62,6 +62,7 @@ struct refspec {\n \tunsigned force : 1;\n \tunsigned pattern : 1;\n \tunsigned matching : 1;\n+\tunsigned exact_sha1 : 1;\n \n \tchar *src;\n \tchar *dst;\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 852efb6..522056f 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1037,6 +1037,40 @@ test_expect_success 'push --prune refspec' '\n \t! check_push_result $the_first_commit tmp/foo tmp/bar\n '\n \n+test_expect_success 'fetch exact SHA1' '\n+\tmk_test heads/master hidden/one &&\n+\tgit push testrepo master:refs/hidden/one &&\n+\t(\n+\t\tcd testrepo &&\n+\t\tgit config transfer.hiderefs refs/hidden\n+\t) &&\n+\tcheck_push_result $the_commit hidden/one &&\n+\n+\tmk_child child &&\n+\t(\n+\t\tcd child &&\n+\n+\t\t# make sure $the_commit does not exist here\n+\t\tgit repack -a -d &&\n+\t\tgit prune &&\n+\t\ttest_must_fail git cat-file -t $the_commit &&\n+\n+\t\t# fetching the hidden object should fail by default\n+\t\ttest_must_fail git fetch -v ../testrepo $the_commit:refs/heads/copy &&\n+\t\ttest_must_fail git rev-parse --verify refs/heads/copy &&\n+\n+\t\t# the server side can allow it to succeed\n+\t\t(\n+\t\t\tcd ../testrepo &&\n+\t\t\tgit config uploadpack.allowtipsha1inwant true\n+\t\t) &&\n+\n+\t\tgit fetch -v ../testrepo $the_commit:refs/heads/copy &&\n+\t\tresult=$(git rev-parse --verify refs/heads/copy) &&\n+\t\ttest \"$the_commit\" = \"$result\"\n+\t)\n+'\n+\n test_expect_success 'push to update a hidden ref' '\n \tmk_test heads/master hidden/one hidden/two hidden/three &&\n \t(\n-- \n1.8.1.2.589.ga9b91ac\n"},{"id":"208291","messageId":"1359571542-19852-9-git-send-email-gitster@pobox.com","threadId":"32780","inReplyTo":"1359571542-19852-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 8/8] WIP: receive.allowupdatestohidden","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-30T18:45:42Z","receivedAt":"2013-01-30T18:45:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This does not work yet, and for a good reason. As the side that\npushes to a hidden ref never sees that the ref already exists, a\nrequest to update such a ref will come in the form of \"please\n_create_ this ref and point it at this object\", which will not pass\nthe compare-and-swap based anti-race safety at the receiving end.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/config.txt | 6 ++++++\n builtin/receive-pack.c | 9 ++++++++-\n t/t5516-fetch-push.sh | 24 ++++++++++++++++++++++++\n 3 files changed, 38 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2dce021..8f13fc0 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1849,6 +1849,12 @@ receive.updateserverinfo::\n \tIf set to true, git-receive-pack will run git-update-server-info\n \tafter receiving data from git-push and updating refs.\n \n+receive.allowupdatestohidden::\n+\tWhen `transfer.hiderefs` is in effect, allow `receive-pack`\n+\tto accept a push request that asks to update or delete a\n+\thidden ref (by default, such a request is rejected).\n+\tsee also `transfer.hiderefs`.\n+\n remote..url::\n \tThe URL of a remote repository. See linkgit:git-fetch[1] or\n \tlinkgit:git-push[1].\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex a8248d9..88500e7 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -41,6 +41,7 @@ static int auto_gc = 1;\n static const char *head_name;\n static void *head_name_to_free;\n static int sent_capabilities;\n+static int allow_updates_to_hidden;\n \n static enum deny_action parse_deny_action(const char *var, const char *value)\n {\n@@ -119,6 +120,11 @@ static int receive_pack_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (strcmp(var, \"receive.allowupdatestohidden\") == 0) {\n+\t\tallow_updates_to_hidden = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \treturn git_default_config(var, value, cb);\n }\n \n@@ -726,7 +732,8 @@ static void execute_commands(struct command *commands, const char *unpacker_erro\n \t\t\t\t 0, &cmd))\n \t\tset_connectivity_errors(commands);\n \n-\treject_updates_to_hidden(commands);\n+\tif (!allow_updates_to_hidden)\n+\t\treject_updates_to_hidden(commands);\n \n \tif (run_receive_hook(commands, pre_receive_hook, 0)) {\n \t\tfor (cmd = commands; cmd; cmd = cmd->next) {\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 522056f..4c8aef9 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1095,4 +1095,28 @@ test_expect_success 'push to update a hidden ref' '\n \tcheck_push_result $the_first_commit hidden/three\n '\n \n+test_expect_failure 'allow push to update a hidden ref' '\n+\tmk_test heads/master hidden/one hidden/two hidden/three &&\n+\t(\n+\t\tcd testrepo &&\n+\t\tgit config transfer.hiderefs refs/hidden &&\n+\t\tgit config receive.allowupdatestohidden yes\n+\t) &&\n+\n+\t# push to unhidden ref succeeds normally\n+\tgit push testrepo master:refs/heads/master &&\n+\tcheck_push_result $the_commit heads/master &&\n+\n+\t# push to update a hidden ref should succeed\n+\tgit push testrepo master:refs/hidden/one &&\n+\tcheck_push_result $the_commit heads/master &&\n+\n+\t# push to delete a hidden ref should succeed\n+\tgit push testrepo :refs/hidden/two &&\n+\t(\n+\t\tcd testrepo &&\n+\t\ttest_must_fail git show-ref -q refs/hidden/one\n+\t)\n+'\n+\n test_done\n-- \n1.8.1.2.589.ga9b91ac\n"},{"id":"208691","messageId":"5110BD18.3080608@alum.mit.edu","threadId":"32780","inReplyTo":"1359571542-19852-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-02-05T08:04:40Z","receivedAt":"2013-02-05T08:04:40Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 01/30/2013 07:45 PM, Junio C Hamano wrote:\n> The third round.\n> \n> - Multi-valued variable transfer.hiderefs lists prefixes of ref\n> hierarchies to be hidden from the requests coming over the\n> network.\n> \n> - A configuration optionally allows uploadpack to accept fetch\n> requests for an object at the tip of a hidden ref.\n> \n> Elsewhere, we discussed \"delaying ref advertisement\" (aka \"expand\n> refs\"), but it is an orthogonal feature and this \"hiding refs\n> completely from advertisement\" series does not attempt to address.\n> \n> Patch #2 (simplify request validation), #4 (clarify the codeflow),\n> and #5 (use struct ref) are new. The are all long overdue clean-ups\n> for these codepaths.\n> \n> The last patch is an illustration why it wouldn't make sense to\n> optionally allow pushing into hidden refs, and not meant to be part\n> of the series proper.\n> \n> For those who missed it, earlier rounds are at:\n> \n> http://thread.gmane.org/gmane.comp.version-control.git/213951\n> http://thread.gmane.org/gmane.comp.version-control.git/214888\n\nI would again like to express my discomfort about this feature, which is\nalready listed as \"will merge to next\". Frankly, I have the feeling\nthat this feature is being steamrolled in before a community consensus\nhas been reached and indeed before many valid points raised by other\nmembers of the community have even been addressed. For example:\n\n* I didn't see a response to Peff's convincing arguments that this\nshould be a client-side feature rather than a server-side feature [1].\n\n* I didn't see an answer to Duy's question [2] about what is different\nbetween the proposed feature and gitnamespaces.\n\n* I didn't see a response to my worries that this feature could be\nabused [3].\n\nI also think that the feature is poorly designed. For example:\n\n* Why should a repository have exactly one setting for what refs should\nbe hidden? Wouldn't it make more sense to allow multiple \"views\" to be\ndefined?:\n\n[view \"official\"]\n\thiderefs = refs/pull\n\thiderefs = refs/heads/??/*\n[view \"pu\"]\n\thiderefs = refs/pull\n[view \"current\"]\n\thiderefs = refs/tags/releases\n\nwith the view perhaps selected via a server-side environment variable?\nThis would allow multiple views to be published via different URLs but\nreferring to the same git repository.\n\n* Is it enough to support only reference exclusion (as opposed to\nexclusion and inclusion rules)? Is it enough to support only reference\nselection by hierarchy (for example, how would you hide contributed\nbranches from your repo)? Can your configuration scheme be expanded in\na backwards-compatible way if these or other extensions are added later?\n\n* Why should this feature only be available remotely? It would be handy\nto clone everything but usually only see some subset of references in my\ndaily work: \"GIT_VIEW=official gitk --all &\". Or to hide some remote\nbranches most of the time without having to remove them from my repo:\n\n[view \"brief\"]\n\trefs = refs\n\trefs = !refs/remotes\n\trefs = refs/remotes/origin\n\trefs = refs/remotes/my-boss\n\nI think there are still more questions than answers about this feature\nand FWIW vote -1 on merging it to next at this time.\n\nMichael\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/214168\n[2] http://article.gmane.org/gmane.comp.version-control.git/214070\n[3] http://article.gmane.org/gmane.comp.version-control.git/213957\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"208692","messageId":"20130205083327.GA4931@elie.Belkin","threadId":"32780","inReplyTo":"5110BD18.3080608@alum.mit.edu","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-02-05T08:33:27Z","receivedAt":"2013-02-05T08:33:27Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Michael,\n\nMichael Haggerty wrote:\n\n> I would again like to express my discomfort about this feature, which is\n> already listed as \"will merge to next\". Frankly, I have the feeling\n> that this feature is being steamrolled in before a community consensus\n> has been reached and indeed before many valid points raised by other\n> members of the community have even been addressed. For example:\n\nIn $dayjob I work with Gerrit, so I think I can start to answer some\nof these questions.\n\n> * I didn't see a response to Peff's convincing arguments that this\n> should be a client-side feature rather than a server-side feature [1].\n\nThe client can't control the size of the ref advertisement. That is\nthe main motivation if I understood correctly.\n\n> * I didn't see an answer to Duy's question [2] about what is different\n> between the proposed feature and gitnamespaces.\n\nNamespaces are more complicated and don't sit well in existing setups\ninvolving git repositories whose refs are not namespaced.\n\n> * I didn't see a response to my worries that this feature could be\n> abused [3].\n\nCan you elaborate? Do you mean that through social engineering an\nattacker would convince the server admin to store secrets using a\nhidden ref and enable the upload-archive service?\n\nThat does sound like a reasonable concern. Perhaps the documentation\nshould be updated along these lines\n\n\ttransfer.hiderefs::\n\t\tString(s) `upload-pack` and `receive-pack` use to decide\n\t\twhich refs to omit from their initial advertisement. Use\n\t\tmore than one transfer.hiderefs configuration variables to\n\t\tspecify multiple prefix strings. A ref that are under the\n\t\thierarchies listed on the value of this variable is excluded,\n\t\tand is hidden from `git ls-remote`, `git fetch`, `git push :`,\n\t\tetc. An attempt to update or delete a hidden ref by `git push`\n\t\tis rejected, and an attempt to fetch a hidden ref by `git fetch`\n\t\twill fail.\n\t+\n\tThis setting does not currently affect the `upload-archive` service.\n\nuntil someone interested implements the same for upload-archive.\n\n> I also think that the feature is poorly designed. For example:\n\nThat's another reasonable concern. It's very hard to get a design\ncorrect right away, which is presumably part of the motivation of\ngetting this into the hands of interested users who can give feedback\non it. What would potentially be worth blocking even that is concerns\nabout the wire protocol, since it is hard to take back mistakes there.\n\n> * Why should a repository have exactly one setting for what refs should\n> be hidden? Wouldn't it make more sense to allow multiple \"views\" to be\n> defined?:\n\nHow do I request a different view of the repository at\n/path/to/repo.git over the network? How can we make the common case\nof only one view easy to achieve? Isn't the multiple-views case\nexactly what gitnamespaces is for?\n\n[...]\n> * Is it enough to support only reference exclusion (as opposed to\n> exclusion and inclusion rules)?\n\nThe motivating example is turning off advertisement of the\nrefs/changes hierarchy. If and when more complicated cases come up,\nthat would presumably be the time to support more complicated\nconfiguration.\n\n[...]\n> * Why should this feature only be available remotely?\n\nIt is about transport. Ref namespaces have their own set of use cases\nand are a distinct feature.\n\nHoping that clarifies,\nJonathan\n"},{"id":"208694","messageId":"20130205085047.GA24973@sigill.intra.peff.net","threadId":"32780","inReplyTo":"1359571542-19852-4-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v3 3/8] upload/receive-pack: allow hiding ref hierarchies","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-05T08:50:48Z","receivedAt":"2013-02-05T08:50:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 30, 2013 at 10:45:37AM -0800, Junio C Hamano wrote:\n\n> Teach upload-pack and receive-pack to omit some refs from their\n> initial advertisements by paying attention to the transfer.hiderefs\n> multi-valued configuration variable. Any ref that is under the\n> hierarchies listed on the value of this variable is excluded from\n> responses to requests made by \"ls-remote\", \"fetch\", \"clone\", \"push\",\n> etc.\n> \n> A typical use case may be\n> \n> \t[transfer]\n> \t\thiderefs = refs/pull\n> \n> to hide the refs that are internally used by the hosting site and\n> should not be exposed over the network.\n\nIn the earlier review, I mentioned making this per-service, but I see\nthat is not the case here. Do you have an argument against doing so?\n\nI'm specifically thinking of the way we do refs/pull at GitHub (which we\nhide only from receive-pack). I know that you think it would be cleaner\nto hide those, and at some level I agree. But at the same time, the\ncurrent mechanism has been in place for some time; changing what we\npresent via upload-pack is likely to break people's workflows. And I\nhave not seen complaints about the current system. So unless there is a\ncompelling reason to do so, I'd rather let the fetcher make the\ndecision.\n\nGerrit's refs/changes may be a different story, if they have a large\nenough number of them to make upload-pack's ref advertisement\noverwhelming.\n\nI'm happy to do the per-service patch on top, but I just expected it\nhere, so I'm wondering if you are against having the feature.\n\n-Peff\n"},{"id":"208698","messageId":"20130205091938.GB24973@sigill.intra.peff.net","threadId":"32780","inReplyTo":"1359571542-19852-8-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v3 7/8] fetch: fetch objects by their exact SHA-1 object names","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-05T09:19:38Z","receivedAt":"2013-02-05T09:19:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 30, 2013 at 10:45:41AM -0800, Junio C Hamano wrote:\n\n> Teach \"git fetch\" to accept an exact SHA-1 object name the user may\n> obtain out of band on the LHS of a pathspec, and send it on a \"want\"\n> message when the server side advertises the allow-tip-sha1-in-want\n> capability.\n\nHmm. The UI on this is a little less nice than I would have hoped. Right\nnow if you want a ref outside of refs/heads, it's up to you to configure\na refspec or do a one-off fetch of the ref:\n\n git config --add remote.origin.fetch '+refs/pull/*:refs/pull/*'\n git fetch\n git checkout refs/pull/123/head\n ... inspect the contents ...\n\nWithout advertisement, we have to learn that refs/pull/123/head exists\nout of band. We can no longer fetch all of the refs/pull hierarchy\npreemptively, but we can in theory grab at least that one ref like this:\n\n git fetch refs/pull/123/head\n git checkout FETCH_HEAD\n ... inspect the contents ...\n\nBut that does not work with your patch; instead you have to learn not\njust the existence of the ref, but also its sha1. This may seem like a\nlittle thing, since you are already learning of the ref out-of-band,\nbut:\n\n 1. The full sha1 is more annoying to work with. You'd have to cut and\n paste or otherwise script getting it to fetch. A human-readable\n ref, though, is much easier to remember. The \"refs/pull/N/head\"\n pattern is simple to learn and type.\n\n 2. Related to (1) above, is that it may be easier to come up with a\n hidden ref name out of band than the full sha1. E.g., if I am\n looking at https://github.com/me/foo.git/pulls/123, I can easily\n construct the ref from that. Getting the sha1 will take extra\n steps.\n\n 3. You have to do the out-of-band step, which may be inconvenient,\n every time the ref is updated. There is no way to say \"just give me\n what is at the tip of refs/pull/123/head\".\n\nI think you could solve it by teaching upload-pack to understand refs on\n\"want\" lines and convert them into the pointed-to object.\n\nBut taking a step back, this really seems quite inferior to an extension\nthat would allow the client to share its refspecs with the server. That\nwould solve the bandwidth efficiency problem for normal fetchers who are\nlooking at \"refs/heads/*\", while still giving people who are interested\nin \"refs/pull/*\" (or even a specific refs/pull tip) the information they\nneed to fetch.\n\nThe obvious problem is that the server speaks first. But I recall\nsomebody suggested a combination of:\n\n 1. For git-over-ssh and git-over-tcp, the server advertises\n tell-me-your-refspecs as it starts advertising. Client interrupts\n advertisement with refspecs once it sees that it is OK to do so.\n\n We waste some bandwidth during the round-trip, but there will still\n be a benefit for repos with many refs (I wonder if we could even\n re-order the advertisement to show refs/heads/ first, as they are\n the most likely case to be requested). And as time goes on and the\n majority of clients support tell-me-your-refspecs, the server side\n can introduce a short delay after the first advertisement.\n\n 2. For git-over-http, the client speaks first via the http protocol.\n We can stuff the refspecs into extra query parameters.\n\nIt's a little more complicated as a solution, but I feel like it gets\nthe efficiency without a loss of functionality. And it helps in more\nsituations than the hidden refs proposal (e.g., fetching refs/heads/foo\ncan avoid enumerating all of refs/heads/*).\n\n-Peff\n"},{"id":"208701","messageId":"5110DF1D.8010505@alum.mit.edu","threadId":"32780","inReplyTo":"20130205083327.GA4931@elie.Belkin","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-02-05T10:29:49Z","receivedAt":"2013-02-05T10:29:49Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 02/05/2013 09:33 AM, Jonathan Nieder wrote:\n> Michael Haggerty wrote:\n> \n>> I would again like to express my discomfort about this feature, which is\n>> already listed as \"will merge to next\". Frankly, I have the feeling\n>> that this feature is being steamrolled in before a community consensus\n>> has been reached and indeed before many valid points raised by other\n>> members of the community have even been addressed. For example:\n> \n> In $dayjob I work with Gerrit, so I think I can start to answer some\n> of these questions.\n> \n>> * I didn't see a response to Peff's convincing arguments that this\n>> should be a client-side feature rather than a server-side feature [1].\n> \n> The client can't control the size of the ref advertisement. That is\n> the main motivation if I understood correctly.\n\nNot according to Junio [4]:\n\n Look at this as a mechanism for the repository owner to control the\n clutter in what is shown to the intended audience of what s/he\n publishes in the repository. Network bandwidth reduction of\n advertisement is a side effect of clutter reduction, and not\n necessarily the primary goal.\n\n>> * I didn't see an answer to Duy's question [2] about what is different\n>> between the proposed feature and gitnamespaces.\n> \n> Namespaces are more complicated and don't sit well in existing setups\n> involving git repositories whose refs are not namespaced.\n\nThanks.\n\n>> * I didn't see a response to my worries that this feature could be\n>> abused [3].\n> \n> Can you elaborate? Do you mean that through social engineering an\n> attacker would convince the server admin to store secrets using a\n> hidden ref and enable the upload-archive service?\n\nThe upload-archive service is not needed; after patch v3 \"7/8\" remote\nclients are able to retrieve hidden content by SHA1.\n\nHiderefs creates a \"dark\" corner of a remote git repo that can hold\narbitrary content that is impossible for anybody to discover but\nnevertheless possible for anybody to download (if they know the name of\na hidden reference). In earlier versions of the patch series I believe\nthat it was possible to push to a hidden reference hierarchy, which made\nit possible to upload dark content. The new version appears (from the\ncode) to prohibit adding references in a hidden hierarchy, which would\nclose the main loophole that I was worried about. But the documentation\nand the unit tests only explicitly say that updates and deletes are\nprohibited; nothing is said about adding references (unless \"update\" is\nunderstood to include \"add\"). I think the true behavior should be\nclarified and tested.\n\nI was worried that somehow this \"dark\" content could be used for\nmalicious purposes; for example, pushing compromised code then\nconvincing somebody to download it by SHA1 with the implicit argument\n\"it's safe since it comes directly from the project's official\nrepository\". If it is indeed impossible to populate the dark namespace\nremotely then I can't think of a way to exploit it.\n\n> That does sound like a reasonable concern. Perhaps the documentation\n> should be updated along these lines\n> \n> \ttransfer.hiderefs::\n> \t\tString(s) `upload-pack` and `receive-pack` use to decide\n> \t\twhich refs to omit from their initial advertisement. Use\n> \t\tmore than one transfer.hiderefs configuration variables to\n> \t\tspecify multiple prefix strings. A ref that are under the\n> \t\thierarchies listed on the value of this variable is excluded,\n> \t\tand is hidden from `git ls-remote`, `git fetch`, `git push :`,\n> \t\tetc. An attempt to update or delete a hidden ref by `git push`\n> \t\tis rejected, and an attempt to fetch a hidden ref by `git fetch`\n> \t\twill fail.\n> \t+\n> \tThis setting does not currently affect the `upload-archive` service.\n> \n> until someone interested implements the same for upload-archive.\n\nYes, this sounds reasonable.\n\n>> I also think that the feature is poorly designed. For example:\n> \n> That's another reasonable concern. It's very hard to get a design\n> correct right away, which is presumably part of the motivation of\n> getting this into the hands of interested users who can give feedback\n> on it. What would potentially be worth blocking even that is concerns\n> about the wire protocol, since it is hard to take back mistakes there.\n> \n>> * Why should a repository have exactly one setting for what refs should\n>> be hidden? Wouldn't it make more sense to allow multiple \"views\" to be\n>> defined?:\n> \n> How do I request a different view of the repository at\n> /path/to/repo.git over the network? How can we make the common case\n> of only one view easy to achieve? Isn't the multiple-views case\n> exactly what gitnamespaces is for?\n\nHidden references can only be configured by somebody with local access\nto the repository being served. Somebody with that access could also\nconfigure views. He could also probably organize a mapping from URL ->\n(GIT_PATH, GIT_VIEW) and offer several different views of the same\nrepository. If the setting of environment variables is thought to\nsometimes be too problematic, the default view could be defined via\nconfiguration, too:\n\n[view]\n\tdefault = official\n[view \"official\"]\n\thiderefs = refs/pull\n\nOr perhaps, if views are made available locally too, then one view could\nbe designated as the default for transfer purposes:\n\n[transfer]\n\tdefaultview = official\n\ngitnamespaces have the disadvantage that you mentioned yourself earlier.\n\n> [...]\n>> * Is it enough to support only reference exclusion (as opposed to\n>> exclusion and inclusion rules)?\n> \n> The motivating example is turning off advertisement of the\n> refs/changes hierarchy. If and when more complicated cases come up,\n> that would presumably be the time to support more complicated\n> configuration.\n\nI suppose should the need arise we could later introduce\n\"transfer.showrefs\" and let the longest match to a given reference\n(i.e., \"transfer.hiderefs\" vs. \"transfer.showrefs\") win.\n\n> [...]\n>> * Why should this feature only be available remotely?\n> \n> It is about transport. Ref namespaces have their own set of use cases\n> and are a distinct feature.\n\ngitnamespaces offer multiple perspectives of a single object database,\nusing independent sets of references to define the subset. Changing a\nreference in one namespace has no effect on similarly-named references\nin another namespace.\n\nThe \"views\" feature that I vaguely suggested, on the other hand, would\noffer multiple subsets of a single set of references, a bit like an SQL\nview. Changing a reference in one view would automatically affect it in\nother views that include the reference. It could be a handy way to\nreduce clutter *locally* in the same way that hiderefs would reduce\nclutter *remotely*. But more importantly, both features could be built\non the same foundation.\n\nMichael\n\n[4] http://article.gmane.org/gmane.comp.version-control.git/213984\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"208705","messageId":"20130205111816.GA25303@sigill.intra.peff.net","threadId":"32780","inReplyTo":"20130205091938.GB24973@sigill.intra.peff.net","subject":"Re: [PATCH v3 7/8] fetch: fetch objects by their exact SHA-1 object names","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-05T11:18:16Z","receivedAt":"2013-02-05T11:18:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 05, 2013 at 04:19:38AM -0500, Jeff King wrote:\n\n> But taking a step back, this really seems quite inferior to an\n> extension that would allow the client to share its refspecs with the\n> server. That would solve the bandwidth efficiency problem for normal\n> fetchers who are\n\nI should have read your cover letter more closely, as I see you make the\npoint there that this is no longer about the efficiency, but about\nuncluttering.\n\nI'm not sure I like it as an uncluttering tool, though; for the reasons\nI stated in my previous mail, it makes it much more awkward for the\nmoments when you actually do want to fetch some of the \"clutter\".\n\n-Peff\n"},{"id":"208730","messageId":"7vwqumvk76.fsf@alter.siamese.dyndns.org","threadId":"32780","inReplyTo":"20130205085047.GA24973@sigill.intra.peff.net","subject":"Re: [PATCH v3 3/8] upload/receive-pack: allow hiding ref hierarchies","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-05T15:45:01Z","receivedAt":"2013-02-05T15:45:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Jan 30, 2013 at 10:45:37AM -0800, Junio C Hamano wrote:\n>\n>> Teach upload-pack and receive-pack to omit some refs from their\n>> initial advertisements by paying attention to the transfer.hiderefs\n>> multi-valued configuration variable. Any ref that is under the\n>> hierarchies listed on the value of this variable is excluded from\n>> responses to requests made by \"ls-remote\", \"fetch\", \"clone\", \"push\",\n>> etc.\n>> \n>> A typical use case may be\n>> \n>> \t[transfer]\n>> \t\thiderefs = refs/pull\n>> \n>> to hide the refs that are internally used by the hosting site and\n>> should not be exposed over the network.\n>\n> In the earlier review, I mentioned making this per-service, but I see\n> that is not the case here. Do you have an argument against doing so?\n\nPerhaps then I misunderstood your intention. By reminding me of the\nreceive-pack side, I thought you were hinting to unify these two\ninto one, which I did. There is no argument against it.\n\n> And I\n> have not seen complaints about the current system.\n\nImmediately after I added github to the set of places I push into,\nwhich I think is long before you joined GitHub, I noticed that _my_\nrepository gets contaminated by second rate commits called pull\nrequests, and I may even have complained, but most likely I didn't,\nas I could easily tell that, even though I know it is _not_ the only\nway, nor even the best way [*1*], to implement the GitHub's pull\nrequest workflow, I perfectly well understood that it would be the\nmost expedient way for GitHub folks to implement this feature.\n\nI think you should take lack of complaints with a huge grain of\nsalt. It does not suggest much.\n\n> Gerrit's refs/changes may be a different story, if they have a large\n> enough number of them to make upload-pack's ref advertisement\n> overwhelming.\n\nThis is probably a stale count, but platform/frameworks/base part of\nAOSP has 3200+ refs; the corresponding repository internal to Google\nhas 60k+ refs (this is because there are many in-between states\nrecorded in the internal repository, even though the end result\npublished to the open source repository may be the same) and results\nin ~4MB advertisement. Which is fairly significant when all you are\ninterested in doing is an \"Am I up to date?\" poll.\n\n\n[Footnote]\n\n*1* From the ownership point of view, objects that are only\nreachable from these refs/pull/* refs do *not* belong to the\nrequestee, until the requestee chooses to accept the changes.\n\nA malicious requestor can fork your repository, add an objectionable\nblob to it, and throw a pull request at you. GitHub shows that the\nblob now belongs to your repository, so the requestor turns around\nand file a DMCA takedown complaint against your repository. A\nclueful judge would then agree with the complaint after running a\n\"clone --mirror\" and seeing the blob in your repository. Oops?\n\nA funny thing is that you cannot \"push :refs/pull/1/head\" to remove\nit anymore (I think in the early days, I took them out by doing this\na few times, but I may be misremembering), so you cannot make\nyourself into compliance, even though you are not the offending\nparty. Your repository is held responsible for whatever the rogue\nrequestor added. That is not very nice, is it?\n\nIn an ideal world, I would have chosen to create a dedicated fork\nmanaged by the hosting company (i.e. GitHub) for your repository\nwhose only purpose is to house these refs/pull/ refs (the hosting\nsite is ultimately who has to respond to DMCA notices anyway, and an\narrangement like this makes it clear who is reponsible for what).\n\nThe e-mail sent to you to let you know about outstanding pull\nrequests and the web UI could just point at that forked repository,\nnot your own (you also could choose to leave the outging pull\nrequests in the requestor's repository, but that is only OK if you\ndo not worry about (1) a requestor sending a pull request, then\nupdating the branch the pull request talks about later, to trick you\nwith bait-and-switch, or (2) a requestor sending a pull request,\nthinks he is done with the topic and removes the repository).\n"},{"id":"208733","messageId":"7vk3qmvjpd.fsf@alter.siamese.dyndns.org","threadId":"32780","inReplyTo":"20130205091938.GB24973@sigill.intra.peff.net","subject":"Re: [PATCH v3 7/8] fetch: fetch objects by their exact SHA-1 object names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-05T15:55:42Z","receivedAt":"2013-02-05T15:55:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Jan 30, 2013 at 10:45:41AM -0800, Junio C Hamano wrote:\n>\n>> Teach \"git fetch\" to accept an exact SHA-1 object name the user may\n>> obtain out of band on the LHS of a pathspec, and send it on a \"want\"\n>> message when the server side advertises the allow-tip-sha1-in-want\n>> capability.\n>\n> Hmm. The UI on this is a little less nice than I would have hoped.\n\nNaming with unadvertised *refname*, not object name, needs protocol\nextension for the serving side to expand the name to object name;\notherwise the receiving end wouldn't know what tip what it asked\nresulted in.\n\nAnd that belongs to a separate \"expand refs\" extension (aka\n\"delaying ref advertisement\") that is outside the scope of this\nseries but can be built on top, as I said in the cover letter.\n"},{"id":"208747","messageId":"7v8v72u0vw.fsf@alter.siamese.dyndns.org","threadId":"32780","inReplyTo":"5110BD18.3080608@alum.mit.edu","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-05T17:27:31Z","receivedAt":"2013-02-05T17:27:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty writes:\n\n> I would again like to express my discomfort about this feature, which is\n> already listed as \"will merge to next\".\n\nDo not take \"will merge to next\" too literally. One major purpose\nof marking a topic as such is exactly to solicit comments like this\n;-)\n\n> * I didn't see a response to Peff's convincing arguments that this\n> should be a client-side feature rather than a server-side feature [1].\n\nUncluttering is not about a choice client should make. \"delayed\nadvertisement\" is an orthogonal issue and requires a larger protocol\nupdate (it needs to make \"git fetch\" speak first instead of the\ncurrent protocol in which \"upload-pack\" speaks first).\n\n> * I didn't see an answer to Duy's question [2] about what is different\n> between the proposed feature and gitnamespaces.\n\nI think Jonathan addressed this already.\n\n> * I didn't see a response to my worries that this feature could be\n> abused [3].\n\nYou can choose not to advertise allow-tip-sha1-in-want capability; I\ndo not think it is making things worse than the status quo.\n\n> * Why should a repository have exactly one setting for what refs should\n> be hidden? Wouldn't it make more sense to allow multiple \"views\" to be\n> defined?:\n\nYou are welcome to extend to have different views, but how would\nyour clients express which view they would want?\n\nGiving a single view that the serving end decides gives us an\nimmediate benefit of showing an uncluttered set of refs of server's\nchoice, without making the problem space larger than necessary.\n\n> * Is it enough to support only reference exclusion (as opposed to\n> exclusion and inclusion rules)?\n\nAgain, I do not think you cannot extend it to do positive and\nnegative filtering \"exclude these, but include those even though\nthey match the 'exclude these' patterns I gave you earlier\".\n\n> * Why should this feature only be available remotely?\n\nThe whole point is to give the server side a choice to show selected\nrefs, so that it can use hidden portion for its own use. These refs\nshould not be hidden from local operations like \"gc\".\n\nI appreciate the comments, but I do not think any point you raised\nin this message is very much relevant as objections.\n"},{"id":"208748","messageId":"7v4nhqu0gn.fsf@alter.siamese.dyndns.org","threadId":"32780","inReplyTo":"20130205083327.GA4931@elie.Belkin","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-05T17:36:40Z","receivedAt":"2013-02-05T17:36:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n>> * I didn't see a response to Peff's convincing arguments that this\n>> should be a client-side feature rather than a server-side feature [1].\n>\n> The client can't control the size of the ref advertisement. That is\n> the main motivation if I understood correctly.\n\nThe answer to this question is more nuanced.\n\nWith the current protocol, it is upload-pack who speaks first, so\nthere is no way for the requestor to say \"I am from an updated Git\nsuite and understand how to tell you to give me limited set of\nrefs\", before upload-pack blasts 4MB of ref advertisement to it.\n\nIf the side that fetches is potentially interested in finding out\nany and all refs, then an alternative solution would be to break\nthe current protocol, open a separate port and have upload-pack-2\nlisten to it, sit silently to let the requestor speak first when it\ngets connection to that port.\n\nBut if the primary thing you are interested in is to hide the\nreferences that:\n\n (1) the server side needs to keep track of for its own use; but\n (2) the requestors do not have to learn about from upload-pack,\n\nwe can do so without breaking older requestors. That is what the\nearly part of this series is about. We can view the last patch to\nadd the allow-tip-sha1-in-want as an icing on the cake.\n\nIt has the side effect of reducing the transfer overhead, because by\nhiding the internal refs, the server side will stop blasting 4MB of\nref advertisements the requestors are not interested in, and that\nwould be the primary observable outcome from the end-user's point of\nview (i.e. your \"git pull --ff-only\" will become a lot faster).\n"},{"id":"208749","messageId":"7vy5f2slsj.fsf@alter.siamese.dyndns.org","threadId":"32780","inReplyTo":"5110DF1D.8010505@alum.mit.edu","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-05T17:38:52Z","receivedAt":"2013-02-05T17:38:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty writes:\n\n> On 02/05/2013 09:33 AM, Jonathan Nieder wrote:\n>> Michael Haggerty wrote:\n>> \n>>> I would again like to express my discomfort about this feature, which is\n>>> already listed as \"will merge to next\". Frankly, I have the feeling\n>>> that this feature is being steamrolled in before a community consensus\n>>> has been reached and indeed before many valid points raised by other\n>>> members of the community have even been addressed. For example:\n>> \n>> In $dayjob I work with Gerrit, so I think I can start to answer some\n>> of these questions.\n>> \n>>> * I didn't see a response to Peff's convincing arguments that this\n>>> should be a client-side feature rather than a server-side feature [1].\n>> \n>> The client can't control the size of the ref advertisement. That is\n>> the main motivation if I understood correctly.\n>\n> Not according to Junio [4]:\n>\n> Look at this as a mechanism for the repository owner to control the\n> clutter in what is shown to the intended audience of what s/he\n> publishes in the repository. Network bandwidth reduction of\n> advertisement is a side effect of clutter reduction, and not\n> necessarily the primary goal.\n\nSee my response to Jonathan.\n\n> Hiderefs creates a \"dark\" corner of a remote git repo that can hold\n> arbitrary content that is impossible for anybody to discover but\n> nevertheless possible for anybody to download (if they know the name of\n> a hidden reference).\n\nThat is why allow-tip-sha1-in-want is a separate opt-in feature only\nthe server side controls.\n"},{"id":"208791","messageId":"51122D9D.9040100@alum.mit.edu","threadId":"32780","inReplyTo":"7v8v72u0vw.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-02-06T10:17:01Z","receivedAt":"2013-02-06T10:17:01Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 02/05/2013 06:27 PM, Junio C Hamano wrote:\n> Michael Haggerty writes:\n>> I would again like to express my discomfort about this feature, which is\n>> already listed as \"will merge to next\".\n> \n> Do not take \"will merge to next\" too literally. One major purpose\n> of marking a topic as such is exactly to solicit comments like this\n> ;-)\n\nI take \"will merge to next\" pretty seriously, because I know how hard it\nis to get *my* patch series to this state :-)\n\n>> * I didn't see a response to Peff's convincing arguments that this\n>> should be a client-side feature rather than a server-side feature [1].\n> \n> Uncluttering is not about a choice client should make. \"delayed\n> advertisement\" is an orthogonal issue and requires a larger protocol\n> update (it needs to make \"git fetch\" speak first instead of the\n> current protocol in which \"upload-pack\" speaks first).\n\nThere seem to be a few issues mixed up in this topic. It is hard to\nreason about your patch series without understanding which scenarios and\nproblems it is meant to address. First the problems that we might like\nto solve:\n\nClutter: The typical user is subjected to much unneeded clutter in the\n form of references that he/she will likely never use.\n\nBandwidth: Interactions with the remote repo (clone, fetch, etc) are\n slowed down by the large volume of unnecessary data.\n\nProvenance: Users mistakenly think that content originates with the\n repository owner whereas it in fact came from some other\n (perhaps untrusted) source.\n\n\nNow, what are some use-case scenarios in which these problems arise? As\nI understand it, there are a few:\n\nScenario 1: Some providers junk up their users' repositories with\ncontent that is not created by the repository's owner and that the owner\ndoesn't want to appear to vouch for (e.g., GitHub pull requests). These\nreferences might sometimes be useful to fetch, singly or in bulk.\n\nScenario 2: Some systems junk up their users' repositories with\nadditional references that are not interesting to most pullers (e.g.,\nGerrit activity markers) though they don't add questionable content.\n\nScenario 3: Some repository owners might *themselves* want to push\nreferences to their repository but hide them from most users (e.g.,\nJunio's topic branches) or make them completely hidden from the rest of\nthe world (e.g., proprietary vs. open-source branches).\n\nIn most of these cases, it would be desirable for at least some users to\nbe able to fetch and/or push hidden content.\n\nA first weakness of your proposal is that even though the hidden refs\nare (optionally) fetchable, there is *no* way to discover them remotely\nor to bulk-download them; they would have to be retrieved one by one\nusing out-of-band information. And if I understand correctly, there\nwould be no way to push hidden references remotely (whether manually or\nfrom some automated process). Such processes would have to be local to\nthe machine holding the repository.\n\nA second weakness of your proposal is that the repository owner would\n*anyway* need local access to the repo server or the help of the\nprovider to implement reference hiding (since hidden references cannot\nbe configured remotely). Who will choose what references to hide? Most\nlikely each provider will pick a one-size-fits-all configuration and\napply it to all of the repos that they manage. All users would be at\nthe mercy of their provider to make wise choices and would not be able\nto override the choice via their client.\n\nA third weakness of your hidden references proposal is that it is\nschizophrenic: some references are hidden and undiscoverable, but their\ncontent can nevertheless be made fetchable if the user happens to know\nthe SHA1. This is more complicated to understand and reason about than\nthe rule \"exactly the content that is referred to by published\nreferences is fetchable\".\n\nWhat would be a better way? Providers could expose multiple views of\nthe same repository; for example, one view with just the uncluttered\ncontent, and a second view that includes *all* fetchable references.\nAccessing the repository via the first view would give all of the\nbenefits provided by your hidden reference proposal. Accessing it via\nthe second view would allow the hidden references to be fetched (even in\nbulk) using purely git tools. The documentation for the second view\ncould explain that it contains un-vetted content.\n\nBut your proposal does not admit two-tiered access to a single\nrepository. You only support one hidden reference configuration that is\napplied to all remote access [1]. See below for more ideas about\nimplementing multiple views.\n\n>> * I didn't see a response to my worries that this feature could be\n>> abused [3].\n> \n> You can choose not to advertise allow-tip-sha1-in-want capability; I\n> do not think it is making things worse than the status quo.\n\nYes, if the feature is turned off then it is not worse than the status\nquo. But what if the feature is turned on?\n\nActually, I'm still not clear about how these hidden references are\nsupposed to be created. I know that you would forbid updating or\ndeleting hidden references via the remote protocol, but would you allow\nthem to be created? If so, then it seems that any pusher can create\ndark content. Or can they only be created via a separate, local channel\nto the repository? In this case, it seems rather limiting that any\nprocess that wants to create hidden references has to be local.\n\n>> * Why should a repository have exactly one setting for what refs should\n>> be hidden? Wouldn't it make more sense to allow multiple \"views\" to be\n>> defined?:\n> \n> You are welcome to extend to have different views, but how would\n> your clients express which view they would want?\n\nThere are several possibilities:\n\n1. Assuming the cooperation of the provider, the provider could offer\ntwo separate URLs: one for the uncluttered view and one for the\ncluttered view. The client would choose the view by choosing which URL\nto clone from. On the provider side, both of these URLs could refer to\nthe same Git repository but, for example, set an environment variable\nGIT_VIEW differently depending on which URL was used. This approach\nwould solve clutter, bandwidth and provenance but require cooperation\nfrom the provider.\n\n2a. Assuming no cooperation from the provider, the git client could have\noptions like \"git fetch --view=uncluttered URL\". This would receive all\nreferences from the server but discard any that are not included in the\nclient's \"uncluttered\" view definition. This would solve clutter.\n\n2b. Again assuming no cooperation from the provider, the user could\nclone all references from the remote repo, but define a local\n\"uncluttered\" view that hides the extra references on the local side.\nThe view could be selected by setting a local environment variable\nGIT_VIEW or via configuration option \"git config view.default\nuncluttered\". This would solve clutter in a more flexible way because\nthe clutter would still be available locally for those occasions when\nthe user wants to see it.\n\nPlease note that none of the above options require a new remote protocol.\n\nIf/when a new protocol is implemented, then the client could tell the\nserver what view it wants and the server would only advertise those refs\nto the client:\n\n3a. The client could tell the host what reference namespaces it wants to\nfetch. Its choice would only be used for the single transaction and\nwould not be recorded on the server side.\n\n3b. The client could pick a server-defined view by name. The server\nwould look up the name in its own configuration to translate it into a\nsubset of references. The views that a particular server supports would\nbe documented in the same place that the URL is documented and might\nalso be queryable by the client. There should probably be some standard\nviews like \"default\" and \"full\" that every server would be expected to\nimplement. Please note that this method can fall back to 2a when\ncommunicating with a server that does not support the new protocol.\n\n>> * Why should this feature only be available remotely?\n> \n> The whole point is to give the server side a choice to show selected\n> refs, so that it can use hidden portion for its own use. These refs\n> should not be hidden from local operations like \"gc\".\n\nCertainly they shouldn't be hidden from \"gc\", but it would be useful to\nbe able to hide references from user-facing commands like \"log --all\",\n\"log --decorate\", \"gitk\", \"grep --all\" etc. For example, here are some\nmore scenarios where clutter is annoying:\n\nScenario 4: I occasionally share with colleague Foo, so I want to\nconfigure his repo as a remote for mine and fetch his latest work:\n\n git remote add foo $URL\n git fetch foo\n\nBut now every time I do a \"gitk --all\" or \"git log --decorate\", the\noutput is cluttered with all of his references (most of which are just\nold versions of references from the upstream repository that we both\nuse). I would like to be able to hide his references most of the time\nbut turn them back on when I need them.\n\nScenario 5: Our upstream repository has gazillions of release tags under\n\"refs/tags/releases/...\", sometimes including customer-specific\nreleases. In my daily life these are just clutter. (This scenario is\nmade worse by the fact that AFAIK there is no way to tell Git to fetch\nsome tags but not others others.) But sometimes I need to track down a\nbug in a particular release and need to access that release tag. So it\nwould be nice to be able to hide and unhide them locally.\n\n> I appreciate the comments, but I do not think any point you raised\n> in this message is very much relevant as objections.\n\nTl;dr summary:\n\n* Hidden refs don't give a way to offer two-tiered remote access to a\n repository (e.g., one uncluttered view and one full view), so\n\n * local access to the repository would (apparently) be required to\n put *anything* in the hidden namespaces.\n\n * they don't help in any scenario where you *sometimes*\n want to bulk fetch the hidden refs, and even make it awkward to\n fetch single hidden refs.\n\n* Hidden refs introduce a confusing schizophrenia between \"advertised\"\n and \"not advertised but nonetheless fetchable\".\n\n* Hidden refs require the cooperation of the provider to configure and\n will therefore be unusable by many repository owners.\n\n* Some small improvements (e.g. allowing *multiple* views to be\n defined) would provide much more benefit for about the same effort,\n and would be a better base for building other features in the future\n (e.g., local views).\n\nThanks for listening.\nMichael\n\n[1] Theoretically one could support multiple views of a single\nrepository by using something like \"GIT_CONFIG=view_1_config git\nupload-pack ...\" or \"git -c transfer.hiderefs=... git upload-pack ...\",\nbut this would be awkward.\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"208792","messageId":"CACsJy8BhL4qDb8BgOVuaUFF_9GXvgu55urYyKqPuZMZCTCoLwA@mail.gmail.com","threadId":"32780","inReplyTo":"5110DF1D.8010505@alum.mit.edu","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-02-06T10:34:48Z","receivedAt":"2013-02-06T10:34:48Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Feb 5, 2013 at 5:29 PM, Michael Haggerty wrote:\n> Hiderefs creates a \"dark\" corner of a remote git repo that can hold\n> arbitrary content that is impossible for anybody to discover but\n> nevertheless possible for anybody to download (if they know the name of\n> a hidden reference). In earlier versions of the patch series I believe\n> that it was possible to push to a hidden reference hierarchy, which made\n> it possible to upload dark content. The new version appears (from the\n> code) to prohibit adding references in a hidden hierarchy, which would\n> close the main loophole that I was worried about. But the documentation\n> and the unit tests only explicitly say that updates and deletes are\n> prohibited; nothing is said about adding references (unless \"update\" is\n> understood to include \"add\"). I think the true behavior should be\n> clarified and tested.\n>\n> I was worried that somehow this \"dark\" content could be used for\n> malicious purposes; for example, pushing compromised code then\n> convincing somebody to download it by SHA1 with the implicit argument\n> \"it's safe since it comes directly from the project's official\n> repository\". If it is indeed impossible to populate the dark namespace\n> remotely then I can't think of a way to exploit it.\n\nOr you can think hiderefs is the first step to addressing the initial\nref advertisment problem. The series says hidden refs are to be\nfetched out of band, but that's not the only way. A new extension can\nbe added to the protocol later to let the client explore this dark\nspace. It's only truly dark for old clients. We could even shed some\nlight to old clients by sending a dummy ref with some loud name like\nPLEASE_UPDATE_TO_LATEST_GIT_TO_FETCH_REMAINING_REFS (new clients\nsilently drop this ref)\n--\nDuy\n"},{"id":"208797","messageId":"20130206113112.GB5267@sigill.intra.peff.net","threadId":"32780","inReplyTo":"7vwqumvk76.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 3/8] upload/receive-pack: allow hiding ref hierarchies","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-06T11:31:12Z","receivedAt":"2013-02-06T11:31:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 05, 2013 at 07:45:01AM -0800, Junio C Hamano wrote:\n\n> > In the earlier review, I mentioned making this per-service, but I see\n> > that is not the case here. Do you have an argument against doing so?\n> \n> Perhaps then I misunderstood your intention. By reminding me of the\n> receive-pack side, I thought you were hinting to unify these two\n> into one, which I did. There is no argument against it.\n\nWhat I meant was that there should be transfer.hiderefs, and an\nindividual {receive,uploadpack}.hiderefs, similar to the way we have\ntransfer.unpacklimit. That makes the easy case (hiding the refs\ncompletely) easy, but leaves the flexibility for more.\n\nLike this:\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex a8248d9..131c163 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -59,7 +59,7 @@ static int receive_pack_config(const char *var, const char *value, void *cb)\n \n static int receive_pack_config(const char *var, const char *value, void *cb)\n {\n-\tint status = parse_hide_refs_config(var, value, cb);\n+\tint status = parse_hide_refs_config(var, value, \"receive\");\n \n \tif (status)\n \t\treturn status;\ndiff --git a/refs.c b/refs.c\nindex e3574ca..9bfea58 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2560,9 +2560,13 @@ int parse_hide_refs_config(const char *var, const char *value, void *unused)\n \n static struct string_list *hide_refs;\n \n-int parse_hide_refs_config(const char *var, const char *value, void *unused)\n+int parse_hide_refs_config(const char *var, const char *value, void *vsection)\n {\n-\tif (!strcmp(\"transfer.hiderefs\", var)) {\n+\tconst char *section = vsection;\n+\n+\tif (!strcmp(\"transfer.hiderefs\", var) ||\n+\t (!prefixcmp(var, section) &&\n+\t !strcmp(var + strlen(section), \".hiderefs\"))) {\n \t\tchar *ref;\n \t\tint len;\n \ndiff --git a/upload-pack.c b/upload-pack.c\nindex 37977e2..c0390af 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -794,7 +794,7 @@ static int upload_pack_config(const char *var, const char *value, void *unused)\n {\n \tif (!strcmp(\"uploadpack.allowtipsha1inwant\", var))\n \t\tallow_tip_sha1_in_want = git_config_bool(var, value);\n-\treturn parse_hide_refs_config(var, value, unused);\n+\treturn parse_hide_refs_config(var, value, \"uploadpack\");\n }\n \n int main(int argc, char **argv)\n\n\nAs an aside, I wonder if there is any point to the void pointer\nparameter of parse_hide_refs_config. It is not used as a git_config\ncallback anywhere.\n\n> > And I\n> > have not seen complaints about the current system.\n> \n> Immediately after I added github to the set of places I push into,\n> which I think is long before you joined GitHub, I noticed that _my_\n> repository gets contaminated by second rate commits called pull\n> requests, and I may even have complained, but most likely I didn't,\n> as I could easily tell that, even though I know it is _not_ the only\n> way, nor even the best way [*1*], to implement the GitHub's pull\n> request workflow, I perfectly well understood that it would be the\n> most expedient way for GitHub folks to implement this feature.\n> \n> I think you should take lack of complaints with a huge grain of\n> salt. It does not suggest much.\n\nSure, I do not pretend that nobody cares. But it is certainly not a\npressing issue, or there would probably be more outcry. And we must also\nweigh it against the silent majority that are perfectly happy with the\nstatus quo, that lets them fetch refs/pull/* as any other ref.\n\nIn your case, I really think the problem is less that you have a problem\nwith PR refs in the repository, and more that you do not care about the\npull request feature at all. To you it is useless noise, both in the\nrepo and on the web. Your arguments about provenance could apply equally\nwell to PRs accessible via the web interface.\n\nI think the refs/ clutter is only an issue if you want to do mirroring,\nand then you have an obvious conflict: did the fetcher want to mirror\neverything, including refs/pull, or do they consider that to be clutter?\nOnly the client knows, which is why I think refspecs are the right place\nto deal with clutter (the fact that we cannot say \"everything except\nrefs/pull/*\" is a weakness in our refspecs).\n\n> *1* From the ownership point of view, objects that are only\n> reachable from these refs/pull/* refs do *not* belong to the\n> requestee, until the requestee chooses to accept the changes.\n> \n> A malicious requestor can fork your repository, add an objectionable\n> blob to it, and throw a pull request at you. GitHub shows that the\n> blob now belongs to your repository, so the requestor turns around\n> and file a DMCA takedown complaint against your repository. A\n> clueful judge would then agree with the complaint after running a\n> \"clone --mirror\" and seeing the blob in your repository. Oops?\n\nI don't think this is a problem in practice. DMCA notices do not go to\nthe repository owner; they go to GitHub. And as far as I know, our\nsupport staff deals with them on a case by case basis (and knows what a\npull request is, and who is responsible for the content in question). It\nis not like they see a report of something in refs/pull and lock down\nthe parent repository; they can see where the request came from and deal\nwith it appropriately.\n\nBut again, such a notice could just as easily come from the list of open\nPRs against your repo in the web interface.\n\n> A funny thing is that you cannot \"push :refs/pull/1/head\" to remove\n> it anymore (I think in the early days, I took them out by doing this\n> a few times, but I may be misremembering),\n\nWe block updates to them explicitly in a hook; it looks like that went\nin around mid-2011.\n\n> The e-mail sent to you to let you know about outstanding pull\n> requests and the web UI could just point at that forked repository,\n> not your own (you also could choose to leave the outging pull\n> requests in the requestor's repository, but that is only OK if you\n> do not worry about (1) a requestor sending a pull request, then\n> updating the branch the pull request talks about later, to trick you\n> with bait-and-switch, or (2) a requestor sending a pull request,\n> thinks he is done with the topic and removes the repository).\n\nYes, point (2) is the main reason they are not simply attached to the\nsender's repository.\n\n-Peff\n"},{"id":"208814","messageId":"7vzjzhmo4m.fsf@alter.siamese.dyndns.org","threadId":"32780","inReplyTo":"20130206113112.GB5267@sigill.intra.peff.net","subject":"Re: [PATCH v3 3/8] upload/receive-pack: allow hiding ref hierarchies","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-06T15:57:13Z","receivedAt":"2013-02-06T15:57:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Feb 05, 2013 at 07:45:01AM -0800, Junio C Hamano wrote:\n>\n>> > In the earlier review, I mentioned making this per-service, but I see\n>> > that is not the case here. Do you have an argument against doing so?\n>> \n>> Perhaps then I misunderstood your intention. By reminding me of the\n>> receive-pack side, I thought you were hinting to unify these two\n>> into one, which I did. There is no argument against it.\n>\n> What I meant was that there should be transfer.hiderefs, and an\n> individual {receive,uploadpack}.hiderefs, similar to the way we have\n> transfer.unpacklimit.\n\nYes, as I said, I misunderstood your intention.\n"},{"id":"208845","messageId":"7v4nhpckwd.fsf@alter.siamese.dyndns.org","threadId":"32780","inReplyTo":"CACsJy8BhL4qDb8BgOVuaUFF_9GXvgu55urYyKqPuZMZCTCoLwA@mail.gmail.com","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-06T19:17:06Z","receivedAt":"2013-02-06T19:17:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Tue, Feb 5, 2013 at 5:29 PM, Michael Haggerty wrote:\n>> Hiderefs creates a \"dark\" corner of a remote git repo that can hold\n>> arbitrary content that is impossible for anybody to discover but\n>> nevertheless possible for anybody to download (if they know the name of\n>> a hidden reference). In earlier versions of the patch series I believe\n>> that it was possible to push to a hidden reference hierarchy, which made\n>> it possible to upload dark content. The new version appears (from the\n>> code) to prohibit adding references in a hidden hierarchy, which would\n>> close the main loophole that I was worried about. But the documentation\n>> and the unit tests only explicitly say that updates and deletes are\n>> prohibited; nothing is said about adding references (unless \"update\" is\n>> understood to include \"add\"). I think the true behavior should be\n>> clarified and tested.\n>>\n>> I was worried that somehow this \"dark\" content could be used for\n>> malicious purposes; for example, pushing compromised code then\n>> convincing somebody to download it by SHA1 with the implicit argument\n>> \"it's safe since it comes directly from the project's official\n>> repository\". If it is indeed impossible to populate the dark namespace\n>> remotely then I can't think of a way to exploit it.\n>\n> Or you can think hiderefs is the first step to addressing the\n> initial ref advertisment problem. The series says hidden refs are\n> to be fetched out of band, but that's not the only way.\n\nLet me help unconfuse this thread.\n\nI think the series as 8-patch series was poorly presented, and\nseparating it into two will help understanding what they are about.\n\nThe first three:\n\n upload-pack: share more code\n upload-pack: simplify request validation\n upload/receive-pack: allow hiding ref hierarchies\n\nis _the_ topic of the series. As far as I am concerned (I am not\nspeaking for Gerrit users, but am speaking as the Git maintainer),\nthe topic is solely about uncluttering. There may be refs that the\nserver end may need to keep for its operation, but that remote users\nhave _no_ business knowing about. Allowing the server to keep these\nrefs in the repository, while not showing these refs over the wire,\nis the problem the series solves.\n\nIn other words, it is not about \"these are *usually* not wanted by\nclients, so do not show them by default\". It is about \"these are\nnot to be shown, ever\".\n\nOK?\n\nNow, there may be some refs that are not *usually* wanted by clients\nbut there may be cases where clients want to\n\n (1) learn about them via the same protocol; and/or\n (2) fetch them over the protocol.\n\nIf you want to solve both of these two issues generally, the\nsolution has to involve a separate protocol from the today's\nprotocol. It would go like this:\n\n * The upload-pack-2 service sits on a port different from today's,\n waits for a ls-remote/fetch/clone client to connect to it, makes\n a default advertisement that only includes the refs that are\n usually wanted by clients with hints on what other refs the\n initial advertisement omitted, to let the client know that it is\n allowed to ask for them.\n\n * An updated client, if it sees that some refs are omitted from the\n initial advertisement *and* what the user told it to fetch or\n list may be one of the omitted ones (this is why the server gives\n hints in the previous step in the first step; when the server\n says it did not omit anything, or when it says it omitted only\n refs/pull/*, a client that wanted to fetch refs/heads/frotz will\n know the request will fail without continuing this step), then\n makes a \"expand-refs\" request to the server, asking for the refs\n it did not see and the server could supply.\n\n * When the server sees \"expand-refs\", it responds with additional\n advertisement. \"expand-refs refs/pull/*\" may result in listing\n of all refs in that hierarchy. \"expand-refs refs/changes/1/1\"\n would result in listing that single ref. \"expand-refs no-such\"\n may result in nothing, indicating an error.\n\n * After the (possible) expand-refs exchange, the client knows\n exactly the same and necessary information as the current\n protocol gives it in order to go to the common ancestor discovery\n step, and the protocol can continue the same way as the current\n protocol.\n\nNote that this cannot sit on the current port in general, as\nexisting clients will not be able to tell some refs are not\nadvertised, so unless you are hiding large and truly unused part of\nthe refspace, interoperability with older clients will render the\nmechanism useless. You cannot use this to delay the refs/tags/\nhierarchy with this mechanism and have older client come to the\nupdated service that by default does not advertise tags, for\nexample.\n\nThe above is what I called the \"delayed advertisement\" in the\ndiscussion, which was brought up several months ago but nothing\nmaterialized as the result. People who are interested in pursuing\nthis can volunteer and start discussing the design refinements now\nand submit implementation for reviews.\n\nBut in the meantime, if there is a niche use case where a solution\nto only the second problem is sufficient (and Gerrit and GitHub pull\nrequests could both be such use cases), the remainder of the series\ncan help, without waiting the solution to solve \"usually not wanted\nbut may need to be learned\" problem. That is the latter 4 patches\n(the very last one is a demonstration to illustrate why allowing a\npush to hidden ref hierarchy would not and should not work, and is\nnot for application):\n\n parse_fetch_refspec(): clarify the codeflow a bit\n fetch: use struct ref to represent refs to be fetched\n upload-pack: optionally allow fetching from the tips of hidden refs\n fetch: fetch objects by their exact SHA-1 object names\n"},{"id":"208849","messageId":"20130206194542.GB21003@google.com","threadId":"32780","inReplyTo":"7v4nhpckwd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-02-06T19:45:43Z","receivedAt":"2013-02-06T19:45:43Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Duy Nguyen writes:\n>> On Tue, Feb 5, 2013 at 5:29 PM, Michael Haggerty wrote:\n\n>>> Hiderefs creates a \"dark\" corner of a remote git repo\n[...]\n>> Or you can think hiderefs is the first step to addressing the\n>> initial ref advertisment problem. The series says hidden refs are\n>> to be fetched out of band, but that's not the only way.\n>\n> Let me help unconfuse this thread.\n>\n> I think the series as 8-patch series was poorly presented, and\n> separating it into two will help understanding what they are about.\n>\n> The first three:\n>\n> upload-pack: share more code\n> upload-pack: simplify request validation\n> upload/receive-pack: allow hiding ref hierarchies\n>\n> is _the_ topic of the series. As far as I am concerned (I am not\n> speaking for Gerrit users, but am speaking as the Git maintainer),\n> the topic is solely about uncluttering. There may be refs that the\n> server end may need to keep for its operation, but that remote users\n> have _no_ business knowing about.\n\nAn obvious question when looking at that alone is, is there ever\nactually need for such private refs? If the refs are not meant to be\nshared with users *at all*, why are they even refs?\n\nAn answer is \"because refs force gc to keep the corresponding\nobjects\". For example, the sysadmin may want to keep refs/archived/\nrefs for dead branches that should not be advertised or accessible to\nthe user any more. Seems sane, though not especially exciting.\n\nWhat is more exciting to me is that it is a first step toward\naddressing the complicated problem of offering access to more refs\nthan can be efficiently presented in the current ref advertisement. I\nthink that's a harder problem but something like this would be needed\nin order to support existing clients without performance degredation.\n\nAnd in the meantime, it helps with the refs/archived case.\n\nThanks for explaining.\nJonathan\n"},{"id":"208851","messageId":"20130206195515.GC21003@google.com","threadId":"32780","inReplyTo":"51122D9D.9040100@alum.mit.edu","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-02-06T19:55:15Z","receivedAt":"2013-02-06T19:55:15Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Michael Haggerty wrote:\n\n> Scenario 1: Some providers junk up their users' repositories with\n> content that is not created by the repository's owner and that the owner\n> doesn't want to appear to vouch for (e.g., GitHub pull requests). These\n> references might sometimes be useful to fetch, singly or in bulk.\n>\n> Scenario 2: Some systems junk up their users' repositories with\n> additional references that are not interesting to most pullers (e.g.,\n> Gerrit activity markers) though they don't add questionable content.\n\nActually Gerrit's refs/changes refs are pretty similar to Github's\nrefs/pull. Both are requests for code review.\n\n[...]\n> But now every time I do a \"gitk --all\" or \"git log --decorate\", the\n> output is cluttered with all of his references (most of which are just\n> old versions of references from the upstream repository that we both\n> use). I would like to be able to hide his references most of the time\n> but turn them back on when I need them.\n>\n> Scenario 5: Our upstream repository has gazillions of release tags under\n> \"refs/tags/releases/...\", sometimes including customer-specific\n> releases. In my daily life these are just clutter.\n\nFor both of these use cases, putting the refs somewhere other than\nrefs/heads, refs/tags, and refs/remotes should be enough to avoid\nclutter.\n\nI agree that a --decorate-glob along the lines of \"git rev-parse\"'s\n--glob would be nice.\n\n[...]\n> * Some small improvements (e.g. allowing *multiple* views to be\n> defined) would provide much more benefit for about the same effort,\n> and would be a better base for building other features in the future\n> (e.g., local views).\n\nWould advertising GIT_CONFIG_PARAMETERS and giving examples for server\nadmins to set it in inetd et al to provide different kinds of access\nto a same repository through different URLs work?\n\n> Thanks for listening.\n> Michael\n>\n> [1] Theoretically one could support multiple views of a single\n> repository by using something like \"GIT_CONFIG=view_1_config git\n> upload-pack ...\" or \"git -c transfer.hiderefs=... git upload-pack ...\",\n> but this would be awkward.\n\nAh, I missed this comment before. What's awkward about that? I\nthink it's a clean way to make many aspects of how a repository is\npresented (including hook actions) configurable.\n\nThanks for your help clarifying this feature. Hopefully some of the\ndiscussion will filter into the documentation.\n\nJonathan\n"},{"id":"208867","messageId":"5112D028.4050005@alum.mit.edu","threadId":"32780","inReplyTo":"7v4nhpckwd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-02-06T21:50:32Z","receivedAt":"2013-02-06T21:50:32Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 02/06/2013 08:17 PM, Junio C Hamano wrote:\n> Duy Nguyen writes:\n> \n>> On Tue, Feb 5, 2013 at 5:29 PM, Michael Haggerty wrote:\n>>> Hiderefs creates a \"dark\" corner of a remote git repo that can hold\n>>> arbitrary content that is impossible for anybody to discover but\n>>> nevertheless possible for anybody to download (if they know the name of\n>>> a hidden reference). In earlier versions of the patch series I believe\n>>> that it was possible to push to a hidden reference hierarchy, which made\n>>> it possible to upload dark content. The new version appears (from the\n>>> code) to prohibit adding references in a hidden hierarchy, which would\n>>> close the main loophole that I was worried about. But the documentation\n>>> and the unit tests only explicitly say that updates and deletes are\n>>> prohibited; nothing is said about adding references (unless \"update\" is\n>>> understood to include \"add\"). I think the true behavior should be\n>>> clarified and tested.\n>>>\n>>> I was worried that somehow this \"dark\" content could be used for\n>>> malicious purposes; for example, pushing compromised code then\n>>> convincing somebody to download it by SHA1 with the implicit argument\n>>> \"it's safe since it comes directly from the project's official\n>>> repository\". If it is indeed impossible to populate the dark namespace\n>>> remotely then I can't think of a way to exploit it.\n>>\n>> Or you can think hiderefs is the first step to addressing the\n>> initial ref advertisment problem. The series says hidden refs are\n>> to be fetched out of band, but that's not the only way.\n> \n> Let me help unconfuse this thread.\n> \n> I think the series as 8-patch series was poorly presented, and\n> separating it into two will help understanding what they are about.\n> \n> The first three:\n> \n> upload-pack: share more code\n> upload-pack: simplify request validation\n> upload/receive-pack: allow hiding ref hierarchies\n> \n> is _the_ topic of the series. As far as I am concerned (I am not\n> speaking for Gerrit users, but am speaking as the Git maintainer),\n> the topic is solely about uncluttering. There may be refs that the\n> server end may need to keep for its operation, but that remote users\n> have _no_ business knowing about. Allowing the server to keep these\n> refs in the repository, while not showing these refs over the wire,\n> is the problem the series solves.\n> \n> In other words, it is not about \"these are *usually* not wanted by\n> clients, so do not show them by default\". It is about \"these are\n> not to be shown, ever\".\n> \n> OK?\n\nYes, the first three patches sound much more reasonable if this is the\ngoal. Do you know of users who want the feature defined by the first\nthree patches, or is it only a stepping stone towards an actually useful\nfeature? (I ask because I have trouble imagining a real-world scenario\nwhere these alone would be useful.)\n\n> Now, there may be some refs that are not *usually* wanted by clients\n> but there may be cases where clients want to\n> \n> (1) learn about them via the same protocol; and/or\n> (2) fetch them over the protocol.\n> \n> If you want to solve both of these two issues generally, the\n> solution has to involve a separate protocol from the today's\n> protocol. It would go like this:\n[... omitted clear explanation of how delayed advertisement could be\nimplemented via a new protocol ...]\n\n> But in the meantime, if there is a niche use case where a solution\n> to only the second problem is sufficient (and Gerrit and GitHub pull\n> requests could both be such use cases), the remainder of the series\n> can help, without waiting the solution to solve \"usually not wanted\n> but may need to be learned\" problem. That is the latter 4 patches\n> (the very last one is a demonstration to illustrate why allowing a\n> push to hidden ref hierarchy would not and should not work, and is\n> not for application):\n\nGiven that some people *do* want to fetch all pull requests, is this a\nfeature that any hosting service would really turn on? True, the\nmajority of users would be spared clutter, but at the cost of completely\npreventing other users from fetching all pull requests, mirroring the\nrepository, etc.\n\nIn other words, I wonder whether your two incremental steps are useful\nat all, in the real world, without yet-to-be-implemented future changes.\n If not, then it doesn't make sense to merge them without at least\nimagining the final goal and gaining confidence that they are not false\nstarts.\n\n\nI think that a more useful interim solution would be to make it easy to\nhave two URLs accessing a single git repository, with different levels\nof reference visibility applied to each. This is something that\nproviders could turn on without sacrificing any existing functionality.\n And it would solve all three problems: clutter, bandwidth, and provenance.\n\nYour first three patches would allow two-tier access to be implemented,\nfor example by setting GIT_CONFIG or GIT_CONFIG_PARAMETERS or\ncommand-line parameters differently for the processes serving the two\nURLs, like:\n\n git upload-pack ...\n\nvs.\n\n GIT_CONFIG=config-with-hidden-refs git upload-pack ...\nor\n git -c transfer.hiderefs=refs/pull upload-pack ...\n\nBut this is a bit awkward because the admin would either have to\nmaintain two config files, or maintain the hiderefs configuration in the\nscript starting upload-pack rather than in the configuration file.\n\nTherefore, I suggest a slight change to how hiderefs are configured to\nmake two-tier URLs easier to configure, such as\n\n # Define one or more views:\n [view \"uncluttered\"]\n hiderefs = refs/pull\n\n # This would set the default view for all services:\n [transfer]\n view = uncluttered\n\n # Peff also wanted the possibility to configure each service\n # independently which could be done like this:\n [receive]\n view = uncluttered\n [uploadpack]\n view = full\n\nI also tentatively suggest that we add a git-level option \"--view\" and\nan environment variable GIT_VIEW (similar to \"--namespace\" and\nGIT_NAMESPACE) to override the default setting:\n\n GIT_VIEW=uncluttered git upload-pack ...\n\nThis way whoever starts the process only needs to choose a particular\nview name; the actual definition would reside in the config file.\n\nI think these changes would make it easier to support two-tier URLs and\nwould also leave the way open to use the \"view\" concept for other things\nin the future.\n\n\nI've said my piece now and am gratified that there has been more\ndiscussion about your proposal, which was my main goal. Therefore FWIW\nI turn my -1 into a -0 and leave it up to the people experiencing more\nclutter-induced pain to decide how to proceed.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"208871","messageId":"5112D2BC.2080407@alum.mit.edu","threadId":"32780","inReplyTo":"20130206195515.GC21003@google.com","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-02-06T22:01:32Z","receivedAt":"2013-02-06T22:01:32Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 02/06/2013 08:55 PM, Jonathan Nieder wrote:\n> Michael Haggerty wrote:\n> [...]\n>> But now every time I do a \"gitk --all\" or \"git log --decorate\", the\n>> output is cluttered with all of his references (most of which are just\n>> old versions of references from the upstream repository that we both\n>> use). I would like to be able to hide his references most of the time\n>> but turn them back on when I need them.\n>>\n>> Scenario 5: Our upstream repository has gazillions of release tags under\n>> \"refs/tags/releases/...\", sometimes including customer-specific\n>> releases. In my daily life these are just clutter.\n> \n> For both of these use cases, putting the refs somewhere other than\n> refs/heads, refs/tags, and refs/remotes should be enough to avoid\n> clutter.\n\nThanks, yes, for release tags in particular your suggestion might be\nworkable. But I also like the idea of being able to turn subsets of\nreferences on and off easily, and have the choice persist until I change it.\n\n> [...]\n>> * Some small improvements (e.g. allowing *multiple* views to be\n>> defined) would provide much more benefit for about the same effort,\n>> and would be a better base for building other features in the future\n>> (e.g., local views).\n> \n> Would advertising GIT_CONFIG_PARAMETERS and giving examples for server\n> admins to set it in inetd et al to provide different kinds of access\n> to a same repository through different URLs work?\n> \n>> Thanks for listening.\n>> Michael\n>>\n>> [1] Theoretically one could support multiple views of a single\n>> repository by using something like \"GIT_CONFIG=view_1_config git\n>> upload-pack ...\" or \"git -c transfer.hiderefs=... git upload-pack ...\",\n>> but this would be awkward.\n> \n> Ah, I missed this comment before. What's awkward about that? I\n> think it's a clean way to make many aspects of how a repository is\n> presented (including hook actions) configurable.\n\nAwkwardness using GIT_CONFIG: the admin would have to maintain two\nseparate config files with mostly overlapping content.\n\nAwkwardness using GIT_CONFIG_PARAMETERS or \"-c transfer.hiderefs=...\":\nthe hiderefs configuration would have to be maintained in some Apache\nconfig or inetd or ... (or multiple places!) rather than in the\nrepository's config file, where it belongs.\n\nAdditional awkwardness using \"-c transfer.hiderefs=...\": AFAIK there is\nno way to turn *off* a configuration variable via a command-line option.\n\nIt's all doable, but I find it awkward.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"208873","messageId":"7vzjzh9jnu.fsf@alter.siamese.dyndns.org","threadId":"32780","inReplyTo":"5112D028.4050005@alum.mit.edu","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-06T22:12:05Z","receivedAt":"2013-02-06T22:12:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty writes:\n\n> On 02/06/2013 08:17 PM, Junio C Hamano wrote:\n> ...\n>\n> Yes, the first three patches sound much more reasonable if this is the\n> goal.\n> ...\n> I think that a more useful interim solution would be to make it easy to\n> have two URLs accessing a single git repository, with different levels\n> of reference visibility applied to each.\n\nI think you said \"more reasonable\" without understanding what you\nare saying is \"more reasonable\", then. The mechanism is about\nserver side wanting to use refs to protect its own metadata from gc\nwithout having to expose them; there is no \"different levels\".\n\n> ...\n> GIT_CONFIG=config-with-hidden-refs git upload-pack ...\n> or\n> git -c transfer.hiderefs=refs/pull upload-pack ...\n>\n> But this is a bit awkward ...\n\nIt is awkward to use hammer to drive screws in wood, too. You want\nto use a screwdriver. The first three patches are to drive a nail\nwith hammer, OK? Screws you keep bringing up is to be handled by\ndelayed ref advertisement.\n"},{"id":"208877","messageId":"CACBZZX6xLvuMEhPnfYLj8W9pMLwdoS7Zb+mTtn+3DanJPiWfXw@mail.gmail.com","threadId":"32780","inReplyTo":"7v4nhpckwd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2013-02-06T22:26:23Z","receivedAt":"2013-02-06T22:26:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Feb 6, 2013 at 8:17 PM, Junio C Hamano wrote:\n\nMaybe this should be split up into a different thread, but:\n\n> The upload-pack-2 service sits on a port different from today's\n> [...].\n\nI think there's a simpler way to do this, which is that:\n\n * New clients supporting v2 of the protocol send some piece of data\n that would break old servers.\n\n * If that fails the new client goes \"oh jeeze, I guess it's an old\n server\", and try again with the old protocol.\n\n * The client then saves a date (or the version the server gave us)\n indicating that it tried the new protocol on that remote, tries\n again sometime later.\n\nWe already covered in previous discussions how this would be simpler\nwith the HTTP protocol, since you could just send an extra header\ninviting the server to speak the new protocol.\n\nBut for the other transports we can just try the new protocol and\nretry with the old one as a fallback if it doesn't work. That'll allow\nus to gracefully migrate without needing to change the git:// port.\n\nBesides, I think the vast majority of users are using Git via http://\nor ssh://, where we can't just change the port, but even so making\npeople change the port when we could handle this more gracefully would\nbe a big PITA. Adding new firewall holes is often a big bureaucratic\nnightmare in some organizations.\n"},{"id":"208881","messageId":"20130206225616.GI27507@sigill.intra.peff.net","threadId":"32780","inReplyTo":"7v4nhpckwd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-06T22:56:16Z","receivedAt":"2013-02-06T22:56:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 06, 2013 at 11:17:06AM -0800, Junio C Hamano wrote:\n\n> Let me help unconfuse this thread.\n> \n> I think the series as 8-patch series was poorly presented, and\n> separating it into two will help understanding what they are about.\n> \n> The first three:\n> \n> upload-pack: share more code\n> upload-pack: simplify request validation\n> upload/receive-pack: allow hiding ref hierarchies\n> \n> is _the_ topic of the series. As far as I am concerned (I am not\n> speaking for Gerrit users, but am speaking as the Git maintainer),\n> the topic is solely about uncluttering. There may be refs that the\n> server end may need to keep for its operation, but that remote users\n> have _no_ business knowing about. Allowing the server to keep these\n> refs in the repository, while not showing these refs over the wire,\n> is the problem the series solves.\n> \n> In other words, it is not about \"these are *usually* not wanted by\n> clients, so do not show them by default\". It is about \"these are\n> not to be shown, ever\".\n> \n> OK?\n\nRight. I am not opposed to this series, as it does have a use-case. And\nif it helps Gerrit folks or other users unclutter, great. The fact that\nI could throw away the custom receive.hiderefs patch we use at GitHub is\na bonus. If people want fancier things, they can do them separately.\n\n_But_. As a potential user of the feature (to hide refs/pull/*), I do\nnot think it is sufficiently flexible for me to use transfer.hiderefs\n(or uploadpack.hiderefs). We use \"fetch\" internally to migrate objects\nbetween forks and our alternates repos. And in that case, we really do\nwant to see all refs. In other words, all fetches are not the same: we\nwould want upload-pack to understand the difference between a client\nfetch and an internal administrative fetch. But this feature does not\nprovide that lee-way. Even if you tried:\n\n git fetch -u 'git -c uploadpack.hiderefs= upload-pack'\n\nthe list nature of the config variable means you cannot reset it.\n\nThis isn't a show-stopper for the series; it may just mean that it is\nnot a good fit for GitHub's use case, but others (like Gerrit) may\nbenefit. But since refs/pull is used as an example of where this could\nbe applied, I wanted to point out that it does not achieve that goal.\n\n-Peff\n"},{"id":"208889","messageId":"7vmwvh9e3p.fsf@alter.siamese.dyndns.org","threadId":"32780","inReplyTo":"CACBZZX6xLvuMEhPnfYLj8W9pMLwdoS7Zb+mTtn+3DanJPiWfXw@mail.gmail.com","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-07T00:12:10Z","receivedAt":"2013-02-07T00:12: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> I think there's a simpler way to do this, which is that:\n>\n> * New clients supporting v2 of the protocol send some piece of data\n> that would break old servers.\n>\n> * If that fails the new client goes \"oh jeeze, I guess it's an old\n> server\", and try again with the old protocol.\n>\n> * The client then saves a date (or the version the server gave us)\n> indicating that it tried the new protocol on that remote, tries\n> again sometime later.\n\nFor that to work, the new server needs to wait for the client to\nspeak first. How would that server handle old clients who expect to\nbe spoken first? Wait with a read timeout (no timeout is the right\ntimeout for everybody)?\n\n> We already covered in previous discussions how this would be simpler\n> with the HTTP protocol,...\n\nYes, that is a solved problem.\n"},{"id":"208891","messageId":"20130207001635.GA29318@sigill.intra.peff.net","threadId":"32780","inReplyTo":"7vmwvh9e3p.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-07T00:16:36Z","receivedAt":"2013-02-07T00:16:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 06, 2013 at 04:12:10PM -0800, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n> \n> > I think there's a simpler way to do this, which is that:\n> >\n> > * New clients supporting v2 of the protocol send some piece of data\n> > that would break old servers.\n> >\n> > * If that fails the new client goes \"oh jeeze, I guess it's an old\n> > server\", and try again with the old protocol.\n> >\n> > * The client then saves a date (or the version the server gave us)\n> > indicating that it tried the new protocol on that remote, tries\n> > again sometime later.\n> \n> For that to work, the new server needs to wait for the client to\n> speak first. How would that server handle old clients who expect to\n> be spoken first? Wait with a read timeout (no timeout is the right\n> timeout for everybody)?\n\nIf the new client can handle the old-style server's response, then the\nserver can start blasting out refs (optionally after a timeout) and stop\nwhen the client interrupts with \"hey, wait, I can speak the new\nprotocol\". The server just has to include \"you can interrupt me\" in its\ncapability advertisement (obviously it would have to send out at least\nthe first ref with the capabilities before the timeout).\n\n-Peff\n"},{"id":"208911","messageId":"CACBZZX4LzW5bbfo+UkcXsBF3nfZSJstC22NEUsJe=7oCenJgpw@mail.gmail.com","threadId":"32780","inReplyTo":"20130207001635.GA29318@sigill.intra.peff.net","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2013-02-07T10:30:24Z","receivedAt":"2013-02-07T10:30:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Feb 7, 2013 at 1:16 AM, Jeff King wrote:\n> On Wed, Feb 06, 2013 at 04:12:10PM -0800, Junio C Hamano wrote:\n>\n>> Ævar Arnfjörð Bjarmason writes:\n>>\n>> > I think there's a simpler way to do this, which is that:\n>> >\n>> > * New clients supporting v2 of the protocol send some piece of data\n>> > that would break old servers.\n>> >\n>> > * If that fails the new client goes \"oh jeeze, I guess it's an old\n>> > server\", and try again with the old protocol.\n>> >\n>> > * The client then saves a date (or the version the server gave us)\n>> > indicating that it tried the new protocol on that remote, tries\n>> > again sometime later.\n>>\n>> For that to work, the new server needs to wait for the client to\n>> speak first. How would that server handle old clients who expect to\n>> be spoken first? Wait with a read timeout (no timeout is the right\n>> timeout for everybody)?\n>\n> If the new client can handle the old-style server's response, then the\n> server can start blasting out refs (optionally after a timeout) and stop\n> when the client interrupts with \"hey, wait, I can speak the new\n> protocol\". The server just has to include \"you can interrupt me\" in its\n> capability advertisement (obviously it would have to send out at least\n> the first ref with the capabilities before the timeout).\n\nCan't this also be handled by passing an extra argument to\nupload-pack? Whether you're talking http, ssh + normal shell, ssh +\ngit-shell or git:// you pass some argument that older clients would\nreject on but would cause newer clients that know about that argument\nto wait for you to speak before blasting refs at you.\n\nIt would mean that older clients (e.g. older git-shell) would reject\nyour initial connection, but you could just try again, and save away\ninfo about that remote's version.\n"},{"id":"208922","messageId":"87pq0c15h3.fsf@59A2.org","threadId":"32780","inReplyTo":"51122D9D.9040100@alum.mit.edu","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Jed Brown","fromEmail":"jed@59a2.org","sentAt":"2013-02-07T15:58:00Z","receivedAt":"2013-02-07T15:58:00Z","isPatch":true,"sender":{"key":"jed@59a2.org","avatar":"https://gravatar.com/avatar/1391d04d82555f9058a9fdf5eead233e909a48e40480db31fc554e7afeb301da?d=mp&s=160"},"body":"Michael Haggerty writes:\n\n> A first weakness of your proposal is that even though the hidden refs\n> are (optionally) fetchable, there is *no* way to discover them remotely\n> or to bulk-download them; they would have to be retrieved one by one\n> using out-of-band information. And if I understand correctly, there\n> would be no way to push hidden references remotely (whether manually or\n> from some automated process). Such processes would have to be local to\n> the machine holding the repository.\n\nI'm the author of git-fat [1], a smudge/clean filter system for managing\nlarge files. I currently store files in the file system\n(.git/fat/objects) and transfer them via rsync because I want to be able\nto transfer exact subsets requested by the user. I would like to put\nthis data in a git repository so that I can take advantage of packfile\ncompression when applicable and so that I can use existing access\ncontrol, but I would need to store a separate reference to each blob (so\nthat I can transfer exact subsets). My refs would be named like\n'fat-' and are known on the client side because\nthey are in the cleaned blob (which contains only this SHA1 and the\nnumber of bytes [2]).\n\nI believe that my use case would be well supported if git could push and\npull unadvertised refs, as long as basic operations were not slowed down\nby the existence of a very large number of such refs.\n\n\n[1] https://github.com/jedbrown/git-fat\n\n[2] We could eliminate the performance problem of needing to buffer the\nentire file if the smudge filter could be passed the object size as an\nargument and if we could forward that size in a stream to 'git\nhash-object --stdin'.\n"},{"id":"208928","messageId":"7vlib07zgz.fsf@alter.siamese.dyndns.org","threadId":"32780","inReplyTo":"20130207001635.GA29318@sigill.intra.peff.net","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-07T18:25:48Z","receivedAt":"2013-02-07T18:25:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> If the new client can handle the old-style server's response, then the\n> server can start blasting out refs (optionally after a timeout) and stop\n> when the client interrupts with \"hey, wait, I can speak the new\n> protocol\". The server just has to include \"you can interrupt me\" in its\n> capability advertisement (obviously it would have to send out at least\n> the first ref with the capabilities before the timeout).\n\nYeah, I would prefer people to come up with a way to share the port\nand autodetect. It is *not* a requirement for the updated server to\nrun on a separate port at all.\n"},{"id":"209102","messageId":"7v38x5ul4s.fsf@alter.siamese.dyndns.org","threadId":"32780","inReplyTo":"87pq0c15h3.fsf@59A2.org","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-09T23:23:47Z","receivedAt":"2013-02-09T23:23:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jed Brown writes:\n\n> I believe that my use case would be well supported if git could push and\n> pull unadvertised refs, as long as basic operations were not slowed down\n> by the existence of a very large number of such refs.\n\nI am not sure about \"pushing\" part, but the jc/fetch-raw-sha1 topic\n(split from the main jc/hidden-refs topic) should allow your script,\nafter the client learns the set of smudged object names, to ask for\n\n git fetch $there $sha1_1 $sha1_2 ...\n\nor\n\n git fetch $there $sha1_1:refs/fat/$sha1_1 $sha1_2:refs/fat/$sha1_2 ...\n\nI think.\n"},{"id":"209114","messageId":"87y5ew6alp.fsf@59A2.org","threadId":"32780","inReplyTo":"7v38x5ul4s.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Jed Brown","fromEmail":"jed@59a2.org","sentAt":"2013-02-10T04:45:06Z","receivedAt":"2013-02-10T04:45:06Z","isPatch":true,"sender":{"key":"jed@59a2.org","avatar":"https://gravatar.com/avatar/1391d04d82555f9058a9fdf5eead233e909a48e40480db31fc554e7afeb301da?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> I am not sure about \"pushing\" part, but the jc/fetch-raw-sha1 topic\n> (split from the main jc/hidden-refs topic) should allow your script,\n> after the client learns the set of smudged object names, to ask for\n>\n> git fetch $there $sha1_1 $sha1_2 ...\n\nWell, my out-of-band knowledge is currently the sha1 of the data\ncontained in the blob I want, not the blob sha1 itself [1]. After\nexperimenting with jc/hidden-refs, I think it already does exactly what\nI want. Specifically, I set this on the server\n\n git config uploadpack.hiderefs refs/fat\n\nso that 'git ls-remote' no longer transfers these refs. Then on the\nclient, I do\n\n contentid=$(sha1sum thefile | cut -f1 -d \\ )\n blobid=$(git hash-object -w thefile)\n git update-ref refs/fat/$contentid $blobid\n\n .... more like this\n\n git push the-remote refs/fat/$contentid ...\n\nand later, I can fetch specific refs using\n\n git fetch the-remote refs/fat/$wanted:refs/fat/$wanted ...\n\nThe client knows the desired refs out-of-band so this looks okay. It\nwould be convenient to have '--stdin' options to 'git push' and 'git\nfetch'. Would a patch for that be welcome?\n\n\n[1] The reason for using $contentid instead of $blobid in the key here\nis to avoid etching the backend=git detail into the cleaned commits.\n"},{"id":"235196","messageId":"CACsJy8Aas3tRoDp9LQw7Nwf6+S3QnvwA7h7s-sHVY+1yFKhTYg@mail.gmail.com","threadId":"32780","inReplyTo":"7vmwvh9e3p.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-02-23T02:44:14Z","receivedAt":"2014-02-23T02:44:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"(Digging up an old thread about initial refs listing in git protocol)\n\nOn Thu, Feb 7, 2013 at 7:12 AM, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> I think there's a simpler way to do this, which is that:\n>>\n>> * New clients supporting v2 of the protocol send some piece of data\n>> that would break old servers.\n>>\n>> * If that fails the new client goes \"oh jeeze, I guess it's an old\n>> server\", and try again with the old protocol.\n>>\n>> * The client then saves a date (or the version the server gave us)\n>> indicating that it tried the new protocol on that remote, tries\n>> again sometime later.\n>\n> For that to work, the new server needs to wait for the client to\n> speak first. How would that server handle old clients who expect to\n> be spoken first? Wait with a read timeout (no timeout is the right\n> timeout for everybody)?\n\nI think the client always speaks first when it asks for a remote\nservice. Earlier in this thread you described the new protocol\nupload-pack-2. Why can't it be a new service \"upload-pack-2\" in\ngit-daemon?\n\nSo new client will try requesting \"upload-pack-2\" service with client\ncapability advertisement before ref listing. Old servers do not\nrecognize this service and disconnect so the new client falls back to\nthe good old \"upload-pack\" (one more round trip though, but you could\nconfigure new client to use old protocol for certain \"old\" hosts).\nSimilar thing happens for ssh transport. \"upload-pack\" service is\nalways there for old clients.\n-- \nDuy\n"},{"id":"236476","messageId":"20140311014945.GB12033@sigill.intra.peff.net","threadId":"32780","inReplyTo":"CACsJy8Aas3tRoDp9LQw7Nwf6+S3QnvwA7h7s-sHVY+1yFKhTYg@mail.gmail.com","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-11T01:49:45Z","receivedAt":"2014-03-11T01:49:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 23, 2014 at 09:44:14AM +0700, Duy Nguyen wrote:\n\n> (Digging up an old thread about initial refs listing in git protocol)\n\nAnd now I am responding to it slowly. :)\n\n> > For that to work, the new server needs to wait for the client to\n> > speak first. How would that server handle old clients who expect to\n> > be spoken first? Wait with a read timeout (no timeout is the right\n> > timeout for everybody)?\n> \n> I think the client always speaks first when it asks for a remote\n> service. Earlier in this thread you described the new protocol\n> upload-pack-2. Why can't it be a new service \"upload-pack-2\" in\n> git-daemon?\n> \n> So new client will try requesting \"upload-pack-2\" service with client\n> capability advertisement before ref listing. Old servers do not\n> recognize this service and disconnect so the new client falls back to\n> the good old \"upload-pack\" (one more round trip though, but you could\n> configure new client to use old protocol for certain \"old\" hosts).\n> Similar thing happens for ssh transport. \"upload-pack\" service is\n> always there for old clients.\n\nRight, I recall the general feeling being that such a system would work,\nand the transition would be managed by a config variable like\n\"remote.*.useUploadPack2\". Probably with settings like:\n\n true:\n always try, but allow fall back to upload-pack\n\n false:\n never try, always use upload-pack\n\n auto:\n try, but if we fail, set remote.*.uploadPackTimestamp, and do not\n try again for N days\n\nThe default would start at false, and people who know their server is\nvery up-to-date can turn it on. And then when many server\nimplementations support it, flip the default to auto. And either leave\nit there forever, or eventually just set it to \"true\" and drop \"auto\"\nentirely as a code cleanup.\n\nIn theory we could do more radical protocol changes here, but I think\nmost people are just interested in adding an opportunity for the client\nto speak before the ref advertisement in order to set a few\nflags/variables. That should be relatively simple, and for http we can\nprobably pass those flags via url parameters without any extra\ncompatibility/round-trip at all.\n\nI think the main flag of interest is giving an fnmatch pattern to limit\nthe advertised refs. There could potentially be others, but I do not\nknow of any offhand.\n\n-Peff\n"},{"id":"236522","messageId":"xmqqtxb4pm3u.fsf@gitster.dls.corp.google.com","threadId":"32780","inReplyTo":"20140311014945.GB12033@sigill.intra.peff.net","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-11T19:32:37Z","receivedAt":"2014-03-11T19:32:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I think the main flag of interest is giving an fnmatch pattern to limit\n> the advertised refs. There could potentially be others, but I do not\n> know of any offhand.\n\nOne thing that comes to mind is where symrefs point at, which we\nfailed to add the last time around because we ran out of the\nhidden-space behind NUL.\n"},{"id":"236527","messageId":"20140311200513.GB29102@sigill.intra.peff.net","threadId":"32780","inReplyTo":"xmqqtxb4pm3u.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-11T20:05:14Z","receivedAt":"2014-03-11T20:05:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 11, 2014 at 12:32:37PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > I think the main flag of interest is giving an fnmatch pattern to limit\n> > the advertised refs. There could potentially be others, but I do not\n> > know of any offhand.\n> \n> One thing that comes to mind is where symrefs point at, which we\n> failed to add the last time around because we ran out of the\n> hidden-space behind NUL.\n\nYeah, good idea. I might be misremembering some complications, but we\ncan probably do it with:\n\n 1. Teach the client to send an \"advertise-symrefs\" flag before the ref\n advertisement.\n\n 2. Teach the server to include symrefs in the ref advertisement; we\n can invent a new syntax because we know the client has asked for\n it.\n\nThat does not have to come immediately, though. Done correctly,\nupload-pack2 is not about implementing the fnmatch feature, but allowing\narbitrary capability strings from the client before the ref\nadvertisement starts. So this just becomes an extension that we can add\nand advertise during that new phase.\n\n-Peff\n"},{"id":"236531","messageId":"xmqq4n34pjnw.fsf@gitster.dls.corp.google.com","threadId":"32780","inReplyTo":"20140311200513.GB29102@sigill.intra.peff.net","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-11T20:25:23Z","receivedAt":"2014-03-11T20:25:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Mar 11, 2014 at 12:32:37PM -0700, Junio C Hamano wrote:\n>\n>> Jeff King writes:\n>> \n>> > I think the main flag of interest is giving an fnmatch pattern to limit\n>> > the advertised refs. There could potentially be others, but I do not\n>> > know of any offhand.\n>> \n>> One thing that comes to mind is where symrefs point at, which we\n>> failed to add the last time around because we ran out of the\n>> hidden-space behind NUL.\n>\n> Yeah, good idea. I might be misremembering some complications, but we\n> can probably do it with:\n>\n> 1. Teach the client to send an \"advertise-symrefs\" flag before the ref\n> advertisement.\n>\n> 2. Teach the server to include symrefs in the ref advertisement; we\n> can invent a new syntax because we know the client has asked for\n> it.\n\nI was thinking more about the underlying protocol, not advertisement\nin particular, and I think we came to the same conclusion.\n\nThe capability advertisement deserves to have its own separate\npacket message type, when both sides say that they understand it, so\nthat we do not have to be limited by the pkt-line length limit. We\ncould do one message per capability, and at the same time can lift\nthe traditional \"capability hidden after the NUL is purged every\ntime, so we need to repeat them if we want to later change it,\nbecause that is how older clients and servers use that information\"\ninsanity, for example.\n"},{"id":"236535","messageId":"20140311203650.GA31173@sigill.intra.peff.net","threadId":"32780","inReplyTo":"xmqq4n34pjnw.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-11T20:36:50Z","receivedAt":"2014-03-11T20:36:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 11, 2014 at 01:25:23PM -0700, Junio C Hamano wrote:\n\n> > Yeah, good idea. I might be misremembering some complications, but we\n> > can probably do it with:\n> >\n> > 1. Teach the client to send an \"advertise-symrefs\" flag before the ref\n> > advertisement.\n> >\n> > 2. Teach the server to include symrefs in the ref advertisement; we\n> > can invent a new syntax because we know the client has asked for\n> > it.\n> \n> I was thinking more about the underlying protocol, not advertisement\n> in particular, and I think we came to the same conclusion.\n> \n> The capability advertisement deserves to have its own separate\n> packet message type, when both sides say that they understand it, so\n> that we do not have to be limited by the pkt-line length limit. We\n> could do one message per capability, and at the same time can lift\n> the traditional \"capability hidden after the NUL is purged every\n> time, so we need to repeat them if we want to later change it,\n> because that is how older clients and servers use that information\"\n> insanity, for example.\n\nSo this may be entering the \"more radical changes\" realm I mentioned\nearlier.\n\nIf the client is limited to setting a few flags, then something like\nhttp can get away with:\n\n GET foo.git/info/refs?service=git-upload-pack&advertise-symrefs&refspec=refs/heads/*\n\nAnd it does not need to worry about upload-pack2 at all. Either the\nserver recognizes and acts on them, or it ignores them.\n\nBut given that we do not have such a magic out-of-band method for\npassing values over ssh and git, maybe it is not worth worrying about.\nHttp can move to upload-pack2 along with the rest.\n\nOne thing that _is_ worth considering for http is how the protocol\nstarts. We do not want to introduce an extra http round-trip to the\nprotocol if we can help it. If the initial GET becomes a POST, then it\ncould pass along the pkt-line of client capabilities with the initial\nrequest, and the server would respond with the ref advertisement as\nusual.\n\n-Peff\n"},{"id":"236724","messageId":"CACsJy8AZ0CfqHRYDrnQD+z0ibVQnsFuSzktEHKRhCVwaXPQryg@mail.gmail.com","threadId":"32780","inReplyTo":"20140311203650.GA31173@sigill.intra.peff.net","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-14T12:37:28Z","receivedAt":"2014-03-14T12:37:28Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Mar 12, 2014 at 3:36 AM, Jeff King wrote:\n> If the client is limited to setting a few flags, then something like\n> http can get away with:\n>\n> GET foo.git/info/refs?service=git-upload-pack&advertise-symrefs&refspec=refs/heads/*\n>\n> And it does not need to worry about upload-pack2 at all. Either the\n> server recognizes and acts on them, or it ignores them.\n>\n> But given that we do not have such a magic out-of-band method for\n> passing values over ssh and git, maybe it is not worth worrying about.\n\ngit could go the same if we lift the restriction in 73bb33a (daemon:\nStrictly parse the \"extra arg\" part of the command - 2009-06-04). It's\nbeen five years. Old daemons hopefully have all died out by now. For\nssh, I suppose upload-pack and receive-pack can take an extra argument\nlike \"advertise-symrefs&refspec=refs/heads/*\" (daemon would use it too\nto pass the advertiment to upload-pack and receive-pack).\n\nThat would make all three not need to change the underlying protocol\nfor capability advertisement. Old git-daemon, upload-pack and\nreceive-pack will fail hard on the new advertisement though, unlike\nhttp. But that's no worse than upload-pack2.\n\n> Http can move to upload-pack2 along with the rest.\n\nOr maybe http may lead the rest to another way.\n-- \nDuy\n"},{"id":"236731","messageId":"CAJo=hJvy6KKMNT9iyZAnKy18Pa+rQkKPQtfqT1e+ddXoVwX0yg@mail.gmail.com","threadId":"32780","inReplyTo":"CACsJy8AZ0CfqHRYDrnQD+z0ibVQnsFuSzktEHKRhCVwaXPQryg@mail.gmail.com","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2014-03-14T16:45:30Z","receivedAt":"2014-03-14T16:45:30Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"On Fri, Mar 14, 2014 at 5:37 AM, Duy Nguyen wrote:\n> On Wed, Mar 12, 2014 at 3:36 AM, Jeff King wrote:\n>> If the client is limited to setting a few flags, then something like\n>> http can get away with:\n>>\n>> GET foo.git/info/refs?service=git-upload-pack&advertise-symrefs&refspec=refs/heads/*\n>>\n>> And it does not need to worry about upload-pack2 at all. Either the\n>> server recognizes and acts on them, or it ignores them.\n>>\n>> But given that we do not have such a magic out-of-band method for\n>> passing values over ssh and git, maybe it is not worth worrying about.\n>\n> git could go the same if we lift the restriction in 73bb33a (daemon:\n> Strictly parse the \"extra arg\" part of the command - 2009-06-04). It's\n> been five years. Old daemons hopefully have all died out by now. For\n> ssh, I suppose upload-pack and receive-pack can take an extra argument\n> like \"advertise-symrefs&refspec=refs/heads/*\" (daemon would use it too\n> to pass the advertiment to upload-pack and receive-pack).\n\nHeh. IIRC you are talking about the DoS attack for git-daemon where\nyou send an extra header and the process infinite loops forever? We\nreally don't want a modern client attempting to upgrade the protocol\nwith an ancient daemon to DoS attack that server.\n\n> That would make all three not need to change the underlying protocol\n> for capability advertisement. Old git-daemon, upload-pack and\n> receive-pack will fail hard on the new advertisement though, unlike\n> http. But that's no worse than upload-pack2.\n\nYou missed the SSH case. It doesn't have this slot to hide the data into.\n\n>> Http can move to upload-pack2 along with the rest.\n>\n> Or maybe http may lead the rest to another way.\n> --\n> Duy\n"},{"id":"236759","messageId":"CACsJy8DtuCCYmmsEFB_m-YPHOOQ4FuchvnYQeuv75-vcSMej_w@mail.gmail.com","threadId":"32780","inReplyTo":"CAJo=hJvy6KKMNT9iyZAnKy18Pa+rQkKPQtfqT1e+ddXoVwX0yg@mail.gmail.com","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-14T23:30:47Z","receivedAt":"2014-03-14T23:30:47Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Mar 14, 2014 at 11:45 PM, Shawn Pearce wrote:\n> On Fri, Mar 14, 2014 at 5:37 AM, Duy Nguyen wrote:\n>> On Wed, Mar 12, 2014 at 3:36 AM, Jeff King wrote:\n>>> If the client is limited to setting a few flags, then something like\n>>> http can get away with:\n>>>\n>>> GET foo.git/info/refs?service=git-upload-pack&advertise-symrefs&refspec=refs/heads/*\n>>>\n>>> And it does not need to worry about upload-pack2 at all. Either the\n>>> server recognizes and acts on them, or it ignores them.\n>>>\n>>> But given that we do not have such a magic out-of-band method for\n>>> passing values over ssh and git, maybe it is not worth worrying about.\n>>\n>> git could go the same if we lift the restriction in 73bb33a (daemon:\n>> Strictly parse the \"extra arg\" part of the command - 2009-06-04). It's\n>> been five years. Old daemons hopefully have all died out by now. For\n>> ssh, I suppose upload-pack and receive-pack can take an extra argument\n>> like \"advertise-symrefs&refspec=refs/heads/*\" (daemon would use it too\n>> to pass the advertiment to upload-pack and receive-pack).\n>\n> Heh. IIRC you are talking about the DoS attack for git-daemon where\n> you send an extra header and the process infinite loops forever? We\n> really don't want a modern client attempting to upgrade the protocol\n> with an ancient daemon to DoS attack that server.\n\nShouldn't vulnerable daemons be upgraded anyway? If they keep using\nthe vulnerable version for all these 5 years, I feel no sorry for new\nclients DoSing them. Jeff's idea about \"remote.*.useUploadPack2\" still\napplies here so after we attack the server once, it'll be black listed\nfor a while (or forever).\n\n>> That would make all three not need to change the underlying protocol\n>> for capability advertisement. Old git-daemon, upload-pack and\n>> receive-pack will fail hard on the new advertisement though, unlike\n>> http. But that's no worse than upload-pack2.\n>\n> You missed the SSH case. It doesn't have this slot to hide the data into.\n\nRight now we run this for ssh case: \"ssh git-upload-pack\n\". New client can do this instead\n\nssh git-upload-pack \n-- \nDuy\n"},{"id":"236765","messageId":"CAJo=hJuGBgkseQ_mvbxFnYbkFDDWEuassf2+ttj_F53AMzU_Nw@mail.gmail.com","threadId":"32780","inReplyTo":"CACsJy8DtuCCYmmsEFB_m-YPHOOQ4FuchvnYQeuv75-vcSMej_w@mail.gmail.com","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2014-03-15T00:09:45Z","receivedAt":"2014-03-15T00:09:45Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"On Fri, Mar 14, 2014 at 4:30 PM, Duy Nguyen wrote:\n> On Fri, Mar 14, 2014 at 11:45 PM, Shawn Pearce wrote:\n>>\n>> You missed the SSH case. It doesn't have this slot to hide the data into.\n>\n> Right now we run this for ssh case: \"ssh git-upload-pack\n> \". New client can do this instead\n>\n> ssh git-upload-pack \n\nOlder servers will fail on this command, and the client must reconnect\nover SSH, which may mean supplying their password/passphrase again.\nBut its remembered that the uploadPack2 didn't work so this can be\nblacklisted and not retried for a while.\n"},{"id":"236768","messageId":"CACsJy8A1=U2=TGoKyo5mo1fLW+hBR1psn1J6S0=391fei2JULw@mail.gmail.com","threadId":"32780","inReplyTo":"20140311014945.GB12033@sigill.intra.peff.net","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-15T01:23:08Z","receivedAt":"2014-03-15T01:23:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Mar 11, 2014 at 8:49 AM, Jeff King wrote:\n> Right, I recall the general feeling being that such a system would work,\n> and the transition would be managed by a config variable like\n> \"remote.*.useUploadPack2\". Probably with settings like:\n>\n> true:\n> always try, but allow fall back to upload-pack\n>\n> false:\n> never try, always use upload-pack\n>\n> auto:\n> try, but if we fail, set remote.*.uploadPackTimestamp, and do not\n> try again for N days\n>\n> The default would start at false, and people who know their server is\n> very up-to-date can turn it on. And then when many server\n> implementations support it, flip the default to auto. And either leave\n> it there forever, or eventually just set it to \"true\" and drop \"auto\"\n> entirely as a code cleanup.\n\nI would add that upload-pack also advertises about the availability of\nupload-pack2 and the client may set the remote.*.useUploadPack2 to\neither yes or auto so next time upload-pack2 will be used.\n-- \nDuy\n"},{"id":"236954","messageId":"20140318041739.GA7252@sigill.intra.peff.net","threadId":"32780","inReplyTo":"CAJo=hJuGBgkseQ_mvbxFnYbkFDDWEuassf2+ttj_F53AMzU_Nw@mail.gmail.com","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-18T04:17:39Z","receivedAt":"2014-03-18T04:17:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 14, 2014 at 05:09:45PM -0700, Shawn Pearce wrote:\n\n> On Fri, Mar 14, 2014 at 4:30 PM, Duy Nguyen wrote:\n> > On Fri, Mar 14, 2014 at 11:45 PM, Shawn Pearce wrote:\n> >>\n> >> You missed the SSH case. It doesn't have this slot to hide the data into.\n> >\n> > Right now we run this for ssh case: \"ssh git-upload-pack\n> > \". New client can do this instead\n> >\n> > ssh git-upload-pack \n> \n> Older servers will fail on this command, and the client must reconnect\n> over SSH, which may mean supplying their password/passphrase again.\n> But its remembered that the uploadPack2 didn't work so this can be\n> blacklisted and not retried for a while.\n\nI wonder if we could use the environment for optional values. E.g., can\nwe run:\n\n ssh host GIT_CAPABILITIES=... git-upload-pack \n\nThat will not work everywhere, of course. Sites with git-shell will\nfail, as will sites with custom ssh handler (GitHub, for example, and I\nimagine Gerrit sites, if they support ssh). So we'd still need some\nfallback, but it would work out-of-the-box in a reasonable number of\ncases (and it is really not that different than the http case, which is\njust stuffing the values into $QUERY_STRING anyway :) ).\n\n-Peff\n"},{"id":"236955","messageId":"20140318041855.GB7252@sigill.intra.peff.net","threadId":"32780","inReplyTo":"CACsJy8A1=U2=TGoKyo5mo1fLW+hBR1psn1J6S0=391fei2JULw@mail.gmail.com","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-18T04:18:55Z","receivedAt":"2014-03-18T04:18:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 15, 2014 at 08:23:08AM +0700, Duy Nguyen wrote:\n\n> > The default would start at false, and people who know their server is\n> > very up-to-date can turn it on. And then when many server\n> > implementations support it, flip the default to auto. And either leave\n> > it there forever, or eventually just set it to \"true\" and drop \"auto\"\n> > entirely as a code cleanup.\n> \n> I would add that upload-pack also advertises about the availability of\n> upload-pack2 and the client may set the remote.*.useUploadPack2 to\n> either yes or auto so next time upload-pack2 will be used.\n\nGood idea. If our auto probe is \"try 1, learn to upgrade to 2 for next\ntime\", we do not have to be so conservative about flipping it on (as\ncompared to my \"try 2, fall back to 1\").\n\n-Peff\n"},{"id":"236985","messageId":"20140318142719.GA9393@lanh","threadId":"32780","inReplyTo":"20140318041739.GA7252@sigill.intra.peff.net","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-18T14:27:19Z","receivedAt":"2014-03-18T14:27:19Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Mar 18, 2014 at 12:17:39AM -0400, Jeff King wrote:\n> On Fri, Mar 14, 2014 at 05:09:45PM -0700, Shawn Pearce wrote:\n> \n> > On Fri, Mar 14, 2014 at 4:30 PM, Duy Nguyen wrote:\n> > > On Fri, Mar 14, 2014 at 11:45 PM, Shawn Pearce wrote:\n> > >>\n> > >> You missed the SSH case. It doesn't have this slot to hide the data into.\n> > >\n> > > Right now we run this for ssh case: \"ssh git-upload-pack\n> > > \". New client can do this instead\n> > >\n> > > ssh git-upload-pack \n> > \n> > Older servers will fail on this command, and the client must reconnect\n> > over SSH, which may mean supplying their password/passphrase again.\n> > But its remembered that the uploadPack2 didn't work so this can be\n> > blacklisted and not retried for a while.\n> \n> I wonder if we could use the environment for optional values. E.g., can\n> we run:\n> \n> ssh host GIT_CAPABILITIES=... git-upload-pack \n> \n> That will not work everywhere, of course. Sites with git-shell will\n> fail, as will sites with custom ssh handler (GitHub, for example, and I\n> imagine Gerrit sites, if they support ssh). So we'd still need some\n> fallback, but it would work out-of-the-box in a reasonable number of\n> cases (and it is really not that different than the http case, which is\n> just stuffing the values into $QUERY_STRING anyway :) ).\n\nAggressively gc'ing linux-2.6 takes forever (and it's being timed so I\ncan't really do any heavy lifting), so I outlined what the new\nprotocol would be instead.\n\nNote that at least for upload-pack client capabilities can be\nadvertised twice: the first time at transport connection level, the\nsecond time in the first \"want\", like in v1. I think this will keep\nthe code change down when we have to support both protocols. Moving\nall capabilities to the first negotiation may touch many places, but\nthat's for now a baseless guess.\n\nThe new capability negotiation is also added for push. We didn't pay\nmuch attention to it so far.\n\nI thought about \"GIT_CAPABILITIES= git-upload-pack ...\" (and actually\nadded it in pack-protocol.txt then deleted). The thing is, if you want\nto new upload-pack, you would need new git-upload-pack at the remote\nend that must understand \"git-upload-pack \"\nalready. Making it aware about GIT_CAPABILITIES is extra cost for\nnothing. And we have to update git-shell to support it eventually.\n\nWell, the \"must understand\" part is not entirely true. If you make\ngit-daemon pass the early capabilities via GIT_CAPABILITIES too,\nupload-pack does not have to support \" \" syntax. The\nupside is if old git-upload-pack ignores this GIT_CAPABILITIES, it'll\nbreak the protocol (see below) and can print friendly error\nmessages. git-daemon has no way of printing friendly messages because\nit can't negotiate side-band.\n\nI'm still not sure. But we should support either way, not both. Anyway\nthe text for new protocols:\n\n-- 8< --\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 79e5768..c329eb1 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2084,6 +2084,16 @@ remote.pushdefault::\n \t`branch..remote` for all branches, and is overridden by\n \t`branch..pushremote` for specific branches.\n \n+remote.useUploadPack2::\n+\tSet to \"always\" to use only upload-pack version 2, \"never\" to\n+\talways use the original \"upload-pack\", \"auto\" to use the\n+\toriginal protocol, but if the remote claims it support version\n+\t2, then set \"remote..useUploadPack2\" to\n+\t\"always\". Default to \"auto\".\n+\n+remote..useUploadPack2::\n+\tOverride remote.useUploadPack2 per remote.\n+\n remote..url::\n \tThe URL of a remote repository. See linkgit:git-fetch[1] or\n \tlinkgit:git-push[1].\ndiff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt\nindex 39c6410..3db4219 100644\n--- a/Documentation/technical/pack-protocol.txt\n+++ b/Documentation/technical/pack-protocol.txt\n@@ -40,15 +40,22 @@ hostname parameter, terminated by a NUL byte.\n \n 0032git-upload-pack /project.git\\0host=myserver.com\\0\n \n+Some service may accept an extra argument (e.g. upload-pack version\n+2). The extra argument must follow \"host\".\n+\n+ 0042git-upload-pack /project.git\\0host=myserver.com\\0flags=someflags\\0\n+\n --\n- git-proto-request = request-command SP pathname NUL [ host-parameter NUL ]\n+ git-proto-request = request-command SP pathname NUL\n+\t\t [ host-parameter NUL [ flags-parameter NUL ] ]\n request-command = \"git-upload-pack\" / \"git-receive-pack\" /\n \t\t \"git-upload-archive\" ; case sensitive\n pathname = *( %x01-ff ) ; exclude NUL\n host-parameter = \"host=\" hostname [ \":\" port ]\n+ flags-parameter = \"flags=\" *( %x01-ff ) ; exclude NULL\n --\n \n-Only host-parameter is allowed in the git-proto-request. Clients\n+No other parameters are allowed in the git-proto-request. Clients\n MUST NOT attempt to send additional parameters. It is used for the\n git-daemon name based virtual hosting. See --interpolated-path\n option to git daemon, with the %H/%CH format characters.\n@@ -77,6 +84,11 @@ It is basically equivalent to running this:\n \n $ ssh git.example.com \"git-upload-pack '/project.git'\"\n \n+Some service may accept an extra argument (e.g. upload-pack version\n+2). The extra argument is appended, e.g.\n+\n+ $ ssh git.example.com \"git-upload-pack '/project.git' 'extra-flags'\"\n+\n For a server to support Git pushing and pulling for a given user over\n SSH, that user needs to be able to execute one or both of those\n commands via the SSH shell that they are provided on login. On some\n@@ -124,6 +136,20 @@ has, the first can 'fetch' from the second. This operation determines\n what data the server has that the client does not then streams that\n data down to the client in packfile format.\n \n+Initial capability negotiation\n+------------------------------\n+\n+When the client connects to the server with the extra argument,\n+upload-pack version 2 is used. Otherwise the original version is\n+used. Unless explicitly stated, the original version is implied.\n+\n+When the client initially connects to the server using upload-pack\n+version 2, the server MUST reply with one pkt-line describing its\n+capabilities. Capabilities that are recognized by both ends are\n+immediately effective.\n+\n+By default, upload-pack version 1's reference discovery will follow\n+unless some capability makes it different.\n \n Reference Discovery\n -------------------\n@@ -447,9 +473,14 @@ Reference Discovery\n \n The reference discovery phase is done nearly the same way as it is in the\n fetching protocol. Each reference obj-id and name on the server is sent\n-in packet-line format to the client, followed by a flush-pkt. The only\n-real difference is that the capability listing is different - the only\n-possible values are 'report-status', 'delete-refs' and 'ofs-delta'.\n+in packet-line format to the client, followed by a flush-pkt. Or with\n+receive-pack version 2, a separate pkt-line containing capabilities is\n+sent back, then followed by reference discovery unless some capability\n+changes it.\n+\n+The only real difference is that the capability listing is different -\n+the only possible values are 'report-status', 'delete-refs' and\n+'ofs-delta'.\n \n Reference Update Request and Packfile Transfer\n ----------------------------------------------\ndiff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt\nindex e174343..a165286 100644\n--- a/Documentation/technical/protocol-capabilities.txt\n+++ b/Documentation/technical/protocol-capabilities.txt\n@@ -250,3 +250,8 @@ allow-tip-sha1-in-want\n If the upload-pack server advertises this capability, fetch-pack may\n send \"want\" lines with SHA-1s that exist at the server but are not\n advertised by upload-pack.\n+\n+uploadpack2\n+-----------\n+\n+upload-pack version 2 is supported.\n-- 8< --\n"},{"id":"236988","messageId":"CACsJy8C4P6Wy=5_nOeB_RSyGpYKpybqaDUUyXUmfF67+-7UFzg@mail.gmail.com","threadId":"32780","inReplyTo":"20140318142719.GA9393@lanh","subject":"Re: [PATCH v3 0/8] Hiding refs","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-18T14:36:01Z","receivedAt":"2014-03-18T14:36:01Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Mar 18, 2014 at 9:27 PM, Duy Nguyen wrote:\n> I thought about \"GIT_CAPABILITIES= git-upload-pack ...\" (and actually\n> added it in pack-protocol.txt then deleted). The thing is, if you want\n> to new upload-pack, you would need new git-upload-pack at the remote\n> end that must understand \"git-upload-pack \"\n> already. Making it aware about GIT_CAPABILITIES is extra cost for\n> nothing. And we have to update git-shell to support it eventually.\n>\n> Well, the \"must understand\" part is not entirely true. If you make\n> git-daemon pass the early capabilities via GIT_CAPABILITIES too,\n> upload-pack does not have to support \" \" syntax. The\n> upside is if old git-upload-pack ignores this GIT_CAPABILITIES, it'll\n> break the protocol (see below) and can print friendly error\n> messages. git-daemon has no way of printing friendly messages because\n> it can't negotiate side-band.\n\nI should have read my mail one more time before sending. The\n\"git-upload-pack ignores...\" sentence is wrong. If it's old, its\nbehavior is fixed and it cannot not send or do anything new.\n\nBut on the other hand, this is good. The new protocol expects\nupload-pack to send its caps in a new pkt-line. The old upload-pack\ndoes not follow this, which should be the indicator for the client\nthat this server does not support v2, so it could fall back to v1\ngracefully. git:// still fails hard because git-daemon is likely old\ntoo and rejects it from the beginning. But ssh:// (without git-shell)\nshould work, http:// too. This is a very good point for\nGIT_CAPABILITIES.\n-- \nDuy\n"}]}