Change check_ref_format() to take a flags argument

Change check_ref_format() to take a flags argument that indicates what is acceptable in the reference name (analogous to "git check-ref-format"'s "--allow-onelevel" and "--refspec-pattern"). This is more convenient for callers and also fixes a failure in the test suite (and likely elsewhere in the code) by enabling "onelevel" and "refspec-pattern" to be allowed independently of each other. Also rename check_ref_format() to check_refname_format() to make it obvious that it deals with refnames rather than references themselves. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Michael Haggerty committed Sep 15, 2011 at 23:10 UTC 8d9c50105f908b2adde4b7c77537cf95f19cd893
20 files changed +69 -126
builtin/check-ref-format.c
+1 -20
@@ -53,9 +53,6 @@ static void refname_format_print(const char *arg)
53 printf("%s\n", refname);
54 }
55
56 -#define REFNAME_ALLOW_ONELEVEL 1
57 -#define REFNAME_REFSPEC_PATTERN 2
58 -
56 int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
57 {
58 int i;
@@ -83,24 +80,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
80 if (! (i == argc - 1))
81 usage(builtin_check_ref_format_usage);
82
86 - switch (check_ref_format(argv[i])) {
87 - case CHECK_REF_FORMAT_OK:
88 - break;
89 - case CHECK_REF_FORMAT_ERROR:
83 + if (check_refname_format(argv[i], flags))
84 return 1;
91 - case CHECK_REF_FORMAT_ONELEVEL:
92 - if (!(flags & REFNAME_ALLOW_ONELEVEL))
93 - return 1;
94 - else
95 - break;
96 - case CHECK_REF_FORMAT_WILDCARD:
97 - if (!(flags & REFNAME_REFSPEC_PATTERN))
98 - return 1;
99 - else
100 - break;
101 - default:
102 - die("internal error: unexpected value from check_ref_format()");
103 - }
85
86 if (print)
87 refname_format_print(argv[i]);
builtin/checkout.c
+1 -1
@@ -882,7 +882,7 @@ static int parse_branchname_arg(int argc, const char **argv,
882 new->name = arg;
883 setup_branch_path(new);
884
885 - if (check_ref_format(new->path) == CHECK_REF_FORMAT_OK &&
885 + if (!check_refname_format(new->path, 0) &&
886 resolve_ref(new->path, branch_rev, 1, NULL))
887 hashcpy(rev, branch_rev);
888 else
builtin/fetch-pack.c
+1 -1
@@ -544,7 +544,7 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
544 for (ref = *refs; ref; ref = next) {
545 next = ref->next;
546 if (!memcmp(ref->name, "refs/", 5) &&
547 - check_ref_format(ref->name + 5))
547 + check_refname_format(ref->name + 5, 0))
548 ; /* trash */
549 else if (args.fetch_all &&
550 (!args.depth || prefixcmp(ref->name, "refs/tags/") )) {
builtin/receive-pack.c
+1 -1
@@ -356,7 +356,7 @@ static const char *update(struct command *cmd)
356 struct ref_lock *lock;
357
358 /* only refs/... are allowed */
359 - if (prefixcmp(name, "refs/") || check_ref_format(name + 5)) {
359 + if (prefixcmp(name, "refs/") || check_refname_format(name + 5, 0)) {
360 rp_error("refusing to create funny ref '%s' remotely", name);
361 return "funny refname";
362 }
builtin/replace.c
+1 -1
@@ -94,7 +94,7 @@ static int replace_object(const char *object_ref, const char *replace_ref,
94 "refs/replace/%s",
95 sha1_to_hex(object)) > sizeof(ref) - 1)
96 die("replace ref name too long: %.*s...", 50, ref);
97 - if (check_ref_format(ref))
97 + if (check_refname_format(ref, 0))
98 die("'%s' is not a valid ref name.", ref);
99
100 if (!resolve_ref(ref, prev, 1, NULL))
builtin/show-ref.c
+1 -1
@@ -145,7 +145,7 @@ static int exclude_existing(const char *match)
145 if (strncmp(ref, match, matchlen))
146 continue;
147 }
148 - if (check_ref_format(ref)) {
148 + if (check_refname_format(ref, 0)) {
149 warning("ref '%s' ignored", ref);
150 continue;
151 }
builtin/tag.c
+2 -2
@@ -407,12 +407,12 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
407 static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
408 {
409 if (name[0] == '-')
410 - return CHECK_REF_FORMAT_ERROR;
410 + return -1;
411
412 strbuf_reset(sb);
413 strbuf_addf(sb, "refs/tags/%s", name);
414
415 - return check_ref_format(sb->buf);
415 + return check_refname_format(sb->buf, 0);
416 }
417
418 int cmd_tag(int argc, const char **argv, const char *prefix)
connect.c
+1 -1
@@ -22,7 +22,7 @@ static int check_ref(const char *name, int len, unsigned int flags)
22 len -= 5;
23
24 /* REF_NORMAL means that we don't want the magic fake tag refs */
25 - if ((flags & REF_NORMAL) && check_ref_format(name) < 0)
25 + if ((flags & REF_NORMAL) && check_refname_format(name, 0))
26 return 0;
27
28 /* REF_HEADS means that we want regular branch heads */
environment.c
+1 -1
@@ -106,7 +106,7 @@ static char *expand_namespace(const char *raw_namespace)
106 if (strcmp((*c)->buf, "/") != 0)
107 strbuf_addf(&buf, "refs/namespaces/%s", (*c)->buf);
108 strbuf_list_free(components);
109 - if (check_ref_format(buf.buf) != CHECK_REF_FORMAT_OK)
109 + if (check_refname_format(buf.buf, 0))
110 die("bad git namespace path \"%s\"", raw_namespace);
111 strbuf_addch(&buf, '/');
112 return strbuf_detach(&buf, NULL);
fast-import.c
+1 -6
@@ -722,13 +722,8 @@ static struct branch *new_branch(const char *name)
722
723 if (b)
724 die("Invalid attempt to create duplicate branch: %s", name);
725 - switch (check_ref_format(name)) {
726 - case 0: break; /* its valid */
727 - case CHECK_REF_FORMAT_ONELEVEL:
728 - break; /* valid, but too few '/', allow anyway */
729 - default:
725 + if (check_refname_format(name, REFNAME_ALLOW_ONELEVEL))
726 die("Branch name doesn't conform to GIT standards: %s", name);
731 - }
727
728 b = pool_calloc(1, sizeof(struct branch));
729 b->name = pool_strdup(name);
git_remote_helpers/git/git.py
+1 -1
@@ -54,7 +54,7 @@ def valid_git_ref (ref_name):
54 # The following is a reimplementation of the git check-ref-format
55 # command. The rules were derived from the git check-ref-format(1)
56 # manual page. This code should be replaced by a call to
57 - # check_ref_format() in the git library, when such is available.
57 + # check_refname_format() in the git library, when such is available.
58 if ref_name.endswith('/') or \
59 ref_name.startswith('.') or \
60 ref_name.count('/.') or \
notes-merge.c
+3 -2
@@ -570,7 +570,8 @@ int notes_merge(struct notes_merge_options *o,
570 /* Dereference o->local_ref into local_sha1 */
571 if (!resolve_ref(o->local_ref, local_sha1, 0, NULL))
572 die("Failed to resolve local notes ref '%s'", o->local_ref);
573 - else if (!check_ref_format(o->local_ref) && is_null_sha1(local_sha1))
573 + else if (!check_refname_format(o->local_ref, 0) &&
574 + is_null_sha1(local_sha1))
575 local = NULL; /* local_sha1 == null_sha1 indicates unborn ref */
576 else if (!(local = lookup_commit_reference(local_sha1)))
577 die("Could not parse local commit %s (%s)",
@@ -583,7 +584,7 @@ int notes_merge(struct notes_merge_options *o,
584 * Failed to get remote_sha1. If o->remote_ref looks like an
585 * unborn ref, perform the merge using an empty notes tree.
586 */
586 - if (!check_ref_format(o->remote_ref)) {
587 + if (!check_refname_format(o->remote_ref, 0)) {
588 hashclr(remote_sha1);
589 remote = NULL;
590 } else {
pack-refs.c
+1 -1
@@ -72,7 +72,7 @@ static void try_remove_empty_parents(char *name)
72 for (i = 0; i < 2; i++) { /* refs/{heads,tags,...}/ */
73 while (*p && *p != '/')
74 p++;
75 - /* tolerate duplicate slashes; see check_ref_format() */
75 + /* tolerate duplicate slashes; see check_refname_format() */
76 while (*p == '/')
77 p++;
78 }
refs.c
+19 -23
@@ -872,10 +872,9 @@ static inline int bad_ref_char(int ch)
872 return 0;
873 }
874
875 -int check_ref_format(const char *ref)
875 +int check_refname_format(const char *ref, int flags)
876 {
877 int ch, level, last;
878 - int ret = CHECK_REF_FORMAT_OK;
878 const char *cp = ref;
879
880 level = 0;
@@ -884,41 +883,42 @@ int check_ref_format(const char *ref)
883 ; /* tolerate duplicated slashes */
884 if (!ch)
885 /* should not end with slashes */
887 - return CHECK_REF_FORMAT_ERROR;
886 + return -1;
887
888 /* we are at the beginning of the path component */
889 if (ch == '.')
891 - return CHECK_REF_FORMAT_ERROR;
890 + return -1;
891 if (bad_ref_char(ch)) {
893 - if (ch == '*' && (!*cp || *cp == '/') &&
894 - ret == CHECK_REF_FORMAT_OK)
895 - ret = CHECK_REF_FORMAT_WILDCARD;
892 + if ((flags & REFNAME_REFSPEC_PATTERN) && ch == '*' &&
893 + (!*cp || *cp == '/'))
894 + /* Accept one wildcard as a full refname component. */
895 + flags &= ~REFNAME_REFSPEC_PATTERN;
896 else
897 - return CHECK_REF_FORMAT_ERROR;
897 + return -1;
898 }
899
900 last = ch;
901 /* scan the rest of the path component */
902 while ((ch = *cp++) != 0) {
903 if (bad_ref_char(ch))
904 - return CHECK_REF_FORMAT_ERROR;
904 + return -1;
905 if (ch == '/')
906 break;
907 if (last == '.' && ch == '.')
908 - return CHECK_REF_FORMAT_ERROR;
908 + return -1;
909 if (last == '@' && ch == '{')
910 - return CHECK_REF_FORMAT_ERROR;
910 + return -1;
911 last = ch;
912 }
913 level++;
914 if (!ch) {
915 if (ref <= cp - 2 && cp[-2] == '.')
916 - return CHECK_REF_FORMAT_ERROR;
917 - if (level < 2)
918 - return CHECK_REF_FORMAT_ONELEVEL;
916 + return -1;
917 + if (level < 2 && !(flags & REFNAME_ALLOW_ONELEVEL))
918 + return -1;
919 if (has_extension(ref, ".lock"))
920 - return CHECK_REF_FORMAT_ERROR;
921 - return ret;
920 + return -1;
921 + return 0;
922 }
923 }
924 }
@@ -1103,7 +1103,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
1103 struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)
1104 {
1105 char refpath[PATH_MAX];
1106 - if (check_ref_format(ref))
1106 + if (check_refname_format(ref, 0))
1107 return NULL;
1108 strcpy(refpath, mkpath("refs/%s", ref));
1109 return lock_ref_sha1_basic(refpath, old_sha1, 0, NULL);
@@ -1111,13 +1111,9 @@ struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)
1111
1112 struct ref_lock *lock_any_ref_for_update(const char *ref, const unsigned char *old_sha1, int flags)
1113 {
1114 - switch (check_ref_format(ref)) {
1115 - default:
1114 + if (check_refname_format(ref, REFNAME_ALLOW_ONELEVEL))
1115 return NULL;
1117 - case 0:
1118 - case CHECK_REF_FORMAT_ONELEVEL:
1119 - return lock_ref_sha1_basic(ref, old_sha1, flags, NULL);
1120 - }
1116 + return lock_ref_sha1_basic(ref, old_sha1, flags, NULL);
1117 }
1118
1119 static struct lock_file packlock;
refs.h
+12 -5
@@ -97,11 +97,18 @@ int for_each_recent_reflog_ent(const char *ref, each_reflog_ent_fn fn, long, voi
97 */
98 extern int for_each_reflog(each_ref_fn, void *);
99
100 -#define CHECK_REF_FORMAT_OK 0
101 -#define CHECK_REF_FORMAT_ERROR (-1)
102 -#define CHECK_REF_FORMAT_ONELEVEL (-2)
103 -#define CHECK_REF_FORMAT_WILDCARD (-3)
104 -extern int check_ref_format(const char *target);
100 +#define REFNAME_ALLOW_ONELEVEL 1
101 +#define REFNAME_REFSPEC_PATTERN 2
102 +
103 +/*
104 + * Return 0 iff ref has the correct format for a refname according to
105 + * the rules described in Documentation/git-check-ref-format.txt. If
106 + * REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level
107 + * reference names. If REFNAME_REFSPEC_PATTERN is set in flags, then
108 + * allow a "*" wildcard character in place of one of the name
109 + * components.
110 + */
111 +extern int check_refname_format(const char *ref, int flags);
112
113 extern const char *prettify_refname(const char *refname);
114 extern char *shorten_unambiguous_ref(const char *ref, int strict);
remote.c
+14 -39
@@ -492,23 +492,6 @@ static void read_config(void)
492 alias_all_urls();
493 }
494
495 -/*
496 - * We need to make sure the remote-tracking branches are well formed, but a
497 - * wildcard refspec in "struct refspec" must have a trailing slash. We
498 - * temporarily drop the trailing '/' while calling check_ref_format(),
499 - * and put it back. The caller knows that a CHECK_REF_FORMAT_ONELEVEL
500 - * error return is Ok for a wildcard refspec.
501 - */
502 -static int verify_refname(char *name, int is_glob)
503 -{
504 - int result;
505 -
506 - result = check_ref_format(name);
507 - if (is_glob && result == CHECK_REF_FORMAT_WILDCARD)
508 - result = CHECK_REF_FORMAT_OK;
509 - return result;
510 -}
511 -
495 /*
496 * This function frees a refspec array.
497 * Warning: code paths should be checked to ensure that the src
@@ -532,13 +515,13 @@ static void free_refspecs(struct refspec *refspec, int nr_refspec)
515 static struct refspec *parse_refspec_internal(int nr_refspec, const char **refspec, int fetch, int verify)
516 {
517 int i;
535 - int st;
518 struct refspec *rs = xcalloc(sizeof(*rs), nr_refspec);
519
520 for (i = 0; i < nr_refspec; i++) {
521 size_t llen;
522 int is_glob;
523 const char *lhs, *rhs;
524 + int flags;
525
526 is_glob = 0;
527
@@ -576,6 +559,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
559
560 rs[i].pattern = is_glob;
561 rs[i].src = xstrndup(lhs, llen);
562 + flags = REFNAME_ALLOW_ONELEVEL | (is_glob ? REFNAME_REFSPEC_PATTERN : 0);
563
564 if (fetch) {
565 /*
@@ -585,26 +569,20 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
569 */
570 if (!*rs[i].src)
571 ; /* empty is ok */
588 - else {
589 - st = verify_refname(rs[i].src, is_glob);
590 - if (st && st != CHECK_REF_FORMAT_ONELEVEL)
591 - goto invalid;
592 - }
572 + else if (check_refname_format(rs[i].src, flags))
573 + goto invalid;
574 /*
575 * RHS
576 * - missing is ok, and is same as empty.
577 * - empty is ok; it means not to store.
578 * - otherwise it must be a valid looking ref.
579 */
599 - if (!rs[i].dst) {
580 + if (!rs[i].dst)
581 ; /* ok */
601 - } else if (!*rs[i].dst) {
582 + else if (!*rs[i].dst)
583 ; /* ok */
603 - } else {
604 - st = verify_refname(rs[i].dst, is_glob);
605 - if (st && st != CHECK_REF_FORMAT_ONELEVEL)
606 - goto invalid;
607 - }
584 + else if (check_refname_format(rs[i].dst, flags))
585 + goto invalid;
586 } else {
587 /*
588 * LHS
@@ -616,8 +594,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
594 if (!*rs[i].src)
595 ; /* empty is ok */
596 else if (is_glob) {
619 - st = verify_refname(rs[i].src, is_glob);
620 - if (st && st != CHECK_REF_FORMAT_ONELEVEL)
597 + if (check_refname_format(rs[i].src, flags))
598 goto invalid;
599 }
600 else
@@ -630,14 +607,12 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
607 * - otherwise it must be a valid looking ref.
608 */
609 if (!rs[i].dst) {
633 - st = verify_refname(rs[i].src, is_glob);
634 - if (st && st != CHECK_REF_FORMAT_ONELEVEL)
610 + if (check_refname_format(rs[i].src, flags))
611 goto invalid;
612 } else if (!*rs[i].dst) {
613 goto invalid;
614 } else {
639 - st = verify_refname(rs[i].dst, is_glob);
640 - if (st && st != CHECK_REF_FORMAT_ONELEVEL)
615 + if (check_refname_format(rs[i].dst, flags))
616 goto invalid;
617 }
618 }
@@ -1427,8 +1402,8 @@ int get_fetch_map(const struct ref *remote_refs,
1402
1403 for (rmp = &ref_map; *rmp; ) {
1404 if ((*rmp)->peer_ref) {
1430 - int st = check_ref_format((*rmp)->peer_ref->name + 5);
1431 - if (st && st != CHECK_REF_FORMAT_ONELEVEL) {
1405 + if (check_refname_format((*rmp)->peer_ref->name + 5,
1406 + REFNAME_ALLOW_ONELEVEL)) {
1407 struct ref *ignore = *rmp;
1408 error("* Ignoring funny ref '%s' locally",
1409 (*rmp)->peer_ref->name);
@@ -1620,7 +1595,7 @@ static int one_local_ref(const char *refname, const unsigned char *sha1, int fla
1595 int len;
1596
1597 /* we already know it starts with refs/ to get here */
1623 - if (check_ref_format(refname + 5))
1598 + if (check_refname_format(refname + 5, 0))
1599 return 0;
1600
1601 len = strlen(refname) + 1;
sha1_name.c
+2 -2
@@ -972,9 +972,9 @@ int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
972 {
973 strbuf_branchname(sb, name);
974 if (name[0] == '-')
975 - return CHECK_REF_FORMAT_ERROR;
975 + return -1;
976 strbuf_splice(sb, 0, 0, "refs/heads/", 11);
977 - return check_ref_format(sb->buf);
977 + return check_refname_format(sb->buf, 0);
978 }
979
980 /*
t/t1402-check-ref-format.sh
+1 -5
@@ -87,11 +87,7 @@ valid_ref "$ref" '--refspec-pattern --allow-onelevel'
87
88 ref='*'
89 invalid_ref "$ref"
90 -
91 -#invalid_ref "$ref" --allow-onelevel
92 -test_expect_failure "ref name '$ref' is invalid with options --allow-onelevel" \
93 - "test_must_fail git check-ref-format --allow-onelevel '$ref'"
94 -
90 +invalid_ref "$ref" --allow-onelevel
91 invalid_ref "$ref" --refspec-pattern
92 valid_ref "$ref" '--refspec-pattern --allow-onelevel'
93
transport.c
+4 -12
@@ -754,18 +754,10 @@ void transport_verify_remote_names(int nr_heads, const char **heads)
754 continue;
755
756 remote = remote ? (remote + 1) : local;
757 - switch (check_ref_format(remote)) {
758 - case 0: /* ok */
759 - case CHECK_REF_FORMAT_ONELEVEL:
760 - /* ok but a single level -- that is fine for
761 - * a match pattern.
762 - */
763 - case CHECK_REF_FORMAT_WILDCARD:
764 - /* ok but ends with a pattern-match character */
765 - continue;
766 - }
767 - die("remote part of refspec is not a valid name in %s",
768 - heads[i]);
757 + if (check_refname_format(remote,
758 + REFNAME_ALLOW_ONELEVEL|REFNAME_REFSPEC_PATTERN))
759 + die("remote part of refspec is not a valid name in %s",
760 + heads[i]);
761 }
762 }
763
walker.c
+1 -1
@@ -190,7 +190,7 @@ static int interpret_target(struct walker *walker, char *target, unsigned char *
190 {
191 if (!get_sha1_hex(target, sha1))
192 return 0;
193 - if (!check_ref_format(target)) {
193 + if (!check_refname_format(target, 0)) {
194 struct ref *ref = alloc_ref(target);
195 if (!walker->fetch_ref(walker, ref)) {
196 hashcpy(sha1, ref->old_sha1);