Skip to content

Commit

Permalink
RepoContext & PackageContext for Label constructors
Browse files Browse the repository at this point in the history
RepoContext = RepositoryName + RepositoryMapping
PackageContext = PackageIdentifier + RepositoryMapping
               = RepositoryName + PathFragment + RepositoryMapping
               = RepoContext + PathFragment

Making these one object stresses the fact that they often need to be used together for correctness.

Work towards bazelbuild#14852

PiperOrigin-RevId: 458437385
Change-Id: I68c0f61ebfcfbb2f5ff929d985bdfa0ac45a912a
  • Loading branch information
Wyverald authored and copybara-github committed Jul 1, 2022
1 parent cfea5d3 commit 98853a7
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -960,8 +960,7 @@ public Label label(String labelString, StarlarkThread thread) throws EvalExcepti
BazelModuleContext moduleContext =
BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(thread));
try {
return Label.parseWithRepoContext(
labelString, moduleContext.label().getRepository(), moduleContext.repoMapping());
return Label.parseWithRepoContext(labelString, moduleContext.packageContext());
} catch (LabelSyntaxException e) {
throw Starlark.errorf("Illegal absolute label syntax: %s", e.getMessage());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,8 @@ public static BazelModuleContext create(
return new AutoValue_BazelModuleContext(
label, repoMapping, filename, loads, bzlTransitiveDigest);
}

public final Label.PackageContext packageContext() {
return Label.PackageContext.of(label().getPackageIdentifier(), repoMapping());
}
}
67 changes: 49 additions & 18 deletions src/main/java/com/google/devtools/build/lib/cmdline/Label.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import static com.google.devtools.build.lib.cmdline.LabelParser.validateAndProcessTargetName;

import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;
import com.google.common.collect.ComparisonChain;
import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -80,6 +81,37 @@ public final class Label implements Comparable<Label>, StarlarkValue, SkyKey, Co

private static final Interner<Label> LABEL_INTERNER = BlazeInterners.newWeakInterner();

/** The context of a current repo, necessary to parse a repo-relative label ("//foo:bar"). */
public interface RepoContext {
static RepoContext of(RepositoryName currentRepo, RepositoryMapping repoMapping) {
return new AutoValue_Label_RepoContextImpl(currentRepo, repoMapping);
}

RepositoryName currentRepo();

RepositoryMapping repoMapping();
}

@AutoValue
abstract static class RepoContextImpl implements RepoContext {}

/** The context of a current package, necessary to parse a package-relative label (":foo"). */
public interface PackageContext extends RepoContext {
static PackageContext of(PackageIdentifier currentPackage, RepositoryMapping repoMapping) {
return new AutoValue_Label_PackageContextImpl(
currentPackage.getRepository(), repoMapping, currentPackage.getPackageFragment());
}

PathFragment packageFragment();

default PackageIdentifier packageIdentifier() {
return PackageIdentifier.create(currentRepo(), packageFragment());
}
}

@AutoValue
abstract static class PackageContextImpl implements PackageContext {}

/**
* Parses a raw label string that contains the canonical form of a label. It must be of the form
* {@code [@repo]//foo/bar[:quux]}. If the {@code @repo} part is present, it must be a canonical
Expand All @@ -96,26 +128,28 @@ public static Label parseCanonical(String raw) throws LabelSyntaxException {

/** Computes the repo name for the label, within the context of a current repo. */
private static RepositoryName computeRepoNameWithRepoContext(
Parts parts, RepositoryName currentRepo, RepositoryMapping repoMapping) {
Parts parts, RepoContext repoContext) {
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) ? RepositoryName.MAIN : currentRepo;
return ABSOLUTE_PACKAGE_NAMES.contains(parts.pkg)
? RepositoryName.MAIN
: repoContext.currentRepo();
}
return repoMapping.get(parts.repo);
return repoContext.repoMapping().get(parts.repo);
}

