Rollback "Allow Merge action to take an interface as primary, not just ResourceContainer", as it breaks some android rule integration tests.

RELNOTES: none
PiperOrigin-RevId: 191304264

6 files changed, 26 insertions, 235 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 1fa50a6628..f23146b3e1 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
@@ -208,7 +208,7 @@ public final class CustomCommandLine extends CommandLine {
       }
 
       /** Each argument is mapped using the supplied map function */
-      public MappedVectorArg<T> mapped(CommandLineItem.MapFn<? super T> mapFn) {
+      public MappedVectorArg<T> mapped(CommandLineItem.MapFn<T> mapFn) {
         return new MappedVectorArg<>(this, mapFn);
       }
     }
@@ -216,9 +216,9 @@ public final class CustomCommandLine extends CommandLine {
     /** A vector arg that maps some type T to strings. */
     static class MappedVectorArg<T> extends VectorArg<String> {
       private final Iterable<T> values;
-      private final CommandLineItem.MapFn<? super T> mapFn;
+      private final CommandLineItem.MapFn<T> mapFn;
 
-      private MappedVectorArg(SimpleVectorArg<T> other, CommandLineItem.MapFn<? super T> mapFn) {
+      private MappedVectorArg(SimpleVectorArg<T> other, CommandLineItem.MapFn<T> mapFn) {
         super(
             other.isNestedSet,
             other.isEmpty,
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
deleted file mode 100644
index cb367d45a2..0000000000
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataConverter.java
+++ /dev/null
@@ -1,151 +0,0 @@
-// Copyright 2018 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.collect.ImmutableList;
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.CommandLineItem.ParametrizedMapFn;
-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.cmdline.Label;
-import com.google.devtools.build.lib.collect.nestedset.NestedSet;
-import com.google.devtools.build.lib.vfs.PathFragment;
-import java.util.Objects;
-import java.util.function.Consumer;
-import java.util.function.Function;
-import java.util.stream.Collectors;
-
-/**
- * Factory for functions to convert a {@code T} to a commandline argument. Uses a certain convention
- * for commandline arguments (e.g., separators, and ordering of container elements).
- */
-public class AndroidDataConverter<T> extends ParametrizedMapFn<T> {
-
-  /** Indicates the type of joiner between options expected by the command line. */
-  public enum JoinerType {
-    COLON_COMMA(":", ","),
-    SEMICOLON_AMPERSAND(";", "&");
-
-    private final String listSeparator;
-    private final String itemSeparator;
-
-    JoinerType(String listSeparator, String itemSeparator) {
-      this.listSeparator = listSeparator;
-      this.itemSeparator = itemSeparator;
-    }
-
-    private String escape(String string) {
-      return string
-          .replace(listSeparator, "\\" + listSeparator)
-          .replace(itemSeparator, "\\" + itemSeparator);
-    }
-  }
-
-  private final ImmutableList<Function<T, String>> suppliers;
-  private final JoinerType joinerType;
-
-  private AndroidDataConverter(
-      ImmutableList<Function<T, String>> suppliers, JoinerType joinerType) {
-    this.suppliers = suppliers;
-    this.joinerType = joinerType;
-  }
-
-  @Override
-  public boolean equals(Object obj) {
-    if (!(obj instanceof AndroidDataConverter)) {
-      return false;
-    }
-
-    AndroidDataConverter<?> other = (AndroidDataConverter) obj;
-    return suppliers.equals(other.suppliers) && joinerType.equals(other.joinerType);
-  }
-
-  @Override
-  public int hashCode() {
-    return Objects.hash(suppliers, joinerType);
-  }
-
-  @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 void expandToCommandLine(T t, Consumer<String> args) {
-    args.accept(map(t));
-  }
-
-  public String map(T t) {
-    return suppliers
-        .stream()
-        .map(s -> (s.apply(t)))
-        .collect(Collectors.joining(joinerType.listSeparator));
-  }
-
-  public static <T> Builder<T> builder(JoinerType joinerType) {
-    return new Builder<>(joinerType);
-  }
-
-  public void addDepsToCommandLine(
-      CustomCommandLine.Builder cmdBuilder,
-      NestedSet<? extends T> direct,
-      NestedSet<? extends T> transitive) {
-    cmdBuilder.addAll("--data", getVectorArg(transitive));
-    cmdBuilder.addAll("--directData", getVectorArg(direct));
-  }
-
-  public VectorArg<String> getVectorArg(NestedSet<? extends T> values) {
-    return VectorArg.join(joinerType.listSeparator).each(values).mapped(this);
-  }
-
-  static class Builder<T> {
-    private final ImmutableList.Builder<Function<T, String>> inner = ImmutableList.builder();
-    private final JoinerType joinerType;
-
-    private Builder(JoinerType joinerType) {
-      this.joinerType = joinerType;
-    }
-
-    Builder<T> withRoots(Function<T, ImmutableList<PathFragment>> rootsFunction) {
-      return with(
-          t ->
-              rootsFunction
-                  .apply(t)
-                  .stream()
-                  .map(PathFragment::toString)
-                  .collect(Collectors.joining("#")));
-    }
-
-    Builder<T> withArtifact(Function<T, Artifact> artifactFunction) {
-      return with(t -> artifactFunction.apply(t).getExecPathString());
-    }
-
-    Builder<T> withLabel(Function<T, Label> labelFunction) {
-      // Escape labels, since they are known to contain separating characters (specifically, ':').
-      return with(t -> joinerType.escape(labelFunction.apply(t).toString()));
-    }
-
-    Builder<T> with(Function<T, String> stringFunction) {
-      inner.add(stringFunction);
-      return this;
-    }
-
-    AndroidDataConverter<T> build() {
-      return new AndroidDataConverter<>(inner.build(), joinerType);
-    }
-  }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java
index 70d70aca56..65afc20314 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java
@@ -25,7 +25,8 @@ import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
 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.NestedSetBuilder;
-import com.google.devtools.build.lib.rules.android.AndroidDataConverter.JoinerType;
+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 java.util.ArrayList;
 import java.util.List;
@@ -39,22 +40,21 @@ import java.util.List;
  */
 public class AndroidResourceMergingActionBuilder {
 
-  private static final AndroidDataConverter<MergableAndroidData> RESOURCE_CONTAINER_TO_ARG =
-      AndroidDataConverter.<MergableAndroidData>builder(JoinerType.SEMICOLON_AMPERSAND)
-          .withRoots(MergableAndroidData::getResourceRoots)
-          .withRoots(MergableAndroidData::getAssetRoots)
-          .withLabel(MergableAndroidData::getLabel)
-          .withArtifact(MergableAndroidData::getSymbols)
-          .build();
-
-  private static final AndroidDataConverter<ResourceContainer>
-      RESOURCE_CONTAINER_TO_ARG_FOR_COMPILED =
-          AndroidDataConverter.<ResourceContainer>builder(JoinerType.SEMICOLON_AMPERSAND)
-              .withRoots(ResourceContainer::getResourceRoots)
-              .withRoots(ResourceContainer::getAssetRoots)
-              .withLabel(ResourceContainer::getLabel)
-              .withArtifact(ResourceContainer::getCompiledSymbols)
-              .build();
+  private static final ResourceContainerConverter.ToArg RESOURCE_CONTAINER_TO_ARG =
+      ResourceContainerConverter.builder()
+          .include(Includes.ResourceRoots)
+          .include(Includes.Label)
+          .include(Includes.SymbolsBin)
+          .withSeparator(ToArg.SeparatorType.SEMICOLON_AMPERSAND)
+          .toArgConverter();
+
+  private static final ResourceContainerConverter.ToArg RESOURCE_CONTAINER_TO_ARG_FOR_COMPILED =
+      ResourceContainerConverter.builder()
+          .include(Includes.ResourceRoots)
+          .include(Includes.Label)
+          .include(Includes.CompiledSymbols)
+          .withSeparator(ToArg.SeparatorType.SEMICOLON_AMPERSAND)
+          .toArgConverter();
 
   private final RuleContext ruleContext;
   private final AndroidSdkProvider sdk;
@@ -172,10 +172,8 @@ public class AndroidResourceMergingActionBuilder {
     inputs.add(primary.getCompiledSymbols());
 
     if (dependencies != null) {
-      RESOURCE_CONTAINER_TO_ARG_FOR_COMPILED.addDepsToCommandLine(
-          builder,
-          dependencies.getDirectResourceContainers(),
-          dependencies.getTransitiveResourceContainers());
+      ResourceContainerConverter.addToCommandLine(
+          dependencies, builder, RESOURCE_CONTAINER_TO_ARG_FOR_COMPILED);
       inputs.addTransitive(dependencies.getTransitiveResources());
       inputs.addTransitive(dependencies.getTransitiveAssets());
       inputs.addTransitive(dependencies.getTransitiveCompiledSymbols());
@@ -210,10 +208,7 @@ public class AndroidResourceMergingActionBuilder {
     inputs.add(primary.getSymbols());
 
     if (dependencies != null) {
-      RESOURCE_CONTAINER_TO_ARG.addDepsToCommandLine(
-          builder,
-          dependencies.getDirectResourceContainers(),
-          dependencies.getTransitiveResourceContainers());
+      ResourceContainerConverter.addToCommandLine(dependencies, builder, RESOURCE_CONTAINER_TO_ARG);
       inputs.addTransitive(dependencies.getTransitiveResources());
       inputs.addTransitive(dependencies.getTransitiveAssets());
       inputs.addTransitive(dependencies.getTransitiveSymbolsBin());
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/MergableAndroidData.java b/src/main/java/com/google/devtools/build/lib/rules/android/MergableAndroidData.java
deleted file mode 100644
index 47e162067a..0000000000
--- a/src/main/java/com/google/devtools/build/lib/rules/android/MergableAndroidData.java
+++ /dev/null
@@ -1,42 +0,0 @@
-// Copyright 2018 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.collect.ImmutableList;
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.vfs.PathFragment;
-
-/**
- * Interface used to indicate a container for resources, assets, or both can be merged.
- *
- * <p>Currently, resources and assets invoke the same action for merging, so virtually all the code
- * to generate that action is identical. Implementing this interface allows it to be reused.
- */
-public interface MergableAndroidData {
-
-  /** @return the roots of all resources to be merged */
-  default ImmutableList<PathFragment> getResourceRoots() {
-    return ImmutableList.of();
-  }
-
-  /** @return the roots of all assets to be merged */
-  default ImmutableList<PathFragment> getAssetRoots() {
-    return ImmutableList.of();
-  }
-
-  Label getLabel();
-
-  Artifact getSymbols();
-}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java
index 2bb66189a7..564597a5bb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java
@@ -40,7 +40,7 @@ import javax.annotation.Nullable;
   category = SkylarkModuleCategory.NONE,
   doc = "The Android resources contributed by a single target."
 )
-public abstract class ResourceContainer implements MergableAndroidData {
+public abstract class ResourceContainer {
   /** The type of resource in question: either asset or a resource. */
   public enum ResourceType {
     ASSETS("assets"),
@@ -92,20 +92,12 @@ public abstract class ResourceContainer implements MergableAndroidData {
         : getResources().getResources();
   }
 
+  /** @deprecated We are moving towards decoupling assets and resources */
+  @Deprecated
   public Iterable<Artifact> getArtifacts() {
     return Iterables.concat(getAssets().getAssets(), getResources().getResources());
   }
 
-  @Override
-  public ImmutableList<PathFragment> getResourceRoots() {
-    return getResources().getResourceRoots();
-  }
-
-  @Override
-  public ImmutableList<PathFragment> getAssetRoots() {
-    return getAssets().getAssetRoots();
-  }
-
   /**
    * Gets the directories containing the resources of a specific type.
    *
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
index df15063d50..102abfc0c2 100644
--- 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
@@ -35,11 +35,8 @@ 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() {