{"thread":{"id":"39807","subject":"[PATCH] git: add optional support for full pattern in fetch refspecs","startedAt":"2015-07-07T22:57:49Z","lastAt":"2015-07-08T04:47:46Z","messageCount":3,"participants":["Jacob Keller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"265781","messageId":"1436309869-19609-1-git-send-email-jacob.e.keller@intel.com","threadId":"39807","inReplyTo":null,"subject":"[PATCH] git: add optional support for full pattern in fetch refspecs","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2015-07-07T22:57:49Z","receivedAt":"2015-07-07T22:57:49Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"This patch updates the refs.c check_refname_component logic in order to\nallow for the possibility of using arbitrary patterns in fetch refspecs.\nSpecifically, patterns similar to `refs/tags/prefix-*:refs/tags/prefix-*`.\n\nIn order to ensure that standard users do not accidentally setup refspecs\nwhich could cause issues, tie this feature to\nremote..arbitrary_pattern boolean configuration option. This ensures\nthat no user will have this enabled on accident.\n\nThe primary reason against this is the ability to pull refs incorrectly\nand then later push them again. Ie:\n\nrefs/tags/some-prefix-*:/refs/tags/other-prefix-*\n\nThis ref will essentially re-name the references locally. This is\ngenerally not what you would want but there is no easy way to validate\nthis doesn't occur. Note this can already occur to normal pattern refspecs\nvia `refs/tags/*:refs/tags/namespace/*` refspecs, but these are a bit more\ndifficult to typo.\n\nHowever, the additional functionality is useful for specifying to pull\ncertain patterns of refs, and can be useful if the power user decides to\nenable it for a given remote.\n\nSigned-off-by: Jacob Keller \nCc: Daniel Barkalow \nCc: Junio C Hamano \n---\n Documentation/config.txt | 7 +++++\n Documentation/git-check-ref-format.txt | 12 ++++++---\n builtin/check-ref-format.c | 2 ++\n refs.c | 48 ++++++++++++++++++++++------------\n refs.h | 15 ++++++-----\n remote.c | 2 ++\n remote.h | 1 +\n 7 files changed, 61 insertions(+), 26 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 3e37b93ed2ac..626492de7a7f 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2331,6 +2331,13 @@ remote..prune::\n \tremote (as if the `--prune` option was given on the command line).\n \tOverrides `fetch.prune` settings, if any.\n \n+remote..arbitrarypattern::\n+\tWhen set to true, fetching from this remote will allow arbitrary complex\n+\tpatterns for the fetch refspecs. For example,\n+\trefs/tags/prefix-*:refs/tags/prefix-* will work as expected. Care should be\n+\ttaken as you could fetch refs into names that don't exist on the remote and\n+\tmay end up pushing them again later by accident.\n+\n remotes.::\n \tThe list of remotes which are fetched by \"git remote update\n \t\". See linkgit:git-remote[1].\ndiff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt\nindex fc02959ba4ab..caab0e3037fa 100644\n--- a/Documentation/git-check-ref-format.txt\n+++ b/Documentation/git-check-ref-format.txt\n@@ -43,8 +43,8 @@ Git imposes the following rules on how references are named:\n caret `^`, or colon `:` anywhere.\n \n . They cannot have question-mark `?`, asterisk `*`, or open\n- bracket `[` anywhere. See the `--refspec-pattern` option below for\n- an exception to this rule.\n+ bracket `[` anywhere. See the `--refspec-pattern` and `--arbitrary-pattern`\n+ options below for exceptions to this rule.\n \n . They cannot begin or end with a slash `/` or contain multiple\n consecutive slashes (see the `--normalize` option below for an\n@@ -95,7 +95,13 @@ OPTIONS\n \t(as used with remote repositories). If this option is\n \tenabled, is allowed to contain a single `*`\n \tin place of a one full pathname component (e.g.,\n-\t`foo/*/bar` but not `foo/bar*`).\n+\t`foo/*/bar` but not `foo/bar*`). If '--arbitrary-pattern' is set, then\n+\ta single `foo/bar*baz` pattern will be accepted.\n+\n+--arbitrary-pattern::\n+\tIf '--refspec-pattern' is set, allow arbitrary patterns instead of the\n+\tdefault simple case. Implies '--refspec-pattern'. Note that only one '*'\n+\tpattern will be accepted.\n \n --normalize::\n \tNormalize 'refname' by removing any leading slash (`/`)\ndiff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c\nindex fd915d59841e..e0b8d00d5337 100644\n--- a/builtin/check-ref-format.c\n+++ b/builtin/check-ref-format.c\n@@ -70,6 +70,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n \t\t\tflags &= ~REFNAME_ALLOW_ONELEVEL;\n \t\telse if (!strcmp(argv[i], \"--refspec-pattern\"))\n \t\t\tflags |= REFNAME_REFSPEC_PATTERN;\n+\t\telse if (!strcmp(argv[i], \"--arbitrary-pattern\"))\n+\t\t\tflags |= REFNAME_ARBITRARY_PATTERN | REFNAME_REFSPEC_PATTERN;\n \t\telse\n \t\t\tusage(builtin_check_ref_format_usage);\n \t}\ndiff --git a/refs.c b/refs.c\nindex 7ac05cf21a25..e4c24bfc778c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -20,11 +20,12 @@ struct ref_lock {\n * 2: ., look for a preceding . to reject .. in refs\n * 3: {, look for a preceding @ to reject @{ in refs\n * 4: A bad character: ASCII control characters, \"~\", \"^\", \":\" or SP\n+ * 5: check for patterns to reject unless REFNAME_REFSPEC_PATTERN is set\n */\n static unsigned char refname_disposition[256] = {\n \t1, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,\n \t4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,\n-\t4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 2, 1,\n+\t4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5, 0, 0, 0, 2, 1,\n \t0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 0, 4,\n \t0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,\n \t0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 4, 0, 4, 0,\n@@ -71,11 +72,14 @@ static unsigned char refname_disposition[256] = {\n * - any path component of it begins with \".\", or\n * - it has double dots \"..\", or\n * - it has ASCII control character, \"~\", \"^\", \":\" or SP, anywhere, or\n- * - it ends with a \"/\".\n- * - it ends with \".lock\"\n- * - it contains a \"\\\" (backslash)\n+ * - it ends with a \"/\", or\n+ * - it ends with \".lock\", or\n+ * - it contains a \"\\\" (backslash), or\n+ * - it contains a \"@{\" portion, or\n+ * - it contains a simple pattern unless REFNAME_REFSPEC_PATTERN is set, or\n+ * - it contains a complex pattern unless REFNAME_ARBITRARY_PATTERN is set\n */\n-static int check_refname_component(const char *refname, int flags)\n+static int check_refname_component(const char *refname, int *flags)\n {\n \tconst char *cp;\n \tchar last = '\\0';\n@@ -96,6 +100,24 @@ static int check_refname_component(const char *refname, int flags)\n \t\t\tbreak;\n \t\tcase 4:\n \t\t\treturn -1;\n+\t\tcase 5:\n+\t\t\tif (!(*flags & REFNAME_REFSPEC_PATTERN))\n+\t\t\t\treturn -1; /* refspec can't be a pattern */\n+\n+\t\t\t/* Unset the pattern flag so that we only accept a single pattern.\n+\t\t\t * This also conveniently makes us only accept the first '*' we\n+\t\t\t * find in a single component. */\n+\t\t\t*flags &= ~ REFNAME_REFSPEC_PATTERN;\n+\n+\t\t\t/* check if entire component is just a '*' */\n+\t\t\tif (refname[0] == '*' &&\n+\t\t\t\t\t(refname[1] == '\\0' || refname[1] == '/'))\n+\t\t\t\treturn 1;\n+\n+\t\t\tif (!(*flags & REFNAME_ARBITRARY_PATTERN))\n+\t\t\t\treturn -1; /* arbitrary patterns not allowed */\n+\n+\t\t\tbreak;\n \t\t}\n \t\tlast = ch;\n \t}\n@@ -120,18 +142,10 @@ int check_refname_format(const char *refname, int flags)\n \n \twhile (1) {\n \t\t/* We are at the start of a path component. */\n-\t\tcomponent_len = check_refname_component(refname, flags);\n-\t\tif (component_len <= 0) {\n-\t\t\tif ((flags & REFNAME_REFSPEC_PATTERN) &&\n-\t\t\t\t\trefname[0] == '*' &&\n-\t\t\t\t\t(refname[1] == '\\0' || refname[1] == '/')) {\n-\t\t\t\t/* Accept one wildcard as a full refname component. */\n-\t\t\t\tflags &= ~REFNAME_REFSPEC_PATTERN;\n-\t\t\t\tcomponent_len = 1;\n-\t\t\t} else {\n-\t\t\t\treturn -1;\n-\t\t\t}\n-\t\t}\n+\t\tcomponent_len = check_refname_component(refname, &flags);\n+\t\tif (component_len <= 0)\n+\t\t\treturn -1;\n+\n \t\tcomponent_count++;\n \t\tif (refname[component_len] == '\\0')\n \t\t\tbreak;\ndiff --git a/refs.h b/refs.h\nindex 8c3d433a8a5b..1429776a975f 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -218,14 +218,17 @@ extern int for_each_reflog(each_ref_fn, void *);\n \n #define REFNAME_ALLOW_ONELEVEL 1\n #define REFNAME_REFSPEC_PATTERN 2\n+#define REFNAME_ARBITRARY_PATTERN 4\n \n /*\n- * Return 0 iff refname has the correct format for a refname according\n- * to the rules described in Documentation/git-check-ref-format.txt.\n- * If REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level\n- * reference names. If REFNAME_REFSPEC_PATTERN is set in flags, then\n- * allow a \"*\" wildcard character in place of one of the name\n- * components. No leading or repeated slashes are accepted.\n+ * Return 0 iff refname has the correct format for a refname according to the\n+ * rules described in Documentation/git-check-ref-format.txt. If\n+ * REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level reference\n+ * names. If REFNAME_REFSPEC_PATTERN is set in flags, then allow a \"*\"\n+ * wildcard character in place of one of the name components. If\n+ * REFNAME_ARBITRARY_PATTERN is set in flags, then allow an arbitrary pattern\n+ * with a single '*' wildcard character as one of the name components as well.\n+ * No leading or repeated slashes are accepted.\n */\n extern int check_refname_format(const char *refname, int flags);\n \ndiff --git a/remote.c b/remote.c\nindex 26504b744786..49df6abad4ef 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -420,6 +420,8 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t\tremote->skip_default_update = git_config_bool(key, value);\n \telse if (!strcmp(subkey, \".prune\"))\n \t\tremote->prune = git_config_bool(key, value);\n+\telse if (!strcmp(subkey, \".arbitrarypattern\"))\n+\t\tremote->arbitrary_pattern = git_config_bool(key, value);\n \telse if (!strcmp(subkey, \".url\")) {\n \t\tconst char *v;\n \t\tif (git_config_string(&v, key, value))\ndiff --git a/remote.h b/remote.h\nindex 312b7ca131c4..c885ab34f42e 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -46,6 +46,7 @@ struct remote {\n \tint skip_default_update;\n \tint mirror;\n \tint prune;\n+\tint arbitrary_pattern;\n \n \tconst char *receivepack;\n \tconst char *uploadpack;\n-- \n2.4.3\n"},{"id":"265802","messageId":"xmqq615v1dlr.fsf@gitster.dls.corp.google.com","threadId":"39807","inReplyTo":"1436309869-19609-1-git-send-email-jacob.e.keller@intel.com","subject":"Re: [PATCH] git: add optional support for full pattern in fetch refspecs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-08T04:24:00Z","receivedAt":"2015-07-08T04:24:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller writes:\n\n> +remote..arbitrarypattern::\n> +\tWhen set to true, fetching from this remote will allow arbitrary complex\n> +\tpatterns for the fetch refspecs. For example,\n> +\trefs/tags/prefix-*:refs/tags/prefix-* will work as expected. Care should be\n> +\ttaken as you could fetch refs into names that don't exist on the remote and\n> +\tmay end up pushing them again later by accident.\n\nBad name and explanation, unless you truly mean \"arbitrary\", like\ntaking something like \"refs/ta*/prefix-*-*-foo:refs/*\".\n\nMore importantly, this is not \"pattern\"; you are talking about\nrefspec, I think.\n\nBut that probably does not matter. I do not think this even needs\nto exist as an option.\n\nPeople's existing settings would not have anything other than an\nasterisk as a whole path component on each side (or no asterisk\nanywhere), and if they had an asterisk anywhere else they would have\ngotten an error and wouldn't have made any progress until they fixed\nit. So if you loosen the current rule sligntly and tell them \"If\nyour refspec has an asterisk in it, then you must have one asterisk\non each side of it. That rule hasn't changed. But your asterisks no\nlonger have to be a whole path component\", such a change would not\nhurt them. Their existing setting that work would not notice, and\nexisting users would not be relying on a refspec with an asterisk as\npart of a path component to error out.\n"},{"id":"265803","messageId":"CA+P7+xpBrZPXFWpmzhkmHH7aAWiY-ews63_eex68kKc=ViHC3A@mail.gmail.com","threadId":"39807","inReplyTo":"xmqq615v1dlr.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] git: add optional support for full pattern in fetch refspecs","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2015-07-08T04:47:46Z","receivedAt":"2015-07-08T04:47:46Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Tue, Jul 7, 2015 at 9:24 PM, Junio C Hamano wrote:\n> Jacob Keller writes:\n>\n>> +remote..arbitrarypattern::\n>> + When set to true, fetching from this remote will allow arbitrary complex\n>> + patterns for the fetch refspecs. For example,\n>> + refs/tags/prefix-*:refs/tags/prefix-* will work as expected. Care should be\n>> + taken as you could fetch refs into names that don't exist on the remote and\n>> + may end up pushing them again later by accident.\n>\n> Bad name and explanation, unless you truly mean \"arbitrary\", like\n> taking something like \"refs/ta*/prefix-*-*-foo:refs/*\".\n>\n\nI couldn't figure out what to use, and the original intent was to add\nan option.. but see below,\n\n> More importantly, this is not \"pattern\"; you are talking about\n> refspec, I think.\n>\n\nIn this case the option was an additional modifier to the\nrefspec_patterns, and I was talking about how the pattern could be\nslightly more arbitrary. I do agree it is a bad name, but i couldn't\nactually come up with anything better.\n\n> But that probably does not matter. I do not think this even needs\n> to exist as an option.\n>\n\nYes, I agree especially as I look at it more. I'll work up a patch\nversion which does this without the option.\n\n> People's existing settings would not have anything other than an\n> asterisk as a whole path component on each side (or no asterisk\n> anywhere), and if they had an asterisk anywhere else they would have\n> gotten an error and wouldn't have made any progress until they fixed\n> it. So if you loosen the current rule sligntly and tell them \"If\n> your refspec has an asterisk in it, then you must have one asterisk\n> on each side of it. That rule hasn't changed. But your asterisks no\n> longer have to be a whole path component\", such a change would not\n> hurt them. Their existing setting that work would not notice, and\n> existing users would not be relying on a refspec with an asterisk as\n> part of a path component to error out.\n>\n\nRight. We aren't breaking anyones current functionality, just adding\nnew functionality. We already check for a * in both sides, and my code\nwill ensure only 1 star total. It will then work for the new somewhat\nmore expanded patterns and we don't need an option.\n\nI'll work up a v2.\n\nRegards,\nJake\n"}]}