/**
* Parses a raw label string within the context of a current repo. It must be of the form {@code
* [@repo]//foo/bar[:quux]}. If the {@code @repo} part is present, it will undergo {@code
* repoMapping}, otherwise the label will be assumed to be in {@code currentRepo}.
* repoContext.repoMapping()}, otherwise the label will be assumed to be in {@code
* repoContext.currentRepo()}.
*/
public static Label parseWithRepoContext(
String raw, RepositoryName currentRepo, RepositoryMapping repoMapping)
public static Label parseWithRepoContext(String raw, RepoContext repoContext)
throws LabelSyntaxException {
Parts parts = Parts.parse(raw);
parts.checkPkgIsAbsolute();
RepositoryName repoName = computeRepoNameWithRepoContext(parts, currentRepo, repoMapping);
RepositoryName repoName = computeRepoNameWithRepoContext(parts, repoContext);
return createUnvalidated(
PackageIdentifier.create(repoName, PathFragment.create(parts.pkg)), parts.target);
}
Expand All @@ -124,23 +158,19 @@ public static Label parseWithRepoContext(
* Parses a raw label string within the context of a current package. It can be of a
* package-relative form ({@code :quux}). Otherwise, it must be of the form {@code
* [@repo]//foo/bar[:quux]}. If the {@code @repo} part is present, it will undergo {@code
* repoMapping}, otherwise the label will be assumed to be in the repo of {@code
* packageIdentifier}.
* packageContext.repoMapping()}, otherwise the label will be assumed to be in the repo of {@code
* packageContext.currentRepo()}.
*/
public static Label parseWithPackageContext(
String raw, PackageIdentifier packageIdentifier, RepositoryMapping repoMapping)
public static Label parseWithPackageContext(String raw, PackageContext packageContext)
throws LabelSyntaxException {
Parts parts = Parts.parse(raw);
// pkg is either absolute or empty
if (!parts.pkg.isEmpty()) {
parts.checkPkgIsAbsolute();
}
RepositoryName repoName =
computeRepoNameWithRepoContext(parts, packageIdentifier.getRepository(), repoMapping);
RepositoryName repoName = computeRepoNameWithRepoContext(parts, packageContext);
PathFragment pkgFragment =
parts.pkgIsAbsolute
? PathFragment.create(parts.pkg)
: packageIdentifier.getPackageFragment();
parts.pkgIsAbsolute ? PathFragment.create(parts.pkg) : packageContext.packageFragment();
return createUnvalidated(PackageIdentifier.create(repoName, pkgFragment), parts.target);
}

Expand Down Expand Up @@ -168,7 +198,7 @@ public static Label parseWithPackageContext(
public static Label parseAbsolute(String absName, RepositoryMapping repositoryMapping)
throws LabelSyntaxException {
Preconditions.checkNotNull(repositoryMapping);
return parseWithRepoContext(absName, RepositoryName.MAIN, repositoryMapping);
return parseWithRepoContext(absName, RepoContext.of(RepositoryName.MAIN, repositoryMapping));
}

// TODO(b/200024947): Remove this.
Expand Down Expand Up @@ -480,7 +510,8 @@ public Label getRelativeWithRemapping(String relName, RepositoryMapping reposito
if (relName.isEmpty()) {
throw new LabelSyntaxException("empty package-relative label");
}
return parseWithPackageContext(relName, packageIdentifier, repositoryMapping);
return parseWithPackageContext(
relName, PackageContext.of(packageIdentifier, repositoryMapping));
}

// TODO(b/200024947): Remove this.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,24 @@ public class LabelConverter {
public static LabelConverter forBzlEvaluatingThread(StarlarkThread thread) {
BazelModuleContext moduleContext =
BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(thread));
return new LabelConverter(
moduleContext.label().getPackageIdentifier(), moduleContext.repoMapping());
return new LabelConverter(moduleContext.packageContext());
}

private final PackageIdentifier base;
private final RepositoryMapping repositoryMapping;
private final Label.PackageContext packageContext;
private final Map<String, Label> labelCache = new HashMap<>();

public LabelConverter(Label.PackageContext packageContext) {
this.packageContext = packageContext;
}

/** Creates a label converter using the given base package and repo mapping. */
public LabelConverter(PackageIdentifier base, RepositoryMapping repositoryMapping) {
this.base = base;
this.repositoryMapping = repositoryMapping;
this(Label.PackageContext.of(base, repositoryMapping));
}

/** Returns the base package identifier that relative labels will be resolved against. */
PackageIdentifier getBasePackage() {
return base;
return packageContext.packageIdentifier();
}

/** Returns the Label corresponding to the input, using the current conversion context. */
Expand All @@ -65,14 +66,14 @@ public Label convert(String input) throws LabelSyntaxException {
// label-strings across all their attribute values.
Label converted = labelCache.get(input);
if (converted == null) {
converted = Label.parseWithPackageContext(input, base, repositoryMapping);
converted = Label.parseWithPackageContext(input, packageContext);
labelCache.put(input, converted);
}
return converted;
}

@Override
public String toString() {
return base.toString();
return getBasePackage().toString();
}
}

0 comments on commit 98853a7

Please sign in to comment.