From 1ad8a8815e5d0f646f12fbcb2ec53a79fb8dcb79 Mon Sep 17 00:00:00 2001
From: asteinb <asteinb@google.com>
Date: Mon, 7 May 2018 11:39:09 -0700
Subject: Stop collapsing NestedSet of deps in RClassGenerator

This collapsing should be avoided when possible, and punted on until execution when needed.

RELNOTES: none
PiperOrigin-RevId: 195695290
---
 .../android/RClassGeneratorActionBuilder.java      | 72 +++++++---------------
 1 file changed, 22 insertions(+), 50 deletions(-)

(limited to 'src/main/java/com/google/devtools')

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
index 016aee5866..01dc6f549b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
@@ -13,26 +13,22 @@
 // limitations under the License.
 package com.google.devtools.build.lib.rules.android;
 
-import com.google.common.base.Function;
 import com.google.common.base.Strings;
-import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ParamFileInfo;
 import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
 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.Builder;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
-import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
-import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion;
+import com.google.devtools.build.lib.rules.android.AndroidDataConverter.JoinerType;
 import java.util.ArrayList;
 import java.util.List;
-import javax.annotation.Nullable;
+import java.util.function.Function;
 
 /** Builds up the spawn action for $android_rclass_generator. */
 public class RClassGeneratorActionBuilder {
@@ -77,7 +73,7 @@ public class RClassGeneratorActionBuilder {
   }
 
   private void build(Artifact rTxt, ProcessedAndroidManifest manifest) {
-    CustomCommandLine.Builder builder = new CustomCommandLine.Builder();
+    Builder builder = new Builder();
 
     // Set the busybox tool.
     builder.add("--tool").add("GENERATE_BINARY_R").add("--");
@@ -98,19 +94,20 @@ public class RClassGeneratorActionBuilder {
       builder.add("--packageForR", manifest.getPackage());
     }
     if (dependencies != null) {
-      // TODO(corysmith): Remove NestedSet as we are already flattening it.
-      Iterable<ValidatedAndroidData> depResources = dependencies.getResourceContainers();
-      if (!Iterables.isEmpty(depResources)) {
+      if (!dependencies.getResourceContainers().isEmpty()) {
         builder.addAll(
             VectorArg.addBefore("--library")
-                .each(
-                    ImmutableList.copyOf(
-                        Iterables.transform(depResources, chooseDepsToArg(version)))));
-        inputs.addTransitive(
-            NestedSetBuilder.wrap(
-                Order.NAIVE_LINK_ORDER,
-                FluentIterable.from(depResources)
-                    .transformAndConcat(chooseDepsToArtifacts(version))));
+                .each(dependencies.getResourceContainers())
+                .mapped(
+                    AndroidDataConverter.<ValidatedAndroidData>builder(JoinerType.COLON_COMMA)
+                        .with(chooseDepsToArg(version))
+                        .build()));
+        if (version == AndroidAaptVersion.AAPT2) {
+          inputs.addTransitive(dependencies.getTransitiveAapt2RTxt());
+        } else {
+          inputs.addTransitive(dependencies.getTransitiveRTxt());
+        }
+        inputs.addTransitive(dependencies.getTransitiveManifests());
       }
     }
     builder.addExecPath("--classJarOutput", classJarOut);
@@ -134,39 +131,14 @@ public class RClassGeneratorActionBuilder {
             .build(ruleContext));
   }
 
-  private static Artifact chooseRTxt(ValidatedAndroidData container, AndroidAaptVersion version) {
-    return version == AndroidAaptVersion.AAPT2 ? container.getAapt2RTxt() : container.getRTxt();
-  }
-
-  private static Function<ValidatedAndroidData, NestedSet<Artifact>> chooseDepsToArtifacts(
-      final AndroidAaptVersion version) {
-    return new Function<ValidatedAndroidData, NestedSet<Artifact>>() {
-      @Override
-      public NestedSet<Artifact> apply(ValidatedAndroidData container) {
-        NestedSetBuilder<Artifact> artifacts = NestedSetBuilder.naiveLinkOrder();
-        addIfNotNull(chooseRTxt(container, version), artifacts);
-        addIfNotNull(container.getManifest(), artifacts);
-        return artifacts.build();
-      }
-
-      private void addIfNotNull(@Nullable Artifact artifact, NestedSetBuilder<Artifact> artifacts) {
-        if (artifact != null) {
-          artifacts.add(artifact);
-        }
-      }
-    };
-  }
-
   private static Function<ValidatedAndroidData, String> chooseDepsToArg(
       final AndroidAaptVersion version) {
-    return new Function<ValidatedAndroidData, String>() {
-      @Override
-      public String apply(ValidatedAndroidData container) {
-        Artifact rTxt = chooseRTxt(container, version);
-        return (rTxt != null ? rTxt.getExecPath() : "")
-            + ","
-            + (container.getManifest() != null ? container.getManifest().getExecPath() : "");
-      }
+    return container -> {
+      Artifact rTxt =
+          version == AndroidAaptVersion.AAPT2 ? container.getAapt2RTxt() : container.getRTxt();
+      return (rTxt != null ? rTxt.getExecPath() : "")
+          + ","
+          + (container.getManifest() != null ? container.getManifest().getExecPath() : "");
     };
   }
 }
-- 
cgit v1.2.3