Automated rollback of commit 2b015c53c89815472923d8ea0c94640b7db2fa20.

RELNOTES[NEW]: Allow @ in package names.

PiperOrigin-RevId: 201487916

7 files changed, 80 insertions, 33 deletions

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 9d14e6d7d8..6f238429eb 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
@@ -155,17 +155,22 @@ public final class Label
       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());
     }
@@ -289,15 +294,25 @@ public final class Label
     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 633946c60f..e63b09684a 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 @@ public final class LabelValidator {
   // 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 @@ 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();
 
   /**
@@ -71,7 +70,7 @@ public final class LabelValidator {
   @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 8d7ad1910a..732e6a0642 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 @@ package com.google.devtools.build.lib.cmdline;
 
 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 final class PackageIdentifier
   }
 
   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 @@ public final class PackageIdentifier
       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 be61b9472a..6f6d444a52 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
@@ -30,8 +30,6 @@ import org.junit.runners.JUnit4;
 @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";
 
@@ -58,6 +56,18 @@ public class LabelTest {
       assertThat(l.getPackageName()).isEmpty();
       assertThat(l.getName()).isEqualTo("foo");
     }
+    {
+      Label l = Label.parseAbsolute("//@foo");
+      assertThat(l.getPackageIdentifier().getRepository().getName()).isEqualTo("@");
+      assertThat(l.getPackageName()).isEqualTo("@foo");
+      assertThat(l.getName()).isEqualTo("@foo");
+    }
+    {
+      Label l = Label.parseAbsolute("//xyz/@foo:abc");
+      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 {
@@ -363,16 +373,6 @@ public class LabelTest {
     Label.parseAbsolute("//$( ):$( )");
   }
 
-  /**
-   * 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",
@@ -442,8 +442,21 @@ public class LabelTest {
       Label.parseAbsolute("foo//bar/baz:bat/boo");
       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");
+      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 fcb187f268..3ec7d0146a 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 @@ public class LabelValidatorTest {
     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 @@ public class LabelValidatorTest {
         .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 @@ public class LabelValidatorTest {
         .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 e33d1f37c3..1506dfd0f3 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 @@ public class TargetPatternTest {
   @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 8cb1ba8fa2..8f0e8ecad7 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 @@ public class CcToolchainTest extends BuildViewTestCase {
   @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(