{"thread":{"id":"8308","subject":"[PATCH] Move refspec pattern matching to match_refs().","startedAt":"2007-05-25T05:20:56Z","lastAt":"2007-05-26T08:20:49Z","messageCount":2,"participants":["Daniel Barkalow","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"43217","messageId":"Pine.LNX.4.64.0705250111200.9778@iabervon.org","threadId":"8308","inReplyTo":null,"subject":"[PATCH] Move refspec pattern matching to match_refs().","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-05-25T05:20:56Z","receivedAt":"2007-05-25T05:20:56Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"This means that send-pack and http-push will support pattern refspecs,\nso builtin-push.c doesn't have to expand them, and also git push can\njust turn --tags into \"refs/tags/*\", further simplifying\nbuiltin-push.c\n\ncheck_ref_format() gets a third \"conditionally okay\" result for\nsomething that's valid as a pattern but not as a particular ref.\n\nSigned-off-by: Daniel Barkalow <barkalow@iabervon.org>\n---\nOn top of my \"remote\" series. Shouldn't change any significant behavior, \nand simplifies a lot of logic. This version takes into account the \ncomments from the previous round (assuming that the ruling on coding style \nis that:\n\nif (condition)\n\t/* Comment */\n\tstatement;\n\nshouldn't have braces).\n\n builtin-push.c |  133 +++++++++----------------------------------------------\n refs.c         |   27 ++++++++---\n remote.c       |   31 ++++++++++++-\n send-pack.c    |    1 +\n 4 files changed, 70 insertions(+), 122 deletions(-)\n\ndiff --git a/builtin-push.c b/builtin-push.c\nindex 6084899..2612f07 100644\n--- a/builtin-push.c\n+++ b/builtin-push.c\n@@ -9,7 +9,7 @@\n \n static const char push_usage[] = \"git-push [--all] [--tags] [--receive-pack=<git-receive-pack>] [--repo=all] [-f | --force] [-v] [<repository> <refspec>...]\";\n \n-static int all, tags, force, thin = 1, verbose;\n+static int all, force, thin = 1, verbose;\n static const char *receivepack;\n \n static const char **refspec;\n@@ -23,114 +23,24 @@ static void add_refspec(const char *ref)\n \trefspec_nr = nr;\n }\n \n-static int expand_one_ref(const char *ref, const unsigned char *sha1, int flag, void *cb_data)\n-{\n-\t/* Ignore the \"refs/\" at the beginning of the refname */\n-\tref += 5;\n-\n-\tif (!prefixcmp(ref, \"tags/\"))\n-\t\tadd_refspec(xstrdup(ref));\n-\treturn 0;\n-}\n-\n-static void expand_refspecs(void)\n-{\n-\tif (all) {\n-\t\tif (refspec_nr)\n-\t\t\tdie(\"cannot mix '--all' and a refspec\");\n-\n-\t\t/*\n-\t\t * No need to expand \"--all\" - we'll just use\n-\t\t * the \"--all\" flag to send-pack\n-\t\t */\n-\t\treturn;\n-\t}\n-\tif (!tags)\n-\t\treturn;\n-\tfor_each_ref(expand_one_ref, NULL);\n-}\n-\n-struct wildcard_cb {\n-\tconst char *from_prefix;\n-\tint from_prefix_len;\n-\tconst char *to_prefix;\n-\tint to_prefix_len;\n-\tint force;\n-};\n-\n-static int expand_wildcard_ref(const char *ref, const unsigned char *sha1, int flag, void *cb_data)\n-{\n-\tstruct wildcard_cb *cb = cb_data;\n-\tint len = strlen(ref);\n-\tchar *expanded, *newref;\n-\n-\tif (len < cb->from_prefix_len ||\n-\t    memcmp(cb->from_prefix, ref, cb->from_prefix_len))\n-\t\treturn 0;\n-\texpanded = xmalloc(len * 2 + cb->force +\n-\t\t\t   (cb->to_prefix_len - cb->from_prefix_len) + 2);\n-\tnewref = expanded + cb->force;\n-\tif (cb->force)\n-\t\texpanded[0] = '+';\n-\tmemcpy(newref, ref, len);\n-\tnewref[len] = ':';\n-\tmemcpy(newref + len + 1, cb->to_prefix, cb->to_prefix_len);\n-\tstrcpy(newref + len + 1 + cb->to_prefix_len,\n-\t       ref + cb->from_prefix_len);\n-\tadd_refspec(expanded);\n-\treturn 0;\n-}\n-\n-static int wildcard_ref(const char *ref)\n-{\n-\tint len;\n-\tconst char *colon;\n-\tstruct wildcard_cb cb;\n-\n-\tmemset(&cb, 0, sizeof(cb));\n-\tif (ref[0] == '+') {\n-\t\tcb.force = 1;\n-\t\tref++;\n-\t}\n-\tlen = strlen(ref);\n-\tcolon = strchr(ref, ':');\n-\tif (! (colon && ref < colon &&\n-\t       colon[-2] == '/' && colon[-1] == '*' &&\n-\t       /* \"<mine>/<asterisk>:<yours>/<asterisk>\" is at least 7 bytes */\n-\t       7 <= len &&\n-\t       ref[len-2] == '/' && ref[len-1] == '*') )\n-\t\treturn 0 ;\n-\tcb.from_prefix = ref;\n-\tcb.from_prefix_len = colon - ref - 1;\n-\tcb.to_prefix = colon + 1;\n-\tcb.to_prefix_len = len - (colon - ref) - 2;\n-\tfor_each_ref(expand_wildcard_ref, &cb);\n-\treturn 1;\n-}\n-\n static void set_refspecs(const char **refs, int nr)\n {\n-\tif (nr) {\n-\t\tint i;\n-\t\tfor (i = 0; i < nr; i++) {\n-\t\t\tconst char *ref = refs[i];\n-\t\t\tif (!strcmp(\"tag\", ref)) {\n-\t\t\t\tchar *tag;\n-\t\t\t\tint len;\n-\t\t\t\tif (nr <= ++i)\n-\t\t\t\t\tdie(\"tag shorthand without <tag>\");\n-\t\t\t\tlen = strlen(refs[i]) + 11;\n-\t\t\t\ttag = xmalloc(len);\n-\t\t\t\tstrcpy(tag, \"refs/tags/\");\n-\t\t\t\tstrcat(tag, refs[i]);\n-\t\t\t\tref = tag;\n-\t\t\t}\n-\t\t\telse if (wildcard_ref(ref))\n-\t\t\t\tcontinue;\n-\t\t\tadd_refspec(ref);\n+\tint i;\n+\tfor (i = 0; i < nr; i++) {\n+\t\tconst char *ref = refs[i];\n+\t\tif (!strcmp(\"tag\", ref)) {\n+\t\t\tchar *tag;\n+\t\t\tint len;\n+\t\t\tif (nr <= ++i)\n+\t\t\t\tdie(\"tag shorthand without <tag>\");\n+\t\t\tlen = strlen(refs[i]) + 11;\n+\t\t\ttag = xmalloc(len);\n+\t\t\tstrcpy(tag, \"refs/tags/\");\n+\t\t\tstrcat(tag, refs[i]);\n+\t\t\tref = tag;\n \t\t}\n+\t\tadd_refspec(ref);\n \t}\n-\texpand_refspecs();\n }\n \n static int do_push(const char *repo)\n@@ -149,11 +59,9 @@ static int do_push(const char *repo)\n \t\tsprintf(rp, \"--receive-pack=%s\", remote->receivepack);\n \t\treceivepack = rp;\n \t}\n-\tif (!refspec && !all && !tags && remote->push_refspec_nr) {\n-\t\tfor (i = 0; i < remote->push_refspec_nr; i++) {\n-\t\t\tif (!wildcard_ref(remote->push_refspec[i]))\n-\t\t\t\tadd_refspec(remote->push_refspec[i]);\n-\t\t}\n+\tif (!refspec && !all && remote->push_refspec_nr) {\n+\t\trefspec = remote->push_refspec;\n+\t\trefspec_nr = remote->push_refspec_nr;\n \t}\n \n \targv = xmalloc((refspec_nr + 10) * sizeof(char *));\n@@ -240,7 +148,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(arg, \"--tags\")) {\n-\t\t\ttags = 1;\n+\t\t\tadd_refspec(\"refs/tags/*\");\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(arg, \"--force\") || !strcmp(arg, \"-f\")) {\n@@ -266,5 +174,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\tusage(push_usage);\n \t}\n \tset_refspecs(argv + i, argc - i);\n+\tif (all && refspec)\n+\t\tusage(push_usage);\n+\n \treturn do_push(repo);\n }\ndiff --git a/refs.c b/refs.c\nindex 2ae3235..ef4484d 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -603,15 +603,20 @@ int get_ref_sha1(const char *ref, unsigned char *sha1)\n \n static inline int bad_ref_char(int ch)\n {\n-\treturn (((unsigned) ch) <= ' ' ||\n-\t\tch == '~' || ch == '^' || ch == ':' ||\n-\t\t/* 2.13 Pattern Matching Notation */\n-\t\tch == '?' || ch == '*' || ch == '[');\n+\tif (((unsigned) ch) <= ' ' ||\n+\t    ch == '~' || ch == '^' || ch == ':')\n+\t\treturn 1;\n+\t/* 2.13 Pattern Matching Notation */\n+\tif (ch == '?' || ch == '[') /* Unsupported */\n+\t\treturn 1;\n+\tif (ch == '*') /* Supported at the end */\n+\t\treturn 2;\n+\treturn 0;\n }\n \n int check_ref_format(const char *ref)\n {\n-\tint ch, level;\n+\tint ch, level, bad_type;\n \tconst char *cp = ref;\n \n \tlevel = 0;\n@@ -622,13 +627,19 @@ int check_ref_format(const char *ref)\n \t\t\treturn -1; /* should not end with slashes */\n \n \t\t/* we are at the beginning of the path component */\n-\t\tif (ch == '.' || bad_ref_char(ch))\n+\t\tif (ch == '.')\n \t\t\treturn -1;\n+\t\tbad_type = bad_ref_char(ch);\n+\t\tif (bad_type) {\n+\t\t\treturn (bad_type == 2 && !*cp) ? -3 : -1;\n+\t\t}\n \n \t\t/* scan the rest of the path component */\n \t\twhile ((ch = *cp++) != 0) {\n-\t\t\tif (bad_ref_char(ch))\n-\t\t\t\treturn -1;\n+\t\t\tbad_type = bad_ref_char(ch);\n+\t\t\tif (bad_type) {\n+\t\t\t\treturn (bad_type == 2 && !*cp) ? -3 : -1;\n+\t\t\t}\n \t\t\tif (ch == '/')\n \t\t\t\tbreak;\n \t\t\tif (ch == '.' && *cp == '.')\ndiff --git a/remote.c b/remote.c\nindex 46fe8d9..d904616 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -415,6 +415,10 @@ static int match_explicit_refs(struct ref *src, struct ref *dst,\n \t\tstruct ref *matched_src, *matched_dst;\n \n \t\tconst char *dst_value = rs[i].dst;\n+\n+\t\tif (rs[i].pattern)\n+\t\t\tcontinue;\n+\n \t\tif (dst_value == NULL)\n \t\t\tdst_value = rs[i].src;\n \n@@ -497,22 +501,43 @@ static struct ref *find_ref_by_name(struct ref *list, const char *name)\n \treturn NULL;\n }\n \n+static int check_pattern_match(struct refspec *rs, int rs_nr, struct ref *src)\n+{\n+\tint i;\n+\tif (!rs_nr)\n+\t\treturn 1;\n+\tfor (i = 0; i < rs_nr; i++) {\n+\t\tif (rs[i].pattern && !prefixcmp(src->name, rs[i].src))\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,\n \t       int nr_refspec, char **refspec, int all)\n {\n \tstruct refspec *rs =\n \t\tparse_ref_spec(nr_refspec, (const char **) refspec);\n \n-\tif (nr_refspec)\n-\t\treturn match_explicit_refs(src, dst, dst_tail, rs, nr_refspec);\n+\tif (match_explicit_refs(src, dst, dst_tail, rs, nr_refspec))\n+\t\treturn -1;\n \n \t/* pick the remainder */\n \tfor ( ; src; src = src->next) {\n \t\tstruct ref *dst_peer;\n \t\tif (src->peer_ref)\n \t\t\tcontinue;\n+\t\tif (!check_pattern_match(rs, nr_refspec, src))\n+\t\t\tcontinue;\n+\n \t\tdst_peer = find_ref_by_name(dst, src->name);\n-\t\tif ((dst_peer && dst_peer->peer_ref) || (!dst_peer && !all))\n+\t\tif (dst_peer && dst_peer->peer_ref)\n+\t\t\t/* We're already sending something to this ref. */\n+\t\t\tcontinue;\n+\t\tif (!dst_peer && !nr_refspec && !all)\n+\t\t\t/* Remote doesn't have it, and we have no\n+\t\t\t * explicit pattern, and we don't have\n+\t\t\t * --all. */\n \t\t\tcontinue;\n \t\tif (!dst_peer) {\n \t\t\t/* Create a new one and link it */\ndiff --git a/send-pack.c b/send-pack.c\nindex 59352c8..697dbbc 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -354,6 +354,7 @@ static void verify_remote_names(int nr_heads, char **heads)\n \t\tcase -2: /* ok but a single level -- that is fine for\n \t\t\t  * a match pattern.\n \t\t\t  */\n+\t\tcase -3: /* ok but ends with a pattern-match character */\n \t\t\tcontinue;\n \t\t}\n \t\tdie(\"remote part of refspec is not a valid name in %s\",\n-- \n1.5.2.rc2.90.gc593-dirty\n"},{"id":"43335","messageId":"7vejl4m1we.fsf@assigned-by-dhcp.cox.net","threadId":"8308","inReplyTo":"Pine.LNX.4.64.0705250111200.9778@iabervon.org","subject":"Re: [PATCH] Move refspec pattern matching to match_refs().","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-26T08:20:49Z","receivedAt":"2007-05-26T08:20:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n> This means that send-pack and http-push will support pattern refspecs,\n> so builtin-push.c doesn't have to expand them, and also git push can\n> just turn --tags into \"refs/tags/*\", further simplifying\n> builtin-push.c\n>\n> check_ref_format() gets a third \"conditionally okay\" result for\n> something that's valid as a pattern but not as a particular ref.\n>\n> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>\n> ---\n> On top of my \"remote\" series. Shouldn't change any significant behavior, \n> and simplifies a lot of logic. This version takes into account the \n> comments from the previous round (assuming that the ruling on coding style \n> is that:\n>\n> if (condition)\n> \t/* Comment */\n> \tstatement;\n>\n> shouldn't have braces).\n>\n>  builtin-push.c |  133 +++++++++----------------------------------------------\n>  refs.c         |   27 ++++++++---\n>  remote.c       |   31 ++++++++++++-\n>  send-pack.c    |    1 +\n>  4 files changed, 70 insertions(+), 122 deletions(-)\n\nWhee.  Removes lots more code than it adds.  Will queue.\n"}]}