Delete supporting code that is now dead after the

--incompatible_load_argument_is_label flag was removed in 940dbc5. Also simplify existing code.

RELNOTES: None
PiperOrigin-RevId: 216464695

Author: haxorz

src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java

@@ -561,7 +561,6 @@ static SkylarkImportResult fetchImportsFromBuildFile(
     ImmutableList<SkylarkImport> imports = buildFileAST.getImports();
     Map<String, Extension> importMap = Maps.newHashMapWithExpectedSize(imports.size());
     ImmutableList.Builder<SkylarkFileDependency> fileDependencies = ImmutableList.builder();
-    ImmutableMap<String, Label> importPathMap;
 
     // Find the labels corresponding to the load statements.
     Label labelForCurrBuildFile;
@@ -571,15 +570,8 @@ static SkylarkImportResult fetchImportsFromBuildFile(
       // Shouldn't happen; the Label is well-formed by construction.
       throw new IllegalStateException(e);
     }
-    try {
-      importPathMap = SkylarkImportLookupFunction.findLabelsForLoadStatements(
-          imports, labelForCurrBuildFile, env);
-      if (importPathMap == null) {
-        return null;
-      }
-    } catch (SkylarkImportFailedException e) {
-      throw propagateSkylarkImportFailedException(packageId, e);
-    }
+    ImmutableMap<String, Label> importPathMap =
+        SkylarkImportLookupFunction.getLabelsForLoadStatements(imports, labelForCurrBuildFile);
 
     // Look up and load the imports.
     ImmutableCollection<Label> importLabels = importPathMap.values();

src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java

@@ -20,7 +20,6 @@
 import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableMultimap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.LinkedHashMultimap;
 import com.google.common.collect.Lists;
@@ -62,6 +61,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.logging.Logger;
+import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 
 /**
@@ -230,18 +230,12 @@ private SkylarkImportLookupValue computeInternal(
 
     // Process the load statements in the file.
     ImmutableList<SkylarkImport> imports = ast.getImports();
-    ImmutableMap<String, Label> labelsForImports;
-
-    // Find the labels corresponding to the load statements.
-    labelsForImports = findLabelsForLoadStatements(imports, fileLabel, env);
-    if (labelsForImports == null) {
-      return null;
-    }
+    ImmutableMap<String, Label> labelsForImports = getLabelsForLoadStatements(imports, fileLabel);
+    ImmutableCollection<Label> importLabels = labelsForImports.values();
 
     // Look up and load the imports.
-    ImmutableCollection<Label> importLabels = labelsForImports.values();
     List<SkyKey> importLookupKeys =
-        Lists.newArrayListWithExpectedSize(importLabels.size());
+        Lists.newArrayListWithExpectedSize(labelsForImports.size());
     for (Label importLabel : importLabels) {
       importLookupKeys.add(SkylarkImportLookupValue.key(importLabel, inWorkspace));
     }
@@ -297,7 +291,7 @@ private SkylarkImportLookupValue computeInternal(
     // Process the loaded imports.
     Map<String, Extension> extensionsForImports = Maps.newHashMapWithExpectedSize(imports.size());
     ImmutableList.Builder<SkylarkFileDependency> fileDependencies =
-        ImmutableList.builderWithExpectedSize(labelsForImports.size());
+        ImmutableList.builderWithExpectedSize(importLabels.size());
     for (Map.Entry<String, Label> importEntry : labelsForImports.entrySet()) {
       String importString = importEntry.getKey();
       Label importLabel = importEntry.getValue();
@@ -399,53 +393,22 @@ private static ImmutableMap<PathFragment, Label> labelsForAbsoluteImports(
   }
 
   /**
-   * Computes the set of {@link Label}s corresponding to a set of Skylark {@link LoadStatement}s.
+   * Given a collection of {@link SkylarkImport}, returns a map from import string to label of
+   * imported file.
    *
    * @param imports a collection of Skylark {@link LoadStatement}s
    * @param containingFileLabel the {@link Label} of the file containing the load statements
-   * @return an {@link ImmutableMap} which maps a {@link String} used in the load statement to its
-   *     corresponding {@Label}. Returns {@code null} if any Skyframe dependencies are unavailable.
-   * @throws SkylarkImportFailedException if no package can be found that contains the loaded file
    */
   @Nullable
-  static ImmutableMap<String, Label> findLabelsForLoadStatements(
-      ImmutableCollection<SkylarkImport> imports, Label containingFileLabel, Environment env)
-      throws SkylarkImportFailedException, InterruptedException {
+  static ImmutableMap<String, Label> getLabelsForLoadStatements(
+      ImmutableCollection<SkylarkImport> imports, Label containingFileLabel) {
     Preconditions.checkArgument(
         !containingFileLabel.getPackageIdentifier().getRepository().isDefault());
-    Map<String, Label> outputMap = Maps.newHashMapWithExpectedSize(imports.size());
-
-    // Filter relative vs. absolute paths.
-    ImmutableSet.Builder<PathFragment> absoluteImportsToLookup = new ImmutableSet.Builder<>();
-    // We maintain a multimap from path fragments to their correspond import strings, to cover the
-    // (unlikely) case where two distinct import strings generate the same path fragment.
-    ImmutableMultimap.Builder<PathFragment, String> pathToImports =
-        new ImmutableMultimap.Builder<>();
-    for (SkylarkImport imp : imports) {
-      if (imp.hasAbsolutePath()) {
-        absoluteImportsToLookup.add(imp.getAbsolutePath());
-        pathToImports.put(imp.getAbsolutePath(), imp.getImportString());
-      } else {
-        outputMap.put(imp.getImportString(), imp.getLabel(containingFileLabel));
-      }
-    }
-
-    // Look up labels for absolute paths.
-    ImmutableMap<PathFragment, Label> absoluteLabels =
-        labelsForAbsoluteImports(absoluteImportsToLookup.build(), env);
-    if (absoluteLabels == null) {
-      return null;
-    }
-    for (Map.Entry<PathFragment, Label> entry : absoluteLabels.entrySet()) {
-      PathFragment currPath = entry.getKey();
-      Label currLabel = entry.getValue();
-      for (String importString : pathToImports.build().get(currPath)) {
-        outputMap.put(importString, currLabel);
-      }
-    }
-
-    ImmutableMap<String, Label> immutableOutputMap = ImmutableMap.copyOf(outputMap);
-    return immutableOutputMap;
+    return ImmutableMap.copyOf(imports.stream().collect(
+        Collectors.toMap(
+            SkylarkImport::getImportString,
+            imp -> imp.getLabel(containingFileLabel),
+            (oldLabel, newLabel) -> oldLabel)));
   }
 
   /**

src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java

@@ -19,11 +19,9 @@
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.causes.LabelCause;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.packages.SkylarkAspect;
 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.syntax.SkylarkImport;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyFunctionException;
@@ -56,22 +54,10 @@ public SkyValue compute(SkyKey skyKey, Environment env)
     SkylarkImport extensionFile = aspectLoadingKey.getSkylarkImport();
 
     // Find label corresponding to skylark file, if one exists.
-    ImmutableMap<String, Label> labelLookupMap;
-    try {
-      labelLookupMap =
-          SkylarkImportLookupFunction.findLabelsForLoadStatements(
-              ImmutableList.of(extensionFile),
-              Label.parseAbsoluteUnchecked("//:empty"),
-              env
-          );
-    } catch (SkylarkImportFailedException e) {
-      env.getListener().handle(Event.error(e.getMessage()));
-      throw new LoadSkylarkAspectFunctionException(
-          new AspectCreationException(e.getMessage(), Label.parseAbsoluteUnchecked("//:empty")));
-    }
-    if (labelLookupMap == null) {
-      return null;
-    }
+    ImmutableMap<String, Label> labelLookupMap =
+        SkylarkImportLookupFunction.getLabelsForLoadStatements(
+            ImmutableList.of(extensionFile),
+            Label.parseAbsoluteUnchecked("//:empty"));
 
     SkylarkAspect skylarkAspect;
     Label extensionFileLabel = Iterables.getOnlyElement(labelLookupMap.values());

src/main/java/com/google/devtools/build/lib/syntax/SkylarkImport.java

@@ -45,16 +45,4 @@ public interface SkylarkImport extends Serializable {
    * @throws IllegalStateException if this import takes the form of an absolute path.
    */
   Label getLabel(@Nullable Label containingFileLabel);
-
-  /**
-   * True if this import takes the form of an absolute path.
-   */
-  boolean hasAbsolutePath();
-
-  /**
-   * Returns a {@link PathFragment} representing the import path.
-   *
-   * @throws IllegalStateException if this import does not take the form of an absolute path.
-   */
-  PathFragment getAbsolutePath();
 }

src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java

@@ -54,16 +54,6 @@ public String getImportString() {
     @Override
     public abstract Label getLabel(Label containingFileLabel);
 
-    @Override
-    public boolean hasAbsolutePath() {
-      return false;
-    }
-
-    @Override
-    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);
@@ -84,76 +74,6 @@ public boolean equals(Object that) {
     }
   }
 
