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

15 files changed, 56 insertions, 341 deletions

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 ae191e8b32..af885be009 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.ImmutableSet;
 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 @@ public class Package {
   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 @@ public class Package {
     }
     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 @@ public class Package {
   }
 
   /**
-   * 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 @@ public class Package {
     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 @@ public class Package {
           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 @@ public class Package {
       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 bcac22b13d..dce457b195 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 @@ public final class PackageFactory {
   }
 
   /**
-   * 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 @@ public final class PackageFactory {
         .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 b7c0a1d915..3c5bf30491 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 @@ public class BlazeQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
       final QueryExpression caller,
       ThreadSafeMutableSet<Target> nodes,
       boolean buildFiles,
-      boolean subincludes,
       boolean loads)
       throws QueryException {
     ThreadSafeMutableSet<Target> dependentFiles = createThreadSafeMutableSet();
@@ -405,28 +404,25 @@ public class BlazeQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
         }
 
         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 @@ public class BlazeQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
     }
   }
 
-  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 9daf252601..b425f2dd9a 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 @@ public class ConfiguredTargetQueryEnvironment
       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 e7ce0a3a29..c96a9ecc71 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 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
       QueryExpression caller,
       ThreadSafeMutableSet<Target> nodes,
       boolean buildFiles,
-      boolean subincludes,
       boolean loads)
       throws QueryException {
     ThreadSafeMutableSet<Target> dependentFiles = createThreadSafeMutableSet();
@@ -750,32 +749,29 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
         }
 
         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 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
     }
   }
 
-  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 e0feac5de1..ac55028c17 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 @@ public class BuildFilesFunction implements QueryFunction {
             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 a008779ae5..e70f8b696b 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 @@ public class LoadFilesFunction implements QueryEnvironment.QueryFunction {
                     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 1edc6584e6..da460e2c5e 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 @@ public interface QueryEnvironment<T> {
   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 2994aa004c..5d21322c1f 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.DependencyFilter;
 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 @@ public interface AspectResolver {
         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 @@ public interface AspectResolver {
       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 1b28bcdbc8..16bc92b0f2 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.Aspect;
 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 @@ public class ConservativeAspectResolver implements AspectResolver {
   }
 
   @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 1b1f9a3b99..f13b9198f7 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.Attribute;
 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 @@ public class NullAspectResolver implements AspectResolver {
   }
 
   @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 007828af53..c132d82278 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 @@ public class PreciseAspectResolver implements AspectResolver {
   }
 
   @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 @@ public class PreciseAspectResolver implements AspectResolver {
       }
     }
 
-    // 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 c052d3855d..edb0796fff 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.OutputFormatterCallback;
 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.io.OutputStream;
 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 @@ public class ProtoOutputFormatter extends AbstractUnorderedFormatter {
       }
 
       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 9af2dd884e..c6102dfc28 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.OutputFormatterCallback;
 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 @@ class XmlOutputFormatter extends AbstractUnorderedFormatter {
       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 @@ class XmlOutputFormatter extends AbstractUnorderedFormatter {
     }
   }
 
-  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 d69dd83137..443fe8b795 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;
 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 @@ public class PackageFunction implements SkyFunction {
     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 @@ public class PackageFunction implements SkyFunction {
       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 @@ public class PackageFunction implements SkyFunction {
         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 @@ public class PackageFunction implements SkyFunction {
     }
     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 @@ public class PackageFunction implements SkyFunction {
           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 @@ public class PackageFunction implements SkyFunction {
       return null;
     }
 
-    if (packageShouldBeConsideredInError) {
+    if (pkgBuilder.containsErrors() || packageShouldBeConsideredInErrorFromGlobDeps) {
       pkgBuilder.setContainsErrors();
     }
     Package pkg = pkgBuilder.finishBuild();
@@ -835,17 +723,6 @@ public class PackageFunction implements SkyFunction {
       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 @@ public class PackageFunction implements SkyFunction {
         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) {