Make intra-package wildcards work for remote repositories and clean up target pattern parsing just a tiny little bit.
This wounds #389 dealing 4d6 fire damage (recursive wildcards, e.g. /... and friends still don't work)
--
MOS_MIGRATED_REVID=103822319
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java
index 6dc2385..d456059 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java
@@ -22,6 +22,8 @@
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.cmdline.LabelValidator.BadLabelException;
import com.google.devtools.build.lib.cmdline.LabelValidator.PackageAndTarget;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier.RepositoryName;
+import com.google.devtools.build.lib.util.StringUtilities;
import java.io.Serializable;
import java.util.ArrayList;
@@ -197,7 +199,7 @@
Preconditions.checkArgument(excludedSubdirectories.isEmpty(),
"Target pattern \"%s\" of type %s cannot be evaluated with excluded subdirectories: %s.",
getOriginalPattern(), getType(), excludedSubdirectories);
- return resolver.getExplicitTarget(targetName);
+ return resolver.getExplicitTarget(label(targetName));
}
@Override
@@ -251,7 +253,7 @@
getOriginalPattern(), getType(), excludedSubdirectories);
if (resolver.isPackage(PackageIdentifier.createInDefaultRepo(path))) {
// User has specified a package name. lookout for default target.
- return resolver.getExplicitTarget("//" + path);
+ return resolver.getExplicitTarget(label("//" + path));
}
List<String> pieces = SLASH_SPLITTER.splitToList(path);
@@ -262,7 +264,7 @@
String packageName = SLASH_JOINER.join(pieces.subList(0, i));
if (resolver.isPackage(PackageIdentifier.createInDefaultRepo(packageName))) {
String targetName = SLASH_JOINER.join(pieces.subList(i, pieces.size()));
- return resolver.getExplicitTarget("//" + packageName + ":" + targetName);
+ return resolver.getExplicitTarget(label("//" + packageName + ":" + targetName));
}
}
@@ -305,16 +307,16 @@
private static final class TargetsInPackage extends TargetPattern {
- private final String pattern;
+ private final PackageIdentifier packageIdentifier;
private final String suffix;
private final boolean isAbsolute;
private final boolean rulesOnly;
private final boolean checkWildcardConflict;
- private TargetsInPackage(String originalPattern, String pattern, String suffix,
- boolean isAbsolute, boolean rulesOnly, boolean checkWildcardConflict) {
+ private TargetsInPackage(String originalPattern, PackageIdentifier packageIdentifier,
+ String suffix, boolean isAbsolute, boolean rulesOnly, boolean checkWildcardConflict) {
super(Type.TARGETS_IN_PACKAGE, originalPattern);
- this.pattern = Preconditions.checkNotNull(pattern);
+ this.packageIdentifier = packageIdentifier;
this.suffix = Preconditions.checkNotNull(suffix);
this.isAbsolute = isAbsolute;
this.rulesOnly = rulesOnly;
@@ -335,17 +337,7 @@
}
}
- String packageName = removeSuffix(pattern, suffix);
-
- try {
- PackageIdentifier.parse(packageName);
- } catch (LabelSyntaxException e) {
- throw new TargetParsingException(
- "Invalid package name '" + packageName + "': " + e.getMessage());
- }
-
- return resolver.getTargetsInPackage(getOriginalPattern(),
- PackageIdentifier.createInDefaultRepo(packageName), rulesOnly);
+ return resolver.getTargetsInPackage(getOriginalPattern(), packageIdentifier, rulesOnly);
}
@Override
@@ -355,7 +347,7 @@
@Override
public String getDirectory() {
- return removeSuffix(pattern, suffix);
+ return packageIdentifier.getPackageFragment().getPathString();
}
@Override
@@ -375,13 +367,13 @@
return isAbsolute == that.isAbsolute && rulesOnly == that.rulesOnly
&& checkWildcardConflict == that.checkWildcardConflict
&& getOriginalPattern().equals(that.getOriginalPattern())
- && pattern.equals(that.pattern) && suffix.equals(that.suffix);
+ && packageIdentifier.equals(that.packageIdentifier) && suffix.equals(that.suffix);
}
@Override
public int hashCode() {
- return Objects.hash(getType(), getOriginalPattern(), pattern, suffix, isAbsolute, rulesOnly,
- checkWildcardConflict);
+ return Objects.hash(getType(), getOriginalPattern(), packageIdentifier, suffix, isAbsolute,
+ rulesOnly, checkWildcardConflict);
}
/**
@@ -397,18 +389,23 @@
return null;
}
- T target = resolver.getTargetOrNull("//" + pattern);
+ T target;
+ Label label;
+ try {
+ label = Label.create(packageIdentifier, suffix);
+ target = resolver.getTargetOrNull(label);
+ } catch (LabelSyntaxException e) {
+ return null;
+ }
+
if (target != null) {
- String name = pattern.lastIndexOf(':') != -1
- ? pattern.substring(pattern.lastIndexOf(':') + 1)
- : pattern.substring(pattern.lastIndexOf('/') + 1);
- resolver.warn(String.format("The Blaze target pattern '%s' is ambiguous: '%s' is " +
+ resolver.warn(String.format("The target pattern '%s' is ambiguous: '%s' is " +
"both a wildcard, and the name of an existing %s; " +
"using the latter interpretation",
- "//" + pattern, ":" + name,
+ getOriginalPattern(), ":" + suffix,
resolver.getTargetKind(target)));
try {
- return resolver.getExplicitTarget("//" + pattern);
+ return resolver.getExplicitTarget(label);
} catch (TargetParsingException e) {
throw new IllegalStateException(
"getTargetOrNull() returned non-null, so target should exist", e);
@@ -548,13 +545,18 @@
String originalPattern = pattern;
final boolean includesRepo = pattern.startsWith("@");
- String repoName = "";
+ RepositoryName repository = PackageIdentifier.DEFAULT_REPOSITORY_NAME;
if (includesRepo) {
int pkgStart = pattern.indexOf("//");
if (pkgStart < 0) {
throw new TargetParsingException("Couldn't find package in target " + pattern);
}
- repoName = pattern.substring(0, pkgStart);
+ try {
+ repository = RepositoryName.create(pattern.substring(0, pkgStart));
+ } catch (LabelSyntaxException e) {
+ throw new TargetParsingException(e.getMessage());
+ }
+
pattern = pattern.substring(pkgStart);
}
final boolean isAbsolute = pattern.startsWith("//");
@@ -598,19 +600,32 @@
}
if (ALL_RULES_IN_SUFFIXES.contains(targetPart)) {
+ PackageIdentifier packageIdentifier;
+ try {
+ packageIdentifier = PackageIdentifier.parse(repository.getName() + "//" + packagePart);
+ } catch (LabelSyntaxException e) {
+ throw new TargetParsingException(
+ "Invalid package name '" + packagePart + "': " + e.getMessage());
+ }
return new TargetsInPackage(
- originalPattern, pattern, ":" + targetPart, isAbsolute, true, true);
+ originalPattern, packageIdentifier, targetPart, isAbsolute, true, true);
}
if (ALL_TARGETS_IN_SUFFIXES.contains(targetPart)) {
+ PackageIdentifier packageIdentifier;
+ try {
+ packageIdentifier = PackageIdentifier.parse(repository.getName() + "//" + packagePart);
+ } catch (LabelSyntaxException e) {
+ throw new TargetParsingException(
+ "Invalid package name '" + packagePart + "': " + e.getMessage());
+ }
return new TargetsInPackage(
- originalPattern, pattern, ":" + targetPart, isAbsolute, false, true);
+ originalPattern, packageIdentifier, targetPart, isAbsolute, false, true);
}
-
if (includesRepo || isAbsolute || pattern.contains(":")) {
PackageAndTarget packageAndTarget;
- String fullLabel = repoName + "//" + pattern;
+ String fullLabel = repository.getName() + "//" + pattern;
try {
packageAndTarget = LabelValidator.validateAbsoluteLabel(fullLabel);
} catch (BadLabelException e) {
@@ -666,6 +681,18 @@
}
}
+ // Parse 'label' as a Label, mapping LabelSyntaxException into
+ // TargetParsingException.
+ private static Label label(String label) throws TargetParsingException {
+ try {
+ return Label.parseAbsolute(label);
+ } catch (LabelSyntaxException e) {
+ throw new TargetParsingException("invalid target format: '"
+ + StringUtilities.sanitizeControlChars(label) + "'; "
+ + StringUtilities.sanitizeControlChars(e.getMessage()));
+ }
+ }
+
/**
* The target pattern type (targets below package, in package, explicit target, etc.)
*/
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPatternResolver.java b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPatternResolver.java
index d106692..abcd516 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPatternResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPatternResolver.java
@@ -32,15 +32,15 @@
void warn(String msg);
/**
- * Returns a single target corresponding to the given name, or null. This method may only throw an
- * exception if the current thread was interrupted.
+ * Returns a single target corresponding to the given label, or null. This method may only throw
+ * an exception if the current thread was interrupted.
*/
- T getTargetOrNull(String targetName) throws InterruptedException;
+ T getTargetOrNull(Label label) throws InterruptedException;
/**
- * Returns a single target corresponding to the given name, or an empty or failed result.
+ * Returns a single target corresponding to the given label, or an empty or failed result.
*/
- ResolvedTargets<T> getExplicitTarget(String targetName)
+ ResolvedTargets<T> getExplicitTarget(Label label)
throws TargetParsingException, InterruptedException;
/**
@@ -83,8 +83,8 @@
throws TargetParsingException, InterruptedException;
/**
- * Returns true, if and only if the given name corresponds to a package, i.e., a file with the
- * name {@code packageName/BUILD} exists.
+ * Returns true, if and only if the given package identifier corresponds to a package, i.e., a
+ * file with the name {@code packageName/BUILD} exists in the appropriat repository.
*/
boolean isPackage(PackageIdentifier packageIdentifier);
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/TargetPatternResolverUtil.java b/src/main/java/com/google/devtools/build/lib/pkgcache/TargetPatternResolverUtil.java
index 60a0d64..0e86f7f 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/TargetPatternResolverUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/TargetPatternResolverUtil.java
@@ -14,8 +14,6 @@
package com.google.devtools.build.lib.pkgcache;
import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.LabelValidator;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.ResolvedTargets;
@@ -23,7 +21,6 @@
import com.google.devtools.build.lib.cmdline.TargetPatternResolver;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.util.StringUtilities;
import com.google.devtools.build.lib.vfs.PathFragment;
/**
@@ -34,26 +31,6 @@
// Utility class.
}
- // Parse 'label' as a Label, mapping LabelSyntaxException into
- // TargetParsingException.
- public static Label label(String label) throws TargetParsingException {
- try {
- return Label.parseAbsolute(label);
- } catch (LabelSyntaxException e) {
- throw invalidTarget(label, e.getMessage());
- }
- }
-
- /**
- * Returns a new exception indicating that a command-line target is invalid.
- */
- private static TargetParsingException invalidTarget(String packageName,
- String additionalMessage) {
- return new TargetParsingException("invalid target format: '" +
- StringUtilities.sanitizeControlChars(packageName) + "'; " +
- StringUtilities.sanitizeControlChars(additionalMessage));
- }
-
public static String getParsingErrorMessage(String message, String originalPattern) {
if (originalPattern == null) {
return message;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
index a48117f..34f5f04 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
@@ -131,7 +131,7 @@
}
@Override
- public Void getTargetOrNull(String targetName) throws InterruptedException {
+ public Void getTargetOrNull(Label label) throws InterruptedException {
// Note:
// This method is used in just one place, TargetPattern.TargetsInPackage#getWildcardConflict.
// Returning null tells #getWildcardConflict that there is not a target with a name like
@@ -142,9 +142,8 @@
}
@Override
- public ResolvedTargets<Void> getExplicitTarget(String targetName)
+ public ResolvedTargets<Void> getExplicitTarget(Label label)
throws TargetParsingException, InterruptedException {
- Label label = TargetPatternResolverUtil.label(targetName);
try {
Target target = packageProvider.getTarget(env.getListener(), label);
SkyKey key = TransitiveTraversalValue.key(target.getLabel());
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java
index 147a4e7..25af04e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java
@@ -15,7 +15,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.PackageIdentifier.RepositoryName;
import com.google.devtools.build.lib.cmdline.ResolvedTargets;
@@ -73,22 +72,20 @@
}
@Override
- public Target getTargetOrNull(String targetName) throws InterruptedException {
+ public Target getTargetOrNull(Label label) throws InterruptedException {
try {
- Label label = Label.parseAbsolute(targetName);
if (!isPackage(label.getPackageIdentifier())) {
return null;
}
return recursivePackageProvider.getTarget(eventHandler, label);
- } catch (LabelSyntaxException | NoSuchThingException e) {
+ } catch (NoSuchThingException e) {
return null;
}
}
@Override
- public ResolvedTargets<Target> getExplicitTarget(String targetName)
+ public ResolvedTargets<Target> getExplicitTarget(Label label)
throws TargetParsingException, InterruptedException {
- Label label = TargetPatternResolverUtil.label(targetName);
try {
Target target = recursivePackageProvider.getTarget(eventHandler, label);
return policy.shouldRetain(target, true)
diff --git a/src/test/shell/bazel/local_repository_test.sh b/src/test/shell/bazel/local_repository_test.sh
index 9fc7a76..e24149a 100755
--- a/src/test/shell/bazel/local_repository_test.sh
+++ b/src/test/shell/bazel/local_repository_test.sh
@@ -824,4 +824,28 @@
expect_log "Hello User"
}
+function test_package_wildcard_in_remote_repository() {
+ local r=$TEST_TMPDIR/r
+ rm -fr $r
+ mkdir -p $r/a
+ touch $r/{x,y,a/g,a/h}
+ cat > $r/BUILD <<EOF
+exports_files(["x", "y"])
+EOF
+
+ cat > $r/a/BUILD <<EOF
+exports_files(["g", "h"])
+EOF
+
+ cat > WORKSPACE <<EOF
+local_repository(name="r", path="$r")
+EOF
+
+ bazel query @r//:all-targets + @r//a:all-targets >& $TEST_log || fail "query failed"
+ expect_log "@r//:x"
+ expect_log "@r//:y"
+ expect_log "@r//a:g"
+ expect_log "@r//a:h"
+}
+
run_suite "local repository tests"
