Remove ResourceContainerConverter

Also, migrate Robolectric action that used it to use AndroidDataConverter
instead.

RELNOTES: none
PiperOrigin-RevId: 207761747

3 files changed, 48 insertions, 327 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataConverter.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataConverter.java
index ffa414f9e1..874f328ad9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataConverter.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataConverter.java
@@ -116,6 +116,10 @@ public class AndroidDataConverter<T> extends ParametrizedMapFn<T> {
    *
    * <p>Because of how Bazel handles these objects, call this method *only* as part of creating a
    * static final field.
+   *
+   * <p>Additionally, the resulting {@link AndroidDataConverter} object should be annotated with
+   * {@link AutoCodec} (and, if relevant, {@link
+   * com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization}.
    */
   public static <T> Builder<T> builder(JoinerType joinerType) {
     return new Builder<>(joinerType);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainerConverter.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainerConverter.java
deleted file mode 100644
index 57cc800b8c..0000000000
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainerConverter.java
+++ /dev/null
@@ -1,235 +0,0 @@
-// Copyright 2016 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//    http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-package com.google.devtools.build.lib.rules.android;
-
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Function;
-import com.google.common.base.Functions;
-import com.google.common.base.Joiner;
-import com.google.common.base.Objects;
-import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterators;
-import com.google.common.collect.Sets;
-import com.google.devtools.build.lib.actions.CommandLineItem;
-import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
-import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg;
-import com.google.devtools.build.lib.rules.android.ResourceContainerConverter.ToArg.Includes;
-import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
-import com.google.devtools.build.lib.vfs.PathFragment;
-import java.util.HashSet;
-import java.util.Set;
-import java.util.function.Consumer;
-
-/**
- * Factory for functions to convert a {@link ResourceContainer} to a commandline argument, or a
- * collection of artifacts. Uses a certain convention for commandline arguments (e.g., separators,
- * and ordering of container elements).
- *
- * @deprecated Use {@link AndroidDataConverter} instead.
- */
-@VisibleForTesting
-@Deprecated
-public class ResourceContainerConverter {
-
-  static Builder builder() {
-    return new Builder();
-  }
-
-  static class ToArg extends CommandLineItem.ParametrizedMapFn<ValidatedAndroidData> {
-
-    private final Set<Includes> includes;
-    private final SeparatorType separatorType;
-    private final Joiner argJoiner;
-    private final Function<String, String> escaper;
-
-    enum Includes {
-      ResourceRoots,
-      Label,
-      Manifest,
-      RTxt,
-      SymbolsBin,
-      CompiledSymbols,
-      StaticLibrary,
-      Aapt2RTxt
-    }
-
-    enum SeparatorType {
-      COLON_COMMA,
-      SEMICOLON_AMPERSAND
-    }
-
-    ToArg(Builder builder) {
-      this.includes = Sets.immutableEnumSet(builder.includes);
-      this.separatorType = builder.separatorType;
-
-      switch (separatorType) {
-        case COLON_COMMA:
-          argJoiner = COLON_JOINER;
-          // We currently use ":" to separate components of an argument and "," to separate
-          // arguments in a list of arguments. Those characters require escaping if used in a label
-          // (part of the set of allowed characters in a label).
-          if (includes.contains(Includes.Label)) {
-            escaper = COLON_COMMA;
-          } else {
-            escaper = IDENTITY;
-          }
-          break;
-        case SEMICOLON_AMPERSAND:
-          argJoiner = SEMICOLON_JOINER;
-          escaper = IDENTITY;
-          break;
-        default:
-          throw new IllegalStateException("Unknown separator type " + separatorType);
-      }
-    }
-
-    @AutoCodec @AutoCodec.VisibleForSerialization
-    static final Function<String, String> IDENTITY = Functions.identity();
-
-    @AutoCodec @AutoCodec.VisibleForSerialization
-    static final Function<String, String> COLON_COMMA =
-        (input) -> input.replace(":", "\\:").replace(",", "\\,");
-
-    @AutoCodec @AutoCodec.VisibleForSerialization
-    static final Joiner SEMICOLON_JOINER = Joiner.on(";");
-
-    @AutoCodec @AutoCodec.VisibleForSerialization static final Joiner COLON_JOINER = Joiner.on(":");
-
-    @Override
-    public void expandToCommandLine(ValidatedAndroidData container, Consumer<String> args) {
-      args.accept(map(container));
-    }
-
-    String map(ValidatedAndroidData container) {
-      ImmutableList.Builder<String> cmdPieces = ImmutableList.builder();
-      if (includes.contains(Includes.ResourceRoots)) {
-        cmdPieces.add(convertRoots(container.getResourceRoots()));
-        cmdPieces.add(convertRoots(container.getAssetRoots()));
-      }
-      if (includes.contains(Includes.Label)) {
-        cmdPieces.add(escaper.apply(container.getLabel().toString()));
-      }
-      if (includes.contains(Includes.Manifest)) {
-        cmdPieces.add(container.getManifest().getExecPathString());
-      }
-      if (includes.contains(Includes.RTxt)) {
-        cmdPieces.add(container.getRTxt() == null ? "" : container.getRTxt().getExecPathString());
-      }
-      if (includes.contains(Includes.Aapt2RTxt)) {
-        cmdPieces.add(
-            container.getAapt2RTxt() == null ? "" : container.getAapt2RTxt().getExecPathString());
-      }
-      if (includes.contains(Includes.StaticLibrary)) {
-        cmdPieces.add(
-            container.getStaticLibrary() == null
-                ? ""
-                : container.getStaticLibrary().getExecPathString());
-      }
-      if (includes.contains(Includes.CompiledSymbols)) {
-        cmdPieces.add(
-            container.getCompiledSymbols() == null
-                ? ""
-                : container.getCompiledSymbols().getExecPathString());
-      }
-      if (includes.contains(Includes.SymbolsBin)) {
-        cmdPieces.add(
-            container.getSymbols() == null ? "" : container.getSymbols().getExecPathString());
-      }
-      return argJoiner.join(cmdPieces.build());
-    }
-
-    String listSeparator() {
-      switch (separatorType) {
-        case COLON_COMMA:
-          return ",";
-        case SEMICOLON_AMPERSAND:
-          return "&";
-        default:
-          Preconditions.checkState(false, "Unknown separator type " + separatorType);
-          return null;
-      }
-    }
-
-    @Override
-    public int maxInstancesAllowed() {
-      // This is the max number of resource converters we expect to statically
-      // construct for any given blaze instance.
-      // Do not increase recklessly.
-      return 10;
-    }
-
-    @Override
-    public boolean equals(Object o) {
-      if (this == o) {
-        return true;
-      }
-      if (o == null || getClass() != o.getClass()) {
-        return false;
-      }
-      ToArg toArg = (ToArg) o;
-      return includes.equals(toArg.includes) && separatorType == toArg.separatorType;
-    }
-
-    @Override
-    public int hashCode() {
-      return Objects.hashCode(includes, separatorType);
-    }
-  }
-
-  static class Builder {
-
-    private final Set<Includes> includes = new HashSet<>();
-    private ToArg.SeparatorType separatorType;
-
-    Builder() {}
-
-    Builder include(Includes include) {
-      includes.add(include);
-      return this;
-    }
-
-    Builder withSeparator(ToArg.SeparatorType type) {
-      separatorType = type;
-      return this;
-    }
-
-    ToArg toArgConverter() {
-      return new ToArg(this);
-    }
-  }
-
-  @VisibleForTesting
-  public static String convertRoots(Iterable<PathFragment> roots) {
-    return Joiner.on("#").join(Iterators.transform(roots.iterator(), Functions.toStringFunction()));
-  }
-
-  /**
-   * Convert ResourceDependencies to commandline args and artifacts, assuming the commandline
-   * arguments should be split into direct deps and transitive deps.
-   */
-  static void addToCommandLine(
-      ResourceDependencies dependencies, CustomCommandLine.Builder cmdBuilder, ToArg toArg) {
-    cmdBuilder.addAll(
-        "--data",
-        VectorArg.join(toArg.listSeparator())
-            .each(dependencies.getTransitiveResourceContainers())
-            .mapped(toArg));
-    cmdBuilder.addAll(
-        "--directData",
-        VectorArg.join(toArg.listSeparator())
-            .each(dependencies.getDirectResourceContainers())
-            .mapped(toArg));
-  }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java
index 6617ddc57b..1fd1d2059a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java
@@ -13,21 +13,13 @@
 // limitations under the License.
 package com.google.devtools.build.lib.rules.android;
 
-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.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.rules.android.AndroidConfiguration.AndroidAaptVersion;
-import com.google.devtools.build.lib.rules.android.ResourceContainerConverter.ToArg;
-import com.google.devtools.build.lib.rules.android.ResourceContainerConverter.ToArg.Includes;
-import com.google.devtools.build.lib.util.OS;
+import com.google.devtools.build.lib.rules.android.AndroidDataConverter.JoinerType;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
 
 /**
  * Builder for generating R classes for robolectric action.
@@ -37,27 +29,28 @@ import com.google.devtools.build.lib.util.OS;
  */
 public class RobolectricResourceSymbolsActionBuilder {
 
-  private static final ResourceContainerConverter.ToArg RESOURCE_CONTAINER_TO_ARG =
-      ResourceContainerConverter.builder()
-          .include(Includes.ResourceRoots)
-          .include(Includes.Manifest)
-          .include(Includes.RTxt)
-          .include(Includes.SymbolsBin)
-          .withSeparator(ToArg.SeparatorType.COLON_COMMA)
-          .toArgConverter();
-
-  private static final ResourceContainerConverter.ToArg RESOURCE_CONTAINER_TO_AAPT2_ARG =
-      ResourceContainerConverter.builder()
-          .include(Includes.ResourceRoots)
-          .include(Includes.Manifest)
-          .include(Includes.Aapt2RTxt)
-          .include(Includes.SymbolsBin)
-          .withSeparator(ToArg.SeparatorType.COLON_COMMA)
-          .toArgConverter();
+  @AutoCodec @VisibleForSerialization
+  static final AndroidDataConverter<ValidatedAndroidData> TO_ARG =
+      AndroidDataConverter.<ValidatedAndroidData>builder(JoinerType.COLON_COMMA)
+          .withRoots(ValidatedAndroidData::getResourceRoots)
+          .withRoots(ValidatedAndroidData::getAssetRoots)
+          .withArtifact(ValidatedAndroidData::getManifest)
+          .maybeWithArtifact(ValidatedAndroidData::getRTxt)
+          .maybeWithArtifact(ValidatedAndroidData::getSymbols)
+          .build();
+
+  @AutoCodec @VisibleForSerialization
+  static final AndroidDataConverter<ValidatedAndroidData> TO_ARG_AAPT2 =
+      AndroidDataConverter.<ValidatedAndroidData>builder(JoinerType.COLON_COMMA)
+          .withRoots(ValidatedAndroidData::getResourceRoots)
+          .withRoots(ValidatedAndroidData::getAssetRoots)
+          .withArtifact(ValidatedAndroidData::getManifest)
+          .maybeWithArtifact(ValidatedAndroidData::getAapt2RTxt)
+          .maybeWithArtifact(ValidatedAndroidData::getSymbols)
+          .build();
 
   private Artifact classJarOut;
   private final ResourceDependencies dependencies;
-  private AndroidSdkProvider sdk;
   private AndroidAaptVersion androidAaptVersion;
 
   private RobolectricResourceSymbolsActionBuilder(ResourceDependencies dependencies) {
@@ -73,77 +66,36 @@ public class RobolectricResourceSymbolsActionBuilder {
     return this;
   }
 
-  public RobolectricResourceSymbolsActionBuilder setSdk(AndroidSdkProvider sdk) {
-    this.sdk = sdk;
-    return this;
-  }
-
   public RobolectricResourceSymbolsActionBuilder targetAaptVersion(
       AndroidAaptVersion androidAaptVersion) {
     this.androidAaptVersion = androidAaptVersion;
     return this;
   }
 
-  public NestedSet<Artifact> buildAsClassPathEntry(RuleContext ruleContext) {
-    CustomCommandLine.Builder builder = new CustomCommandLine.Builder();
-    // Set the busybox tool.
-    builder.add("--tool").add("GENERATE_ROBOLECTRIC_R").add("--");
-
-    NestedSetBuilder<Artifact> inputs = NestedSetBuilder.stableOrder();
-
-    builder.addExecPath("--androidJar", sdk.getAndroidJar());
-    inputs.add(sdk.getAndroidJar());
-
-    ToArg resourceContainerToArg;
-
-    if (androidAaptVersion == AndroidAaptVersion.AAPT2) {
-      inputs.addTransitive(dependencies.getTransitiveAapt2RTxt());
-      resourceContainerToArg = RESOURCE_CONTAINER_TO_AAPT2_ARG;
-    } else {
-      inputs.addTransitive(dependencies.getTransitiveRTxt());
-      resourceContainerToArg = RESOURCE_CONTAINER_TO_ARG;
-    }
-    if (!Iterables.isEmpty(dependencies.getResourceContainers())) {
-      builder.addAll(
-          "--data",
-          VectorArg.join(resourceContainerToArg.listSeparator())
-              .each(dependencies.getResourceContainers())
-              .mapped(resourceContainerToArg));
+  public NestedSet<Artifact> buildAsClassPathEntry(AndroidDataContext dataContext) {
+    BusyBoxActionBuilder builder =
+        BusyBoxActionBuilder.create(dataContext, "GENERATE_ROBOLECTRIC_R").addAndroidJar();
+
+    if (!dependencies.getResourceContainers().isEmpty()) {
+      builder
+          .addTransitiveFlag(
+              "--data",
+              dependencies.getResourceContainers(),
+              androidAaptVersion == AndroidAaptVersion.AAPT2 ? TO_ARG_AAPT2 : TO_ARG)
+          .addTransitiveInputValues(
+              androidAaptVersion == AndroidAaptVersion.AAPT2
+                  ? dependencies.getTransitiveAapt2RTxt()
+                  : dependencies.getTransitiveRTxt())
+          .addTransitiveInputValues(dependencies.getTransitiveResources())
+          .addTransitiveInputValues(dependencies.getTransitiveAssets())
+          .addTransitiveInputValues(dependencies.getTransitiveManifests())
+          .addTransitiveInputValues(dependencies.getTransitiveSymbolsBin());
     }
 
-    inputs
-        .addTransitive(dependencies.getTransitiveResources())
-        .addTransitive(dependencies.getTransitiveAssets())
-        .addTransitive(dependencies.getTransitiveManifests())
-        .addTransitive(dependencies.getTransitiveSymbolsBin());
-
-    builder.addExecPath("--classJarOutput", classJarOut);
-    builder.addLabel("--targetLabel", ruleContext.getLabel());
-
-    SpawnAction.Builder spawnActionBuilder = new SpawnAction.Builder();
-
-    ParamFileInfo.Builder paramFile = ParamFileInfo.builder(ParameterFileType.SHELL_QUOTED);
-    // Some flags (e.g. --mainData) may specify lists (or lists of lists) separated by special
-    // characters (colon, semicolon, hashmark, ampersand) that don't work on Windows, and quoting
-    // semantics are very complicated (more so than in Bash), so let's just always use a parameter
-    // file.
-    // TODO(laszlocsomor), TODO(corysmith): restructure the Android BusyBux's flags by deprecating
-    // list-type and list-of-list-type flags that use such problematic separators in favor of
-    // multi-value flags (to remove one level of listing) and by changing all list separators to a
-    // platform-safe character (= comma).
-    paramFile.setUseAlways(OS.getCurrent() == OS.WINDOWS);
-
-    ruleContext.registerAction(
-        spawnActionBuilder
-            .useDefaultShellEnvironment()
-            .addTransitiveInputs(inputs.build())
-            .addOutput(classJarOut)
-            .addCommandLine(builder.build(), paramFile.build())
-            .setExecutable(
-                ruleContext.getExecutablePrerequisite("$android_resources_busybox", Mode.HOST))
-            .setProgressMessage("Generating R classes for %s", ruleContext.getLabel())
-            .setMnemonic("GenerateRobolectricRClasses")
-            .build(ruleContext));
+    builder
+        .addOutput("--classJarOutput", classJarOut)
+        .addLabelFlag("--targetLabel")
+        .buildAndRegister("Generating R classes", "GenerateRobolectricRClasses");
 
     return NestedSetBuilder.<Artifact>naiveLinkOrder().add(classJarOut).build();
   }