As promised in an earlier commit, remove subinclude machinery from PackageFactory, Package, PackageFunction, and also all things that make use of Package#getSubincludeLabels.
This code is completely dead, and has been for a while.
RELNOTES: None
PiperOrigin-RevId: 190486792
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index ae191e8..af885be 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -22,8 +22,6 @@
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
-import com.google.common.collect.Sets;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
@@ -169,11 +167,6 @@
private boolean containsErrors;
/**
- * The set of labels subincluded by this package.
- */
- private Set<Label> subincludes;
-
- /**
* The list of transitive closure of the Skylark file dependencies.
*/
private ImmutableList<Label> skylarkFileDependencies;
@@ -334,7 +327,6 @@
}
this.buildFile = builder.buildFile;
this.containsErrors = builder.containsErrors;
- this.subincludes = builder.subincludes.keySet();
this.skylarkFileDependencies = builder.skylarkFileDependencies;
this.defaultLicense = builder.defaultLicense;
this.defaultDistributionSet = builder.defaultDistributionSet;
@@ -346,13 +338,6 @@
}
/**
- * Returns the list of subincluded labels on which the validity of this package depends.
- */
- public Set<Label> getSubincludeLabels() {
- return subincludes;
- }
-
- /**
* Returns the list of transitive closure of the Skylark file dependencies of this package.
*/
public ImmutableList<Label> getSkylarkFileDependencies() {
@@ -764,7 +749,6 @@
protected Map<String, Target> targets = new HashMap<>();
protected Map<Label, EnvironmentGroup> environmentGroups = new HashMap<>();
- protected Map<Label, Path> subincludes = null;
protected ImmutableList<Label> skylarkFileDependencies = ImmutableList.of();
protected List<String> registeredExecutionPlatforms = new ArrayList<>();
@@ -1064,32 +1048,6 @@
implicitOutputsFunction);
}
- /**
- * Called by the parser when a "mocksubinclude" is encountered, to record the
- * mappings from labels to absolute paths upon which that the validity of
- * this package depends.
- */
- void addSubinclude(Label label, Path resolvedPath) {
- if (subincludes == null) {
- // This is a TreeMap because the order needs to be deterministic.
- subincludes = Maps.newTreeMap();
- }
-
- Path oldResolvedPath = subincludes.put(label, resolvedPath);
- if (oldResolvedPath != null && !oldResolvedPath.equals(resolvedPath)){
- // The same label should have been resolved to the same path
- throw new IllegalStateException("Ambiguous subinclude path");
- }
- }
-
- public Set<Label> getSubincludeLabels() {
- return subincludes == null ? Sets.<Label>newHashSet() : subincludes.keySet();
- }
-
- public Map<Label, Path> getSubincludes() {
- return subincludes == null ? Maps.<Label, Path>newHashMap() : subincludes;
- }
-
@Nullable
public Target getTarget(String name) {
return targets.get(name);
@@ -1301,10 +1259,6 @@
if (ioException != null) {
throw new NoSuchPackageException(getPackageIdentifier(), ioExceptionMessage, ioException);
}
- // Freeze subincludes.
- subincludes = (subincludes == null)
- ? Collections.<Label, Path>emptyMap()
- : Collections.unmodifiableMap(subincludes);
// We create the original BUILD InputFile when the package filename is set; however, the
// visibility may be overridden with an exports_files directive, so we need to obtain the
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index bcac22b..dce457b 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -591,51 +591,6 @@
}
/**
- * Returns a function value implementing the "mocksubinclude" function, emitted by the
- * PythonPreprocessor. We annotate the package with additional dependencies. (A 'real' subinclude
- * will never be seen by the parser, because the presence of "subinclude" triggers preprocessing.)
- */
- // TODO(b/35913039): Remove this and all references to 'mocksubinclude'.
- @SkylarkSignature(
- name = "mocksubinclude",
- returnType = Runtime.NoneType.class,
- doc = "implement the mocksubinclude function emitted by the PythonPreprocessor.",
- parameters = {
- @Param(name = "label", type = Object.class, doc = "a label designator."),
- @Param(name = "path", type = String.class, doc = "a path.")
- },
- documented = false,
- useLocation = true
- )
- private static final BuiltinFunction.Factory newMockSubincludeFunction =
- new BuiltinFunction.Factory("mocksubinclude") {
- public BuiltinFunction create(final PackageContext context) {
- return new BuiltinFunction("mocksubinclude", this) {
- public Runtime.NoneType invoke(Object labelO, String pathString, Location loc)
- throws ConversionException {
- Label label =
- BuildType.LABEL.convert(
- labelO, "'mocksubinclude' argument", context.pkgBuilder.getBuildFileLabel());
- Path path =
- pathString.isEmpty()
- ? null
- : context.pkgBuilder.getFilename().getRelative(pathString);
- // A subinclude within a package counts as a file declaration.
- if (label.getPackageIdentifier().equals(context.pkgBuilder.getPackageIdentifier())) {
- if (loc == null) {
- loc = Location.fromFile(context.pkgBuilder.getFilename());
- }
- context.pkgBuilder.createInputFileMaybe(label, loc);
- }
-
- context.pkgBuilder.addSubinclude(label, path);
- return Runtime.NONE;
- }
- };
- }
- };
-
- /**
* Returns a function value implementing "environment_group" in the specified package context.
* Syntax is as follows:
*
@@ -1573,7 +1528,6 @@
.setup("distribs", newDistribsFunction.apply(context))
.setup("glob", newGlobFunction.apply(context))
.setup("licenses", newLicensesFunction.apply(context))
- .setup("mocksubinclude", newMockSubincludeFunction.apply(context))
.setup("exports_files", newExportsFilesFunction.apply())
.setup("package_group", newPackageGroupFunction.apply())
.setup("package", newPackageFunction(packageArguments))
diff --git a/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java
index b7c0a1d..3c5bf30 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java
@@ -386,7 +386,6 @@
final QueryExpression caller,
ThreadSafeMutableSet<Target> nodes,
boolean buildFiles,
- boolean subincludes,
boolean loads)
throws QueryException {
ThreadSafeMutableSet<Target> dependentFiles = createThreadSafeMutableSet();
@@ -405,28 +404,25 @@
}
List<Label> extensions = new ArrayList<>();
- if (subincludes) {
- extensions.addAll(pkg.getSubincludeLabels());
- }
if (loads) {
extensions.addAll(pkg.getSkylarkFileDependencies());
}
- for (Label subinclude : extensions) {
+ for (Label extension : extensions) {
- Node<Target> subincludeTarget = getSubincludeTarget(subinclude, pkg);
- addIfUniqueLabel(subincludeTarget, seenLabels, dependentFiles);
+ Node<Target> loadTarget = getLoadTarget(extension, pkg);
+ addIfUniqueLabel(loadTarget, seenLabels, dependentFiles);
- // Also add the BUILD file of the subinclude.
+ // Also add the BUILD file of the extension.
if (buildFiles) {
- Path buildFileForSubinclude =
+ Path buildFileForLoad =
cachingPackageLocator.getBuildFileForPackage(
- subincludeTarget.getLabel().getLabel().getPackageIdentifier());
- if (buildFileForSubinclude != null) {
+ loadTarget.getLabel().getLabel().getPackageIdentifier());
+ if (buildFileForLoad != null) {
Label buildFileLabel =
Label.createUnvalidated(
- subincludeTarget.getLabel().getLabel().getPackageIdentifier(),
- buildFileForSubinclude.getBaseName());
+ loadTarget.getLabel().getLabel().getPackageIdentifier(),
+ buildFileForLoad.getBaseName());
addIfUniqueLabel(
getNode(new FakeLoadTarget(buildFileLabel, pkg)), seenLabels, dependentFiles);
}
@@ -456,7 +452,7 @@
}
}
- private Node<Target> getSubincludeTarget(Label label, Package pkg) {
+ private Node<Target> getLoadTarget(Label label, Package pkg) {
return getNode(new FakeLoadTarget(label, pkg));
}
diff --git a/src/main/java/com/google/devtools/build/lib/query2/ConfiguredTargetQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/ConfiguredTargetQueryEnvironment.java
index 9daf252..b425f2d 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/ConfiguredTargetQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/ConfiguredTargetQueryEnvironment.java
@@ -591,7 +591,6 @@
QueryExpression caller,
ThreadSafeMutableSet<ConfiguredTarget> nodes,
boolean buildFiles,
- boolean subincludes,
boolean loads)
throws QueryException, InterruptedException {
throw new QueryException("buildfiles() doesn't make sense for the configured target graph");
diff --git a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
index e7ce0a3..c96a9ec 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
@@ -731,7 +731,6 @@
QueryExpression caller,
ThreadSafeMutableSet<Target> nodes,
boolean buildFiles,
- boolean subincludes,
boolean loads)
throws QueryException {
ThreadSafeMutableSet<Target> dependentFiles = createThreadSafeMutableSet();
@@ -750,32 +749,29 @@
}
List<Label> extensions = new ArrayList<>();
- if (subincludes) {
- extensions.addAll(pkg.getSubincludeLabels());
- }
if (loads) {
extensions.addAll(pkg.getSkylarkFileDependencies());
}
- for (Label subinclude : extensions) {
+ for (Label extension : extensions) {
- Target subincludeTarget = getSubincludeTarget(subinclude, pkg);
- addIfUniqueLabel(subincludeTarget, seenLabels, dependentFiles);
+ Target loadTarget = getLoadTarget(extension, pkg);
+ addIfUniqueLabel(loadTarget, seenLabels, dependentFiles);
- // Also add the BUILD file of the subinclude.
+ // Also add the BUILD file of the extension.
if (buildFiles) {
- Path buildFileForSubinclude = null;
+ Path buildFileForLoad = null;
try {
- buildFileForSubinclude =
- pkgPath.getPackageBuildFile(subincludeTarget.getLabel().getPackageIdentifier());
+ buildFileForLoad =
+ pkgPath.getPackageBuildFile(loadTarget.getLabel().getPackageIdentifier());
} catch (NoSuchPackageException e) {
throw new QueryException(
- subincludeTarget.getLabel().getPackageIdentifier() + " does not exist in graph");
+ loadTarget.getLabel().getPackageIdentifier() + " does not exist in graph");
}
Label buildFileLabel =
Label.createUnvalidated(
- subincludeTarget.getLabel().getPackageIdentifier(),
- buildFileForSubinclude.getBaseName());
+ loadTarget.getLabel().getPackageIdentifier(),
+ buildFileForLoad.getBaseName());
addIfUniqueLabel(new FakeLoadTarget(buildFileLabel, pkg), seenLabels, dependentFiles);
}
@@ -791,7 +787,7 @@
}
}
- private Target getSubincludeTarget(Label label, Package pkg) {
+ private Target getLoadTarget(Label label, Package pkg) {
return new FakeLoadTarget(label, pkg);
}
diff --git a/src/main/java/com/google/devtools/build/lib/query2/engine/BuildFilesFunction.java b/src/main/java/com/google/devtools/build/lib/query2/engine/BuildFilesFunction.java
index e0feac5..ac55028 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/engine/BuildFilesFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/engine/BuildFilesFunction.java
@@ -58,7 +58,7 @@
Iterables.addAll(result, partialResult);
callback.process(uniquifier.unique(
env.getBuildFiles(
- expression, result, /* BUILD */ true, /* subinclude */ true, /* load */ true)));
+ expression, result, /* BUILD */ true, /* load */ true)));
}
});
}
diff --git a/src/main/java/com/google/devtools/build/lib/query2/engine/LoadFilesFunction.java b/src/main/java/com/google/devtools/build/lib/query2/engine/LoadFilesFunction.java
index a008779..e70f8b6 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/engine/LoadFilesFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/engine/LoadFilesFunction.java
@@ -57,7 +57,6 @@
expression,
result,
/* BUILD */ false,
- /* subinclude */ false,
/* load */ true)));
}
});
diff --git a/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java
index 1edc658..da460e2 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java
@@ -395,12 +395,14 @@
void reportBuildFileError(QueryExpression expression, String msg) throws QueryException;
/**
- * Returns the set of BUILD, and optionally sub-included and Skylark files that define the given
- * set of targets. Each such file is itself represented as a target in the result.
+ * Returns the set of BUILD, and optionally Skylark files that define the given set of targets.
+ * Each such file is itself represented as a target in the result.
*/
ThreadSafeMutableSet<T> getBuildFiles(
- QueryExpression caller, ThreadSafeMutableSet<T> nodes, boolean buildFiles,
- boolean subincludes, boolean loads) throws QueryException, InterruptedException;
+ QueryExpression caller,
+ ThreadSafeMutableSet<T> nodes,
+ boolean buildFiles,
+ boolean loads) throws QueryException, InterruptedException;
/**
* Returns an object that can be used to query information about targets. Implementations should
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/AspectResolver.java b/src/main/java/com/google/devtools/build/lib/query2/output/AspectResolver.java
index 2994aa0..5d21322 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/AspectResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/AspectResolver.java
@@ -21,7 +21,6 @@
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.pkgcache.PackageProvider;
-import java.util.Collection;
import java.util.Set;
/** Utility class that determines additional dependencies of a target from its aspects. */
@@ -63,28 +62,6 @@
PackageProvider provider, ExtendedEventHandler eventHandler);
}
- /** The way aspect dependencies for a BUILD file are calculated. */
- enum BuildFileDependencyMode {
-
- /** Return all the subincluded files that may affect the package. */
- SUBINCLUDE {
- @Override
- protected Collection<Label> getDependencies(Package pkg) {
- return pkg.getSubincludeLabels();
- }
- },
-
- /** Return all Skylark files that may affect the package. */
- SKYLARK {
- @Override
- protected Collection<Label> getDependencies(Package pkg) {
- return pkg.getSkylarkFileDependencies();
- }
- };
-
- protected abstract Collection<Label> getDependencies(Package pkg);
- }
-
/**
* Compute additional dependencies of target from aspects. This method may load the direct deps
* of target to determine their types. Returns map of attributes and corresponding label values.
@@ -94,9 +71,8 @@
throws InterruptedException;
/**
- * Compute the labels of the BUILD and subinclude and Skylark files on which the results of the
- * other two methods depend for a target in the given package.
+ * Compute the labels of the BUILD Skylark files on which the results of the other two methods
+ * depend for a target in the given package.
*/
- Set<Label> computeBuildFileDependencies(Package pkg, BuildFileDependencyMode mode)
- throws InterruptedException;
+ Set<Label> computeBuildFileDependencies(Package pkg) throws InterruptedException;
}
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/ConservativeAspectResolver.java b/src/main/java/com/google/devtools/build/lib/query2/output/ConservativeAspectResolver.java
index 1b28bcd..16bc92b 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/ConservativeAspectResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/ConservativeAspectResolver.java
@@ -22,9 +22,9 @@
import com.google.devtools.build.lib.packages.AspectDefinition;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.DependencyFilter;
+import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Target;
-
import java.util.Set;
/**
@@ -54,9 +54,9 @@
}
@Override
- public Set<Label> computeBuildFileDependencies(com.google.devtools.build.lib.packages.Package pkg,
- BuildFileDependencyMode mode) throws InterruptedException {
+ public Set<Label> computeBuildFileDependencies(Package pkg)
+ throws InterruptedException {
// We do a conservative estimate precisely so that we don't depend on any other BUILD files.
- return ImmutableSet.copyOf(mode.getDependencies(pkg));
+ return ImmutableSet.copyOf(pkg.getSkylarkFileDependencies());
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/NullAspectResolver.java b/src/main/java/com/google/devtools/build/lib/query2/output/NullAspectResolver.java
index 1b1f9a3..f13b919 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/NullAspectResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/NullAspectResolver.java
@@ -20,7 +20,6 @@
import com.google.devtools.build.lib.packages.DependencyFilter;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Target;
-
import java.util.Set;
/**
@@ -37,8 +36,7 @@
}
@Override
- public Set<Label> computeBuildFileDependencies(
- Package pkg, BuildFileDependencyMode mode) throws InterruptedException {
- return ImmutableSet.copyOf(mode.getDependencies(pkg));
+ public Set<Label> computeBuildFileDependencies(Package pkg) throws InterruptedException {
+ return ImmutableSet.copyOf(pkg.getSkylarkFileDependencies());
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/PreciseAspectResolver.java b/src/main/java/com/google/devtools/build/lib/query2/output/PreciseAspectResolver.java
index 007828a..c132d82 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/PreciseAspectResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/PreciseAspectResolver.java
@@ -79,10 +79,10 @@
}
@Override
- public Set<Label> computeBuildFileDependencies(Package pkg, BuildFileDependencyMode mode)
+ public Set<Label> computeBuildFileDependencies(Package pkg)
throws InterruptedException {
Set<Label> result = new LinkedHashSet<>();
- result.addAll(mode.getDependencies(pkg));
+ result.addAll(pkg.getSkylarkFileDependencies());
Set<PackageIdentifier> dependentPackages = new LinkedHashSet<>();
// First compute with packages can possibly affect the aspect attributes of this package:
@@ -116,12 +116,12 @@
}
}
- // Then add all the subinclude labels of the packages thus found to the result.
+ // Then add all the labels of all the bzl files loaded by the packages found.
for (PackageIdentifier packageIdentifier : dependentPackages) {
try {
result.add(Label.create(packageIdentifier, "BUILD"));
Package dependentPackage = packageProvider.getPackage(eventHandler, packageIdentifier);
- result.addAll(mode.getDependencies(dependentPackage));
+ result.addAll(dependentPackage.getSkylarkFileDependencies());
} catch (NoSuchPackageException e) {
// If the package is not found, just add its BUILD file, which is already done above.
// Hopefully this error is not raised when there is a syntax error in a subincluded file
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java
index c052d38..edb0796 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java
@@ -46,7 +46,6 @@
import com.google.devtools.build.lib.query2.engine.QueryEnvironment;
import com.google.devtools.build.lib.query2.engine.SynchronizedDelegatingOutputFormatterCallback;
import com.google.devtools.build.lib.query2.engine.ThreadSafeOutputFormatterCallback;
-import com.google.devtools.build.lib.query2.output.AspectResolver.BuildFileDependencyMode;
import com.google.devtools.build.lib.query2.output.OutputFormatter.AbstractUnorderedFormatter;
import com.google.devtools.build.lib.query2.output.QueryOptions.OrderOutput;
import com.google.devtools.build.lib.query2.proto.proto2api.Build;
@@ -60,11 +59,9 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
-import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
-import java.util.Set;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
@@ -298,18 +295,12 @@
}
if (inputFile.getName().equals("BUILD")) {
- Set<Label> subincludeLabels = new LinkedHashSet<>();
- subincludeLabels.addAll(aspectResolver == null
- ? inputFile.getPackage().getSubincludeLabels()
- : aspectResolver.computeBuildFileDependencies(
- inputFile.getPackage(), BuildFileDependencyMode.SUBINCLUDE));
- subincludeLabels.addAll(aspectResolver == null
+ Iterable<Label> skylarkLoadLabels = aspectResolver == null
? inputFile.getPackage().getSkylarkFileDependencies()
- : aspectResolver.computeBuildFileDependencies(
- inputFile.getPackage(), BuildFileDependencyMode.SKYLARK));
+ : aspectResolver.computeBuildFileDependencies(inputFile.getPackage());
- for (Label skylarkFileDep : subincludeLabels) {
- input.addSubinclude(skylarkFileDep.toString());
+ for (Label skylarkLoadLabel : skylarkLoadLabels) {
+ input.addSubinclude(skylarkLoadLabel.toString());
}
for (String feature : inputFile.getPackage().getFeatures()) {
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java
index 9af2dd8..c6102df 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java
@@ -31,7 +31,6 @@
import com.google.devtools.build.lib.query2.engine.QueryEnvironment;
import com.google.devtools.build.lib.query2.engine.SynchronizedDelegatingOutputFormatterCallback;
import com.google.devtools.build.lib.query2.engine.ThreadSafeOutputFormatterCallback;
-import com.google.devtools.build.lib.query2.output.AspectResolver.BuildFileDependencyMode;
import com.google.devtools.build.lib.query2.output.OutputFormatter.AbstractUnorderedFormatter;
import com.google.devtools.build.lib.syntax.Type;
import java.io.IOException;
@@ -191,7 +190,6 @@
elem = doc.createElement("source-file");
InputFile inputFile = (InputFile) target;
if (inputFile.getName().equals("BUILD")) {
- addSubincludedFilesToElement(doc, elem, inputFile);
addSkylarkFilesToElement(doc, elem, inputFile);
addFeaturesToElement(doc, elem, inputFile);
elem.setAttribute("package_contains_errors",
@@ -254,22 +252,10 @@
}
}
- private void addSubincludedFilesToElement(Document doc, Element parent, InputFile inputFile)
- throws InterruptedException {
- Iterable<Label> dependencies = aspectResolver.computeBuildFileDependencies(
- inputFile.getPackage(), BuildFileDependencyMode.SUBINCLUDE);
-
- for (Label subinclude : dependencies) {
- Element elem = doc.createElement("subinclude");
- elem.setAttribute("name", subinclude.toString());
- parent.appendChild(elem);
- }
- }
-
private void addSkylarkFilesToElement(Document doc, Element parent, InputFile inputFile)
throws InterruptedException {
- Iterable<Label> dependencies = aspectResolver.computeBuildFileDependencies(
- inputFile.getPackage(), BuildFileDependencyMode.SKYLARK);
+ Iterable<Label> dependencies =
+ aspectResolver.computeBuildFileDependencies(inputFile.getPackage());
for (Label skylarkFileDep : dependencies) {
Element elem = doc.createElement("load");
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index d69dd83..443fe8b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -68,7 +68,6 @@
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
-import com.google.devtools.build.skyframe.ValueOrException;
import com.google.devtools.build.skyframe.ValueOrException2;
import com.google.devtools.build.skyframe.ValueOrException3;
import java.io.IOException;
@@ -291,31 +290,6 @@
return Pair.of(builder.build(), packageShouldBeInError);
}
- private static boolean markFileDepsAndPropagateFilesystemExceptions(
- PackageIdentifier packageIdentifier,
- Iterable<SkyKey> depKeys,
- Environment env,
- boolean packageWasInError)
- throws InternalInconsistentFilesystemException, InterruptedException {
- Preconditions.checkState(
- Iterables.all(depKeys, SkyFunctions.isSkyFunction(SkyFunctions.FILE)), depKeys);
- boolean packageShouldBeInError = packageWasInError;
- for (Map.Entry<SkyKey, ValueOrException<IOException>> entry :
- env.getValuesOrThrow(depKeys, IOException.class).entrySet()) {
- try {
- entry.getValue().get();
- } catch (InconsistentFilesystemException e) {
- throw new InternalInconsistentFilesystemException(packageIdentifier, e);
- } catch (FileSymlinkException e) {
- // Legacy doesn't detect symlink cycles.
- packageShouldBeInError = true;
- } catch (IOException e) {
- maybeThrowFilesystemInconsistency(packageIdentifier, e, packageWasInError);
- }
- }
- return packageShouldBeInError;
- }
-
/**
* These deps have already been marked (see {@link SkyframeHybridGlobber}) but we need to properly
* handle some errors that legacy package loading can't handle gracefully.
@@ -328,7 +302,7 @@
throws InternalInconsistentFilesystemException, InterruptedException {
Preconditions.checkState(
Iterables.all(depKeys, SkyFunctions.isSkyFunction(SkyFunctions.GLOB)), depKeys);
- boolean packageShouldBeInError = packageWasInError;
+ boolean packageShouldBeInErrorFromGlobDeps = false;
for (Map.Entry<SkyKey, ValueOrException2<IOException, BuildFileNotFoundException>> entry :
env.getValuesOrThrow(
depKeys, IOException.class, BuildFileNotFoundException.class).entrySet()) {
@@ -338,97 +312,12 @@
throw new InternalInconsistentFilesystemException(packageIdentifier, e);
} catch (FileSymlinkException e) {
// Legacy doesn't detect symlink cycles.
- packageShouldBeInError = true;
+ packageShouldBeInErrorFromGlobDeps = true;
} catch (IOException | BuildFileNotFoundException e) {
maybeThrowFilesystemInconsistency(packageIdentifier, e, packageWasInError);
}
}
- return packageShouldBeInError;
- }
-
- /**
- * Marks dependencies implicitly used by legacy package loading code, after the fact. Note that
- * the given package might already be in error.
- *
- * <p>Most skyframe exceptions encountered here are ignored, as similar errors should have already
- * been encountered by legacy package loading (if not, then the filesystem is inconsistent). Some
- * exceptions that Skyframe is stricter about (disallowed access to files outside package roots)
- * are propagated.
- */
- private static boolean markDependenciesAndPropagateFilesystemExceptions(
- Environment env,
- Set<SkyKey> globDepKeys,
- Map<Label, Path> subincludes,
- PackageIdentifier packageIdentifier,
- boolean containsErrors)
- throws InternalInconsistentFilesystemException, InterruptedException {
- boolean packageShouldBeInError = containsErrors;
-
- Set<SkyKey> subincludePackageLookupDepKeys = Sets.newHashSet();
- for (Label label : subincludes.keySet()) {
- // Declare a dependency on the package lookup for the package giving access to the label.
- subincludePackageLookupDepKeys.add(PackageLookupValue.key(label.getPackageIdentifier()));
- }
- Pair<? extends Map<PathFragment, PackageLookupValue>, Boolean> subincludePackageLookupResult =
- getPackageLookupDepsAndPropagateInconsistentFilesystemExceptions(
- packageIdentifier, subincludePackageLookupDepKeys, env, containsErrors);
- Map<PathFragment, PackageLookupValue> subincludePackageLookupDeps =
- subincludePackageLookupResult.getFirst();
- packageShouldBeInError |= subincludePackageLookupResult.getSecond();
- List<SkyKey> subincludeFileDepKeys = Lists.newArrayList();
- for (Entry<Label, Path> subincludeEntry : subincludes.entrySet()) {
- // Ideally, we would have a direct dependency on the target with the given label, but then
- // subincluding a file from the same package will cause a dependency cycle, since targets
- // depend on their containing packages.
- Label label = subincludeEntry.getKey();
- PackageLookupValue subincludePackageLookupValue =
- subincludePackageLookupDeps.get(label.getPackageFragment());
- if (subincludePackageLookupValue != null) {
- // Declare a dependency on the actual file that was subincluded.
- Path subincludeFilePath = subincludeEntry.getValue();
- if (subincludeFilePath != null && !subincludePackageLookupValue.packageExists()) {
- // Legacy blaze puts a non-null path when only when the package does indeed exist.
- throw new InternalInconsistentFilesystemException(
- packageIdentifier,
- String.format(
- "Unexpected package in %s. Was it modified during the build?",
- subincludeFilePath));
- }
- if (subincludePackageLookupValue.packageExists()) {
- // Sanity check for consistency of Skyframe and legacy blaze.
- Path subincludeFilePathSkyframe =
- subincludePackageLookupValue.getRoot().getRelative(label.toPathFragment());
- if (subincludeFilePath != null
- && !subincludeFilePathSkyframe.equals(subincludeFilePath)) {
- throw new InternalInconsistentFilesystemException(
- packageIdentifier,
- String.format(
- "Inconsistent package location for %s: '%s' vs '%s'. "
- + "Was the source tree modified during the build?",
- label.getPackageFragment(),
- subincludeFilePathSkyframe,
- subincludeFilePath));
- }
- // The actual file may be under a different package root than the package being
- // constructed.
- SkyKey subincludeSkyKey =
- FileValue.key(
- RootedPath.toRootedPath(
- subincludePackageLookupValue.getRoot(),
- label.getPackageFragment().getRelative(label.getName())));
- subincludeFileDepKeys.add(subincludeSkyKey);
- }
- }
- }
- packageShouldBeInError |=
- markFileDepsAndPropagateFilesystemExceptions(
- packageIdentifier, subincludeFileDepKeys, env, containsErrors);
-
- packageShouldBeInError |=
- handleGlobDepsAndPropagateFilesystemExceptions(
- packageIdentifier, globDepKeys, env, containsErrors);
-
- return packageShouldBeInError;
+ return packageShouldBeInErrorFromGlobDeps;
}
/**
@@ -606,7 +495,7 @@
}
try {
// Since the Skyframe dependencies we request below in
- // markDependenciesAndPropagateFilesystemExceptions are requested independently of
+ // handleGlobDepsAndPropagateFilesystemExceptions are requested independently of
// the ones requested here in
// handleLabelsCrossingSubpackagesAndPropagateInconsistentFilesystemExceptions, we don't
// bother checking for missing values and instead piggyback on the env.missingValues() call
@@ -620,12 +509,11 @@
e.isTransient() ? Transience.TRANSIENT : Transience.PERSISTENT);
}
Set<SkyKey> globKeys = packageCacheEntry.globDepKeys;
- Map<Label, Path> subincludes = pkgBuilder.getSubincludes();
- boolean packageShouldBeConsideredInError;
+ boolean packageShouldBeConsideredInErrorFromGlobDeps;
try {
- packageShouldBeConsideredInError =
- markDependenciesAndPropagateFilesystemExceptions(
- env, globKeys, subincludes, packageId, pkgBuilder.containsErrors());
+ packageShouldBeConsideredInErrorFromGlobDeps =
+ handleGlobDepsAndPropagateFilesystemExceptions(
+ packageId, globKeys, env, pkgBuilder.containsErrors());
} catch (InternalInconsistentFilesystemException e) {
packageFunctionCache.invalidate(packageId);
throw new PackageFunctionException(
@@ -636,7 +524,7 @@
return null;
}
- if (packageShouldBeConsideredInError) {
+ if (pkgBuilder.containsErrors() || packageShouldBeConsideredInErrorFromGlobDeps) {
pkgBuilder.setContainsErrors();
}
Package pkg = pkgBuilder.finishBuild();
@@ -835,17 +723,6 @@
targetToKey.put(target, key);
containingPkgLookupKeys.add(key);
}
- Map<Label, SkyKey> subincludeToKey = new HashMap<>();
- for (Label subincludeLabel : pkgBuilder.getSubincludeLabels()) {
- PathFragment dir = getContainingDirectory(subincludeLabel);
- if (dir.equals(pkgDir)) {
- continue;
- }
- PackageIdentifier dirId = PackageIdentifier.create(pkgId.getRepository(), dir);
- SkyKey key = ContainingPackageLookupValue.key(dirId);
- subincludeToKey.put(subincludeLabel, key);
- containingPkgLookupKeys.add(ContainingPackageLookupValue.key(dirId));
- }
Map<SkyKey, ValueOrException3<BuildFileNotFoundException, InconsistentFilesystemException,
FileSymlinkException>> containingPkgLookupValues = env.getValuesOrThrow(
containingPkgLookupKeys, BuildFileNotFoundException.class,
@@ -867,19 +744,6 @@
pkgBuilder.setContainsErrors();
}
}
- for (Label subincludeLabel : pkgBuilder.getSubincludeLabels()) {
- SkyKey key = subincludeToKey.get(subincludeLabel);
- if (!containingPkgLookupValues.containsKey(key)) {
- continue;
- }
- ContainingPackageLookupValue containingPackageLookupValue =
- getContainingPkgLookupValueAndPropagateInconsistentFilesystemExceptions(
- pkgId, containingPkgLookupValues.get(key), env);
- if (maybeAddEventAboutLabelCrossingSubpackage(pkgBuilder, pkgRoot, subincludeLabel,
- /*location=*/null, containingPackageLookupValue)) {
- pkgBuilder.setContainsErrors();
- }
- }
}
private static PathFragment getContainingDirectory(Label label) {
diff --git a/src/main/protobuf/build.proto b/src/main/protobuf/build.proto
index d4e7915..4af9893 100644
--- a/src/main/protobuf/build.proto
+++ b/src/main/protobuf/build.proto
@@ -399,10 +399,10 @@
// machine-parseable form.
optional Location DEPRECATED_parseable_location = 7;
- // Labels of files that are transitively subincluded in this BUILD file. This
- // is present only when the SourceFile represents a BUILD file that
- // subincludes other files. The subincluded file can be either a Python
- // preprocessed build extension or a Skylark file.
+ // Labels of .bzl (Skylark) files that are transitively loaded in this BUILD
+ // file. This is present only when the SourceFile represents a BUILD file that
+ // loaded .bzl files.
+ // TODO(bazel-team): Rename this field.
repeated string subinclude = 3;
// Labels of package groups that are mentioned in the visibility declaration
