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/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index cc99514..0282489 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -1215,7 +1215,7 @@
// Check visibility attribute
for (PackageSpecification specification :
prerequisite.getProvider(VisibilityProvider.class).getVisibility()) {
- if (specification.containsPackage(rule.getLabel().getPackageFragment())) {
+ if (specification.containsPackage(rule.getLabel().getPackageIdentifier())) {
return true;
}
}
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 64851d6..13f795e 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
@@ -301,7 +301,7 @@
repo = input.substring(0, packageStartPos);
packageName = input.substring(packageStartPos + 2);
} else if (input.startsWith("@")) {
- throw new LabelSyntaxException("invalid package name '" + input + "'");
+ throw new LabelSyntaxException("starts with a '@' but does not contain '//'");
} else if (packageStartPos == 0) {
repo = PackageIdentifier.DEFAULT_REPOSITORY;
packageName = input.substring(2);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageGroup.java b/src/main/java/com/google/devtools/build/lib/packages/PackageGroup.java
index 3695e36..67ea4ed 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageGroup.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageGroup.java
@@ -51,7 +51,7 @@
for (String containedPackage : packages) {
PackageSpecification specification = null;
try {
- specification = PackageSpecification.fromString(containedPackage);
+ specification = PackageSpecification.fromString(label, containedPackage);
} catch (PackageSpecification.InvalidPackageSpecificationException e) {
containsErrors = true;
eventHandler.handle(Event.error(location, e.getMessage()));
@@ -74,7 +74,7 @@
public boolean contains(Package pkg) {
for (PackageSpecification specification : packageSpecifications) {
- if (specification.containsPackage(pkg.getNameFragment())) {
+ if (specification.containsPackage(pkg.getPackageIdentifier())) {
return true;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageSpecification.java b/src/main/java/com/google/devtools/build/lib/packages/PackageSpecification.java
index c81123a..fb2073e 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageSpecification.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageSpecification.java
@@ -13,8 +13,10 @@
// limitations under the License.
package com.google.devtools.build.lib.packages;
+import com.google.common.base.Verify;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.cmdline.LabelValidator;
+import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.vfs.PathFragment;
/**
@@ -24,10 +26,14 @@
private static final String PACKAGE_LABEL = "__pkg__";
private static final String SUBTREE_LABEL = "__subpackages__";
private static final String ALL_BENEATH_SUFFIX = "/...";
- public static final PackageSpecification EVERYTHING =
- new AllPackagesBeneath(new PathFragment(""));
+ public static final PackageSpecification EVERYTHING = new PackageSpecification() {
+ @Override
+ public boolean containsPackage(PackageIdentifier packageName) {
+ return true;
+ }
+ };
- public abstract boolean containsPackage(PathFragment packageName);
+ public abstract boolean containsPackage(PackageIdentifier packageName);
@Override
public int hashCode() {
@@ -53,39 +59,39 @@
*
* <p>Note that these strings are different from what {@link #fromLabel} understands.
*/
- public static PackageSpecification fromString(final String spec)
+ public static PackageSpecification fromString(Label context, final String spec)
throws InvalidPackageSpecificationException {
String result = spec;
boolean allBeneath = false;
-
- if (result.startsWith("//")) {
- result = spec.substring(2);
- } else {
- throw new InvalidPackageSpecificationException("invalid package label: " + spec);
- }
-
- if (result.indexOf(':') >= 0) {
- throw new InvalidPackageSpecificationException("invalid package label: " + spec);
- }
-
- if (result.equals("...")) {
- // Special case: //... will not end in /...
- return EVERYTHING;
- }
-
if (result.endsWith(ALL_BENEATH_SUFFIX)) {
allBeneath = true;
result = result.substring(0, result.length() - ALL_BENEATH_SUFFIX.length());
+ if (result.equals("/")) {
+ // Special case: //... will not end in /...
+ return EVERYTHING;
+ }
}
- String errorMessage = LabelValidator.validatePackageName(result);
- if (errorMessage == null) {
- return allBeneath ?
- new AllPackagesBeneath(new PathFragment(result)) :
- new SinglePackage(new PathFragment(result));
- } else {
- throw new InvalidPackageSpecificationException(errorMessage);
+ if (!spec.startsWith("//")) {
+ throw new InvalidPackageSpecificationException("invalid package name '" + spec
+ + "': must start with '//'");
}
+
+ PackageIdentifier packageId;
+ try {
+ packageId = PackageIdentifier.parse(result);
+ } catch (LabelSyntaxException e) {
+ throw new InvalidPackageSpecificationException(
+ "invalid package name '" + spec + "': " + e.getMessage());
+ }
+
+ Verify.verify(packageId.getRepository().isDefault());
+ packageId = PackageIdentifier.create(
+ context.getPackageIdentifier().getRepository(), packageId.getPackageFragment());
+
+ return allBeneath ?
+ new AllPackagesBeneath(packageId) :
+ new SinglePackage(packageId);
}
/**
@@ -95,23 +101,23 @@
*/
public static PackageSpecification fromLabel(Label label) {
if (label.getName().equals(PACKAGE_LABEL)) {
- return new SinglePackage(label.getPackageFragment());
+ return new SinglePackage(label.getPackageIdentifier());
} else if (label.getName().equals(SUBTREE_LABEL)) {
- return new AllPackagesBeneath(label.getPackageFragment());
+ return new AllPackagesBeneath(label.getPackageIdentifier());
} else {
return null;
}
}
private static class SinglePackage extends PackageSpecification {
- private PathFragment singlePackageName;
+ private PackageIdentifier singlePackageName;
- public SinglePackage(PathFragment packageName) {
+ public SinglePackage(PackageIdentifier packageName) {
this.singlePackageName = packageName;
}
@Override
- public boolean containsPackage(PathFragment packageName) {
+ public boolean containsPackage(PackageIdentifier packageName) {
return this.singlePackageName.equals(packageName);
}
@@ -122,15 +128,16 @@
}
private static class AllPackagesBeneath extends PackageSpecification {
- private PathFragment prefix;
+ private PackageIdentifier prefix;
- public AllPackagesBeneath(PathFragment prefix) {
+ public AllPackagesBeneath(PackageIdentifier prefix) {
this.prefix = prefix;
}
@Override
- public boolean containsPackage(PathFragment packageName) {
- return packageName.startsWith(prefix);
+ public boolean containsPackage(PackageIdentifier packageName) {
+ return packageName.getRepository().equals(prefix.getRepository())
+ && packageName.getPackageFragment().startsWith(prefix.getPackageFragment());
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryVisibility.java b/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryVisibility.java
index 92be032..427ea84 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryVisibility.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryVisibility.java
@@ -29,7 +29,7 @@
@Override
public boolean contains(Target target) {
- return packageSpecification.containsPackage(target.getPackage().getNameFragment());
+ return packageSpecification.containsPackage(target.getLabel().getPackageIdentifier());
}
@Override
