From 158dda74f4d6d33b88be4ab4c40322eca5f1c2fd Mon Sep 17 00:00:00 2001 From: asteinb Date: Thu, 3 May 2018 08:09:09 -0700 Subject: Fix up newly discovered bugs in decoupled Android data processing - Always generate a symbols file - the new resource processing pipeline likes knowing that this is non-null, and it shouldn't cost much extra to create. - A misleading method signature (which I made) led to me forgetting about Proguard artifacts. Properly propogate them into the ResourceApk object. - Don't get Aapt version directly from AndroidConfiguration - there's some additional logic in AndroidAaptVersion not exposed elsewhere - Split library tests that expect resources and assets to be processed together to have a new version where they're processed seperately. - Tests use ValidatedAndroidData interface rather than ResourceContainer object - Properly move some LocalTest magic around resource JAR out of the old pipeline only, as it should apply to both old and new pipelines. - Processing action defaults to empty resource and asset deps rather than null RELNOTES: none PiperOrigin-RevId: 195253161 --- .../build/lib/rules/android/AarImport.java | 2 +- .../build/lib/rules/android/AndroidAssets.java | 2 +- .../build/lib/rules/android/AndroidLibrary.java | 2 +- .../lib/rules/android/AndroidLocalTestBase.java | 3 +- .../build/lib/rules/android/AndroidManifest.java | 38 +++- .../android/AndroidResourcesProcessorBuilder.java | 8 +- .../lib/rules/android/MergedAndroidAssets.java | 1 - .../lib/rules/android/MergedAndroidResources.java | 7 +- .../lib/rules/android/ParsedAndroidResources.java | 4 +- .../lib/rules/android/ProcessedAndroidData.java | 33 ++- .../build/lib/rules/android/ResourceApk.java | 19 +- .../build/lib/rules/android/ResourceContainer.java | 2 +- .../rules/android/ValidatedAndroidResources.java | 22 +- .../build/lib/rules/android/AarImportTest.java | 224 +++++++++++++++++++-- .../build/lib/rules/android/AndroidBinaryTest.java | 38 ++-- .../rules/android/AndroidBuildViewTestCase.java | 26 ++- .../lib/rules/android/AndroidLibraryTest.java | 84 ++++++++ .../lib/rules/android/AndroidResourcesTest.java | 30 ++- 18 files changed, 451 insertions(+), 94 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java b/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java index 056bc8a537..2293f2abbd 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java @@ -105,7 +105,7 @@ public class AarImport implements RuleConfiguredTargetFactory { MergedAndroidAssets mergedAssets = AndroidAssets.forAarImport(assets).process(ruleContext, neverlink); - resourceApk = ResourceApk.of(validatedResources, mergedAssets); + resourceApk = ResourceApk.of(validatedResources, mergedAssets, null, null); } else { ApplicationManifest androidManifest = ApplicationManifest.fromExplicitManifest(ruleContext, androidManifestArtifact); 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 a20b818cc1..fc112ae174 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 @@ -99,7 +99,7 @@ public class AndroidAssets { ImmutableList.of(assetsDir), ImmutableList.of(assetsDir.getExecPath().getChild("assets"))); } - static AndroidAssets empty() { + public static AndroidAssets empty() { return new AndroidAssets(ImmutableList.of(), ImmutableList.of()); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java index df7601b51a..f2fd46f9f2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java @@ -161,7 +161,7 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory { MergedAndroidAssets assets = AndroidAssets.from(ruleContext).process(ruleContext, isNeverLink); - resourceApk = ResourceApk.of(resources, assets); + resourceApk = ResourceApk.of(resources, assets, null, null); } else { ApplicationManifest applicationManifest = androidSemantics diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java index 6f324cd2b2..05a9cc32e1 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java @@ -125,9 +125,10 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor : null, null, /* featureOfArtifact */ null /* featureAfterArtifact */); - attributesBuilder.addRuntimeClassPathEntry(resourceApk.getResourceJavaClassJar()); } + attributesBuilder.addRuntimeClassPathEntry(resourceApk.getResourceJavaClassJar()); + // Exclude the Rs from the library from the runtime classpath. NestedSet excludedRuntimeArtifacts = getLibraryResourceJars(ruleContext); attributesBuilder.addExcludedArtifacts(excludedRuntimeArtifacts); diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifest.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifest.java index 12dae484b6..02b741dd1d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifest.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifest.java @@ -77,20 +77,40 @@ public class AndroidManifest { exported); } + /** + * Stamps the manifest with values from the "manifest_values" attributes. + * + *

If no manifest values are specified, the manifest will remain unstamped. + */ + public StampedAndroidManifest stampWithManifestValues(RuleContext ruleContext) { + return mergeWithDeps(ruleContext, ResourceDependencies.empty()); + } + /** * Merges the manifest with any dependent manifests. * - *

The resulting manifest will be stamped, even if no merging was done. + *

The manifest will also be stamped with any manifest values specified in the target's + * attributes + * + *

If there is no merging to be done and no manifest values are specified, the manifest will + * remain unstamped. */ public StampedAndroidManifest mergeWithDeps(RuleContext ruleContext) { - return ApplicationManifest.maybeMergeWith( - ruleContext, - manifest, - ResourceDependencies.fromRuleDeps(ruleContext, /* neverlink = */ false), - ApplicationManifest.getManifestValues(ruleContext)) - .map(merged -> new StampedAndroidManifest(merged, pkg, exported)) - // If we don't merge, we still need to guarantee the manifest is stamped correctly - .orElseGet(() -> stamp(ruleContext)); + return mergeWithDeps( + ruleContext, ResourceDependencies.fromRuleDeps(ruleContext, /* neverlink = */ false)); + } + + private StampedAndroidManifest mergeWithDeps( + RuleContext ruleContext, ResourceDependencies resourceDeps) { + Artifact newManifest = + ApplicationManifest.maybeMergeWith( + ruleContext, + manifest, + resourceDeps, + ApplicationManifest.getManifestValues(ruleContext)) + .orElse(manifest); + + return new StampedAndroidManifest(newManifest, pkg, exported); } public Artifact getManifest() { diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java index 93a20a2b55..a93740d3b1 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java @@ -63,8 +63,8 @@ public class AndroidResourcesProcessorBuilder { .withSeparator(ToArg.SeparatorType.COLON_COMMA) .toArgConverter(); - private ResourceDependencies resourceDependencies; - private AssetDependencies assetDependencies; + private ResourceDependencies resourceDependencies = ResourceDependencies.empty(); + private AssetDependencies assetDependencies = AssetDependencies.empty(); private Artifact proguardOut; private Artifact mainDexProguardOut; @@ -305,7 +305,9 @@ public class AndroidResourcesProcessorBuilder { sourceJarOut, apkOut, dataBindingInfoZip, - resourceDependencies); + resourceDependencies, + proguardOut, + mainDexProguardOut); } public AndroidResourcesProcessorBuilder setJavaPackage(String customJavaPackage) { 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 608d56be38..6890108769 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 @@ -46,7 +46,6 @@ public class MergedAndroidAssets extends ParsedAndroidAssets { builder .addOutput("--assetsOutput", mergedAssets) - .addInput("--androidJar", AndroidSdkProvider.fromRuleContext(ruleContext).getAndroidJar()) .addInput( "--primaryData", AndroidDataConverter.MERGABLE_DATA_CONVERTER.map(parsed), diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/MergedAndroidResources.java b/src/main/java/com/google/devtools/build/lib/rules/android/MergedAndroidResources.java index 717159a143..07c274ac81 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/MergedAndroidResources.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/MergedAndroidResources.java @@ -39,12 +39,12 @@ public class MergedAndroidResources extends ParsedAndroidResources { public static MergedAndroidResources mergeFrom( RuleContext ruleContext, ParsedAndroidResources parsed, boolean neverlink) - throws InterruptedException { + throws InterruptedException, RuleErrorException { AndroidConfiguration androidConfiguration = AndroidCommon.getAndroidConfig(ruleContext); boolean useCompiledMerge = - androidConfiguration.getAndroidAaptVersion().equals(AndroidAaptVersion.AAPT2) + AndroidAaptVersion.chooseTargetAaptVersion(ruleContext) == AndroidAaptVersion.AAPT2 && androidConfiguration.skipParsingAction(); Preconditions.checkState( @@ -141,7 +141,8 @@ public class MergedAndroidResources extends ParsedAndroidResources { *

See {@link ValidatedAndroidResources#validateFrom(RuleContext, MergedAndroidResources)}. * This method is a convenience method for calling that one. */ - public ValidatedAndroidResources validate(RuleContext ruleContext) throws InterruptedException { + public ValidatedAndroidResources validate(RuleContext ruleContext) + throws InterruptedException, RuleErrorException { return ValidatedAndroidResources.validateFrom(ruleContext, this); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ParsedAndroidResources.java b/src/main/java/com/google/devtools/build/lib/rules/android/ParsedAndroidResources.java index d5f73af10d..44b08e07ae 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ParsedAndroidResources.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ParsedAndroidResources.java @@ -36,7 +36,7 @@ public class ParsedAndroidResources extends AndroidResources throws RuleErrorException, InterruptedException { boolean isAapt2 = - AndroidAaptVersion.chooseTargetAaptVersion(ruleContext).equals(AndroidAaptVersion.AAPT2); + AndroidAaptVersion.chooseTargetAaptVersion(ruleContext) == AndroidAaptVersion.AAPT2; AndroidResourceParsingActionBuilder builder = new AndroidResourceParsingActionBuilder(ruleContext); @@ -124,7 +124,7 @@ public class ParsedAndroidResources extends AndroidResources /** Merges this target's resources with resources from dependencies. */ public MergedAndroidResources merge(RuleContext ruleContext, boolean neverlink) - throws InterruptedException { + throws InterruptedException, RuleErrorException { return MergedAndroidResources.mergeFrom(ruleContext, this, neverlink); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ProcessedAndroidData.java b/src/main/java/com/google/devtools/build/lib/rules/android/ProcessedAndroidData.java index 637afabf25..8a0d0e66ab 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ProcessedAndroidData.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ProcessedAndroidData.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.rules.android; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.config.CompilationMode; +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.rules.android.AndroidConfiguration.AndroidAaptVersion; import com.google.devtools.build.lib.rules.java.ProguardHelper; @@ -46,6 +47,8 @@ public class ProcessedAndroidData { private final Artifact apk; @Nullable private final Artifact dataBindingInfoZip; private final ResourceDependencies resourceDeps; + @Nullable private final Artifact resourceProguardConfig; + @Nullable private final Artifact mainDexProguardConfig; /** Processes Android data (assets, resources, and manifest) for android_binary targets. */ public static ProcessedAndroidData processBinaryDataFrom( @@ -72,6 +75,18 @@ public class ProcessedAndroidData { .setDataBindingInfoZip( DataBinding.isEnabled(ruleContext) ? DataBinding.getLayoutInfoFile(ruleContext) + : null) + .setFeatureOf( + ruleContext.attributes().isAttributeValueExplicitlySpecified("feature_of") + ? ruleContext + .getPrerequisite("feature_of", Mode.TARGET, ApkInfo.PROVIDER) + .getApk() + : null) + .setFeatureAfter( + ruleContext.attributes().isAttributeValueExplicitlySpecified("feature_after") + ? ruleContext + .getPrerequisite("feature_after", Mode.TARGET, ApkInfo.PROVIDER) + .getApk() : null); return buildActionForBinary(ruleContext, builder, manifest); } @@ -185,7 +200,8 @@ public class ProcessedAndroidData { .setApkOut(ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_APK)) .setRTxtOut(ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT)) .setSourceJarOut( - ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_JAVA_SOURCE_JAR)); + ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_JAVA_SOURCE_JAR)) + .setSymbols(ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_MERGED_SYMBOLS)); } /** @@ -219,9 +235,12 @@ public class ProcessedAndroidData { Artifact sourceJar, Artifact apk, @Nullable Artifact dataBindingInfoZip, - ResourceDependencies resourceDeps) { + ResourceDependencies resourceDeps, + @Nullable Artifact resourceProguardConfig, + @Nullable Artifact mainDexProguardConfig) { return new ProcessedAndroidData( - resources, assets, manifest, rTxt, sourceJar, apk, dataBindingInfoZip, resourceDeps); + resources, assets, manifest, rTxt, sourceJar, apk, dataBindingInfoZip, resourceDeps, + resourceProguardConfig, mainDexProguardConfig); } private ProcessedAndroidData( @@ -232,7 +251,9 @@ public class ProcessedAndroidData { Artifact sourceJar, Artifact apk, @Nullable Artifact dataBindingInfoZip, - ResourceDependencies resourceDeps) { + ResourceDependencies resourceDeps, + @Nullable Artifact resourceProguardConfig, + @Nullable Artifact mainDexProguardConfig) { this.resources = resources; this.assets = assets; this.manifest = manifest; @@ -241,6 +262,8 @@ public class ProcessedAndroidData { this.apk = apk; this.dataBindingInfoZip = dataBindingInfoZip; this.resourceDeps = resourceDeps; + this.resourceProguardConfig = resourceProguardConfig; + this.mainDexProguardConfig = mainDexProguardConfig; } /** @@ -278,7 +301,7 @@ public class ProcessedAndroidData { // Combined resource processing does not produce aapt2 artifacts; they're nulled out ValidatedAndroidResources validated = ValidatedAndroidResources.of(merged, rTxt, sourceJar, apk, null, null, null); - return ResourceApk.of(validated, assets); + return ResourceApk.of(validated, assets, resourceProguardConfig, mainDexProguardConfig); } public MergedAndroidAssets getAssets() { diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java index 9466e5fd01..96115475d9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java @@ -74,7 +74,11 @@ public final class ResourceApk { mainDexProguardConfig); } - public static ResourceApk of(ValidatedAndroidResources resources, MergedAndroidAssets assets) { + public static ResourceApk of( + ValidatedAndroidResources resources, + MergedAndroidAssets assets, + @Nullable Artifact resourceProguardConfig, + @Nullable Artifact mainDexProguardConfig) { return new ResourceApk( resources.getApk(), resources.getJavaSourceJar(), @@ -86,8 +90,8 @@ public final class ResourceApk { assets, resources.getManifest(), resources.getRTxt(), - null, - null); + resourceProguardConfig, + mainDexProguardConfig); } private ResourceApk( @@ -239,9 +243,12 @@ public final class ResourceApk { AndroidAssetsInfo assetsInfo = merged.toProvider(); builder.addNativeDeclaredProvider(assetsInfo); - // Asset merging output isn't consumed by anything. Require it to be run by top-level targets - // so we can validate there are no asset merging conflicts. - builder.addOutputGroup(OutputGroupInfo.HIDDEN_TOP_LEVEL, assetsInfo.getValidationResult()); + if (assetsInfo.getValidationResult() != null) { + // Asset merging output isn't consumed by anything. Require it to be run by top-level + // targets + // so we can validate there are no asset merging conflicts. + builder.addOutputGroup(OutputGroupInfo.HIDDEN_TOP_LEVEL, assetsInfo.getValidationResult()); + } } else if (primaryAssets == null) { builder.addNativeDeclaredProvider(assetDeps.toInfo(label)); 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 90573075f4..035d77e856 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 @@ -89,7 +89,7 @@ public abstract class ResourceContainer implements ValidatedAndroidData { return getAndroidAssets().getAssets(); } - abstract AndroidAssets getAndroidAssets(); + public abstract AndroidAssets getAndroidAssets(); @Override public ImmutableList getResources() { diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ValidatedAndroidResources.java b/src/main/java/com/google/devtools/build/lib/rules/android/ValidatedAndroidResources.java index c6158b5d43..528c53fd35 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ValidatedAndroidResources.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ValidatedAndroidResources.java @@ -55,8 +55,8 @@ public class ValidatedAndroidResources extends MergedAndroidResources * */ public static ValidatedAndroidResources validateFrom( - RuleContext ruleContext, MergedAndroidResources merged) throws InterruptedException { - AndroidConfiguration config = AndroidCommon.getAndroidConfig(ruleContext); + RuleContext ruleContext, MergedAndroidResources merged) + throws InterruptedException, RuleErrorException { AndroidResourceValidatorActionBuilder builder = new AndroidResourceValidatorActionBuilder(ruleContext) .setJavaPackage(merged.getJavaPackage()) @@ -72,7 +72,7 @@ public class ValidatedAndroidResources extends MergedAndroidResources ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_LIBRARY_APK)) .withDependencies(merged.getResourceDependencies()); - if (config.getAndroidAaptVersion() == AndroidAaptVersion.AAPT2) { + if (AndroidAaptVersion.chooseTargetAaptVersion(ruleContext) == AndroidAaptVersion.AAPT2) { builder .setCompiledSymbols(merged.getCompiledSymbols()) .setAapt2RTxtOut( @@ -169,13 +169,7 @@ public class ValidatedAndroidResources extends MergedAndroidResources .map( merged -> ValidatedAndroidResources.of( - merged, - rTxt, - sourceJar, - apk, - aapt2RTxt, - aapt2SourceJar, - staticLibrary)); + merged, rTxt, sourceJar, apk, aapt2RTxt, aapt2SourceJar, staticLibrary)); } @Override @@ -196,12 +190,6 @@ public class ValidatedAndroidResources extends MergedAndroidResources @Override public int hashCode() { return Objects.hash( - super.hashCode(), - rTxt, - sourceJar, - apk, - aapt2RTxt, - aapt2SourceJar, - staticLibrary); + super.hashCode(), rTxt, sourceJar, apk, aapt2RTxt, aapt2SourceJar, staticLibrary); } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java index 4e5f05e1d7..54ca21a176 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java @@ -91,7 +91,9 @@ public class AarImportTest extends BuildViewTestCase { } @Test - public void testResourcesProvided() throws Exception { + public void testResourcesProvided_NotDecoupled() throws Exception { + useConfiguration("--noandroid_decouple_data_processing"); + ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:foo"); NestedSet directResources = @@ -113,7 +115,37 @@ public class AarImportTest extends BuildViewTestCase { } @Test - public void testResourcesExtractor() throws Exception { + public void testResourcesProvided() throws Exception { + useConfiguration("--android_decouple_data_processing"); + + ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:foo"); + + NestedSet directResources = + aarImportTarget.get(AndroidResourcesInfo.PROVIDER).getDirectAndroidResources(); + assertThat(directResources).hasSize(1); + + ValidatedAndroidData resourceContainer = directResources.iterator().next(); + assertThat(resourceContainer.getManifest()).isNotNull(); + + Artifact resourceTreeArtifact = Iterables.getOnlyElement(resourceContainer.getResources()); + assertThat(resourceTreeArtifact.isTreeArtifact()).isTrue(); + assertThat(resourceTreeArtifact.getExecPathString()).endsWith("_aar/unzipped/resources/foo"); + + NestedSet directAssets = + aarImportTarget.get(AndroidAssetsInfo.PROVIDER).getDirectParsedAssets(); + assertThat(directAssets).hasSize(1); + + ParsedAndroidAssets assets = directAssets.iterator().next(); + assertThat(assets.getSymbols()).isNotNull(); + + Artifact assetsTreeArtifact = Iterables.getOnlyElement(assets.getAssets()); + assertThat(assetsTreeArtifact.isTreeArtifact()).isTrue(); + assertThat(assetsTreeArtifact.getExecPathString()).endsWith("_aar/unzipped/assets/foo"); + } + + @Test + public void testResourcesExtractor_NotDecoupled() throws Exception { + useConfiguration("--noandroid_decouple_data_processing"); ValidatedAndroidData resourceContainer = getConfiguredTarget("//a:foo") .get(AndroidResourcesInfo.PROVIDER) @@ -125,9 +157,9 @@ public class AarImportTest extends BuildViewTestCase { Artifact assetsTreeArtifact = resourceContainer.getAssets().get(0); Artifact aarResourcesExtractor = getHostConfiguredTarget( - ruleClassProvider.getToolsRepository() + "//tools/android:aar_resources_extractor") - .getProvider(FilesToRunProvider.class) - .getExecutable(); + ruleClassProvider.getToolsRepository() + "//tools/android:aar_resources_extractor") + .getProvider(FilesToRunProvider.class) + .getExecutable(); assertThat(getGeneratingSpawnAction(resourceTreeArtifact).getArguments()) .containsExactly( @@ -141,8 +173,45 @@ public class AarImportTest extends BuildViewTestCase { } @Test - public void testDepsCheckerActionExistsForLevelError() throws Exception { - useConfiguration("--experimental_import_deps_checking=ERROR"); + public void testResourcesExtractor() throws Exception { + useConfiguration("--android_decouple_data_processing"); + ValidatedAndroidData resourceContainer = + getConfiguredTarget("//a:foo") + .get(AndroidResourcesInfo.PROVIDER) + .getDirectAndroidResources() + .toList() + .get(0); + + Artifact resourceTreeArtifact = resourceContainer.getResources().get(0); + Artifact aarResourcesExtractor = + getHostConfiguredTarget( + ruleClassProvider.getToolsRepository() + "//tools/android:aar_resources_extractor") + .getProvider(FilesToRunProvider.class) + .getExecutable(); + + ParsedAndroidAssets assets = + getConfiguredTarget("//a:foo") + .get(AndroidAssetsInfo.PROVIDER) + .getDirectParsedAssets() + .toList() + .get(0); + Artifact assetsTreeArtifact = assets.getAssets().get(0); + + assertThat(getGeneratingSpawnAction(resourceTreeArtifact).getArguments()) + .containsExactly( + aarResourcesExtractor.getExecPathString(), + "--input_aar", + "a/foo.aar", + "--output_res_dir", + resourceTreeArtifact.getExecPathString(), + "--output_assets_dir", + assetsTreeArtifact.getExecPathString()); + } + + @Test + public void testDepsCheckerActionExistsForLevelErrorNotDecoupled() throws Exception { + useConfiguration( + "--experimental_import_deps_checking=ERROR", "--noandroid_decouple_data_processing"); ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:last"); OutputGroupInfo outputGroupInfo = aarImportTarget.get(OutputGroupInfo.SKYLARK_CONSTRUCTOR); NestedSet outputGroup = @@ -172,8 +241,50 @@ public class AarImportTest extends BuildViewTestCase { } @Test - public void testDepsCheckerActionExistsForLevelStrictError() throws Exception { - useConfiguration("--experimental_import_deps_checking=STRICT_ERROR"); + public void testDepsCheckerActionExistsForLevelError() throws Exception { + useConfiguration( + "--experimental_import_deps_checking=ERROR", "--android_decouple_data_processing"); + ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:last"); + OutputGroupInfo outputGroupInfo = aarImportTarget.get(OutputGroupInfo.SKYLARK_CONSTRUCTOR); + NestedSet outputGroup = + outputGroupInfo.getOutputGroup(OutputGroupInfo.HIDDEN_TOP_LEVEL); + assertThat(outputGroup).hasSize(2); + + // We should force asset merging to happen + Artifact mergedAssetsZip = + aarImportTarget.get(AndroidAssetsInfo.PROVIDER).getValidationResult(); + assertThat(outputGroup).contains(mergedAssetsZip); + + // Get the other artifact from the output group + Artifact artifact = ActionsTestUtil.getFirstArtifactEndingWith(outputGroup, ".txt"); + + assertThat(artifact.isTreeArtifact()).isFalse(); + assertThat(artifact.getExecPathString()) + .endsWith("_aar/last/aar_import_deps_checker_result.txt"); + + SpawnAction checkerAction = getGeneratingSpawnAction(artifact); + List arguments = checkerAction.getArguments(); + assertThat(arguments) + .containsAllOf( + "--bootclasspath_entry", + "--classpath_entry", + "--input", + "--output", + "--fail_on_errors"); + ensureArgumentsHaveClassEntryOptionWithSuffix(arguments, "/bar/classes_and_libs_merged.jar"); + ensureArgumentsHaveClassEntryOptionWithSuffix(arguments, "/baz/java/baz-ijar.jar"); + ensureArgumentsHaveClassEntryOptionWithSuffix(arguments, "/baz/classes_and_libs_merged.jar"); + ensureArgumentsHaveClassEntryOptionWithSuffix(arguments, "/foo/classes_and_libs_merged.jar"); + ensureArgumentsHaveClassEntryOptionWithSuffix( + arguments, "/intermediate/classes_and_libs_merged.jar"); + assertThat(arguments.stream().filter(arg -> "--classpath_entry".equals(arg)).count()) + .isEqualTo(5); + } + + @Test + public void testDepsCheckerActionExistsForLevelStrictError_NotDecoupled() throws Exception { + useConfiguration( + "--experimental_import_deps_checking=STRICT_ERROR", "--noandroid_decouple_data_processing"); ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:last"); OutputGroupInfo outputGroupInfo = aarImportTarget.get(OutputGroupInfo.SKYLARK_CONSTRUCTOR); NestedSet outputGroup = @@ -199,13 +310,84 @@ public class AarImportTest extends BuildViewTestCase { } @Test - public void testDepsCheckerActionExistsForLevelWarning() throws Exception { - useConfiguration("--experimental_import_deps_checking=WARNING"); + public void testDepsCheckerActionExistsForLevelStrictError() throws Exception { + useConfiguration( + "--experimental_import_deps_checking=STRICT_ERROR", "--android_decouple_data_processing"); + ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:last"); + OutputGroupInfo outputGroupInfo = aarImportTarget.get(OutputGroupInfo.SKYLARK_CONSTRUCTOR); + NestedSet outputGroup = + outputGroupInfo.getOutputGroup(OutputGroupInfo.HIDDEN_TOP_LEVEL); + assertThat(outputGroup).hasSize(2); + + // We should force asset merging to happen + Artifact mergedAssetsZip = + aarImportTarget.get(AndroidAssetsInfo.PROVIDER).getValidationResult(); + assertThat(outputGroup).contains(mergedAssetsZip); + + // Get the other artifact from the output group + Artifact artifact = ActionsTestUtil.getFirstArtifactEndingWith(outputGroup, ".txt"); + assertThat(artifact.isTreeArtifact()).isFalse(); + assertThat(artifact.getExecPathString()) + .endsWith("_aar/last/aar_import_deps_checker_result.txt"); + + SpawnAction checkerAction = getGeneratingSpawnAction(artifact); + List arguments = checkerAction.getArguments(); + assertThat(arguments) + .containsAllOf( + "--bootclasspath_entry", + "--classpath_entry", + "--input", + "--output", + "--fail_on_errors"); + ensureArgumentsHaveClassEntryOptionWithSuffix( + arguments, "/intermediate/classes_and_libs_merged.jar"); + assertThat(arguments.stream().filter(arg -> "--classpath_entry".equals(arg)).count()) + .isEqualTo(1); + } + + @Test + public void testDepsCheckerActionExistsForLevelWarning_NotDecoupled() throws Exception { + useConfiguration( + "--experimental_import_deps_checking=WARNING", "--noandroid_decouple_data_processing"); ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:bar"); OutputGroupInfo outputGroupInfo = aarImportTarget.get(OutputGroupInfo.SKYLARK_CONSTRUCTOR); NestedSet outputGroup = outputGroupInfo.getOutputGroup(OutputGroupInfo.HIDDEN_TOP_LEVEL); Artifact artifact = Iterables.getOnlyElement(outputGroup); + + assertThat(artifact.isTreeArtifact()).isFalse(); + assertThat(artifact.getExecPathString()) + .endsWith("_aar/bar/aar_import_deps_checker_result.txt"); + + SpawnAction checkerAction = getGeneratingSpawnAction(artifact); + List arguments = checkerAction.getArguments(); + assertThat(arguments) + .containsAllOf( + "--bootclasspath_entry", + "--classpath_entry", + "--input", + "--output", + "--nofail_on_errors"); + } + + @Test + public void testDepsCheckerActionExistsForLevelWarning() throws Exception { + useConfiguration( + "--experimental_import_deps_checking=WARNING", "--android_decouple_data_processing"); + ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:bar"); + OutputGroupInfo outputGroupInfo = aarImportTarget.get(OutputGroupInfo.SKYLARK_CONSTRUCTOR); + NestedSet outputGroup = + outputGroupInfo.getOutputGroup(OutputGroupInfo.HIDDEN_TOP_LEVEL); + assertThat(outputGroup).hasSize(2); + + // We should force asset merging to happen + Artifact mergedAssetsZip = + aarImportTarget.get(AndroidAssetsInfo.PROVIDER).getValidationResult(); + assertThat(outputGroup).contains(mergedAssetsZip); + + // Get the other artifact from the output group + Artifact artifact = ActionsTestUtil.getFirstArtifactEndingWith(outputGroup, ".txt"); + assertThat(artifact.isTreeArtifact()).isFalse(); assertThat(artifact.getExecPathString()) .endsWith("_aar/bar/aar_import_deps_checker_result.txt"); @@ -246,8 +428,9 @@ public class AarImportTest extends BuildViewTestCase { } @Test - public void testDepsCheckerActionNotExistsForLevelOff() throws Exception { - useConfiguration("--experimental_import_deps_checking=OFF"); + public void testDepsCheckerActionNotExistsForLevelOff_NotDecoupled() throws Exception { + useConfiguration( + "--experimental_import_deps_checking=OFF", "--noandroid_decouple_data_processing"); ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:bar"); OutputGroupInfo outputGroupInfo = aarImportTarget.get(OutputGroupInfo.SKYLARK_CONSTRUCTOR); NestedSet outputGroup = @@ -255,6 +438,21 @@ public class AarImportTest extends BuildViewTestCase { assertThat(outputGroup).isEmpty(); } + @Test + public void testDepsCheckerActionNotExistsForLevelOff() throws Exception { + useConfiguration( + "--experimental_import_deps_checking=OFF", "--android_decouple_data_processing"); + ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:bar"); + OutputGroupInfo outputGroupInfo = aarImportTarget.get(OutputGroupInfo.SKYLARK_CONSTRUCTOR); + + // The deps checker action should not exist, but we should still create an action for asset + // merging + NestedSet outputGroup = + outputGroupInfo.getOutputGroup(OutputGroupInfo.HIDDEN_TOP_LEVEL); + assertThat(outputGroup).hasSize(1); + assertThat(outputGroup.toList().get(0).getFilename()).isEqualTo("assets.zip"); + } + @Test public void testNativeLibsProvided() throws Exception { ConfiguredTarget androidLibraryTarget = getConfiguredTarget("//java:lib"); diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java index d24f2dc8e3..1dd3e80eed 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java @@ -1062,7 +1062,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { "java/com/google/android/neversayneveragain/libb1.jar_desugared.jar"); assertThat( resourceInputPaths( - "java/com/google/android/neversayneveragain", getResourceContainer(b1))) + "java/com/google/android/neversayneveragain", getValidatedData(b1))) .doesNotContain("res/values/resource.xml"); ConfiguredTarget b2 = getConfiguredTarget("//java/com/google/android/neversayneveragain:b2"); @@ -1079,7 +1079,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { "java/com/google/android/neversayneveragain/libb2.jar_desugared.jar"); assertThat( resourceInputPaths( - "java/com/google/android/neversayneveragain", getResourceContainer(b2))) + "java/com/google/android/neversayneveragain", getValidatedData(b2))) .doesNotContain("res/values/resource.xml"); ConfiguredTarget b3 = getConfiguredTarget("//java/com/google/android/neversayneveragain:b3"); @@ -1096,7 +1096,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { .doesNotContain("java/com/google/android/neversayneveragain/libl2.jar_desugared.jar"); assertThat( resourceInputPaths( - "java/com/google/android/neversayneveragain", getResourceContainer(b3))) + "java/com/google/android/neversayneveragain", getValidatedData(b3))) .contains("res/values/resource.xml"); } @@ -1189,7 +1189,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { // Ensure that the args are present ConfiguredTarget binary = getConfiguredTarget("//java/com/google/android:b"); - List args = resourceArguments(getResourceContainer(binary)); + List args = resourceArguments(getValidatedData(binary)); assertThat(flagValue("--resourceConfigs", args)).contains("en,fr"); } @@ -1243,7 +1243,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " aapt_version = 'aapt2',", " densities = ['hdpi, , ', 'xhdpi'],", " resource_files = ['" + Joiner.on("', '").join(resources) + "'])"); - ResourceContainer directResources = getResourceContainer(binary, /* transitive= */ false); + ValidatedAndroidData directResources = getValidatedData(binary, /* transitive= */ false); // Validate that the AndroidResourceProvider for this binary contains all values. assertThat(resourceContentsPaths(dir, directResources)).containsExactlyElementsIn(resources); @@ -1592,7 +1592,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { .getOutputDirectoryName()) .isNull(); - ResourceContainer directResources = getResourceContainer(binary, /* transitive= */ false); + ValidatedAndroidData directResources = getValidatedData(binary, /* transitive= */ false); // Validate that the AndroidResourceProvider for this binary contains only the filtered values. assertThat(resourceContentsPaths(dir, directResources)) @@ -1649,8 +1649,8 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " deps = [':lib'],", " resource_configuration_filters = ['en'])"); - ResourceContainer directResources = getResourceContainer(binary, /* transitive= */ false); - ResourceContainer transitiveResources = getResourceContainer(binary, /* transitive= */ true); + ValidatedAndroidData directResources = getValidatedData(binary, /* transitive= */ false); + ValidatedAndroidData transitiveResources = getValidatedData(binary, /* transitive= */ true); assertThat(resourceContentsPaths(dir, directResources)).isEmpty(); @@ -1698,7 +1698,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " 'multiple/res/drawable-en-mdpi/foo.png'],", ")"); - ResourceContainer directResources = getResourceContainer(binary, /* transitive= */ false); + ValidatedAndroidData directResources = getValidatedData(binary, /* transitive= */ false); // All of the resources are transitive assertThat(resourceContentsPaths(dir, directResources)).isEmpty(); @@ -1752,7 +1752,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " ])"); List resourceProcessingArgs = - getGeneratingSpawnActionArgs(getResourceContainer(binary).getRTxt()); + getGeneratingSpawnActionArgs(getValidatedData(binary).getRTxt()); assertThat( Iterables.filter( @@ -1784,7 +1784,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " manifest = 'AndroidManifest.xml')"); List resourceProcessingArgs = - getGeneratingSpawnActionArgs(getResourceContainer(binary).getRTxt()); + getGeneratingSpawnActionArgs(getValidatedData(binary).getRTxt()); assertThat(resourceProcessingArgs).containsAllOf("--resourceConfigs", "ar-rXB,en,en-rXA"); } @@ -1801,7 +1801,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { useConfiguration("--experimental_android_throw_on_resource_conflict"); ConfiguredTarget binary = getConfiguredTarget("//java/r/android:r"); - List resourceProcessingArgs = resourceArguments(getResourceContainer(binary)); + List resourceProcessingArgs = resourceArguments(getValidatedData(binary)); assertThat(resourceProcessingArgs).contains("--throwOnResourceConflict"); } @@ -1813,7 +1813,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { * @return the paths to all artifacts from the input that are contained within the given * directory, relative to that directory. */ - private List resourceContentsPaths(String dir, ResourceContainer resource) { + private List resourceContentsPaths(String dir, ValidatedAndroidData resource) { return pathsToArtifacts(dir, resource.getArtifacts()); } @@ -1826,7 +1826,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { * @return the paths to all artifacts used as inputs to resource processing that are contained * within the given directory, relative to that directory. */ - private List resourceInputPaths(String dir, ResourceContainer resource) { + private List resourceInputPaths(String dir, ValidatedAndroidData resource) { return pathsToArtifacts(dir, resourceGeneratingAction(resource).getInputs()); } @@ -3250,7 +3250,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " nocompress_extensions = ['.apk', '.so'],", ")"); ConfiguredTarget binary = getConfiguredTarget("//java/r/android:r"); - ResourceContainer resource = getResourceContainer(binary); + ValidatedAndroidData resource = getValidatedData(binary); List args = resourceArguments(resource); Artifact inputManifest = getFirstArtifactEndingWith( @@ -4211,7 +4211,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { ConfiguredTarget b = getDirectPrerequisite(a, "//java/b:b"); List resourceProcessingArgs = - getGeneratingSpawnActionArgs(getResourceContainer(a).getRTxt()); + getGeneratingSpawnActionArgs(getValidatedData(a).getRTxt()); assertThat(resourceProcessingArgs).contains("AAPT2_PACKAGE"); String directData = @@ -4222,7 +4222,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { assertThat(resourceProcessingArgs).contains("--useCompiledResourcesForMerge"); List resourceMergingArgs = - getGeneratingSpawnActionArgs(getResourceContainer(b).getJavaClassJar()); + getGeneratingSpawnActionArgs(getValidatedData(b).getJavaClassJar()); assertThat(resourceMergingArgs).contains("MERGE_COMPILED"); } @@ -4255,7 +4255,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { ConfiguredTarget b = getDirectPrerequisite(a, "//java/b:b"); List resourceProcessingArgs = - getGeneratingSpawnActionArgs(getResourceContainer(a).getRTxt()); + getGeneratingSpawnActionArgs(getValidatedData(a).getRTxt()); assertThat(resourceProcessingArgs).contains("PACKAGE"); String directData = @@ -4265,7 +4265,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { assertThat(directData).contains("merged.bin"); List compiledResourceMergingArgs = - getGeneratingSpawnActionArgs(getResourceContainer(b).getJavaClassJar()); + getGeneratingSpawnActionArgs(getValidatedData(b).getJavaClassJar()); assertThat(compiledResourceMergingArgs).contains("MERGE_COMPILED"); diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java index e85c1c1bcf..2848520e2f 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java @@ -115,12 +115,12 @@ public abstract class AndroidBuildViewTestCase extends BuildViewTestCase { .isNotNull(); } - protected List resourceArguments(ResourceContainer resource) + protected List resourceArguments(ValidatedAndroidData resource) throws CommandLineExpansionException { return getGeneratingSpawnActionArgs(resource.getApk()); } - protected SpawnAction resourceGeneratingAction(ResourceContainer resource) { + protected SpawnAction resourceGeneratingAction(ValidatedAndroidData resource) { return getGeneratingSpawnAction(resource.getApk()); } @@ -130,15 +130,22 @@ public abstract class AndroidBuildViewTestCase extends BuildViewTestCase { protected static ResourceContainer getResourceContainer( ConfiguredTarget target, boolean transitive) { + ValidatedAndroidData validated = getValidatedData(target, transitive); + assertThat(validated).isInstanceOf(ResourceContainer.class); + return (ResourceContainer) validated; + } + + protected static ValidatedAndroidData getValidatedData(ConfiguredTarget target) { + return getValidatedData(target, /* transitive = */ false); + } + protected static ValidatedAndroidData getValidatedData( + ConfiguredTarget target, boolean transitive) { Preconditions.checkNotNull(target); final AndroidResourcesInfo info = target.get(AndroidResourcesInfo.PROVIDER); assertThat(info).named("No android resources exported from the target.").isNotNull(); - ValidatedAndroidData validated = - getOnlyElement( - transitive ? info.getTransitiveAndroidResources() : info.getDirectAndroidResources()); - assertThat(validated).isInstanceOf(ResourceContainer.class); - return (ResourceContainer) validated; + return getOnlyElement( + transitive ? info.getTransitiveAndroidResources() : info.getDirectAndroidResources()); } protected Artifact getResourceClassJar(final ConfiguredTargetAndData target) { @@ -233,6 +240,11 @@ public abstract class AndroidBuildViewTestCase extends BuildViewTestCase { .getJavaClassJar(); } + // Returns an artifact that will be generated when a rule has assets that are processed seperately + static Artifact getDecoupledAssetArtifact(ConfiguredTarget target) { + return target.get(AndroidAssetsInfo.PROVIDER).getValidationResult(); + } + protected static Set getNonToolInputs(Action action) { return Sets.difference( ImmutableSet.copyOf(action.getInputs()), ImmutableSet.copyOf(action.getTools())); diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidLibraryTest.java index 3b97bcbe96..4acc3483b2 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidLibraryTest.java @@ -1233,6 +1233,8 @@ public class AndroidLibraryTest extends AndroidBuildViewTestCase { @Test public void testMultipleDirectDependentResourceDirectories_LocalResources() throws Exception { + useConfiguration("--noandroid_decouple_data_processing"); + scratch.file("java/android/resources/d1/BUILD", "android_library(name = 'd1',", " manifest = 'AndroidManifest.xml',", @@ -1256,10 +1258,44 @@ public class AndroidLibraryTest extends AndroidBuildViewTestCase { assertNoEvents(); } + @Test + public void testMultipleDirectDependentResourceDirectories_DecoupledLocalResources() + throws Exception { + useConfiguration("--android_decouple_data_processing"); + + scratch.file( + "java/android/resources/d1/BUILD", + "android_library(name = 'd1',", + " manifest = 'AndroidManifest.xml',", + " resource_files = ['d1-res/values/strings.xml'],", + " assets = ['assets-d1/some/random/file'],", + " assets_dir = 'assets-d1',", + " deps = ['//java/android/resources/d2:d2'])"); + scratch.file( + "java/android/resources/d2/BUILD", + "android_library(name = 'd2',", + " manifest = 'AndroidManifest.xml',", + " assets = ['assets-d2/some/random/file'],", + " assets_dir = 'assets-d2',", + " resource_files = ['d2-res/values/strings.xml'],", + " )"); + ConfiguredTarget resource = getConfiguredTarget("//java/android/resources/d1:d1"); + List args = getGeneratingSpawnActionArgs(getResourceArtifact(resource)); + assertPrimaryResourceDirs(ImmutableList.of("java/android/resources/d1/d1-res"), args); + assertThat(getDirectDependentResourceDirs(args)).contains("java/android/resources/d2/d2-res"); + + List assetArgs = getGeneratingSpawnActionArgs(getDecoupledAssetArtifact(resource)); + assertThat(getDependentAssetDirs("--directData", assetArgs)) + .contains("java/android/resources/d2/assets-d2"); + + assertNoEvents(); + } @Test public void testTransitiveDependentResourceDirectories_LocalResources() throws Exception { + useConfiguration("--noandroid_decouple_data_processing"); + scratch.file("java/android/resources/d1/BUILD", "android_library(name = 'd1',", " manifest = 'AndroidManifest.xml',", @@ -1297,6 +1333,54 @@ public class AndroidLibraryTest extends AndroidBuildViewTestCase { assertNoEvents(); } + @Test + public void testTransitiveDependentResourceDirectories_DecoupledLocalResources() + throws Exception { + useConfiguration("--android_decouple_data_processing"); + + scratch.file( + "java/android/resources/d1/BUILD", + "android_library(name = 'd1',", + " manifest = 'AndroidManifest.xml',", + " resource_files = ['d1-res/values/strings.xml'],", + " assets = ['assets-d1/some/random/file'],", + " assets_dir = 'assets-d1',", + " deps = ['//java/android/resources/d2:d2'])"); + scratch.file( + "java/android/resources/d2/BUILD", + "android_library(name = 'd2',", + " manifest = 'AndroidManifest.xml',", + " assets = ['assets-d2/some/random/file'],", + " assets_dir = 'assets-d2',", + " resource_files = ['d2-res/values/strings.xml'],", + " deps = ['//java/android/resources/d3:d3'],", + " )"); + scratch.file( + "java/android/resources/d3/BUILD", + "android_library(name = 'd3',", + " manifest = 'AndroidManifest.xml',", + " assets = ['assets-d3/some/random/file'],", + " assets_dir = 'assets-d3',", + " resource_files = ['d3-res/values/strings.xml'],", + " )"); + + ConfiguredTarget resource = getConfiguredTarget("//java/android/resources/d1:d1"); + List args = getGeneratingSpawnActionArgs(getResourceArtifact(resource)); + assertPrimaryResourceDirs(ImmutableList.of("java/android/resources/d1/d1-res"), args); + Truth.assertThat(getDirectDependentResourceDirs(args)) + .contains("java/android/resources/d2/d2-res"); + Truth.assertThat(getTransitiveDependentResourceDirs(args)) + .contains("java/android/resources/d3/d3-res"); + + List assetArgs = getGeneratingSpawnActionArgs(getDecoupledAssetArtifact(resource)); + Truth.assertThat(getDependentAssetDirs("--directData", assetArgs)) + .contains("java/android/resources/d2/assets-d2"); + Truth.assertThat(getDependentAssetDirs("--data", assetArgs)) + .contains("java/android/resources/d3/assets-d3"); + + assertNoEvents(); + } + @Test public void testCustomJavacopts() throws Exception { scratch.file("java/android/BUILD", diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java index 6e8a32a84d..1ab8083626 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java @@ -430,7 +430,9 @@ public class AndroidResourcesTest extends ResourceTestBase { ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_JAVA_SOURCE_JAR), ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_APK), /* dataBindingInfoZip = */ null, - ResourceDependencies.fromRuleDeps(ruleContext, /* neverlink = */ false)); + ResourceDependencies.fromRuleDeps(ruleContext, /* neverlink = */ false), + null, + null); ValidatedAndroidData validated = processedData.generateRClass(ruleContext).getValidatedResources(); @@ -442,6 +444,18 @@ public class AndroidResourcesTest extends ResourceTestBase { /* outputs = */ ImmutableList.of(validated.getJavaClassJar())); } + @Test + public void testProcessBinaryDataGeneratesProguardOutput() throws Exception { + RuleContext ruleContext = getRuleContext("android_binary", "manifest='AndroidManifest.xml',"); + + ResourceApk resourceApk = + ProcessedAndroidData.processBinaryDataFrom(ruleContext, getManifest(), false) + .generateRClass(ruleContext); + + assertThat(resourceApk.getResourceProguardConfig()).isNotNull(); + assertThat(resourceApk.getMainDexProguardConfig()).isNotNull(); + } + /** * Validates that a parse action was invoked correctly. Returns the {@link ParsedAndroidResources} * for further validation. @@ -484,13 +498,21 @@ public class AndroidResourcesTest extends ResourceTestBase { /** Gets a dummy rule context object by creating a dummy target. */ private RuleContext getRuleContext(boolean useDataBinding) throws Exception { + return getRuleContext("android_library", useDataBinding ? " enable_data_binding = True" : ""); + } + + /** Gets a dummy rule context object by creating a dummy target. */ + private RuleContext getRuleContext(String kind, String... additionalLines) throws Exception { ConfiguredTarget target = scratchConfiguredTarget( "java/foo", "target", - "android_library(name = 'target',", - useDataBinding ? " enable_data_binding = True" : "", - ")"); + ImmutableList.builder() + .add(kind + "(name = 'target',") + .add(additionalLines) + .add(")") + .build() + .toArray(new String[0])); return getRuleContextForActionTesting(target); } } -- cgit v1.2.3