Rollback of https://github.com/bazelbuild/bazel/commit/732dc512801c32207c252a76ca8d9e5544560339.
RELNOTES: Allow @ in package names.
PiperOrigin-RevId: 203270369
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
index 873d85e..831c408 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
@@ -151,17 +151,22 @@
repo = absName;
absName = "//:" + absName.substring(1);
}
+ String error = RepositoryName.validate(repo);
+ if (error != null) {
+ throw new LabelSyntaxException(
+ "invalid repository name '" + StringUtilities.sanitizeControlChars(repo) + "': " + error);
+ }
try {
LabelValidator.PackageAndTarget labelParts = LabelValidator.parseAbsoluteLabel(absName);
- PackageIdentifier pkgIdWithoutRepo =
- validatePackageName(labelParts.getPackageName(), labelParts.getTargetName());
- PathFragment packageFragment = pkgIdWithoutRepo.getPackageFragment();
+ PackageIdentifier pkgId =
+ validatePackageName(
+ labelParts.getPackageName(), labelParts.getTargetName(), repo, repositoryMapping);
+ PathFragment packageFragment = pkgId.getPackageFragment();
if (repo.isEmpty() && ABSOLUTE_PACKAGE_NAMES.contains(packageFragment)) {
- repo = "@";
+ pkgId =
+ PackageIdentifier.create(getGlobalRepoName("@", repositoryMapping), packageFragment);
}
- RepositoryName globalRepoName = getGlobalRepoName(repo, repositoryMapping);
- return create(
- PackageIdentifier.create(globalRepoName, packageFragment), labelParts.getTargetName());
+ return create(pkgId, labelParts.getTargetName());
} catch (BadLabelException e) {
throw new LabelSyntaxException(e.getMessage());
}
@@ -288,15 +293,25 @@
return name;
}
+ private static PackageIdentifier validatePackageName(String packageIdentifier, String name)
+ throws LabelSyntaxException {
+ return validatePackageName(
+ packageIdentifier, name, /* repo= */ null, /* repositoryMapping= */ null);
+ }
+
/**
* Validates the given package name and returns a canonical {@link PackageIdentifier} instance if
* it is valid. Otherwise it throws a SyntaxException.
*/
- private static PackageIdentifier validatePackageName(String packageIdentifier, String name)
+ private static PackageIdentifier validatePackageName(
+ String packageIdentifier,
+ String name,
+ String repo,
+ ImmutableMap<RepositoryName, RepositoryName> repositoryMapping)
throws LabelSyntaxException {
String error = null;
try {
- return PackageIdentifier.parse(packageIdentifier);
+ return PackageIdentifier.parse(packageIdentifier, repo, repositoryMapping);
} catch (LabelSyntaxException e) {
error = e.getMessage();
error = "invalid package name '" + packageIdentifier + "': " + error;
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 633946c..e63b096 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
@@ -45,7 +45,6 @@
// Package names allow all 7-bit ASCII characters except
// 0-31 (control characters)
// 58 ':' (colon) - target name separator
- // 64 '@' (at-sign) - workspace name prefix
// 92 '\' (backslash) - directory separator (on Windows); may be allowed in the future
// 127 (delete)
/** Matches characters allowed in package name. */
@@ -53,7 +52,7 @@
CharMatcher.inRange('0', '9')
.or(CharMatcher.inRange('a', 'z'))
.or(CharMatcher.inRange('A', 'Z'))
- .or(CharMatcher.anyOf(" !\"#$%&'()*+,-./;<=>?[]^_`{|}~"))
+ .or(CharMatcher.anyOf(" !\"#$%&'()*+,-./;<=>?@[]^_`{|}~"))
.precomputed();
/**
@@ -71,7 +70,7 @@
@VisibleForTesting
static final String PACKAGE_NAME_ERROR =
"package names may contain A-Z, a-z, 0-9, or any of ' !\"#$%&'()*+,-./;<=>?[]^_`{|}~'"
- + " (most 127-bit ascii characters except 0-31, 127, ':', '@', or '\\')";
+ + " (most 127-bit ascii characters except 0-31, 127, ':', or '\\')";
@VisibleForTesting
static final String PACKAGE_NAME_DOT_ERROR =
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
index 8d7ad19..732e6a0 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
@@ -16,6 +16,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ComparisonChain;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Interner;
import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
@@ -119,10 +120,17 @@
}
public static PackageIdentifier parse(String input) throws LabelSyntaxException {
- String repo;
+ return parse(input, /* repo= */ null, /* repositoryMapping= */ null);
+ }
+
+ public static PackageIdentifier parse(
+ String input, String repo, ImmutableMap<RepositoryName, RepositoryName> repositoryMapping)
+ throws LabelSyntaxException {
String packageName;
int packageStartPos = input.indexOf("//");
- if (input.startsWith("@") && packageStartPos > 0) {
+ if (repo != null) {
+ packageName = input;
+ } else if (input.startsWith("@") && packageStartPos > 0) {
repo = input.substring(0, packageStartPos);
packageName = input.substring(packageStartPos + 2);
} else if (input.startsWith("@")) {
@@ -145,7 +153,13 @@
throw new LabelSyntaxException(error);
}
- return create(repo, PathFragment.create(packageName));
+ if (repositoryMapping != null) {
+ RepositoryName repositoryName = RepositoryName.create(repo);
+ repositoryName = repositoryMapping.getOrDefault(repositoryName, repositoryName);
+ return create(repositoryName, PathFragment.create(packageName));
+ } else {
+ return create(repo, PathFragment.create(packageName));
+ }
}
public RepositoryName getRepository() {
diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java
index 17bc1e2..fdfb30bd 100644
--- a/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java
+++ b/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java
@@ -31,8 +31,6 @@
@RunWith(JUnit4.class)
public class LabelTest {
- private static final String BAD_PACKAGE_CHARS =
- "package names may contain A-Z, a-z, 0-9, or any of";
private static final String INVALID_TARGET_NAME = "invalid target name";
private static final String INVALID_PACKAGE_NAME = "invalid package name";
@@ -59,6 +57,18 @@
assertThat(l.getPackageName()).isEmpty();
assertThat(l.getName()).isEqualTo("foo");
}
+ {
+ Label l = Label.parseAbsolute("//@foo", ImmutableMap.of());
+ assertThat(l.getPackageIdentifier().getRepository().getName()).isEqualTo("@");
+ assertThat(l.getPackageName()).isEqualTo("@foo");
+ assertThat(l.getName()).isEqualTo("@foo");
+ }
+ {
+ Label l = Label.parseAbsolute("//xyz/@foo:abc", ImmutableMap.of());
+ assertThat(l.getPackageIdentifier().getRepository().getName()).isEqualTo("@");
+ assertThat(l.getPackageName()).isEqualTo("xyz/@foo");
+ assertThat(l.getName()).isEqualTo("abc");
+ }
}
private static String parseCommandLine(String label, String prefix) throws LabelSyntaxException {
@@ -365,16 +375,6 @@
Label.parseAbsolute("//$( ):$( )", ImmutableMap.of());
}
- /**
- * Regression test: we previously expanded the set of characters which are considered label chars
- * to include "@" (see test above). An unexpected side-effect is that "@D" in genrule(cmd) was
- * considered to be a valid relative label! The fix is to forbid "@x" in package names.
- */
- @Test
- public void testAtVersionIsIllegal() throws Exception {
- assertSyntaxError(BAD_PACKAGE_CHARS, "//foo/bar@123:baz");
- }
-
@Test
public void testDoubleSlashPathSeparator() throws Exception {
assertSyntaxError("package names may not contain '//' path separators",
@@ -444,8 +444,21 @@
Label.parseAbsolute("foo//bar/baz:bat/boo", ImmutableMap.of());
fail();
} catch (LabelSyntaxException e) {
- assertThat(e).hasMessage(
- "invalid repository name 'foo': workspace names must start with '@'");
+ assertThat(e)
+ .hasMessageThat()
+ .isEqualTo("invalid repository name 'foo': workspace names must start with '@'");
+ }
+ }
+
+ @Test
+ public void testInvalidRepoWithColon() throws Exception {
+ try {
+ Label.parseAbsolute("@foo:xyz", ImmutableMap.of());
+ fail();
+ } catch (LabelSyntaxException e) {
+ assertThat(e)
+ .hasMessageThat()
+ .containsMatch("invalid repository name '@foo:xyz': workspace names may contain only");
}
}
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 fcb187f..3ec7d01 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
@@ -74,6 +74,7 @@
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]bar")).isNull();
assertThat(LabelValidator.validatePackageName("foo^bar")).isNull();
@@ -91,8 +92,6 @@
.isEqualTo("package names may not end with '/'");
assertThat(LabelValidator.validatePackageName("foo:bar"))
.isEqualTo(LabelValidator.PACKAGE_NAME_ERROR);
- assertThat(LabelValidator.validatePackageName("baz@12345"))
- .isEqualTo(LabelValidator.PACKAGE_NAME_ERROR);
assertThat(LabelValidator.validatePackageName("bar/../baz"))
.isEqualTo(LabelValidator.PACKAGE_NAME_DOT_ERROR);
@@ -171,6 +170,12 @@
.isEqualTo(new PackageAndTarget("f$( )oo", "b$() ar"));
assertThat(LabelValidator.validateAbsoluteLabel("@//f$( )oo:b$() ar"))
.isEqualTo(new PackageAndTarget("f$( )oo", "b$() ar"));
+ assertThat(LabelValidator.validateAbsoluteLabel("//f@oo"))
+ .isEqualTo(new PackageAndTarget("f@oo", "f@oo"));
+ assertThat(LabelValidator.validateAbsoluteLabel("//@foo"))
+ .isEqualTo(new PackageAndTarget("@foo", "@foo"));
+ assertThat(LabelValidator.validateAbsoluteLabel("//@foo:@bar"))
+ .isEqualTo(new PackageAndTarget("@foo", "@bar"));
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java
index e33d1f3..1506dfd 100644
--- a/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java
+++ b/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java
@@ -50,7 +50,7 @@
@Test
public void testInvalidPatterns() throws TargetParsingException {
try {
- parse("Bar@java");
+ parse("Bar\\java");
fail();
} catch (TargetParsingException expected) {
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java
index 8cb1ba8..8f0e8ec 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java
@@ -499,7 +499,8 @@
@Test
public void testInvalidIncludeDirectory() throws Exception {
assertInvalidIncludeDirectoryMessage("%package(//a", "has an unrecognized %prefix%");
- assertInvalidIncludeDirectoryMessage("%package(//a@@a)%", "The package '//a@@a' is not valid");
+ assertInvalidIncludeDirectoryMessage(
+ "%package(//a:@@a)%", "The package '//a:@@a' is not valid");
assertInvalidIncludeDirectoryMessage(
"%package(//a)%foo", "The path in the package.*is not valid");
assertInvalidIncludeDirectoryMessage(
