Skip to content

Commit

Permalink
More usages of Label.parseAbsolute to Label.parseCanonical
Browse files Browse the repository at this point in the history
These are either Google-internal-only usages (so no repo mapping needed), or parsing special labels such as `//external:X` or `//visibility:X`.

PiperOrigin-RevId: 459249803
Change-Id: I9584b5c1fd99f0a2b232951dbfbb6ae83af9c029
  • Loading branch information
Wyverald authored and copybara-github committed Jul 6, 2022
1 parent 89301f4 commit 8e03d82
Show file tree
Hide file tree
Showing 8 changed files with 23 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,14 @@ public Builder clearWorkspaceFileSuffixForTesting() {

@CanIgnoreReturnValue
public Builder setPrelude(String preludeLabelString) {
Preconditions.checkArgument(
preludeLabelString.startsWith("//"),
"Prelude label '%s' must start with '//'",
preludeLabelString);
try {
this.preludeLabel = Label.parseAbsolute(preludeLabelString, ImmutableMap.of());
// We're parsing this label as if it's in the main repository but it will actually get
// massaged into a label in the repository where the package being loaded resides.
this.preludeLabel = Label.parseCanonical(preludeLabelString);
} catch (LabelSyntaxException e) {
String errorMsg =
String.format("Prelude label '%s' is invalid: %s", preludeLabelString, e.getMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,14 @@ public static Label parseCanonical(String raw) throws LabelSyntaxException {
PackageIdentifier.create(repoName, PathFragment.create(parts.pkg)), parts.target);
}

public static Label parseCanonicalUnchecked(String raw) {
try {
return parseCanonical(raw);
} catch (LabelSyntaxException e) {
throw new IllegalArgumentException(e);
}
}

/** Computes the repo name for the label, within the context of a current repo. */
private static RepositoryName computeRepoNameWithRepoContext(
Parts parts, RepoContext repoContext) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import com.google.common.base.MoreObjects;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
Expand Down Expand Up @@ -45,8 +44,8 @@ public class ConstantRuleVisibility implements RuleVisibility {

static {
try {
PUBLIC_LABEL = Label.parseAbsolute("//visibility:public", ImmutableMap.of());
PRIVATE_LABEL = Label.parseAbsolute("//visibility:private", ImmutableMap.of());
PUBLIC_LABEL = Label.parseCanonical("//visibility:public");
PRIVATE_LABEL = Label.parseCanonical("//visibility:private");
} catch (LabelSyntaxException e) {
throw new IllegalStateException(e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import com.google.common.base.Strings;
import com.google.common.base.Verify;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
Expand Down Expand Up @@ -55,7 +54,7 @@ public static Rule createAndAddRepositoryRule(
overwriteRule(pkg, rule);
for (Map.Entry<String, Label> entry :
ruleClass.getExternalBindingsFunction().apply(rule).entrySet()) {
Label nameLabel = Label.parseAbsolute("//external:" + entry.getKey(), ImmutableMap.of());
Label nameLabel = Label.parseCanonical("//external:" + entry.getKey());
addBindRule(pkg, bindRuleClass, nameLabel, entry.getValue(), semantics, callstack);
}
// NOTE(wyv): What is this madness?? This is the only instance where a repository rule can
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public void bind(String name, Object actual, StarlarkThread thread)
throws EvalException, InterruptedException {
Label nameLabel;
try {
nameLabel = Label.parseAbsolute("//external:" + name, ImmutableMap.of());
nameLabel = Label.parseCanonical("//external:" + name);
} catch (LabelSyntaxException e) {
throw Starlark.errorf("%s", e.getMessage());
}
Expand All @@ -156,7 +156,7 @@ public void bind(String name, Object actual, StarlarkThread thread)
builder,
ruleClass,
nameLabel,
actual == NONE ? null : Label.parseAbsolute((String) actual, ImmutableMap.of()),
actual == NONE ? null : Label.parseCanonical((String) actual),
thread.getSemantics(),
thread.getCallStack());
} catch (InvalidRuleException | Package.NameConflictException | LabelSyntaxException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.io.BaseEncoding;
import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
Expand Down Expand Up @@ -206,7 +205,7 @@ private static boolean verifyLabelMarkerData(Rule rule, String key, String value
RootedPath rootedPath;
String fileKey = key.substring(5);
if (LabelValidator.isAbsolute(fileKey)) {
rootedPath = getRootedPathFromLabel(Label.parseAbsolute(fileKey, ImmutableMap.of()), env);
rootedPath = getRootedPathFromLabel(Label.parseCanonical(fileKey), env);
} else {
// TODO(pcloudy): Removing checking absolute path, they should all be absolute label.
PathFragment filePathFragment = PathFragment.create(fileKey);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1226,11 +1226,7 @@ private LoadedPackage loadPackage(
Label preludeLabel = null;
// Can be null in tests.
if (packageFactory != null) {
// Load the prelude from the same repository as the package being loaded. Can't use
// Label.resolveRepositoryRelative because rawPreludeLabel is in the main repository, not the
// default one, so it is resolved to itself.
// TODO(brandjon): Why can't we just replace the use of the main repository with the default
// repository in the prelude label?
// Load the prelude from the same repository as the package being loaded.
Label rawPreludeLabel = packageFactory.getRuleClassProvider().getPreludeLabel();
if (rawPreludeLabel != null) {
PackageIdentifier preludePackage =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,7 @@ public StarlarkCallable rule(
@Override
public Label label(String labelString, StarlarkThread thread) throws EvalException {
try {
return Label.parseAbsolute(
labelString,
/* repositoryMapping= */ ImmutableMap.of());
return Label.parseCanonical(labelString);
} catch (LabelSyntaxException e) {
throw Starlark.errorf("Illegal absolute label syntax: %s", labelString);
}
Expand Down

0 comments on commit 8e03d82

Please sign in to comment.