Make JavaCompileAction and friends emit new-style arguments.

Instead of --direct_dependency, --indirect_dependency args we now emit --direct_dependencies. We no longer need to emit any jar owner information since that is baked into the jar by JavaBuilder.

This CL also contains the deletion of CustomMultiArgv and the injecting_rule_kind aspect parameter, as the deleted code was the last remaining usage.

RELNOTES: None
PiperOrigin-RevId: 187558628

9 files changed, 8 insertions, 144 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java
index e90f856709..8e999b5518 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java
@@ -102,25 +102,6 @@ public final class CustomCommandLine extends CommandLine {
     abstract void addToFingerprint(ActionKeyContext actionKeyContext, Fingerprint fingerprint);
   }
 
-  /** Deprecated. Do not use. TODO(b/64841073): Remove this */
-  @Deprecated
-  public abstract static class CustomMultiArgv extends StandardArgvFragment {
-
-    @Override
-    void eval(ImmutableList.Builder<String> builder) {
-      builder.addAll(argv());
-    }
-
-    public abstract Iterable<String> argv();
-
-    @Override
-    final void addToFingerprint(ActionKeyContext actionKeyContext, Fingerprint fingerprint) {
-      for (String arg : argv()) {
-        fingerprint.addString(arg);
-      }
-    }
-  }
-
   /**
    * An ArgvFragment that expands a collection of objects in a user-specified way.
    *
@@ -1014,13 +995,6 @@ public final class CustomCommandLine extends CommandLine {
       return addVectorArgInternal(arg, vectorArg);
     }
 
-    public Builder addCustomMultiArgv(@Nullable CustomMultiArgv arg) {
-      if (arg != null) {
-        arguments.add(arg);
-      }
-      return this;
-    }
-
     /**
      * Adds a placeholder TreeArtifact exec path. When the command line is used in an action
      * template, the placeholder will be replaced by the exec path of a {@link TreeFileArtifact}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaLiteProtoLibraryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaLiteProtoLibraryRule.java
index f8e5725c59..1e468b866c 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaLiteProtoLibraryRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaLiteProtoLibraryRule.java
@@ -15,21 +15,17 @@
 package com.google.devtools.build.lib.bazel.rules.java.proto;
 
 import static com.google.devtools.build.lib.bazel.rules.java.proto.BazelJavaLiteProtoAspect.DEFAULT_PROTO_TOOLCHAIN_LABEL;
-import static com.google.devtools.build.lib.packages.Aspect.INJECTING_RULE_KIND_PARAMETER_KEY;
 import static com.google.devtools.build.lib.packages.Attribute.attr;
 import static com.google.devtools.build.lib.packages.BuildType.LABEL;
 import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
 import static com.google.devtools.build.lib.rules.java.proto.JavaLiteProtoAspect.getProtoToolchainLabel;
 import static com.google.devtools.build.lib.syntax.Type.BOOLEAN;
 
-import com.google.common.base.Function;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.analysis.BaseRuleClasses;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
 import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
-import com.google.devtools.build.lib.packages.AspectParameters;
-import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
 import com.google.devtools.build.lib.rules.java.JavaConfiguration;
@@ -49,12 +45,6 @@ public class BazelJavaLiteProtoLibraryRule implements RuleDefinition {
 
   @Override
   public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment environment) {
-    Function<Rule, AspectParameters> aspectParameters =
-        rule ->
-            new AspectParameters.Builder()
-                .addAttribute(INJECTING_RULE_KIND_PARAMETER_KEY, "java_lite_proto_library")
-                .build();
-
     return builder
         .requiresConfigurationFragments(JavaConfiguration.class)
         /* <!-- #BLAZE_RULE(java_lite_proto_library).ATTRIBUTE(deps) -->
@@ -65,7 +55,7 @@ public class BazelJavaLiteProtoLibraryRule implements RuleDefinition {
             attr("deps", LABEL_LIST)
                 .allowedRuleClasses("proto_library")
                 .allowedFileTypes()
-                .aspect(javaProtoAspect, aspectParameters))
+                .aspect(javaProtoAspect))
         .add(attr("strict_deps", BOOLEAN).value(true).undocumented("for migration"))
         .add(
             attr(JavaProtoAspectCommon.LITE_PROTO_TOOLCHAIN_ATTR, LABEL)
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaProtoLibraryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaProtoLibraryRule.java
index e514089f1f..f6faa2c77b 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaProtoLibraryRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaProtoLibraryRule.java
@@ -14,17 +14,13 @@
 
 package com.google.devtools.build.lib.bazel.rules.java.proto;
 
-import static com.google.devtools.build.lib.packages.Aspect.INJECTING_RULE_KIND_PARAMETER_KEY;
 import static com.google.devtools.build.lib.packages.Attribute.attr;
 import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
 import static com.google.devtools.build.lib.syntax.Type.BOOLEAN;
 
-import com.google.common.base.Function;
 import com.google.devtools.build.lib.analysis.BaseRuleClasses;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
-import com.google.devtools.build.lib.packages.AspectParameters;
-import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
 import com.google.devtools.build.lib.rules.java.JavaConfiguration;
@@ -43,12 +39,6 @@ public class BazelJavaProtoLibraryRule implements RuleDefinition {
 
   @Override
   public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment environment) {
-    Function<Rule, AspectParameters> aspectParameters =
-        rule ->
-            new AspectParameters.Builder()
-                .addAttribute(INJECTING_RULE_KIND_PARAMETER_KEY, "java_proto_library")
-                .build();
-
     return builder
         .requiresConfigurationFragments(JavaConfiguration.class, ProtoConfiguration.class)
         /* <!-- #BLAZE_RULE(java_proto_library).ATTRIBUTE(deps) -->
@@ -59,7 +49,7 @@ public class BazelJavaProtoLibraryRule implements RuleDefinition {
             attr("deps", LABEL_LIST)
                 .allowedRuleClasses("proto_library")
                 .allowedFileTypes()
-                .aspect(javaProtoAspect, aspectParameters))
+                .aspect(javaProtoAspect))
         .add(attr("strict_deps", BOOLEAN).value(true).undocumented("for migration"))
         .advertiseSkylarkProvider(SkylarkProviderIdentifier.forKey(JavaInfo.PROVIDER.getKey()))
         .build();
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Aspect.java b/src/main/java/com/google/devtools/build/lib/packages/Aspect.java
index 602ab8129a..14e9d5dbf0 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Aspect.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Aspect.java
@@ -29,9 +29,6 @@ import java.util.concurrent.ConcurrentHashMap;
 @Immutable
 public final class Aspect implements DependencyFilter.AttributeInfoProvider {
 
-  /** */
-  public static final String INJECTING_RULE_KIND_PARAMETER_KEY = "$injecting_rule_kind";
-
   /**
    * The aspect definition is a function of the aspect class + its parameters, so we can cache that.
    *
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryRule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryRule.java
index dd02eaaadf..f65f76287d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryRule.java
@@ -14,16 +14,12 @@
 
 package com.google.devtools.build.lib.rules.cpp.proto;
 
-import static com.google.devtools.build.lib.packages.Aspect.INJECTING_RULE_KIND_PARAMETER_KEY;
 import static com.google.devtools.build.lib.packages.Attribute.attr;
 import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
 
-import com.google.common.base.Function;
 import com.google.devtools.build.lib.analysis.BaseRuleClasses;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
-import com.google.devtools.build.lib.packages.AspectParameters;
-import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClass;
 
 /** Declaration part of cc_proto_library. */
