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 be40a4d900a81d..52b81fb0876a2c 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
@@ -120,11 +120,12 @@ abstract static class PackageContextImpl implements PackageContext {}
*/
public static Label parseCanonical(String raw) throws LabelSyntaxException {
Parts parts = Parts.parse(raw);
+ parts.checkPkgDoesNotEndWithTripleDots();
parts.checkPkgIsAbsolute();
RepositoryName repoName =
- parts.repo == null ? RepositoryName.MAIN : RepositoryName.createUnvalidated(parts.repo);
+ parts.repo() == null ? RepositoryName.MAIN : RepositoryName.createUnvalidated(parts.repo());
return createUnvalidated(
- PackageIdentifier.create(repoName, PathFragment.create(parts.pkg)), parts.target);
+ PackageIdentifier.create(repoName, PathFragment.create(parts.pkg())), parts.target());
}
/** Like {@link #parseCanonical}, but throws an unchecked exception instead. */
@@ -139,18 +140,18 @@ public static Label parseCanonicalUnchecked(String raw) {
/** Computes the repo name for the label, within the context of a current repo. */
private static RepositoryName computeRepoNameWithRepoContext(
Parts parts, RepoContext repoContext) {
- if (parts.repo == null) {
+ if (parts.repo() == null) {
// Certain package names when used without a "@" part are always absolutely in the main repo,
// disregarding the current repo and repo mappings.
- return ABSOLUTE_PACKAGE_NAMES.contains(parts.pkg)
+ return ABSOLUTE_PACKAGE_NAMES.contains(parts.pkg())
? RepositoryName.MAIN
: repoContext.currentRepo();
}
- if (parts.repoIsCanonical) {
+ if (parts.repoIsCanonical()) {
// This label uses the canonical label literal syntax starting with two @'s ("@@foo//bar").
- return RepositoryName.createUnvalidated(parts.repo);
+ return RepositoryName.createUnvalidated(parts.repo());
}
- return repoContext.repoMapping().get(parts.repo);
+ return repoContext.repoMapping().get(parts.repo());
}
/**
@@ -162,10 +163,11 @@ private static RepositoryName computeRepoNameWithRepoContext(
public static Label parseWithRepoContext(String raw, RepoContext repoContext)
throws LabelSyntaxException {
Parts parts = Parts.parse(raw);
+ parts.checkPkgDoesNotEndWithTripleDots();
parts.checkPkgIsAbsolute();
RepositoryName repoName = computeRepoNameWithRepoContext(parts, repoContext);
return createUnvalidated(
- PackageIdentifier.create(repoName, PathFragment.create(parts.pkg)), parts.target);
+ PackageIdentifier.create(repoName, PathFragment.create(parts.pkg())), parts.target());
}
/**
@@ -178,14 +180,15 @@ public static Label parseWithRepoContext(String raw, RepoContext repoContext)
public static Label parseWithPackageContext(String raw, PackageContext packageContext)
throws LabelSyntaxException {
Parts parts = Parts.parse(raw);
+ parts.checkPkgDoesNotEndWithTripleDots();
// pkg is either absolute or empty
- if (!parts.pkg.isEmpty()) {
+ if (!parts.pkg().isEmpty()) {
parts.checkPkgIsAbsolute();
}
RepositoryName repoName = computeRepoNameWithRepoContext(parts, packageContext);
PathFragment pkgFragment =
- parts.pkgIsAbsolute ? PathFragment.create(parts.pkg) : packageContext.packageFragment();
- return createUnvalidated(PackageIdentifier.create(repoName, pkgFragment), parts.target);
+ parts.pkgIsAbsolute() ? PathFragment.create(parts.pkg()) : packageContext.packageFragment();
+ return createUnvalidated(PackageIdentifier.create(repoName, pkgFragment), parts.target());
}
/**
@@ -200,7 +203,7 @@ public static Label parseWithPackageContext(String raw, PackageContext packageCo
public static Label create(String packageName, String targetName) throws LabelSyntaxException {
return createUnvalidated(
PackageIdentifier.parse(packageName),
- validateAndProcessTargetName(packageName, targetName));
+ validateAndProcessTargetName(packageName, targetName, /* pkgEndsWithTripleDots= */ false));
}
/**
@@ -211,7 +214,10 @@ public static Label create(PackageIdentifier packageId, String targetName)
throws LabelSyntaxException {
return createUnvalidated(
packageId,
- validateAndProcessTargetName(packageId.getPackageFragment().getPathString(), targetName));
+ validateAndProcessTargetName(
+ packageId.getPackageFragment().getPathString(),
+ targetName,
+ /* pkgEndsWithTripleDots= */ false));
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/LabelParser.java b/src/main/java/com/google/devtools/build/lib/cmdline/LabelParser.java
index 6dc3f6c8dff459..5666e1ab8fc55b 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/LabelParser.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/LabelParser.java
@@ -14,6 +14,8 @@
package com.google.devtools.build.lib.cmdline;
+import com.google.auto.value.AutoValue;
+import com.google.common.annotations.VisibleForTesting;
import com.google.devtools.build.lib.util.StringUtilities;
import com.google.errorprone.annotations.FormatMethod;
import javax.annotation.Nullable;
@@ -26,82 +28,83 @@ private LabelParser() {}
* Contains the parsed elements of a label string. The parts are validated (they don't contain
* invalid characters). See {@link #parse} for valid label patterns.
*/
- static final class Parts {
+ @AutoValue
+ abstract static class Parts {
/**
* The {@code @repo} or {@code @@canonical_repo} part of the string (sans any leading
* {@literal @}s); can be null if it doesn't have such a part (i.e. if it doesn't start with a
* {@literal @}).
*/
- @Nullable final String repo;
+ @Nullable
+ abstract String repo();
/**
* Whether the repo part is using the canonical repo syntax (two {@literal @}s) or not (one
* {@literal @}). If there is no repo part, this is false.
*/
- final boolean repoIsCanonical;
+ abstract boolean repoIsCanonical();
/**
* Whether the package part of the string is prefixed by double-slash. This can only be false if
* the repo part is missing.
*/
- final boolean pkgIsAbsolute;
- /** The package part of the string (sans double-slash, if any). */
- final String pkg;
+ abstract boolean pkgIsAbsolute();
+ /**
+ * The package part of the string (sans the leading double-slash, if present; also sans the
+ * final '...' segment, if present).
+ */
+ abstract String pkg();
+ /** Whether the package part of the string ends with a '...' segment. */
+ abstract boolean pkgEndsWithTripleDots();
/** The target part of the string (sans colon). */
- final String target;
+ abstract String target();
/** The original unparsed raw string. */
- final String raw;
+ abstract String raw();
- private Parts(
- @Nullable String repo,
- boolean repoIsCanonical,
- boolean pkgIsAbsolute,
- String pkg,
- String target,
- String raw) {
- this.repo = repo;
- this.repoIsCanonical = repoIsCanonical;
- this.pkgIsAbsolute = pkgIsAbsolute;
- this.pkg = pkg;
- this.target = target;
- this.raw = raw;
- }
-
- private static Parts validateAndCreate(
+ @VisibleForTesting
+ static Parts validateAndCreate(
@Nullable String repo,
boolean repoIsCanonical,
boolean pkgIsAbsolute,
String pkg,
+ boolean pkgEndsWithTripleDots,
String target,
String raw)
throws LabelSyntaxException {
validateRepoName(repo);
validatePackageName(pkg, target);
- return new Parts(
+ return new AutoValue_LabelParser_Parts(
repo,
repoIsCanonical,
pkgIsAbsolute,
pkg,
- validateAndProcessTargetName(pkg, target),
+ pkgEndsWithTripleDots,
+ validateAndProcessTargetName(pkg, target, pkgEndsWithTripleDots),
raw);
}
/**
* Parses a raw label string into parts. The logic can be summarized by the following table:
*
- * {@code
- * raw | repo | repoIsCanonical | pkgIsAbsolute | pkg | target
- * ----------------------+--------+-----------------+---------------+-----------+-----------
- * foo/bar | null | false | false | "" | "foo/bar"
- * //foo/bar | null | false | true | "foo/bar" | "bar"
- * @repo | "repo" | false | true | "" | "repo"
- * @@repo | "repo" | true | true | "" | "repo"
- * @repo//foo/bar | "repo" | false | true | "foo/bar" | "bar"
- * @@repo//foo/bar | "repo" | true | true | "foo/bar" | "bar"
- * :quux | null | false | false | "" | "quux"
- * foo/bar:quux | null | false | false | "foo/bar" | "quux"
- * //foo/bar:quux | null | false | true | "foo/bar" | "quux"
- * @repo//foo/bar:quux | "repo" | false | true | "foo/bar" | "quux"
- * @@repo//foo/bar:quux | "repo" | true | true | "foo/bar" | "quux"
- * }
+ * {@code
+ * raw | repo | repoIs- | pkgIs- | pkg | pkgEndsWith- | target
+ * | | Canonical | Absolute | | TripleDots |
+ * ----------------------+--------+-----------+----------+-----------+--------------+-----------
+ * "foo/bar" | null | false | false | "" | false | "foo/bar"
+ * "..." | null | false | false | "" | true | ""
+ * "...:all" | null | false | false | "" | true | "all"
+ * "foo/..." | null | false | false | "foo" | true | ""
+ * "//foo/bar" | null | false | true | "foo/bar" | false | "bar"
+ * "//foo/..." | null | false | true | "foo" | true | ""
+ * "//foo/...:all" | null | false | true | "foo" | true | "all"
+ * "//foo/all" | null | false | true | "foo/all" | false | "all"
+ * "@repo" | "repo" | false | true | "" | false | "repo"
+ * "@@repo" | "repo" | true | true | "" | false | "repo"
+ * "@repo//foo/bar" | "repo" | false | true | "foo/bar" | false | "bar"
+ * "@@repo//foo/bar" | "repo" | true | true | "foo/bar" | false | "bar"
+ * ":quux" | null | false | false | "" | false | "quux"
+ * "foo/bar:quux" | null | false | false | "foo/bar" | false | "quux"
+ * "//foo/bar:quux" | null | false | true | "foo/bar" | false | "quux"
+ * "@repo//foo/bar:quux" | "repo" | false | true | "foo/bar" | false | "quux"
+ * }
*/
static Parts parse(String rawLabel) throws LabelSyntaxException {
@Nullable final String repo;
@@ -116,9 +119,10 @@ static Parts parse(String rawLabel) throws LabelSyntaxException {
return validateAndCreate(
repo,
repoIsCanonical,
- /*pkgIsAbsolute=*/ true,
- /*pkg=*/ "",
- /*target=*/ repo,
+ /* pkgIsAbsolute= */ true,
+ /* pkg= */ "",
+ /* pkgEndsWithTripleDots= */ false,
+ /* target= */ repo,
rawLabel);
} else {
repo = rawLabel.substring(repoIsCanonical ? 2 : 1, doubleSlashIndex);
@@ -136,20 +140,39 @@ static Parts parse(String rawLabel) throws LabelSyntaxException {
final String pkg;
final String target;
final int colonIndex = rawLabel.indexOf(':', startOfPackage);
- if (colonIndex >= 0) {
- pkg = rawLabel.substring(startOfPackage, colonIndex);
- target = rawLabel.substring(colonIndex + 1);
- } else if (pkgIsAbsolute) {
- // Special case: the label "[@repo]//foo/bar" is synonymous with "[@repo]//foo/bar:bar".
- pkg = rawLabel.substring(startOfPackage);
- // The target name is the last package segment (works even if `pkg` contains no slash)
- target = pkg.substring(pkg.lastIndexOf('/') + 1);
- } else {
+ final String rawPkg =
+ rawLabel.substring(startOfPackage, colonIndex >= 0 ? colonIndex : rawLabel.length());
+ final boolean pkgEndsWithTripleDots = rawPkg.endsWith("/...") || rawPkg.equals("...");
+ if (colonIndex < 0 && pkgEndsWithTripleDots) {
+ // Special case: if the entire label ends in '...', the target name is empty.
+ pkg = stripTrailingTripleDots(rawPkg);
+ target = "";
+ } else if (colonIndex < 0 && !pkgIsAbsolute) {
// Special case: the label "foo/bar" is synonymous with ":foo/bar".
pkg = "";
target = rawLabel.substring(startOfPackage);
+ } else {
+ pkg = stripTrailingTripleDots(rawPkg);
+ if (colonIndex >= 0) {
+ target = rawLabel.substring(colonIndex + 1);
+ } else {
+ // Special case: the label "[@repo]//foo/bar" is synonymous with "[@repo]//foo/bar:bar".
+ // The target name is the last package segment (works even if `pkg` contains no slash)
+ target = pkg.substring(pkg.lastIndexOf('/') + 1);
+ }
+ }
+ return validateAndCreate(
+ repo, repoIsCanonical, pkgIsAbsolute, pkg, pkgEndsWithTripleDots, target, rawLabel);
+ }
+
+ private static String stripTrailingTripleDots(String pkg) {
+ if (pkg.endsWith("/...")) {
+ return pkg.substring(0, pkg.length() - 4);
}
- return validateAndCreate(repo, repoIsCanonical, pkgIsAbsolute, pkg, target, rawLabel);
+ if (pkg.equals("...")) {
+ return "";
+ }
+ return pkg;
}
private static void validateRepoName(@Nullable String repo) throws LabelSyntaxException {
@@ -167,8 +190,14 @@ private static void validatePackageName(String pkg, String target) throws LabelS
}
void checkPkgIsAbsolute() throws LabelSyntaxException {
- if (!pkgIsAbsolute) {
- throw syntaxErrorf("invalid label '%s': absolute label must begin with '@' or '//'", raw);
+ if (!pkgIsAbsolute()) {
+ throw syntaxErrorf("invalid label '%s': absolute label must begin with '@' or '//'", raw());
+ }
+ }
+
+ void checkPkgDoesNotEndWithTripleDots() throws LabelSyntaxException {
+ if (pkgEndsWithTripleDots()) {
+ throw syntaxErrorf("invalid label '%s': package name cannot contain '...'", raw());
}
}
}
@@ -183,8 +212,12 @@ private static String perhapsYouMeantMessage(String pkg, String target) {
return pkg.endsWith('/' + target) ? " (perhaps you meant \":" + target + "\"?)" : "";
}
- static String validateAndProcessTargetName(String pkg, String target)
- throws LabelSyntaxException {
+ static String validateAndProcessTargetName(
+ String pkg, String target, boolean pkgEndsWithTripleDots) throws LabelSyntaxException {
+ if (pkgEndsWithTripleDots && target.isEmpty()) {
+ // Allow empty target name if the package part ends in '...'.
+ return target;
+ }
String targetError = LabelValidator.validateTargetName(target);
if (targetError != null) {
throw syntaxErrorf(
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 86a12c07b8e5d6..b945c6198381b0 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
@@ -121,8 +121,8 @@ public static PackageIdentifier parse(String input) throws LabelSyntaxException
}
LabelParser.Parts parts = LabelParser.Parts.parse(input + ":dummy_target");
RepositoryName repoName =
- parts.repo == null ? RepositoryName.MAIN : RepositoryName.createUnvalidated(parts.repo);
- return create(repoName, PathFragment.create(parts.pkg));
+ parts.repo() == null ? RepositoryName.MAIN : RepositoryName.createUnvalidated(parts.repo());
+ return create(repoName, PathFragment.create(parts.pkg()));
}
public RepositoryName getRepository() {
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 d034efdb960f1f..d9886b3d60579d 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
@@ -41,7 +41,6 @@
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
-import java.util.regex.Pattern;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
@@ -850,12 +849,6 @@ public String toString() {
@Immutable
public static final class Parser {
- // A valid pattern either starts with exactly 0 slashes (relative pattern) or exactly two
- // slashes (absolute pattern).
- private static final Pattern VALID_SLASH_PREFIX = Pattern.compile("(//)?([^/]|$)");
-
- // TODO(bazel-team): Merge the Label functionality that requires similar constants into this
- // class.
/**
* The set of target-pattern suffixes which indicate wildcards over all rules in a
* single package.
@@ -869,34 +862,6 @@ public static final class Parser {
private static final ImmutableList ALL_TARGETS_IN_SUFFIXES =
ImmutableList.of("*", "all-targets");
- private static final List SUFFIXES;
-
- static {
- SUFFIXES =
- ImmutableList.builder()
- .addAll(ALL_RULES_IN_SUFFIXES)
- .addAll(ALL_TARGETS_IN_SUFFIXES)
- .add("/...")
- .build();
- }
-
- /**
- * Returns whether the given pattern is simple, i.e., not starting with '-' and using none of
- * the target matching suffixes.
- */
- public static boolean isSimpleTargetPattern(String pattern) {
- if (pattern.startsWith("-")) {
- return false;
- }
-
- for (String suffix : SUFFIXES) {
- if (pattern.endsWith(":" + suffix)) {
- return false;
- }
- }
- return true;
- }
-
/**
* Directory prefix to use when resolving relative labels (rather than absolute ones). For
* example, if the working directory is "/foo", then this should be "foo", which
@@ -930,117 +895,73 @@ public Parser(
* @throws TargetParsingException if the pattern is invalid
*/
public TargetPattern parse(String pattern) throws TargetParsingException {
- // The structure of this method is by cases, according to the usage string
- // constant (see lib/blaze/commands/target-syntax.txt).
-
- String originalPattern = pattern;
- final boolean includesRepo = pattern.startsWith("@");
- RepositoryName repository;
- if (!includesRepo) {
- repository = currentRepo;
- } else {
- int pkgStart = pattern.indexOf("//");
- if (pkgStart < 0) {
- throw new TargetParsingException(
- "Couldn't find package in target " + pattern, TargetPatterns.Code.PACKAGE_NOT_FOUND);
- }
- boolean isCanonicalRepoName = pattern.startsWith("@@");
- String repoPart = pattern.substring(isCanonicalRepoName ? 2 : 1, pkgStart);
- try {
- RepositoryName.validate(repoPart);
- } catch (LabelSyntaxException e) {
- throw new TargetParsingException(e.getMessage(), TargetPatterns.Code.LABEL_SYNTAX_ERROR);
- }
- if (isCanonicalRepoName) {
- repository = RepositoryName.createUnvalidated(repoPart);
- } else {
- repository = repoMapping.get(repoPart);
- if (!repository.isVisible()) {
- throw new TargetParsingException(
- String.format(
- "No repository visible as '@%s' from %s",
- repository.getName(), repository.getOwnerRepoDisplayString()),
- Code.PACKAGE_NOT_FOUND);
- }
- }
-
- pattern = pattern.substring(pkgStart);
- }
-
- if (!VALID_SLASH_PREFIX.matcher(pattern).lookingAt()) {
- throw new TargetParsingException(
- "not a valid absolute pattern (absolute target patterns "
- + "must start with exactly two slashes): '"
- + pattern
- + "'",
- TargetPatterns.Code.ABSOLUTE_TARGET_PATTERN_INVALID);
- }
-
- final boolean wasOriginallyAbsolute = pattern.startsWith("//");
- // We now ensure the relativeDirectory is applied to relative patterns.
- pattern = absolutize(pattern).substring(2);
-
- if (pattern.isEmpty()) {
- throw new TargetParsingException(
- "the empty string is not a valid target",
- TargetPatterns.Code.TARGET_CANNOT_BE_EMPTY_STRING);
- }
-
- int colonIndex = pattern.lastIndexOf(':');
- String packagePart = colonIndex < 0 ? pattern : pattern.substring(0, colonIndex);
- String targetPart = colonIndex < 0 ? "" : pattern.substring(colonIndex + 1);
-
- if (packagePart.equals("...")) {
- packagePart = "/..."; // special case this for easier parsing
+ LabelParser.Parts parts;
+ try {
+ parts = LabelParser.Parts.parse(pattern);
+ } catch (LabelSyntaxException e) {
+ throw new TargetParsingException(e.getMessage(), TargetPatterns.Code.LABEL_SYNTAX_ERROR);
}
- if (packagePart.endsWith("/")) {
- throw new TargetParsingException(
- "The package part of '" + originalPattern + "' should not end in a slash",
- TargetPatterns.Code.PACKAGE_PART_CANNOT_END_IN_SLASH);
+ // Special case: For a target pattern that just looks like `foo/bar/baz`, we treat this as a
+ // file path. LabelParser parses it as `:foo/bar/baz`, so we need to distinguish this case by
+ // checking if the original pattern contains a colon.
+ if (!parts.pkgIsAbsolute()
+ && currentRepo.isMain()
+ && parts.pkg().isEmpty()
+ && !parts.pkgEndsWithTripleDots()
+ && !pattern.contains(":")) {
+ return new InterpretPathAsTarget(
+ pattern, relativeDirectory.getRelative(parts.target()).getPathString());
}
- if (packagePart.endsWith("/...")) {
- String realPackagePart = packagePart.substring(0, packagePart.length() - "/...".length());
- PackageIdentifier packageIdentifier = createPackageIdentifier(repository, realPackagePart);
- if (targetPart.isEmpty() || ALL_RULES_IN_SUFFIXES.contains(targetPart)) {
- return new TargetsBelowDirectory(originalPattern, packageIdentifier, true);
- } else if (ALL_TARGETS_IN_SUFFIXES.contains(targetPart)) {
- return new TargetsBelowDirectory(originalPattern, packageIdentifier, false);
+ PackageIdentifier packageIdentifier = createPackageIdentifierFromParts(parts);
+ if (parts.pkgEndsWithTripleDots()) {
+ if (parts.target().isEmpty() || ALL_RULES_IN_SUFFIXES.contains(parts.target())) {
+ return new TargetsBelowDirectory(pattern, packageIdentifier, true);
+ } else if (ALL_TARGETS_IN_SUFFIXES.contains(parts.target())) {
+ return new TargetsBelowDirectory(pattern, packageIdentifier, false);
}
+ throw new TargetParsingException(
+ "Invalid target pattern " + pattern + ": '...' can only be used with wildcard targets",
+ Code.LABEL_SYNTAX_ERROR);
}
- if (ALL_RULES_IN_SUFFIXES.contains(targetPart)) {
+ if (pattern.contains(":") && ALL_RULES_IN_SUFFIXES.contains(parts.target())) {
return new TargetsInPackage(
- originalPattern,
- createPackageIdentifier(repository, packagePart),
- targetPart,
- wasOriginallyAbsolute,
- true);
+ pattern, packageIdentifier, parts.target(), parts.pkgIsAbsolute(), true);
}
- if (ALL_TARGETS_IN_SUFFIXES.contains(targetPart)) {
+ if (pattern.contains(":") && ALL_TARGETS_IN_SUFFIXES.contains(parts.target())) {
return new TargetsInPackage(
- originalPattern,
- createPackageIdentifier(repository, packagePart),
- targetPart,
- wasOriginallyAbsolute,
- false);
+ pattern, packageIdentifier, parts.target(), parts.pkgIsAbsolute(), false);
}
- if (includesRepo || !repository.isMain() || wasOriginallyAbsolute || pattern.contains(":")) {
- Label label;
- try {
- label = Label.parseCanonical(repository.getNameWithAt() + "//" + pattern);
- } catch (LabelSyntaxException e) {
+ return new SingleTarget(pattern, Label.createUnvalidated(packageIdentifier, parts.target()));
+ }
+
+ private PackageIdentifier createPackageIdentifierFromParts(LabelParser.Parts parts)
+ throws TargetParsingException {
+ RepositoryName repo;
+ if (parts.repo() == null) {
+ repo = currentRepo;
+ } else if (parts.repoIsCanonical()) {
+ repo = RepositoryName.createUnvalidated(parts.repo());
+ } else {
+ repo = repoMapping.get(parts.repo());
+ if (!repo.isVisible()) {
throw new TargetParsingException(
- "invalid target format '" + originalPattern + "': " + e.getMessage(),
- TargetPatterns.Code.TARGET_FORMAT_INVALID);
+ String.format(
+ "No repository visible as '@%s' from %s",
+ repo.getName(), repo.getOwnerRepoDisplayString()),
+ Code.PACKAGE_NOT_FOUND);
}
- return new SingleTarget(originalPattern, label);
}
- return new InterpretPathAsTarget(originalPattern, pattern);
+ PathFragment packagePathFragment =
+ parts.pkgIsAbsolute()
+ ? PathFragment.create(parts.pkg())
+ : relativeDirectory.getRelative(parts.pkg());
+ return PackageIdentifier.create(repo, packagePathFragment);
}
public RepositoryMapping getRepoMapping() {
@@ -1055,16 +976,6 @@ public PathFragment getRelativeDirectory() {
return relativeDirectory;
}
- private PackageIdentifier createPackageIdentifier(RepositoryName repoName, String pkg)
- throws TargetParsingException {
- String pkgError = LabelValidator.validatePackageName(pkg);
- if (pkgError != null) {
- throw new TargetParsingException(
- "Invalid package name '" + pkg + "': " + pkgError, Code.LABEL_SYNTAX_ERROR);
- }
- return PackageIdentifier.create(repoName, PathFragment.create(pkg));
- }
-
/**
* Parses a constant string TargetPattern, throwing IllegalStateException on invalid pattern.
*/
diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/LabelParserTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/LabelParserTest.java
new file mode 100644
index 00000000000000..3039f001e42d41
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/cmdline/LabelParserTest.java
@@ -0,0 +1,69 @@
+// Copyright 2023 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.cmdline;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.lib.cmdline.LabelParser.Parts.parse;
+import static com.google.devtools.build.lib.cmdline.LabelParser.Parts.validateAndCreate;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link LabelParser}. */
+@RunWith(JUnit4.class)
+public class LabelParserTest {
+
+ /** Checks that the javadoc of {@link LabelParser.Parts#parse} is accurate. */
+ @Test
+ public void parserTable() throws Exception {
+ assertThat(parse("foo/bar"))
+ .isEqualTo(validateAndCreate(null, false, false, "", false, "foo/bar", "foo/bar"));
+ assertThat(parse("...")).isEqualTo(validateAndCreate(null, false, false, "", true, "", "..."));
+ assertThat(parse("...:all"))
+ .isEqualTo(validateAndCreate(null, false, false, "", true, "all", "...:all"));
+ assertThat(parse("foo/..."))
+ .isEqualTo(validateAndCreate(null, false, false, "foo", true, "", "foo/..."));
+ assertThat(parse("//foo/bar"))
+ .isEqualTo(validateAndCreate(null, false, true, "foo/bar", false, "bar", "//foo/bar"));
+ assertThat(parse("//foo/..."))
+ .isEqualTo(validateAndCreate(null, false, true, "foo", true, "", "//foo/..."));
+ assertThat(parse("//foo/...:all"))
+ .isEqualTo(validateAndCreate(null, false, true, "foo", true, "all", "//foo/...:all"));
+ assertThat(parse("//foo/all"))
+ .isEqualTo(validateAndCreate(null, false, true, "foo/all", false, "all", "//foo/all"));
+ assertThat(parse("@repo"))
+ .isEqualTo(validateAndCreate("repo", false, true, "", false, "repo", "@repo"));
+ assertThat(parse("@@repo"))
+ .isEqualTo(validateAndCreate("repo", true, true, "", false, "repo", "@@repo"));
+ assertThat(parse("@repo//foo/bar"))
+ .isEqualTo(
+ validateAndCreate("repo", false, true, "foo/bar", false, "bar", "@repo//foo/bar"));
+ assertThat(parse("@@repo//foo/bar"))
+ .isEqualTo(
+ validateAndCreate("repo", true, true, "foo/bar", false, "bar", "@@repo//foo/bar"));
+ assertThat(parse(":quux"))
+ .isEqualTo(validateAndCreate(null, false, false, "", false, "quux", ":quux"));
+ assertThat(parse("foo/bar:quux"))
+ .isEqualTo(validateAndCreate(null, false, false, "foo/bar", false, "quux", "foo/bar:quux"));
+ assertThat(parse("//foo/bar:quux"))
+ .isEqualTo(
+ validateAndCreate(null, false, true, "foo/bar", false, "quux", "//foo/bar:quux"));
+ assertThat(parse("@repo//foo/bar:quux"))
+ .isEqualTo(
+ validateAndCreate(
+ "repo", false, true, "foo/bar", false, "quux", "@repo//foo/bar:quux"));
+ }
+}
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 eaeb528a698d57..69345ff84d69e2 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
@@ -55,6 +55,7 @@ public void validPatterns_mainRepo_atRepoRoot() throws TargetParsingException {
ImmutableMap.of("repo", RepositoryName.createUnvalidated("canonical_repo")),
RepositoryName.MAIN));
+ assertThat(parser.parse(":foo")).isEqualTo(new SingleTarget(":foo", label("@@//:foo")));
assertThat(parser.parse("foo:bar"))
.isEqualTo(new SingleTarget("foo:bar", label("@@//foo:bar")));
assertThat(parser.parse("foo:all"))
@@ -80,6 +81,8 @@ public void validPatterns_mainRepo_atRepoRoot() throws TargetParsingException {
assertThat(parser.parse("//..."))
.isEqualTo(new TargetsBelowDirectory("//...", pkg("@@//"), true));
+ assertThat(parser.parse("@repo"))
+ .isEqualTo(new SingleTarget("@repo", label("@@canonical_repo//:repo")));
assertThat(parser.parse("@repo//foo:bar"))
.isEqualTo(new SingleTarget("@repo//foo:bar", label("@@canonical_repo//foo:bar")));
assertThat(parser.parse("@repo//foo:all"))
@@ -90,6 +93,9 @@ public void validPatterns_mainRepo_atRepoRoot() throws TargetParsingException {
.isEqualTo(new SingleTarget("@repo//:bar", label("@@canonical_repo//:bar")));
assertThat(parser.parse("@repo//..."))
.isEqualTo(new TargetsBelowDirectory("@repo//...", pkg("@@canonical_repo//"), true));
+
+ assertThat(parser.parse("@@repo"))
+ .isEqualTo(new SingleTarget("@@repo", label("@@repo//:repo")));
assertThat(parser.parse("@@repo//foo:all"))
.isEqualTo(new TargetsInPackage("@@repo//foo:all", pkg("@@repo//foo"), "all", true, true));
assertThat(parser.parse("@@repo//:bar"))
@@ -106,6 +112,7 @@ public void validPatterns_mainRepo_inSomeRelativeDirectory() throws TargetParsin
ImmutableMap.of("repo", RepositoryName.createUnvalidated("canonical_repo")),
RepositoryName.MAIN));
+ assertThat(parser.parse(":foo")).isEqualTo(new SingleTarget(":foo", label("@@//base:foo")));
assertThat(parser.parse("foo:bar"))
.isEqualTo(new SingleTarget("foo:bar", label("@@//base/foo:bar")));
assertThat(parser.parse("foo:all"))
@@ -133,6 +140,8 @@ public void validPatterns_mainRepo_inSomeRelativeDirectory() throws TargetParsin
assertThat(parser.parse("//..."))
.isEqualTo(new TargetsBelowDirectory("//...", pkg("@@//"), true));
+ assertThat(parser.parse("@repo"))
+ .isEqualTo(new SingleTarget("@repo", label("@@canonical_repo//:repo")));
assertThat(parser.parse("@repo//foo:bar"))
.isEqualTo(new SingleTarget("@repo//foo:bar", label("@@canonical_repo//foo:bar")));
assertThat(parser.parse("@repo//foo:all"))
@@ -143,6 +152,9 @@ public void validPatterns_mainRepo_inSomeRelativeDirectory() throws TargetParsin
.isEqualTo(new SingleTarget("@repo//:bar", label("@@canonical_repo//:bar")));
assertThat(parser.parse("@repo//..."))
.isEqualTo(new TargetsBelowDirectory("@repo//...", pkg("@@canonical_repo//"), true));
+
+ assertThat(parser.parse("@@repo"))
+ .isEqualTo(new SingleTarget("@@repo", label("@@repo//:repo")));
assertThat(parser.parse("@@repo//foo:all"))
.isEqualTo(new TargetsInPackage("@@repo//foo:all", pkg("@@repo//foo"), "all", true, true));
assertThat(parser.parse("@@repo//:bar"))
@@ -159,6 +171,7 @@ public void validPatterns_nonMainRepo() throws TargetParsingException {
ImmutableMap.of("repo", RepositoryName.createUnvalidated("canonical_repo")),
RepositoryName.createUnvalidated("my_repo")));
+ assertThat(parser.parse(":foo")).isEqualTo(new SingleTarget(":foo", label("@@my_repo//:foo")));
assertThat(parser.parse("foo:bar"))
.isEqualTo(new SingleTarget("foo:bar", label("@@my_repo//foo:bar")));
assertThat(parser.parse("foo:all"))
@@ -167,15 +180,11 @@ public void validPatterns_nonMainRepo() throws TargetParsingException {
.isEqualTo(new TargetsBelowDirectory("foo/...:all", pkg("@@my_repo//foo"), true));
assertThat(parser.parse("foo:*"))
.isEqualTo(new TargetsInPackage("foo:*", pkg("@@my_repo//foo"), "*", false, false));
- // TODO(wyv): This is surprising. This means that in a non-main repo,
- // `register_toolchains("foo")` is equivalent to `r_t("//foo:foo")`, but in the main repo, it's
- // equivalent to `r_t("//:foo")` (which is the more intuitive behavior).
- // Granted, nobody probably ever writes this, but we should look into fixing it nonetheless.
- assertThat(parser.parse("foo")).isEqualTo(new SingleTarget("foo", label("@@my_repo//foo:foo")));
+ assertThat(parser.parse("foo")).isEqualTo(new SingleTarget("foo", label("@@my_repo//:foo")));
assertThat(parser.parse("..."))
.isEqualTo(new TargetsBelowDirectory("...", pkg("@@my_repo//"), true));
assertThat(parser.parse("foo/bar"))
- .isEqualTo(new SingleTarget("foo/bar", label("@@my_repo//foo/bar:bar")));
+ .isEqualTo(new SingleTarget("foo/bar", label("@@my_repo//:foo/bar")));
assertThat(parser.parse("//foo"))
.isEqualTo(new SingleTarget("//foo", label("@@my_repo//foo:foo")));
@@ -191,6 +200,8 @@ public void validPatterns_nonMainRepo() throws TargetParsingException {
assertThat(parser.parse("//..."))
.isEqualTo(new TargetsBelowDirectory("//...", pkg("@@my_repo//"), true));
+ assertThat(parser.parse("@repo"))
+ .isEqualTo(new SingleTarget("@repo", label("@@canonical_repo//:repo")));
assertThat(parser.parse("@repo//foo:bar"))
.isEqualTo(new SingleTarget("@repo//foo:bar", label("@@canonical_repo//foo:bar")));
assertThat(parser.parse("@repo//foo:all"))
@@ -201,6 +212,9 @@ public void validPatterns_nonMainRepo() throws TargetParsingException {
.isEqualTo(new SingleTarget("@repo//:bar", label("@@canonical_repo//:bar")));
assertThat(parser.parse("@repo//..."))
.isEqualTo(new TargetsBelowDirectory("@repo//...", pkg("@@canonical_repo//"), true));
+
+ assertThat(parser.parse("@@repo"))
+ .isEqualTo(new SingleTarget("@@repo", label("@@repo//:repo")));
assertThat(parser.parse("@@repo//foo:all"))
.isEqualTo(new TargetsInPackage("@@repo//foo:all", pkg("@@repo//foo"), "all", true, true));
assertThat(parser.parse("@@repo//:bar"))
@@ -210,7 +224,7 @@ public void validPatterns_nonMainRepo() throws TargetParsingException {
@Test
public void invalidPatterns() throws Exception {
ImmutableList badPatterns =
- ImmutableList.of("//Bar\\java", "", "@", "@foo", "@foo//", "@@", "@@foo");
+ ImmutableList.of("//Bar\\java", "", "/foo", "///foo", "@", "@foo//", "@@");
ImmutableMap repoMappingEntries =
ImmutableMap.of("repo", RepositoryName.createUnvalidated("canonical_repo"));
for (TargetPattern.Parser parser :
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
index 29ac1530eabd0a..403bf501c392b0 100644
--- a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
@@ -249,8 +249,7 @@ public void testMistypedTarget() {
assertThat(e)
.hasMessageThat()
.contains(
- "invalid target format 'foo//bar:missing': invalid package name 'foo//bar': package"
- + " names may not contain '//' path separators");
+ "invalid package name 'foo//bar': package names may not contain '//' path separators");
ParsingFailedEvent err = tester.findPostOnce(ParsingFailedEvent.class);
assertThat(err.getPattern()).isEqualTo("foo//bar:missing");
}
@@ -258,7 +257,7 @@ public void testMistypedTarget() {
@Test
public void testEmptyTarget() {
TargetParsingException e = assertThrows(TargetParsingException.class, () -> tester.load(""));
- assertThat(e).hasMessageThat().contains("the empty string is not a valid target");
+ assertThat(e).hasMessageThat().contains("invalid target name '': empty target name");
}
@Test
@@ -266,8 +265,7 @@ public void testMistypedTargetKeepGoing() throws Exception {
TargetPatternPhaseValue result = tester.loadKeepGoing("foo//bar:missing");
assertThat(result.hasError()).isTrue();
tester.assertContainsError(
- "invalid target format 'foo//bar:missing': invalid package name 'foo//bar': package names"
- + " may not contain '//' path separators");
+ "invalid package name 'foo//bar': package names may not contain '//' path separators");
ParsingFailedEvent err = tester.findPostOnce(ParsingFailedEvent.class);
assertThat(err.getPattern()).isEqualTo("foo//bar:missing");
}
@@ -1143,8 +1141,7 @@ private void expectError(String pattern, String message) {
public void testPatternWithSingleSlashIsError() {
expectError(
"/single/slash",
- "not a valid absolute pattern (absolute target patterns must start with exactly "
- + "two slashes): '/single/slash'");
+ "invalid target name '/single/slash': target names may not start with '/'");
}
@Test
@@ -1152,26 +1149,26 @@ public void testPatternWithSingleSlashIsErrorAndOffset() {
tester.setRelativeWorkingDirectory("base");
expectError(
"/single/slash",
- "not a valid absolute pattern (absolute target patterns must start with exactly "
- + "two slashes): '/single/slash'");
+ "invalid target name '/single/slash': target names may not start with '/'");
}
@Test
public void testPatternWithTripleSlashIsError() {
expectError(
"///triple/slash",
- "not a valid absolute pattern (absolute target patterns must start with exactly "
- + "two slashes): '///triple/slash'");
+ "invalid package name '/triple/slash': package names may not start with '/'");
}
@Test
public void testPatternEndingWithSingleSlashIsError() {
- expectError("foo/", "The package part of 'foo/' should not end in a slash");
+ expectError("foo/", "invalid target name 'foo/': target names may not end with '/'");
}
@Test
public void testPatternStartingWithDotDotSlash() {
- expectError("../foo", "couldn't determine target from filename '../foo'");
+ expectError(
+ "../foo",
+ "invalid target name '../foo': target names may not contain up-level references '..'");
}
private void runTestPackageLoadingError(boolean keepGoing, String... patterns) throws Exception {
diff --git a/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java b/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java
index 8c24a7e41a6e04..9fd8a59da6d62c 100644
--- a/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java
@@ -82,11 +82,6 @@ public abstract class AbstractQueryTest {
private static final String DEFAULT_UNIVERSE = "//...:*";
- protected static final String BAD_PACKAGE_NAME =
- "package names may contain "
- + "A-Z, a-z, 0-9, or any of ' !\"#$%&'()*+,-./;<=>?[]^_`{|}~' "
- + "(most 7-bit ascii characters except 0-31, 127, ':', or '\\')";
-
protected MockToolsConfig mockToolsConfig;
protected QueryHelper helper;
protected AnalysisMock analysisMock;
@@ -323,7 +318,7 @@ public void testBadTargetLiterals() throws Exception {
protected final void checkResultofBadTargetLiterals(String message, FailureDetail failureDetail) {
assertThat(failureDetail.getTargetPatterns().getCode())
.isEqualTo(TargetPatterns.Code.LABEL_SYNTAX_ERROR);
- assertThat(message).isEqualTo("Invalid package name 'bad:*': " + BAD_PACKAGE_NAME);
+ assertThat(message).isEqualTo("invalid target name '*:*': target names may not contain ':'");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
index 929c23a7bea459..93d2ce512af385 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
@@ -259,7 +259,7 @@ public void testRegisterToolchainsInvalidPattern() throws Exception {
EvaluationResult evaluationResult = eval(key);
Package pkg = evaluationResult.get(key).getPackage();
assertThat(pkg.containsErrors()).isTrue();
- assertContainsEvent("not a valid absolute pattern");
+ assertContainsEvent("error parsing target pattern");
}
@Test
diff --git a/src/test/shell/integration/loading_phase_test.sh b/src/test/shell/integration/loading_phase_test.sh
index 478410cd79427c..da44fb8589b24c 100755
--- a/src/test/shell/integration/loading_phase_test.sh
+++ b/src/test/shell/integration/loading_phase_test.sh
@@ -597,18 +597,6 @@ EOF
assert_contains "^${filename}$" $(bazel info "${PRODUCT_NAME}-bin")/$pkg/paths.txt
}
-function test_path_from_subdir() {
- local -r pkg="${FUNCNAME}"
- mkdir -p "$pkg/subdir" || fail "could not create \"$pkg/subdir\""
- touch "$pkg/subdir/BUILD" || fail "Could not touch"
- echo 'filegroup(name = "foo", srcs = [])' > "$pkg/BUILD" || fail "echo"
- cd "$pkg/subdir"
- bazel query '../BUILD + ../foo' >output 2> "$TEST_log" \
- || fail "Expected success"
- assert_contains "^//$pkg:BUILD" output
- assert_contains "^//$pkg:foo" output
-}
-
function test_target_with_BUILD() {
local -r pkg="${FUNCNAME}"
mkdir -p "$pkg" || fail "could not create \"$pkg\""
diff --git a/src/test/shell/integration/toolchain_test.sh b/src/test/shell/integration/toolchain_test.sh
index 24623e24840f8c..035802af91cde3 100755
--- a/src/test/shell/integration/toolchain_test.sh
+++ b/src/test/shell/integration/toolchain_test.sh
@@ -888,7 +888,7 @@ use_toolchain(
EOF
bazel build "//${pkg}/demo:use" &> $TEST_log && fail "Build failure expected"
- expect_log "error parsing target pattern \"/:invalid:label:syntax\": not a valid absolute pattern"
+ expect_log "error parsing target pattern \"/:invalid:label:syntax\": invalid package name '/': package names may not start with '/'"
}
function test_register_toolchain_error_invalid_target() {