Automated rollback of commit 5a9befc5602e71f7512074c303afbdcff5617cca.

RELNOTES: None

*** Reason for rollback ***

PiperOrigin-RevId: 201686843

4 files changed, 18 insertions, 37 deletions

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 43875b2791..4465a049f1 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,6 +31,8 @@ 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";
 
@@ -57,18 +59,6 @@ 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 {
@@ -374,6 +364,16 @@ 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",
@@ -443,21 +443,8 @@ public class LabelTest {
       Label.parseAbsolute("foo//bar/baz:bat/boo");
       fail();
     } catch (LabelSyntaxException e) {
-      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");
+      assertThat(e).hasMessage(
+          "invalid repository name 'foo': workspace names must start with '@'");
     }
   }
 
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 3ec7d0146a..fcb187f268 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,7 +74,6 @@ 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();
@@ -92,6 +91,8 @@ 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);
@@ -170,12 +171,6 @@ 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 1506dfd0f3..e33d1f37c3 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 8f0e8ecad7..8cb1ba8fa2 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,8 +499,7 @@ 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(