Skip to content

Commit

Permalink
Automated rollback of commit 24f6fe8.
Browse files Browse the repository at this point in the history 
*** Reason for rollback ***

Breakages in depot[1]

*** Original change description ***

Switch TargetPattern.Parser to use LabelParser

- Target patterns and labels have very similar syntax, yet are parsed completely separately. Furthermore, the code for target pattern parsing wasn't written with repos in mind, causing some corner cases such as bazelbuild#4385.
- This CL augments LabelParser to deal with some syntax specific to target patterns (mostly the '...' thing), and switches TargetPattern.Parser to use LabelParser instead.
- This fixes some in...

***

PiperOrigin-RevId: 521412117
Change-Id: I34f182cc9567678b07e45db82fbdafeab17e67a3
  • Loading branch information
@hvadehra @fweikert
hvadehra authored and fweikert committed May 25, 2023
1 parent d7b0191 commit e9cbbdb
Show file tree
Hide file tree
Showing 11 changed files with 252 additions and 265 deletions.
32 changes: 13 additions & 19 deletions src/main/java/com/google/devtools/build/lib/cmdline/Label.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,11 @@ 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. */
Expand All @@ -140,18 +139,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);
}

/**
Expand All @@ -163,11 +162,10 @@ 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);
}

/**
Expand All @@ -180,15 +178,14 @@ 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);
}

/**
Expand All @@ -203,7 +200,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, /* pkgEndsWithTripleDots= */ false));
validateAndProcessTargetName(packageName, targetName));
}

/**
Expand All @@ -214,10 +211,7 @@ public static Label create(PackageIdentifier packageId, String targetName)
throws LabelSyntaxException {
return createUnvalidated(
packageId,
validateAndProcessTargetName(
packageId.getPackageFragment().getPathString(),
targetName,
/* pkgEndsWithTripleDots= */ false));
validateAndProcessTargetName(packageId.getPackageFragment().getPathString(), targetName));
}

/**
Expand Down
149 changes: 58 additions & 91 deletions src/main/java/com/google/devtools/build/lib/cmdline/LabelParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

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;
Expand All @@ -28,83 +26,82 @@ 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.
*/
@AutoValue
abstract static class Parts {
static final 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
abstract String repo();
@Nullable final 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.
*/
abstract boolean repoIsCanonical();
final 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.
*/
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();
final boolean pkgIsAbsolute;
/** The package part of the string (sans double-slash, if any). */
final String pkg;
/** The target part of the string (sans colon). */
abstract String target();
final String target;
/** The original unparsed raw string. */
abstract String raw();
final String raw;

@VisibleForTesting
static Parts validateAndCreate(
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(
@Nullable String repo,
boolean repoIsCanonical,
boolean pkgIsAbsolute,
String pkg,
boolean pkgEndsWithTripleDots,
String target,
String raw)
throws LabelSyntaxException {
validateRepoName(repo);
validatePackageName(pkg, target);
return new AutoValue_LabelParser_Parts(
return new Parts(
repo,
repoIsCanonical,
pkgIsAbsolute,
pkg,
pkgEndsWithTripleDots,
validateAndProcessTargetName(pkg, target, pkgEndsWithTripleDots),
validateAndProcessTargetName(pkg, target),
raw);
}

/**
* Parses a raw label string into parts. The logic can be summarized by the following table:
*
* <pre>{@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"
* }</pre>
* {@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"
* }
*/
static Parts parse(String rawLabel) throws LabelSyntaxException {
@Nullable final String repo;
Expand All @@ -119,10 +116,9 @@ static Parts parse(String rawLabel) throws LabelSyntaxException {
return validateAndCreate(
repo,
repoIsCanonical,
/* pkgIsAbsolute= */ true,
/* pkg= */ "",
/* pkgEndsWithTripleDots= */ false,
/* target= */ repo,
/*pkgIsAbsolute=*/ true,
/*pkg=*/ "",
/*target=*/ repo,
rawLabel);
} else {
repo = rawLabel.substring(repoIsCanonical ? 2 : 1, doubleSlashIndex);
Expand All @@ -140,39 +136,20 @@ static Parts parse(String rawLabel) throws LabelSyntaxException {
final String pkg;
final String target;
final int colonIndex = rawLabel.indexOf(':', startOfPackage);
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) {
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 {
// 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);
}
if (pkg.equals("...")) {
return "";
}
return pkg;
return validateAndCreate(repo, repoIsCanonical, pkgIsAbsolute, pkg, target, rawLabel);
}

private static void validateRepoName(@Nullable String repo) throws LabelSyntaxException {
Expand All @@ -190,14 +167,8 @@ 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());
}
}

void checkPkgDoesNotEndWithTripleDots() throws LabelSyntaxException {
if (pkgEndsWithTripleDots()) {
throw syntaxErrorf("invalid label '%s': package name cannot contain '...'", raw());
if (!pkgIsAbsolute) {
throw syntaxErrorf("invalid label '%s': absolute label must begin with '@' or '//'", raw);
}
}
}
Expand All @@ -212,12 +183,8 @@ private static String perhapsYouMeantMessage(String pkg, String target) {
return pkg.endsWith('/' + target) ? " (perhaps you meant \":" + target + "\"?)" : "";
}

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;
}
static String validateAndProcessTargetName(String pkg, String target)
throws LabelSyntaxException {
String targetError = LabelValidator.validateTargetName(target);
if (targetError != null) {
throw syntaxErrorf(
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Loading

0 comments on commit e9cbbdb

Please sign in to comment.