{"thread":{"id":"29143","subject":"[PATCH 0/4] exact ref-matching for fetch-pack","startedAt":"2011-12-13T00:39:25Z","lastAt":"2011-12-15T21:46:22Z","messageCount":8,"participants":["Jeff King","Michael Haggerty","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"181005","messageId":"20111213003925.GA28403@sigill.intra.peff.net","threadId":"29143","inReplyTo":null,"subject":"[PATCH 0/4] exact ref-matching for fetch-pack","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-13T00:39:25Z","receivedAt":"2011-12-13T00:39:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"I was passed a bug report of clone failing on a repo with an oddly named\nref in it: \"refs/for/refs/heads/master\". This is probably an error\nsomebody made while pushing and should simply be deleted. However, it's\ncurious that \"clone\" would fail on this, since it should be ignoring\neverything outside the \"refs/heads/*\" refspec.\n\nIt turns out that during the fetch-pack phase, we accidentally ask for\nthe sha1 of \"refs/for/refs/heads/master\" instead of that of\n\"refs/heads/master\". This can cause \"clone\" to fail, as we may not have\nasked for the object pointed to by \"refs/heads/master\" at all. This\nbehavior is due to some questionable suffix-matching deep inside\nfetch-pack. The code in question dates back to the beginning of git; I\nthink it simply hasn't triggered much because the refname you need to\nhave is so specific (you must be asking to fetch a ref \"refs/X\", have\n\"refs/Y/X\" on the remote, and \"Y\" must come alphabetically before \"X\",\nsince we match refs in alphabetical order).\n\nAs I said, this is probably just a silly one-off error. But I could\nimagine this happening legitimately. Here are two possible scenarios:\n\n  1. You are mirroring some meta-information about your refs in a\n     parallel namespace. E.g., \"refs/descriptions/refs/heads/master\"\n     contains information about \"refs/heads/master\".\n\n  2. One of the things we've discussed for future git is mirroring the\n     remote ref namespaces more literally. E.g., having something like\n     \"refs/remotes/origin/refs/heads/master\". That won't actually\n     trigger this bug because \"heads\" is alphabetically before\n     \"remotes\". But \"tags\", for example, is not.\n\nThis could possibly be considered a behavior regression for users of the\nfetch-pack command line. See patches 2 and 3 for details.\n\n  [1/4]: drop \"match\" parameter from get_remote_heads\n  [2/4]: t5500: give fully-qualified refs to fetch-pack\n  [3/4]: fetch-pack: match refs exactly\n  [4/4]: connect.c: drop path_match function\n\n-Peff\n"},{"id":"181006","messageId":"20111213004137.GA3699@sigill.intra.peff.net","threadId":"29143","inReplyTo":"20111213003925.GA28403@sigill.intra.peff.net","subject":"[PATCH 1/4] drop \"match\" parameter from get_remote_heads","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-13T00:41:37Z","receivedAt":"2011-12-13T00:41:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The get_remote_heads function reads the list of remote refs\nduring git protocol session. It dates all the way back to\ndef88e9 (Commit first cut at \"git-fetch-pack\", 2005-07-04).\nAt that time, the idea was to come up with a list of refs we\nwere interested in, and then filter the list as we got it\nfrom the remote side.\n\nLater, 1baaae5 (Make maximal use of the remote refs,\n2005-10-28) stopped filtering at the get_remote_heads layer,\nletting us use the non-matching refs to find common history.\n\nAs a result, all callers now simply pass an empty match\nlist (and any future callers will want to do the same). So\nlet's drop these now-useless parameters.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis one isn't necessary for the bugfix, but since it is the only other\ncaller of path_match besides fetch-pack, it gives us freedom to modify\nor get rid of path_match later.\n\n builtin/fetch-pack.c |    2 +-\n builtin/send-pack.c  |    3 +--\n cache.h              |    2 +-\n connect.c            |    3 ---\n remote-curl.c        |    2 +-\n transport.c          |    7 +++----\n 6 files changed, 7 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex c6bc8eb..46688dc 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -976,7 +976,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\t   args.verbose ? CONNECT_VERBOSE : 0);\n \t}\n \n-\tget_remote_heads(fd[0], &ref, 0, NULL, 0, NULL);\n+\tget_remote_heads(fd[0], &ref, 0, NULL);\n \n \tref = fetch_pack(&args, fd, conn, ref, dest,\n \t\tnr_heads, heads, pack_lockfile_ptr);\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex e0b8030..cd1115f 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -494,8 +494,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \n \tmemset(&extra_have, 0, sizeof(extra_have));\n \n-\tget_remote_heads(fd[0], &remote_refs, 0, NULL, REF_NORMAL,\n-\t\t\t &extra_have);\n+\tget_remote_heads(fd[0], &remote_refs, REF_NORMAL, &extra_have);\n \n \ttransport_verify_remote_names(nr_refspecs, refspecs);\n \ndiff --git a/cache.h b/cache.h\nindex 8c98d05..408e880 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1034,7 +1034,7 @@ struct extra_have_objects {\n \tint nr, alloc;\n \tunsigned char (*array)[20];\n };\n-extern struct ref **get_remote_heads(int in, struct ref **list, int nr_match, char **match, unsigned int flags, struct extra_have_objects *);\n+extern struct ref **get_remote_heads(int in, struct ref **list, unsigned int flags, struct extra_have_objects *);\n extern int server_supports(const char *feature);\n \n extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path);\ndiff --git a/connect.c b/connect.c\nindex 51990fa..48df90b 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -53,7 +53,6 @@ static void add_extra_have(struct extra_have_objects *extra, unsigned char *sha1\n  * Read all the refs from the other end\n  */\n struct ref **get_remote_heads(int in, struct ref **list,\n-\t\t\t      int nr_match, char **match,\n \t\t\t      unsigned int flags,\n \t\t\t      struct extra_have_objects *extra_have)\n {\n@@ -92,8 +91,6 @@ struct ref **get_remote_heads(int in, struct ref **list,\n \n \t\tif (!check_ref(name, name_len, flags))\n \t\t\tcontinue;\n-\t\tif (nr_match && !path_match(name, nr_match, match))\n-\t\t\tcontinue;\n \t\tref = alloc_ref(buffer + 41);\n \t\thashcpy(ref->old_sha1, old_sha1);\n \t\t*list = ref;\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 0e720ee..94dc488 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -200,7 +200,7 @@ static struct ref *parse_git_refs(struct discovery *heads)\n \n \tif (start_async(&async))\n \t\tdie(\"cannot start thread to parse advertised refs\");\n-\tget_remote_heads(async.out, &list, 0, NULL, 0, NULL);\n+\tget_remote_heads(async.out, &list, 0, NULL);\n \tclose(async.out);\n \tif (finish_async(&async))\n \t\tdie(\"ref parsing thread failed\");\ndiff --git a/transport.c b/transport.c\nindex 51814b5..c2245d4 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -502,7 +502,7 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus\n \tstruct ref *refs;\n \n \tconnect_setup(transport, for_push, 0);\n-\tget_remote_heads(data->fd[0], &refs, 0, NULL,\n+\tget_remote_heads(data->fd[0], &refs,\n \t\t\t for_push ? REF_NORMAL : 0, &data->extra_have);\n \tdata->got_remote_heads = 1;\n \n@@ -537,7 +537,7 @@ static int fetch_refs_via_pack(struct transport *transport,\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, 0, NULL);\n+\t\tget_remote_heads(data->fd[0], &refs_tmp, 0, NULL);\n \t\tdata->got_remote_heads = 1;\n \t}\n \n@@ -772,8 +772,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re\n \t\tstruct ref *tmp_refs;\n \t\tconnect_setup(transport, 1, 0);\n \n-\t\tget_remote_heads(data->fd[0], &tmp_refs, 0, NULL, REF_NORMAL,\n-\t\t\t\t NULL);\n+\t\tget_remote_heads(data->fd[0], &tmp_refs, REF_NORMAL, NULL);\n \t\tdata->got_remote_heads = 1;\n \t}\n \n-- \n1.7.8.13.g74677\n"},{"id":"181007","messageId":"20111213004440.GB3699@sigill.intra.peff.net","threadId":"29143","inReplyTo":"20111213003925.GA28403@sigill.intra.peff.net","subject":"[PATCH 2/4] t5500: give fully-qualified refs to fetch-pack","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-13T00:44:40Z","receivedAt":"2011-12-13T00:44:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The fetch-pack documentation is very clear that refs given\non the command line are to be full refs:\n\n  <refs>...::\n          The remote heads to update from. This is relative to\n          $GIT_DIR (e.g. \"HEAD\", \"refs/heads/master\").  When\n          unspecified, update from all heads the remote side has.\n\nand this has been the case since fetch-pack was originally documented in\n8b3d9dc ([PATCH] Documentation: clone/fetch/upload., 2005-07-14).\n\nLet's follow our own documentation to set a good example,\nand to avoid breaking when this restriction is enforced in\nthe next patch.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis patch by itself isn't a big deal for backwards compatibility But\nif we are violating the constraints in the documentation, then I worry\nthat others are, too. On the other hand, I seriously doubt anyone is\nactually using fetch-pack directly anymore at all.\n\n t/t5500-fetch-pack.sh |    6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\nindex bafcca7..9bf69e9 100755\n--- a/t/t5500-fetch-pack.sh\n+++ b/t/t5500-fetch-pack.sh\n@@ -97,7 +97,7 @@ test_expect_success 'setup' '\n \tgit symbolic-ref HEAD refs/heads/B\n '\n \n-pull_to_client 1st \"B A\" $((11*3))\n+pull_to_client 1st \"refs/heads/B refs/heads/A\" $((11*3))\n \n test_expect_success 'post 1st pull setup' '\n \tadd A11 $A10 &&\n@@ -110,9 +110,9 @@ test_expect_success 'post 1st pull setup' '\n \tdone\n '\n \n-pull_to_client 2nd \"B\" $((64*3))\n+pull_to_client 2nd \"refs/heads/B\" $((64*3))\n \n-pull_to_client 3rd \"A\" $((1*3))\n+pull_to_client 3rd \"refs/heads/A\" $((1*3))\n \n test_expect_success 'clone shallow' '\n \tgit clone --depth 2 \"file://$(pwd)/.\" shallow\n-- \n1.7.8.13.g74677\n"},{"id":"181010","messageId":"20111213004808.GC3699@sigill.intra.peff.net","threadId":"29143","inReplyTo":"20111213003925.GA28403@sigill.intra.peff.net","subject":"[PATCH 3/4] fetch-pack: match refs exactly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-13T00:48:08Z","receivedAt":"2011-12-13T00:48:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we are determining the list of refs to fetch via\nfetch-pack, we have two sets of refs to compare: those on\nthe remote side, and a \"match\" list of things we want to\nfetch. We iterate through the remote refs alphabetically,\nseeing if each one is wanted by the \"match\" list.\n\nSince def88e9 (Commit first cut at \"git-fetch-pack\",\n2005-07-04), we have used the \"path_match\" function to do a\nsuffix match, where a remote ref is considered wanted if\nany of the \"match\" elements is a suffix of the remote\nrefname.\n\nThis enables callers of fetch-pack to specify unqualified\nrefs and have them matched up with remote refs (e.g., ask\nfor \"A\" and get remote's \"refs/heads/A\"). However, if you\nprovide a fully qualified ref, then there are corner cases\nwhere we provide the wrong answer. For example, given a\nremote with two refs:\n\n   refs/foo/refs/heads/master\n   refs/heads/master\n\nasking for \"refs/heads/master\" will first match\n\"refs/foo/refs/heads/master\" by the suffix rule, and we will\nerroneously fetch it instead of refs/heads/master.\n\nAs it turns out, all callers of fetch_pack do provide\nfully-qualified refs for the match list. There are two ways\nfetch_pack can get match lists:\n\n  1. Through the transport code (i.e., via git-fetch)\n\n  2. On the command-line of git-fetch-pack\n\nIn the first case, we will always be providing the names of\nfully-qualified refs from \"struct ref\" objects. We will have\npre-matched those ref objects already (since we have to\nhandle more advanced matching, like wildcard refspecs), and\nare just providing a list of the refs whose objects we need.\n\nIn the second case, users could in theory be providing\nnon-qualified refs on the command-line. However, the\nfetch-pack documentation claims that refs should be fully\nqualified (and has always done so since it was written in\n2005).\n\nLet's change this path_match call to simply check for string\nequality, matching what the callers of fetch_pack are\nexpecting.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis is obviously the one that can break existing fetch-pack users. I\ndoubt they exist. If they do, there are a few alternatives:\n\n  1. Come up with some more sane rules for path_match (e.g., try full\n     strings first, use full-string matching for things starting with\n     \"refs/\", etc).\n\n  2. Leave the matching in-place for git-fetch-pack, but use exact\n     matching for internal users that will always provide qualified refs\n     (i.e., \"git fetch\").\n\nI don't personally think it's worth the trouble.\n\n builtin/fetch-pack.c      |   13 +++++++++----\n t/t5527-fetch-odd-refs.sh |   29 +++++++++++++++++++++++++++++\n 2 files changed, 38 insertions(+), 4 deletions(-)\n create mode 100755 t/t5527-fetch-odd-refs.sh\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 46688dc..6207ecd 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -556,11 +556,16 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)\n \t\t\tcontinue;\n \t\t}\n \t\telse {\n-\t\t\tint order = path_match(ref->name, nr_match, match);\n-\t\t\tif (order) {\n-\t\t\t\treturn_refs[order-1] = ref;\n-\t\t\t\tcontinue; /* we will link it later */\n+\t\t\tint i;\n+\t\t\tfor (i = 0; i < nr_match; i++) {\n+\t\t\t\tif (!strcmp(ref->name, match[i])) {\n+\t\t\t\t\tmatch[i][0] = '\\0';\n+\t\t\t\t\treturn_refs[i] = ref;\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n \t\t\t}\n+\t\t\tif (i < nr_match)\n+\t\t\t\tcontinue; /* we will link it later */\n \t\t}\n \t\tfree(ref);\n \t}\ndiff --git a/t/t5527-fetch-odd-refs.sh b/t/t5527-fetch-odd-refs.sh\nnew file mode 100755\nindex 0000000..edea9f9\n--- /dev/null\n+++ b/t/t5527-fetch-odd-refs.sh\n@@ -0,0 +1,29 @@\n+#!/bin/sh\n+\n+test_description='test fetching of oddly-named refs'\n+. ./test-lib.sh\n+\n+# afterwards we will have:\n+#  HEAD - two\n+#  refs/for/refs/heads/master - one\n+#  refs/heads/master - three\n+test_expect_success 'setup repo with odd suffix ref' '\n+\techo content >file &&\n+\tgit add . &&\n+\tgit commit -m one &&\n+\tgit update-ref refs/for/refs/heads/master HEAD &&\n+\techo content >>file &&\n+\tgit commit -a -m two &&\n+\techo content >>file &&\n+\tgit commit -a -m three &&\n+\tgit checkout HEAD^\n+'\n+\n+test_expect_success 'suffix ref is ignored during fetch' '\n+\tgit clone --bare file://\"$PWD\" suffix &&\n+\techo three >expect &&\n+\tgit --git-dir=suffix log -1 --format=%s refs/heads/master >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \n1.7.8.13.g74677\n"},{"id":"181011","messageId":"20111213004959.GD3699@sigill.intra.peff.net","threadId":"29143","inReplyTo":"20111213003925.GA28403@sigill.intra.peff.net","subject":"[PATCH 4/4] connect.c: drop path_match function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-13T00:49:59Z","receivedAt":"2011-12-13T00:49:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This function was used for comparing local and remote ref\nnames during fetch (which makes it a candidate for \"most\nconfusingly named function of the year\").\n\nIt no longer has any callers, so let's get rid of it.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI just did the exact-string match inline in the previous patch. I could\nalso have modified path_match to do it. But really, I can't think of a\nworse name for a global function in a system which is all about\nmanaging content in paths. Unless, you know, it actually matched paths.\nWhich it doesn't.\n\n cache.h   |    1 -\n connect.c |   21 ---------------------\n 2 files changed, 0 insertions(+), 22 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 408e880..2ad063f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1029,7 +1029,6 @@ struct ref {\n extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);\n extern int finish_connect(struct child_process *conn);\n extern int git_connection_is_socket(struct child_process *conn);\n-extern int path_match(const char *path, int nr, char **match);\n struct extra_have_objects {\n \tint nr, alloc;\n \tunsigned char (*array)[20];\ndiff --git a/connect.c b/connect.c\nindex 48df90b..2a0a040 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -105,27 +105,6 @@ int server_supports(const char *feature)\n \t\tstrstr(server_capabilities, feature) != NULL;\n }\n \n-int path_match(const char *path, int nr, char **match)\n-{\n-\tint i;\n-\tint pathlen = strlen(path);\n-\n-\tfor (i = 0; i < nr; i++) {\n-\t\tchar *s = match[i];\n-\t\tint len = strlen(s);\n-\n-\t\tif (!len || len > pathlen)\n-\t\t\tcontinue;\n-\t\tif (memcmp(path + pathlen - len, s, len))\n-\t\t\tcontinue;\n-\t\tif (pathlen > len && path[pathlen - len - 1] != '/')\n-\t\t\tcontinue;\n-\t\t*s = 0;\n-\t\treturn (i + 1);\n-\t}\n-\treturn 0;\n-}\n-\n enum protocol {\n \tPROTO_LOCAL = 1,\n \tPROTO_SSH,\n-- \n1.7.8.13.g74677\n"},{"id":"181012","messageId":"20111213005429.GA3812@sigill.intra.peff.net","threadId":"29143","inReplyTo":"20111213004808.GC3699@sigill.intra.peff.net","subject":"Re: [PATCH 3/4] fetch-pack: match refs exactly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-13T00:54:29Z","receivedAt":"2011-12-13T00:54:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 12, 2011 at 07:48:08PM -0500, Jeff King wrote:\n\n> diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\n> index 46688dc..6207ecd 100644\n> --- a/builtin/fetch-pack.c\n> +++ b/builtin/fetch-pack.c\n> @@ -556,11 +556,16 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)\n>  \t\t\tcontinue;\n>  \t\t}\n>  \t\telse {\n> -\t\t\tint order = path_match(ref->name, nr_match, match);\n> -\t\t\tif (order) {\n> -\t\t\t\treturn_refs[order-1] = ref;\n> -\t\t\t\tcontinue; /* we will link it later */\n> +\t\t\tint i;\n> +\t\t\tfor (i = 0; i < nr_match; i++) {\n> +\t\t\t\tif (!strcmp(ref->name, match[i])) {\n> +\t\t\t\t\tmatch[i][0] = '\\0';\n> +\t\t\t\t\treturn_refs[i] = ref;\n> +\t\t\t\t\tbreak;\n> +\t\t\t\t}\n>  \t\t\t}\n> +\t\t\tif (i < nr_match)\n> +\t\t\t\tcontinue; /* we will link it later */\n\nAstute readers may notice that our matching scheme is now something\nlike:\n\n  for ref in refs\n    for match in matches\n      strcmp(ref, match)\n\nIf you are fetching everything, that's O(n^2) strcmps (where n is the\nnumber of remote refs). If we sorted the match list (the refs list is\nalready sorted), we could turn it into an O(n lg n) sort+merge.  This is\nan optimization we couldn't do with the old suffix-matching rule.\n\nI doubt it matters in any practical case. Even for our crazy 100K ref\ncases at GitHub, we don't tend to actually fetch all of those at one\ntime. So it is more like O(n * m), where m is probably in the dozens at\nmost.\n\n-Peff\n"},{"id":"181017","messageId":"4EE6C532.9060306@alum.mit.edu","threadId":"29143","inReplyTo":"20111213004959.GD3699@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] connect.c: drop path_match function","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-12-13T03:23:30Z","receivedAt":"2011-12-13T03:23:30Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 12/13/2011 01:49 AM, Jeff King wrote:\n> I just did the exact-string match inline in the previous patch. I could\n> also have modified path_match to do it. But really, I can't think of a\n> worse name for a global function in a system which is all about\n> managing content in paths. Unless, you know, it actually matched paths.\n> Which it doesn't.\n\nThe tacit assumption that a reference is always a file is all over the\ncode and is often confusing.  My\n\n    [PATCH v2 02/51] refs: rename \"refname\" variables\n\nis a step towards making the distinction more explicit, at least in the\nrefs code.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"181260","messageId":"7vaa6tmrtd.fsf@alter.siamese.dyndns.org","threadId":"29143","inReplyTo":"20111213004808.GC3699@sigill.intra.peff.net","subject":"Re: [PATCH 3/4] fetch-pack: match refs exactly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-15T21:46:22Z","receivedAt":"2011-12-15T21:46:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> This is obviously the one that can break existing fetch-pack users. I\n> doubt they exist. If they do, there are a few alternatives:\n>\n>   1. Come up with some more sane rules for path_match (e.g., try full\n>      strings first, use full-string matching for things starting with\n>      \"refs/\", etc).\n>\n>   2. Leave the matching in-place for git-fetch-pack, but use exact\n>      matching for internal users that will always provide qualified refs\n>      (i.e., \"git fetch\").\n\nI think the latter is the sane thing to do _if_ this becomes an issue, and\nas you mention, it is in line with what \"git fetch\" wrapper already\ndoes.\n\nGiven that fetch-pack was meant to be driven by \"git fetch\" wrapper that\nturns the command line and other refspecs into full refnames on the remote\nend before calling it, and also as you mentionied that it is clearly\ndocumented as \"relative to $GIT_DIR\", I do not think we should support the\ntail-match semantics at all in the first place.\n"}]}