-  @VisibleForSerialization
-  @AutoCodec
-  static final class AbsolutePathImport extends SkylarkImportImpl {
-    private final PathFragment importPath;
-
-    @VisibleForSerialization
-    AbsolutePathImport(String importString, PathFragment importPath) {
-      super(importString);
-      this.importPath = importPath;
-    }
-
-    @Override
-    public PathFragment asPathFragment() {
-      return importPath;
-    }
-
-    @Override
-    public Label getLabel(Label containingFileLabel) {
-      throw new IllegalStateException("can't request a label from an absolute path import");
-    }
-
-    @Override
-    public boolean hasAbsolutePath() {
-      return true;
-    }
-
-    @Override
-    public PathFragment getAbsolutePath() {
-      return this.importPath;
-    }
-
-  }
-
-  @VisibleForSerialization
-  @AutoCodec
-  static final class RelativePathImport extends SkylarkImportImpl {
-    private final String importFile;
-
-    @VisibleForSerialization
-    RelativePathImport(String importString, String importFile) {
-      super(importString);
-      this.importFile = importFile;
-    }
-
-    @Override
-    public PathFragment asPathFragment() {
-      return PathFragment.create(importFile);
-    }
-
-    @Override
-    public Label getLabel(Label containingFileLabel) {
-      // The twistiness of the code below is due to the fact that the containing file may be in
-      // a subdirectory of the package that contains it. We need to construct a Label with
-      // the imported file in the same subdirectory of the package.
-      PathFragment containingDirInPkg =
-          PathFragment.create(containingFileLabel.getName()).getParentDirectory();
-      String targetNameForImport = containingDirInPkg.getRelative(importFile).toString();
-      try {
-        // This is for imports relative to the current repository, so repositoryMapping can be
-        // empty
-        return containingFileLabel.getRelativeWithRemapping(targetNameForImport, ImmutableMap.of());
-      } catch (LabelSyntaxException e) {
-        // Shouldn't happen because the parent label is assumed to be valid and the target string is
-        // validated on construction.
-        throw new IllegalStateException(e);
-      }
-    }
-
-  }
-
   @VisibleForSerialization
   @AutoCodec
   static final class AbsoluteLabelImport extends SkylarkImportImpl {

src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java

@@ -119,45 +119,45 @@ public void testLoadFromSkylarkFileInRemoteRepo() throws Exception {
   }
 
   @Test
-  public void testLoadRelativePath() throws Exception {
+  public void testLoadRelativeLabel() throws Exception {
     scratch.file("pkg/BUILD");
     scratch.file("pkg/ext1.bzl", "a = 1");
     scratch.file("pkg/ext2.bzl", "load(':ext1.bzl', 'a')");
-    get(key("//pkg:ext2.bzl"));
+    checkSuccessfulLookup("//pkg:ext2.bzl");
   }
 
   @Test
-  public void testLoadAbsolutePath() throws Exception {
+  public void testLoadAbsoluteLabel() throws Exception {
     scratch.file("pkg2/BUILD");
     scratch.file("pkg3/BUILD");
     scratch.file("pkg2/ext.bzl", "b = 1");
     scratch.file("pkg3/ext.bzl", "load('//pkg2:ext.bzl', 'b')");
-    get(key("//pkg3:ext.bzl"));
+    checkSuccessfulLookup("//pkg3:ext.bzl");
   }
 
   @Test
-  public void testLoadFromSameAbsolutePathTwice() throws Exception {
+  public void testLoadFromSameAbsoluteLabelTwice() throws Exception {
     scratch.file("pkg1/BUILD");
     scratch.file("pkg2/BUILD");
     scratch.file("pkg1/ext.bzl", "a = 1", "b = 2");
     scratch.file("pkg2/ext.bzl", "load('//pkg1:ext.bzl', 'a')", "load('//pkg1:ext.bzl', 'b')");
-    get(key("//pkg2:ext.bzl"));
+    checkSuccessfulLookup("//pkg2:ext.bzl");
   }
 
   @Test
-  public void testLoadFromSameRelativePathTwice() throws Exception {
+  public void testLoadFromSameRelativeLabelTwice() throws Exception {
     scratch.file("pkg/BUILD");
     scratch.file("pkg/ext1.bzl", "a = 1", "b = 2");
     scratch.file("pkg/ext2.bzl", "load(':ext1.bzl', 'a')", "load(':ext1.bzl', 'b')");
-    get(key("//pkg:ext2.bzl"));
+    checkSuccessfulLookup("//pkg:ext2.bzl");
   }
 
   @Test
-  public void testLoadFromRelativePathInSubdir() throws Exception {
+  public void testLoadFromRelativeLabelInSubdir() throws Exception {
     scratch.file("pkg/BUILD");
     scratch.file("pkg/subdir/ext1.bzl", "a = 1");
     scratch.file("pkg/subdir/ext2.bzl", "load(':subdir/ext1.bzl', 'a')");
-    get(key("//pkg:subdir/ext2.bzl"));
+    checkSuccessfulLookup("//pkg:subdir/ext2.bzl");
   }
 
   private EvaluationResult<SkylarkImportLookupValue> get(SkyKey skylarkImportLookupKey)
@@ -171,7 +171,7 @@ private EvaluationResult<SkylarkImportLookupValue> get(SkyKey skylarkImportLooku
     return result;
   }
 
-  private SkyKey key(String label) throws Exception {
+  private SkyKey key(String label) {
     return SkylarkImportLookupValue.key(Label.parseAbsoluteUnchecked(label), false);
   }
 

src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java

@@ -1099,7 +1099,6 @@ private void validNonAbsoluteImportTest(String importString, String containingFi
     SkylarkImport imp = SkylarkImports.create(stmt.getImport().getValue());
 
     assertThat(imp.getImportString()).named("getImportString()").isEqualTo(importString);
-    assertThat(imp.hasAbsolutePath()).named("hasAbsolutePath()").isFalse();
 
     Label containingFileLabel = Label.parseAbsoluteUnchecked(containingFileLabelString);
     assertThat(imp.getLabel(containingFileLabel)).named("containingFileLabel()")