Make package names in the package_group.packages attribute refer to the repository where the package group is.
There is currently no way to refer to packages in other repositories and that doesn't seem to be useful, because visibility currently checks the repository name in the label and that can be changed in the main WORKSPACE file. If needed, it'd be pretty easy to implement, though.
As a drive-by fix, made the parsing of the package name call into the same logic implemented in the cmdline package because code duplication is bad, mmmkay?
Fixes #767.
--
MOS_MIGRATED_REVID=112032508
diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
index c615b9d..d8d799a 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
@@ -811,7 +811,7 @@
@Test
public void testPackageGroupSpecBad() throws Exception {
- expectEvalError("invalid package label", "package_group(name='skin', packages=['--25:17--'])");
+ expectEvalError("invalid package name", "package_group(name='skin', packages=['--25:17--'])");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageGroupStaticInitializationTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageGroupStaticInitializationTest.java
index c527a10..8f993ab 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/PackageGroupStaticInitializationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/PackageGroupStaticInitializationTest.java
@@ -15,6 +15,7 @@
import static org.junit.Assert.assertFalse;
+import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.util.EventCollectionApparatus;
import com.google.devtools.build.lib.packages.util.PackageFactoryApparatus;
import com.google.devtools.build.lib.testutil.Scratch;
@@ -50,7 +51,8 @@
@Override
public void run() {
try {
- groupQueue.put(PackageSpecification.fromString("//fruits/..."));
+ groupQueue.put(PackageSpecification.fromString(
+ Label.parseAbsoluteUnchecked("//context"), "//fruits/..."));
} catch (Exception e) {
// Can't throw from Runnable, but this will cause the test to timeout
// when the consumer can't take the object.
diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageGroupTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageGroupTest.java
index f5c540a..24002f2 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/PackageGroupTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/PackageGroupTest.java
@@ -81,7 +81,31 @@
events.setFailFast(false);
getPackageGroup("fruits", "apple");
- events.assertContainsError("invalid package label: vegetables");
+ events.assertContainsError("invalid package name 'vegetables'");
+ }
+
+ @Test
+ public void testPackagesWithRepositoryDoNotWork() throws Exception {
+ scratch.file(
+ "fruits/BUILD",
+ "package_group(name = 'banana',",
+ " packages = ['@veggies//:cucumber'])");
+
+ events.setFailFast(false);
+ getPackageGroup("fruits", "banana");
+ events.assertContainsError("invalid package name '@veggies//:cucumber'");
+ }
+
+ @Test
+ public void testAllPackagesInMainRepositoryDoesNotWork() throws Exception {
+ scratch.file(
+ "fruits/BUILD",
+ "package_group(name = 'apple',",
+ " packages = ['@//...'])");
+
+ events.setFailFast(false);
+ getPackageGroup("fruits", "apple");
+ events.assertContainsError("invalid package name '@//...'");
}
@Test
@@ -96,7 +120,7 @@
events.setFailFast(false);
getPackageGroup("fruits", "apple");
- events.assertContainsError("invalid package label: //vegetables:carrot");
+ events.assertContainsError("invalid package name '//vegetables:carrot'");
}
@Test
@@ -109,7 +133,7 @@
events.setFailFast(false);
getPackageGroup("fruits", "apple");
- events.assertContainsError("invalid package label: :carrot");
+ events.assertContainsError("invalid package name ':carrot'");
}
@Test
