diff --git a/site/en/concepts/labels.md b/site/en/concepts/labels.md index 346a0d74d5204c..11a9efa010696c 100644 --- a/site/en/concepts/labels.md +++ b/site/en/concepts/labels.md @@ -136,7 +136,8 @@ file; the name of a file is its pathname relative to the directory containing the `BUILD` file. Target names must be composed entirely of characters drawn from the set `a`–`z`, -`A`–`Z`, `0`–`9`, and the punctuation symbols `!%-@^_"#$&'()*-+,;<=>?[]{|}~/.`. +`A`–`Z`, `0`–`9`, and the punctuation symbols `!%-@^_"#$&'()*-+,:;<=>?[]{|}~/.`. +They must not start with `:`. Filenames must be relative pathnames in normal form, which means they must neither start nor end with a slash (for example, `/foo` and `foo/` are diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/LabelParser.java b/src/main/java/com/google/devtools/build/lib/cmdline/LabelParser.java index 5666e1ab8fc55b..d4518541289df5 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/LabelParser.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/LabelParser.java @@ -101,7 +101,9 @@ static Parts validateAndCreate( * "@repo//foo/bar" | "repo" | false | true | "foo/bar" | false | "bar" * "@@repo//foo/bar" | "repo" | true | true | "foo/bar" | false | "bar" * ":quux" | null | false | false | "" | false | "quux" + * ":qu:ux" | null | false | false | "" | false | "qu:ux" * "foo/bar:quux" | null | false | false | "foo/bar" | false | "quux" + * "foo/bar:qu:ux" | null | false | false | "foo/bar" | false | "qu:ux" * "//foo/bar:quux" | null | false | true | "foo/bar" | false | "quux" * "@repo//foo/bar:quux" | "repo" | false | true | "foo/bar" | false | "quux" * } diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/LabelValidator.java b/src/main/java/com/google/devtools/build/lib/cmdline/LabelValidator.java index 6080cb3426b8b7..c04c0686db941d 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/LabelValidator.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/LabelValidator.java @@ -26,7 +26,6 @@ public final class LabelValidator { // Target names allow all 7-bit ASCII characters except // 0-31 (control characters) - // 58 ':' (colon) // 92 '\' (backslash) - directory separator (on Windows); may be allowed in the future // 127 (delete) /** Matches punctuation in target names which requires quoting in a blaze query. */ @@ -36,11 +35,11 @@ public final class LabelValidator { /** * Matches punctuation in target names which doesn't require quoting in a blaze query. * - * Note that . is also allowed in target names, and doesn't require quoting, but has restrictions - * on its surrounding characters; see {@link #validateTargetName(String)}. + *

Note that . is also allowed in target names, and doesn't require quoting, but has + * restrictions on its surrounding characters; see {@link #validateTargetName(String)}. The same + * applies to :. */ - private static final CharMatcher PUNCTUATION_NOT_REQUIRING_QUOTING = - CharMatcher.anyOf("!%-@^_`"); + private static final CharMatcher PUNCTUATION_NOT_REQUIRING_QUOTING = CharMatcher.anyOf("!%-@^_`"); // Package names allow all 7-bit ASCII characters except // 0-31 (control characters) @@ -52,7 +51,7 @@ public final class LabelValidator { CharMatcher.inRange('0', '9') .or(CharMatcher.inRange('a', 'z')) .or(CharMatcher.inRange('A', 'Z')) - .or(CharMatcher.anyOf(" !\"#$%&'()*+,-./;<=>?@[]^_`{|}~")) + .or(CharMatcher.anyOf(" !\"#$%&'()*+,-./:;<=>?@[]^_`{|}~")) .precomputed(); /** @@ -153,6 +152,8 @@ public static String validateTargetName(String targetName) { char c = targetName.charAt(0); if (c == '/') { return "target names may not start with '/'"; + } else if (c == ':') { + return "target names may not start with ':'"; } else if (c == '.') { if (targetName.startsWith("../") || targetName.equals("..")) { return "target names may not contain up-level references '..'"; @@ -174,7 +175,7 @@ public static String validateTargetName(String targetName) { if (ALWAYS_ALLOWED_TARGET_CHARACTERS.matches(c)) { continue; } - if (c == '.') { + if (c == '.' || c == ':') { continue; } if (c == '/') { diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java b/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java index 3a0f8d1f4b7373..d0a398a1421678 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java @@ -68,8 +68,7 @@ public static Rule createRule( String name = (String) nameObject; Label label; try { - // Test that this would form a valid label name -- in particular, this - // catches cases where Makefile variables $(foo) appear in "name". + // Test that this would form a valid label name. label = pkgBuilder.createLabel(name); } catch (LabelSyntaxException e) { throw new InvalidRuleException("illegal rule name: " + name + ": " + e.getMessage()); diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java index 5d8f06458ae6b0..4d86e2b70963b9 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java @@ -117,8 +117,8 @@ public Sequence glob( ArrayList result = new ArrayList<>(matches.size()); for (String match : matches) { - if (match.charAt(0) == '@') { - // Add explicit colon to disambiguate from external repository. + if (match.charAt(0) == '@' || match.contains(":")) { + // Add explicit colon to disambiguate from external repository or target reference. match = ":" + match; } result.add(match); diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/LabelParserTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/LabelParserTest.java index 3039f001e42d41..c0cec36325fcf9 100644 --- a/src/test/java/com/google/devtools/build/lib/cmdline/LabelParserTest.java +++ b/src/test/java/com/google/devtools/build/lib/cmdline/LabelParserTest.java @@ -56,8 +56,12 @@ public void parserTable() throws Exception { validateAndCreate("repo", true, true, "foo/bar", false, "bar", "@@repo//foo/bar")); assertThat(parse(":quux")) .isEqualTo(validateAndCreate(null, false, false, "", false, "quux", ":quux")); + assertThat(parse(":qu:ux")) + .isEqualTo(validateAndCreate(null, false, false, "", false, "qu:ux", ":qu:ux")); assertThat(parse("foo/bar:quux")) .isEqualTo(validateAndCreate(null, false, false, "foo/bar", false, "quux", "foo/bar:quux")); + assertThat(parse("foo/bar:qu:ux")) + .isEqualTo(validateAndCreate(null, false, false, "foo/bar", false, "qu:ux", "foo/bar:qu:ux")); assertThat(parse("//foo/bar:quux")) .isEqualTo( validateAndCreate(null, false, true, "foo/bar", false, "quux", "//foo/bar:quux")); diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/LabelValidatorTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/LabelValidatorTest.java index 0d0310be98f5f9..9577834cd52ff7 100644 --- a/src/test/java/com/google/devtools/build/lib/cmdline/LabelValidatorTest.java +++ b/src/test/java/com/google/devtools/build/lib/cmdline/LabelValidatorTest.java @@ -68,7 +68,6 @@ public void testValidatePackageName() throws Exception { assertThat(LabelValidator.validatePackageName("foo,bar")).isNull(); assertThat(LabelValidator.validatePackageName("foo-bar")).isNull(); assertThat(LabelValidator.validatePackageName("foo.bar")).isNull(); - assertThat(LabelValidator.validatePackageName("foo+bar")).isNull(); assertThat(LabelValidator.validatePackageName("foo;bar")).isNull(); assertThat(LabelValidator.validatePackageName("foo