Expose merge_assets method to skylark

RELNOTES: none
PiperOrigin-RevId: 195266230

5 files changed, 173 insertions, 24 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidAssets.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidAssets.java
index fc112ae174..32c5989cf1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidAssets.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidAssets.java
@@ -23,19 +23,38 @@ import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
+import com.google.devtools.build.lib.packages.RuleErrorConsumer;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Objects;
+import javax.annotation.Nullable;
 
 /** Wraps this target's Android assets */
 public class AndroidAssets {
   private static final String ASSETS_ATTR = "assets";
   private static final String ASSETS_DIR_ATTR = "assets_dir";
 
+  /**
+   * Validates that either neither or both of assets and assets_dir are set.
+   *
+   * <p>TODO(b/77574966): Remove this method and just validate assets as part of {@link
+   * #from(RuleErrorConsumer, Iterable, PathFragment)} once assets are fully decoupled from
+   * resources.
+   *
+   * @deprecated Instead, validate assets as part of creating an AndroidAssets object.
+   */
+  @Deprecated
   static void validateAssetsAndAssetsDir(RuleContext ruleContext) throws RuleErrorException {
-    if (ruleContext.attributes().isAttributeValueExplicitlySpecified(ASSETS_ATTR)
-        ^ ruleContext.attributes().isAttributeValueExplicitlySpecified(ASSETS_DIR_ATTR)) {
-      ruleContext.throwWithRuleError(
+    validateAssetsAndAssetsDir(ruleContext, getAssetTargets(ruleContext), getAssetDir(ruleContext));
+  }
+
+  private static void validateAssetsAndAssetsDir(
+      RuleErrorConsumer errorConsumer,
+      @Nullable Iterable<? extends TransitiveInfoCollection> assetTargets,
+      @Nullable PathFragment assetsDir)
+      throws RuleErrorException {
+    if (assetTargets == null ^ assetsDir == null) {
+      errorConsumer.throwWithRuleError(
           String.format(
               "'%s' and '%s' should be either both empty or both non-empty",
               ASSETS_ATTR, ASSETS_DIR_ATTR));
@@ -44,19 +63,24 @@ public class AndroidAssets {
 
   /** Collects this rule's android assets. */
   public static AndroidAssets from(RuleContext ruleContext) throws RuleErrorException {
-    validateAssetsAndAssetsDir(ruleContext);
+    return from(ruleContext, getAssetTargets(ruleContext), getAssetDir(ruleContext));
+  }
 
-    if (!ruleContext.attributes().has(ASSETS_ATTR)) {
-      return new AndroidAssets(ImmutableList.of(), ImmutableList.of());
-    }
+  static AndroidAssets from(
+      RuleErrorConsumer errorConsumer,
+      @Nullable Iterable<? extends TransitiveInfoCollection> assetTargets,
+      @Nullable PathFragment assetsDir)
+      throws RuleErrorException {
+    validateAssetsAndAssetsDir(errorConsumer, assetTargets, assetsDir);
 
-    PathFragment assetsDir = getAssetDir(ruleContext);
+    if (assetTargets == null) {
+      return empty();
+    }
 
     ImmutableList.Builder<Artifact> assets = ImmutableList.builder();
     ImmutableList.Builder<PathFragment> assetRoots = ImmutableList.builder();
 
-    for (TransitiveInfoCollection target :
-        ruleContext.getPrerequisitesIf(ASSETS_ATTR, Mode.TARGET, FileProvider.class)) {
+    for (TransitiveInfoCollection target : assetTargets) {
       for (Artifact file : target.getProvider(FileProvider.class).getFilesToBuild()) {
         PathFragment packageFragment =
             file.getArtifactOwner().getLabel().getPackageIdentifier().getSourceRoot();
@@ -66,7 +90,7 @@ public class AndroidAssets {
           PathFragment path = file.getExecPath();
           assetRoots.add(path.subFragment(0, path.segmentCount() - relativePath.segmentCount()));
         } else {
-          ruleContext.attributeError(
+          errorConsumer.attributeError(
               ASSETS_ATTR,
               String.format(
                   "'%s' (generated by '%s') is not beneath '%s'",
@@ -80,7 +104,21 @@ public class AndroidAssets {
     return new AndroidAssets(assets.build(), assetRoots.build());
   }
 
+  @Nullable
+  private static Iterable<? extends TransitiveInfoCollection> getAssetTargets(
+      RuleContext ruleContext) {
+    if (!ruleContext.attributes().isAttributeValueExplicitlySpecified(ASSETS_ATTR)) {
+      return null;
+    }
+
+    return ruleContext.getPrerequisitesIf(ASSETS_ATTR, Mode.TARGET, FileProvider.class);
+  }
+
+  @Nullable
   private static PathFragment getAssetDir(RuleContext ruleContext) {
+    if (!ruleContext.attributes().isAttributeValueExplicitlySpecified(ASSETS_DIR_ATTR)) {
+      return null;
+    }
     return PathFragment.create(ruleContext.attributes().get(ASSETS_DIR_ATTR, Type.STRING));
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSkylarkData.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSkylarkData.java
index a6b8466a73..b4e3508489 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSkylarkData.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSkylarkData.java
@@ -14,11 +14,18 @@
 package com.google.devtools.build.lib.rules.android;
 
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.skylark.SkylarkRuleContext;
+import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
 import com.google.devtools.build.lib.skylarkinterface.Param;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
+import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Runtime;
+import com.google.devtools.build.lib.syntax.SkylarkList;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import java.util.List;
 import javax.annotation.Nullable;
 
 /** Skylark-visible methods for working with Android data (manifests, resources, and assets). */
@@ -91,6 +98,91 @@ public class AndroidSkylarkData {
   }
 
   /**
+   * Skylark API for merging android_library assets
+   *
+   * <p>TODO(b/79159379): Stop passing SkylarkRuleContext here
+   *
+   * @param ctx the SkylarkRuleContext. We will soon change to using an ActionConstructionContext
+   *     instead. See b/79159379
+   */
+  @SkylarkCallable(
+      name = "merge_assets",
+      mandatoryPositionals = 1, // context
+      parameters = {
+        @Param(
+            name = "assets",
+            positional = false,
+            defaultValue = "None",
+            type = SkylarkList.class,
+            generic1 = ConfiguredTarget.class,
+            noneable = true,
+            named = true,
+            doc =
+                "Targets containing raw assets for this target. If passed, 'assets_dir' must also"
+                    + " be passed."),
+        @Param(
+            name = "assets_dir",
+            positional = false,
+            defaultValue = "None",
+            type = String.class,
+            noneable = true,
+            named = true,
+            doc =
+                "Directory the assets are contained in. Must be passed if and only if 'assets' is"
+                    + " passed. This path will be split off of the asset paths on the device."),
+        @Param(
+            name = "deps",
+            positional = false,
+            defaultValue = "[]",
+            type = SkylarkList.class,
+            generic1 = AndroidAssetsInfo.class,
+            named = true,
+            doc =
+                "Providers containing assets from dependencies. These assets will be merged"
+                    + " together with each other and this target's assets."),
+        @Param(
+            name = "neverlink",
+            positional = false,
+            defaultValue = "False",
+            type = Boolean.class,
+            named = true,
+            doc =
+                "Defaults to False. If passed as True, these assets will not be inherited by"
+                    + " targets that depend on this one.")
+      },
+      doc =
+          "Merges this target's assets together with assets inherited from dependencies. Note that,"
+              + " by default, actions for validating the merge are created but may not be called."
+              + " You may want to force these actions to be called - see the 'validation_result'"
+              + " field in AndroidAssetsInfo")
+  public AndroidAssetsInfo mergeAssets(
+      SkylarkRuleContext ctx,
+      Object assets,
+      Object assetsDir,
+      SkylarkList<AndroidAssetsInfo> deps,
+      boolean neverlink)
+      throws EvalException, InterruptedException {
+    try {
+      return AndroidAssets.from(
+              ctx.getRuleContext(),
+              listFromNoneable(assets, ConfiguredTarget.class),
+              isNone(assetsDir) ? null : PathFragment.create(fromNoneable(assetsDir, String.class)))
+          .parse(ctx.getRuleContext())
+          .merge(
+              ctx.getRuleContext(),
+              AssetDependencies.fromProviders(deps.getImmutableList(), neverlink))
+          .toProvider();
+    } catch (RuleErrorException e) {
+      throw new EvalException(Location.BUILTIN, e);
+    }
+  }
+
+  /** Checks if a "Noneable" object passed by Skylark is "None", which Java should treat as null. */
+  private static boolean isNone(Object object) {
+    return object == Runtime.NONE;
+  }
+
+  /**
    * Converts a "Noneable" Object passed by Skylark to an nullable object of the appropriate type.
    *
    * <p>Skylark "Noneable" types are passed in as an Object that may be either the correct type or a
@@ -104,10 +196,26 @@ public class AndroidSkylarkData {
    */
   @Nullable
   private static <T> T fromNoneable(Object object, Class<T> clazz) {
-    if (object == Runtime.NONE) {
+    if (isNone(object)) {
       return null;
     }
 
     return clazz.cast(object);
   }
+
+  /**
+   * Converts a "Noneable" Object passed by Skylark to a List of the appropriate type.
+   *
+   * <p>This first calls {@link #fromNoneable(Object, Class)} to get a SkylarkList<?>, then safely
+   * casts it to a list with the appropriate generic.
+   */
+  @Nullable
+  private static <T> List<T> listFromNoneable(Object object, Class<T> clazz) throws EvalException {
+    SkylarkList<?> asList = fromNoneable(object, SkylarkList.class);
+    if (asList == null) {
+      return null;
+    }
+
+    return SkylarkList.castList(asList, clazz, null);
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AssetDependencies.java b/src/main/java/com/google/devtools/build/lib/rules/android/AssetDependencies.java
index ef088993a8..cc9c8dc5ae 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AssetDependencies.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AssetDependencies.java
@@ -39,14 +39,19 @@ public class AssetDependencies {
   private final NestedSet<Artifact> transitiveSymbols;
 
   static AssetDependencies fromRuleDeps(RuleContext ruleContext, boolean neverlink) {
+    return fromProviders(
+        AndroidCommon.getTransitivePrerequisites(
+            ruleContext, Mode.TARGET, AndroidAssetsInfo.PROVIDER),
+        neverlink);
+  }
+
+  static AssetDependencies fromProviders(Iterable<AndroidAssetsInfo> providers, boolean neverlink) {
     NestedSetBuilder<ParsedAndroidAssets> direct = NestedSetBuilder.naiveLinkOrder();
     NestedSetBuilder<ParsedAndroidAssets> transitive = NestedSetBuilder.naiveLinkOrder();
     NestedSetBuilder<Artifact> assets = NestedSetBuilder.naiveLinkOrder();
     NestedSetBuilder<Artifact> symbols = NestedSetBuilder.naiveLinkOrder();
 
-    for (AndroidAssetsInfo info :
-        AndroidCommon.getTransitivePrerequisites(
-            ruleContext, Mode.TARGET, AndroidAssetsInfo.PROVIDER)) {
+    for (AndroidAssetsInfo info : providers) {
       direct.addTransitive(info.getDirectParsedAssets());
       transitive.addTransitive(info.getTransitiveParsedAssets());
       assets.addTransitive(info.getAssets());
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/MergedAndroidAssets.java b/src/main/java/com/google/devtools/build/lib/rules/android/MergedAndroidAssets.java
index 6890108769..25c74122c5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/MergedAndroidAssets.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/MergedAndroidAssets.java
@@ -13,7 +13,6 @@
 // limitations under the License.
 package com.google.devtools.build.lib.rules.android;
 
-import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
@@ -25,13 +24,6 @@ public class MergedAndroidAssets extends ParsedAndroidAssets {
   private final Artifact mergedAssets;
   private final AssetDependencies assetDependencies;
 
-  public static MergedAndroidAssets mergeFrom(
-      RuleContext ruleContext, ParsedAndroidAssets parsed, boolean neverlink)
-      throws InterruptedException {
-    return mergeFrom(ruleContext, parsed, AssetDependencies.fromRuleDeps(ruleContext, neverlink));
-  }
-
-  @VisibleForTesting
   static MergedAndroidAssets mergeFrom(
       RuleContext ruleContext, ParsedAndroidAssets parsed, AssetDependencies deps)
       throws InterruptedException {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ParsedAndroidAssets.java b/src/main/java/com/google/devtools/build/lib/rules/android/ParsedAndroidAssets.java
index 3419ceeca5..696643328f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ParsedAndroidAssets.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ParsedAndroidAssets.java
@@ -47,7 +47,13 @@ public class ParsedAndroidAssets extends AndroidAssets implements MergableAndroi
   /** Merges these assets with assets from dependencies. */
   public MergedAndroidAssets merge(RuleContext ruleContext, boolean neverlink)
       throws InterruptedException {
-    return MergedAndroidAssets.mergeFrom(ruleContext, this, neverlink);
+    return MergedAndroidAssets.mergeFrom(
+        ruleContext, this, AssetDependencies.fromRuleDeps(ruleContext, neverlink));
+  }
+
+  MergedAndroidAssets merge(RuleContext ruleContext, AssetDependencies assetDeps)
+      throws InterruptedException {
+    return MergedAndroidAssets.mergeFrom(ruleContext, this, assetDeps);
   }
 
   @Override