@@ -37,12 +33,6 @@ public class CcProtoLibraryRule implements RuleDefinition {
 
   @Override
   public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment environment) {
-    Function<Rule, AspectParameters> aspectParameters =
-        rule ->
-            new AspectParameters.Builder()
-                .addAttribute(INJECTING_RULE_KIND_PARAMETER_KEY, "cc_proto_library")
-                .build();
-
     return builder
         /* <!-- #BLAZE_RULE(cc_proto_library).ATTRIBUTE(deps) -->
         The list of <a href="protocol-buffer.html#proto_library"><code>proto_library</code></a>
@@ -52,7 +42,7 @@ public class CcProtoLibraryRule implements RuleDefinition {
             attr("deps", LABEL_LIST)
                 .allowedRuleClasses("proto_library")
                 .allowedFileTypes()
-                .aspect(ccProtoAspect, aspectParameters))
+                .aspect(ccProtoAspect))
         .build();
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
index 91ec9f9fbe..9846e0f7ec 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
@@ -15,14 +15,11 @@
 package com.google.devtools.build.lib.rules.java;
 
 import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.common.collect.Iterables.getOnlyElement;
-import static com.google.devtools.build.lib.packages.Aspect.INJECTING_RULE_KIND_PARAMETER_KEY;
 import static java.nio.charset.StandardCharsets.ISO_8859_1;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -31,7 +28,6 @@ import com.google.devtools.build.lib.actions.ActionEnvironment;
 import com.google.devtools.build.lib.actions.ActionKeyContext;
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
 import com.google.devtools.build.lib.actions.ArtifactRoot;
 import com.google.devtools.build.lib.actions.CommandLine;
 import com.google.devtools.build.lib.actions.CommandLineExpansionException;
@@ -44,7 +40,6 @@ import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
-import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.CustomMultiArgv;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg;
 import com.google.devtools.build.lib.analysis.actions.LazyWritePathsFileAction;
 import com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction;
@@ -58,7 +53,6 @@ import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
 import com.google.devtools.build.lib.rules.java.JavaConfiguration.JavaClasspathMode;
-import com.google.devtools.build.lib.skyframe.AspectValue;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
 import com.google.devtools.build.lib.util.LazyString;
@@ -68,7 +62,6 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.stream.Stream;
 import javax.annotation.Nullable;
 
@@ -385,54 +378,6 @@ public final class JavaCompileAction extends SpawnAction {
   }
 
   /**
-   * Builds the list of mappings between jars on the classpath and their originating targets names.
-   */
-  @VisibleForTesting
-  static class JarsToTargetsArgv extends CustomMultiArgv {
-    private final Iterable<Artifact> classpath;
-    private final NestedSet<Artifact> directJars;
-
-    @VisibleForTesting
-    JarsToTargetsArgv(Iterable<Artifact> classpath, NestedSet<Artifact> directJars) {
-      this.classpath = classpath;
-      this.directJars = directJars;
-    }
-
-    @Override
-    public Iterable<String> argv() {
-      Set<Artifact> directJarSet = directJars.toSet();
-      ImmutableList.Builder<String> builder = ImmutableList.builder();
-      for (Artifact jar : classpath) {
-        builder.add(directJarSet.contains(jar) ? "--direct_dependency" : "--indirect_dependency");
-        builder.add(jar.getExecPathString());
-        builder.add(getArtifactOwnerGeneralizedLabel(jar));
-      }
-      return builder.build();
-    }
-
-    private String getArtifactOwnerGeneralizedLabel(Artifact artifact) {
-      // The simple case can simply return the label,
-      // avoiding any concatenation or StringBuilder garbage
-      ArtifactOwner owner = checkNotNull(artifact.getArtifactOwner(), artifact);
-      Label label = owner.getLabel();
-      boolean isDefaultOrMain =
-          label.getPackageIdentifier().getRepository().isDefault()
-              || label.getPackageIdentifier().getRepository().isMain();
-      String result =
-          isDefaultOrMain ? label.toString() : "@" + label; // Escape '@' prefix for .params file.
-      if (owner instanceof AspectValue.AspectKey) {
-        AspectValue.AspectKey aspectOwner = (AspectValue.AspectKey) owner;
-        ImmutableCollection<String> injectingRuleKind =
-            aspectOwner.getParameters().getAttribute(INJECTING_RULE_KIND_PARAMETER_KEY);
-        if (injectingRuleKind.size() == 1) {
-          result += ' ' + getOnlyElement(injectingRuleKind);
-        }
-      }
-      return result;
-    }
-  }
-
-  /**
    * Tells {@link Builder} how to create new artifacts. Is there so that {@link Builder} can be
    * exercised in tests without creating a full {@link RuleContext}.
    */
@@ -743,7 +688,7 @@ public final class JavaCompileAction extends SpawnAction {
       // written out and whether we try to minimize the compile-time classpath.
       if (strictJavaDeps != BuildConfiguration.StrictDepsMode.OFF) {
         result.add("--strict_java_deps", strictJavaDeps.toString());
-        result.addCustomMultiArgv(new JarsToTargetsArgv(classpathEntries, directJars));
+        result.addExecPaths("--direct_dependencies", directJars);
 
         if (configuration.getFragment(JavaConfiguration.class).getReduceJavaClasspath()
             == JavaClasspathMode.JAVABUILDER) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java
index 7fc16ddfe9..fca4ffc847 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java
@@ -585,8 +585,7 @@ public class JavaHeaderCompileAction extends SpawnAction {
         result.addExecPaths("--processorpath", processorPath);
       }
       if (strictJavaDeps != BuildConfiguration.StrictDepsMode.OFF) {
-        result.addCustomMultiArgv(
-            new JavaCompileAction.JarsToTargetsArgv(classpathEntries, directJars));
+        result.addExecPaths("--direct_dependencies", directJars);
         if (!compileTimeDependencyArtifacts.isEmpty()) {
           result.addExecPaths("--deps_artifacts", compileTimeDependencyArtifacts);
         }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaTargetAttributes.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaTargetAttributes.java
index 5e080aec8d..5ff0c25b12 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaTargetAttributes.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaTargetAttributes.java
@@ -232,17 +232,13 @@ public class JavaTargetAttributes {
     }
 
     /**
-     * In tandem with strictJavaDeps, directJars represents a subset of the compile-time, classpath
+     * In tandem with strictJavaDeps, directJars represents a subset of the compile-time classpath
      * jars that were provided by direct dependencies. When strictJavaDeps is OFF, there is no need
      * to provide directJars, and no extra information is passed to javac. When strictJavaDeps is
      * set to WARN or ERROR, the compiler command line will include extra flags to indicate the
      * warning/error policy and to map the classpath jars to direct or transitive dependencies,
-     * using the information in directJars. The extra flags are formatted like this (same for
-     * --indirect_dependency): <pre>
-     * --direct_dependency
-     * foo/bar/lib.jar
-     * //java/com/google/foo:bar
-     * </pre>
+     * using the information in directJars. The compiler command line will include an extra flag to
+     * indicate which classpath jars are direct dependencies.
      */
     public Builder addDirectJars(NestedSet<Artifact> directJars) {
       Preconditions.checkArgument(!built);
diff --git a/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java b/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java
index b9f341a588..022ecdd124 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java
@@ -23,7 +23,6 @@ import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
 import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
 import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
-import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.CustomMultiArgv;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -813,21 +812,6 @@ public class CustomCommandLineTest {
   }
 
   @Test
-  public void testCustomMultiArgs() {
-    CustomCommandLine cl =
-        builder()
-            .addCustomMultiArgv(
-                new CustomMultiArgv() {
-                  @Override
-                  public ImmutableList<String> argv() {
-                    return ImmutableList.of("--arg1", "--arg2");
-                  }
-                })
-            .build();
-    assertThat(cl.arguments()).containsExactly("--arg1", "--arg2").inOrder();
-  }
-
-  @Test
   public void testCombinedArgs() {
     CustomCommandLine cl =
         builder()
@@ -891,7 +875,6 @@ public class CustomCommandLineTest {
             .addExecPaths("foo", NestedSetBuilder.emptySet(Order.STABLE_ORDER))
             .addAll("foo", VectorArg.of((NestedSet<String>) null))
             .addAll("foo", VectorArg.of(NestedSetBuilder.<String>emptySet(Order.STABLE_ORDER)))
-            .addCustomMultiArgv(null)
             .addPlaceholderTreeArtifactExecPath("foo", null)
             .build();
     assertThat(cl.arguments()).isEmpty();