Allow labels in the '--aspects' parameter.
--
MOS_MIGRATED_REVID=139573590
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index b29d2a2..5cd90d2 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -75,6 +75,9 @@
import com.google.devtools.build.lib.skyframe.SkyframeBuildView;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.syntax.EvalException;
+import com.google.devtools.build.lib.syntax.SkylarkImport;
+import com.google.devtools.build.lib.syntax.SkylarkImports;
+import com.google.devtools.build.lib.syntax.SkylarkImports.SkylarkImportSyntaxException;
import com.google.devtools.build.lib.util.OrderedSetMultimap;
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.util.RegexFilter;
@@ -465,7 +468,22 @@
// TODO(jfield): For consistency with Skylark loads, the aspect should be specified
// as an absolute path. Also, we probably need to do at least basic validation of
// path well-formedness here.
- PathFragment bzlFile = new PathFragment("/" + aspect.substring(0, delimiterPosition));
+ String bzlFileLoadLikeString = aspect.substring(0, delimiterPosition);
+ if (!bzlFileLoadLikeString.startsWith("//") && !bzlFileLoadLikeString.startsWith("@")) {
+ // "Legacy" behavior of '--aspects' parameter.
+ bzlFileLoadLikeString = new PathFragment("/" + bzlFileLoadLikeString).toString();
+ if (bzlFileLoadLikeString.endsWith(".bzl")) {
+ bzlFileLoadLikeString = bzlFileLoadLikeString.substring(0,
+ bzlFileLoadLikeString.length() - ".bzl".length());
+ }
+ }
+ SkylarkImport skylarkImport;
+ try {
+ skylarkImport = SkylarkImports.create(bzlFileLoadLikeString);
+ } catch (SkylarkImportSyntaxException e) {
+ throw new ViewCreationFailedException(
+ String.format("Invalid aspect '%s': %s", aspect, e.getMessage()), e);
+ }
String skylarkFunctionName = aspect.substring(delimiterPosition + 1);
for (TargetAndConfiguration targetSpec : topLevelTargetsWithConfigs) {
@@ -479,7 +497,7 @@
// aspect and the base target while the top-level configuration is untrimmed.
targetSpec.getConfiguration(),
targetSpec.getConfiguration(),
- bzlFile,
+ skylarkImport,
skylarkFunctionName));
}
} else {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
index 1d3b0fd..d4366fa 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
@@ -26,9 +26,8 @@
import com.google.devtools.build.lib.packages.AspectClass;
import com.google.devtools.build.lib.packages.AspectParameters;
import com.google.devtools.build.lib.packages.Package;
-import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.syntax.SkylarkImport;
import com.google.devtools.build.skyframe.SkyFunctionName;
-
import javax.annotation.Nullable;
/**
@@ -223,20 +222,20 @@
private final Label targetLabel;
private final BuildConfiguration aspectConfiguration;
private final BuildConfiguration targetConfiguration;
- private final PathFragment extensionFile;
+ private final SkylarkImport skylarkImport;
private final String skylarkValueName;
private SkylarkAspectLoadingKey(
Label targetLabel,
BuildConfiguration aspectConfiguration,
BuildConfiguration targetConfiguration,
- PathFragment extensionFile,
+ SkylarkImport skylarkImport,
String skylarkFunctionName) {
this.targetLabel = targetLabel;
this.aspectConfiguration = aspectConfiguration;
this.targetConfiguration = targetConfiguration;
- this.extensionFile = extensionFile;
+ this.skylarkImport = skylarkImport;
this.skylarkValueName = skylarkFunctionName;
}
@@ -245,16 +244,16 @@
return SkyFunctions.LOAD_SKYLARK_ASPECT;
}
- public PathFragment getExtensionFile() {
- return extensionFile;
+ public Label getTargetLabel() {
+ return targetLabel;
}
public String getSkylarkValueName() {
return skylarkValueName;
}
- public Label getTargetLabel() {
- return targetLabel;
+ public SkylarkImport getSkylarkImport() {
+ return skylarkImport;
}
/**
@@ -274,7 +273,35 @@
@Override
public String getDescription() {
// Skylark aspects are referred to on command line with <file>%<value ame>
- return String.format("%s%%%s of %s", extensionFile.toString(), skylarkValueName, targetLabel);
+ return String.format("%s%%%s of %s", skylarkImport.getImportString(),
+ skylarkValueName, targetLabel);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(targetLabel,
+ aspectConfiguration,
+ targetConfiguration,
+ skylarkImport,
+ skylarkValueName);
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (o == this) {
+ return true;
+ }
+
+ if (!(o instanceof SkylarkAspectLoadingKey)) {
+ return false;
+ }
+ SkylarkAspectLoadingKey that = (SkylarkAspectLoadingKey) o;
+ return Objects.equal(targetLabel, that.targetLabel)
+ && Objects.equal(aspectConfiguration, that.aspectConfiguration)
+ && Objects.equal(targetConfiguration, that.targetConfiguration)
+ && Objects.equal(skylarkImport, that.skylarkImport)
+ && Objects.equal(skylarkValueName, that.skylarkValueName);
+
}
}
@@ -350,9 +377,9 @@
Label targetLabel,
BuildConfiguration aspectConfiguration,
BuildConfiguration targetConfiguration,
- PathFragment skylarkFile,
+ SkylarkImport skylarkImport,
String skylarkExportName) {
return new SkylarkAspectLoadingKey(
- targetLabel, aspectConfiguration, targetConfiguration, skylarkFile, skylarkExportName);
+ targetLabel, aspectConfiguration, targetConfiguration, skylarkImport, skylarkExportName);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java
index 5d2c163..4d6dbf2 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java
@@ -14,8 +14,9 @@
package com.google.devtools.build.lib.skyframe;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.analysis.AspectDescriptor;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.Event;
@@ -24,12 +25,11 @@
import com.google.devtools.build.lib.skyframe.AspectFunction.AspectCreationException;
import com.google.devtools.build.lib.skyframe.AspectValue.SkylarkAspectLoadingKey;
import com.google.devtools.build.lib.skyframe.SkylarkImportLookupFunction.SkylarkImportFailedException;
-import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.syntax.SkylarkImport;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
-
import javax.annotation.Nullable;
/**
@@ -47,13 +47,17 @@
throws LoadSkylarkAspectFunctionException, InterruptedException {
SkylarkAspectLoadingKey aspectLoadingKey = (SkylarkAspectLoadingKey) skyKey.argument();
String skylarkValueName = aspectLoadingKey.getSkylarkValueName();
- PathFragment extensionFile = aspectLoadingKey.getExtensionFile();
+ SkylarkImport extensionFile = aspectLoadingKey.getSkylarkImport();
// Find label corresponding to skylark file, if one exists.
- ImmutableMap<PathFragment, Label> labelLookupMap;
+ ImmutableMap<String, Label> labelLookupMap;
try {
labelLookupMap =
- SkylarkImportLookupFunction.labelsForAbsoluteImports(ImmutableSet.of(extensionFile), env);
+ SkylarkImportLookupFunction.findLabelsForLoadStatements(
+ ImmutableList.of(extensionFile),
+ Label.parseAbsoluteUnchecked("//:empty"),
+ env
+ );
} catch (SkylarkImportFailedException e) {
env.getListener().handle(Event.error(e.getMessage()));
throw new LoadSkylarkAspectFunctionException(
@@ -63,16 +67,17 @@
return null;
}
- SkylarkAspect skylarkAspect = null;
+ SkylarkAspect skylarkAspect;
+ Label extensionFileLabel = Iterables.getOnlyElement(labelLookupMap.values());
try {
skylarkAspect = AspectFunction.loadSkylarkAspect(
- env, labelLookupMap.get(extensionFile), skylarkValueName);
+ env, extensionFileLabel, skylarkValueName);
if (skylarkAspect == null) {
return null;
}
if (!skylarkAspect.getParamAttributes().isEmpty()) {
throw new AspectCreationException("Cannot instantiate parameterized aspect "
- + skylarkAspect.getName() + " at the top level.", labelLookupMap.get(extensionFile));
+ + skylarkAspect.getName() + " at the top level.", extensionFileLabel);
}
} catch (AspectCreationException e) {
throw new LoadSkylarkAspectFunctionException(e);
@@ -101,4 +106,4 @@
super(cause, Transience.PERSISTENT);
}
}
-}
+}
\ No newline at end of file
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImport.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImport.java
index 95ddf96..e581436 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImport.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImport.java
@@ -17,6 +17,7 @@
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.Serializable;
+import javax.annotation.Nullable;
/**
* Encapsulates the four syntactic variants of Skylark imports: Absolute paths, relative
@@ -45,7 +46,7 @@
*
* @throws IllegalStateException if this import takes the form of an absolute path.
*/
- Label getLabel(Label containingFileLabel);
+ Label getLabel(@Nullable Label containingFileLabel);
/**
* True if this import takes the form of an absolute path.
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java
index 2910beb..44d4a9b 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java
@@ -19,6 +19,7 @@
import com.google.devtools.build.lib.cmdline.LabelValidator;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.vfs.PathFragment;
+import java.util.Objects;
/**
* Factory class for creating appropriate instances of {@link SkylarkImports}.
@@ -31,7 +32,11 @@
// Default implementation class for SkylarkImport.
private abstract static class SkylarkImportImpl implements SkylarkImport {
- protected String importString;
+ private final String importString;
+
+ protected SkylarkImportImpl(String importString) {
+ this.importString = importString;
+ }
@Override
public String getImportString() {
@@ -53,13 +58,32 @@
public PathFragment getAbsolutePath() {
throw new IllegalStateException("can't request absolute path from a non-absolute import");
}
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(getClass(), importString);
+ }
+
+ @Override
+ public boolean equals(Object that) {
+ if (this == that) {
+ return true;
+ }
+
+ if (!(that instanceof SkylarkImportImpl)) {
+ return false;
+ }
+
+ return Objects.equals(getClass(), that.getClass())
+ && Objects.equals(importString, ((SkylarkImportImpl) that).importString);
+ }
}
private static final class AbsolutePathImport extends SkylarkImportImpl {
private final PathFragment importPath;
private AbsolutePathImport(String importString, PathFragment importPath) {
- this.importString = importString;
+ super(importString);
this.importPath = importPath;
}
@@ -82,13 +106,14 @@
public PathFragment getAbsolutePath() {
return this.importPath;
}
+
}
private static final class RelativePathImport extends SkylarkImportImpl {
private final String importFile;
private RelativePathImport(String importString, String importFile) {
- this.importString = importString;
+ super(importString);
this.importFile = importFile;
}
@@ -113,13 +138,14 @@
throw new IllegalStateException(e);
}
}
+
}
private static final class AbsoluteLabelImport extends SkylarkImportImpl {
private final Label importLabel;
private AbsoluteLabelImport(String importString, Label importLabel) {
- this.importString = importString;
+ super(importString);
this.importLabel = importLabel;
}
@@ -134,13 +160,14 @@
// to the repo of the containing file.
return containingFileLabel.resolveRepositoryRelative(importLabel);
}
+
}
private static final class RelativeLabelImport extends SkylarkImportImpl {
private final String importTarget;
private RelativeLabelImport(String importString, String importTarget) {
- this.importString = importString;
+ super(importString);
this.importTarget = importTarget;
}
@@ -161,6 +188,7 @@
throw new IllegalStateException(e);
}
}
+
}
/**
