diff options
author | 2017-08-18 16:33:01 +0200 | |
---|---|---|
committer | 2017-08-21 14:15:46 +0200 | |
commit | e8909945ea1c566f90b864eb42a34d2f76cb72fd (patch) | |
tree | be87a8aa3541ca24dd7acf39aa4d93c4cb3893d0 /src/main/java/com/google | |
parent | 74a8c3e529f0c3ec9ab02db684e9d0ec4f71bf64 (diff) |
Prevent using android libraries as resources if they have non-resource info
android_libraries can now be used in the resources attribute. However, if these
libraries contain non-resource information, it won't get picked up. To prevent
unexpected behavior, fail if such libraries are used as resources.
Adding a temporary boolean to AndroidResourcesProvider seems to be the most
straightforward way of doing this. The alternative, having the consuming target
check all relevant providers for non-resource information, would be much
messier.
RELNOTES: None
PiperOrigin-RevId: 165703578
Diffstat (limited to 'src/main/java/com/google')
7 files changed, 117 insertions, 29 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 9143e4f5fa..05c667c76f 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 @@ -155,7 +155,8 @@ public class AarImport implements RuleConfiguredTargetFactory { JavaSkylarkApiProvider.NAME, JavaSkylarkApiProvider.fromRuleContext()) .addProvider(RunfilesProvider.class, RunfilesProvider.EMPTY) .addProvider( - AndroidResourcesProvider.class, resourceApk.toResourceProvider(ruleContext.getLabel())) + AndroidResourcesProvider.class, + resourceApk.toResourceProvider(ruleContext.getLabel(), /* isResourcesOnly = */ false)) .addProvider( NativeLibsZipsProvider.class, new NativeLibsZipsProvider( diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java index f13b70e70c..1beadb87b7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java @@ -808,7 +808,8 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { resourceApk, zipAlignedApk, apksUnderTest, - nativeLibs); + nativeLibs, + /* isResourcesOnly = */ false); if (proguardOutput.getMapping() != null) { builder.add( diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java index 3c938067c1..c7ca121ab2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java @@ -743,7 +743,8 @@ public class AndroidCommon { ResourceApk resourceApk, Artifact zipAlignedApk, Iterable<Artifact> apksUnderTest, - NativeLibs nativeLibs) { + NativeLibs nativeLibs, + boolean isResourcesOnly) { idlHelper.addTransitiveInfoProviders(builder, classJar, manifestProtoOutput); @@ -777,14 +778,16 @@ public class AndroidCommon { .setFilesToBuild(filesToBuild) .addSkylarkTransitiveInfo( JavaSkylarkApiProvider.NAME, JavaSkylarkApiProvider.fromRuleContext()) - .add(JavaRuleOutputJarsProvider.class, ruleOutputJarsProvider) - .add(JavaSourceJarsProvider.class, sourceJarsProvider) - .add( + .addProvider(JavaRuleOutputJarsProvider.class, ruleOutputJarsProvider) + .addProvider(JavaSourceJarsProvider.class, sourceJarsProvider) + .addProvider( JavaRuntimeJarProvider.class, new JavaRuntimeJarProvider(javaCommon.getJavaCompilationArtifacts().getRuntimeJars())) - .add(RunfilesProvider.class, RunfilesProvider.simple(getRunfiles())) - .add(AndroidResourcesProvider.class, resourceApk.toResourceProvider(ruleContext.getLabel())) - .add( + .addProvider(RunfilesProvider.class, RunfilesProvider.simple(getRunfiles())) + .addProvider( + AndroidResourcesProvider.class, + resourceApk.toResourceProvider(ruleContext.getLabel(), isResourcesOnly)) + .addProvider( AndroidIdeInfoProvider.class, createAndroidIdeInfoProvider( ruleContext, @@ -796,7 +799,7 @@ public class AndroidCommon { zipAlignedApk, apksUnderTest, nativeLibs)) - .add(JavaCompilationArgsProvider.class, compilationArgsProvider) + .addProvider(JavaCompilationArgsProvider.class, compilationArgsProvider) .addSkylarkTransitiveInfo(AndroidSkylarkApiProvider.NAME, new AndroidSkylarkApiProvider()) .addOutputGroup( OutputGroupProvider.HIDDEN_TOP_LEVEL, collectHiddenTopLevelArtifacts(ruleContext)) @@ -831,10 +834,24 @@ public class AndroidCommon { if (prerequisite == null) { return null; } + + AndroidResourcesProvider provider = prerequisite.getProvider(AndroidResourcesProvider.class); + + if (!provider.getIsResourcesOnly()) { + ruleContext.attributeError( + "resources", + "android_library target " + + prerequisite.getLabel() + + " cannot be used in the 'resources' attribute as it specifies information (probably" + + " 'srcs' or 'deps') not directly related to android_resources. Consider moving this" + + " target from 'resources' to 'deps'."); + return null; + } + ruleContext.ruleWarning( "The use of the android_resources rule and the resources attribute is deprecated. " + "Please use the resource_files, assets, and manifest attributes of android_library."); - return prerequisite.getProvider(AndroidResourcesProvider.class); + return provider; } /** 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 b9a1471ac0..71f8847e12 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 @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.rules.android; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.ConfiguredTarget; @@ -57,6 +58,48 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory { } } + /** + * Attributes provided by android_library targets that provide information also supported by + * android_resources targets. + * + * <p>As part of migrating away from android_resources, we are allowing android_library targets to + * be used in the 'resources' attribute of android_binary, android_library, and android_test + * targets. However, android_library targets can specify information that cannot be propagated by + * the 'resources' attribute. By enumerating those attributes which can be propagated by + * 'resources' and having the {@link AndroidResourcesProvider} specify whether any other + * attributes are used, we can error out if an android_library is specified in a resources + * attribute despite having information incompatible with that output. + * + * TODO(b/30307842): Remove this support once the resources attribute is completely removed. + * + * <p>With the exception of 'resource_files', these attributes are simply those provided by both + * android_library and android_resources. android_resources does provide the 'resources' + * attribute, but its behavior is like the android_library 'resource_files' attribute, not the + * android_library 'resources' attribute (which indicates a dependency on an android_resources + * target). + */ + private static final ImmutableSet<String> ATTRS_COMPATIBLE_WITH_ANDROID_RESOURCES = + ImmutableSet.of( + "assets", + "assets_dir", + "compatible_with", + "custom_package", + "deprecation", + "distribs", + "exports_manifest", + "features", + "inline_constants", + "javacopts", + "licenses", + "manifest", + "name", + "plugins", + "resource_files", + "restricted_to", + "tags", + "testonly", + "visibility"); + @Override public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedException, RuleErrorException { @@ -197,6 +240,15 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory { ruleContext.getFragment(AndroidConfiguration.class).throwOnResourceConflict()) .build(ruleContext); + boolean isResourcesOnly = true; + for (String attr : ruleContext.attributes().getAttributeNames()) { + if (ruleContext.attributes().isAttributeValueExplicitlySpecified(attr) + && !ATTRS_COMPATIBLE_WITH_ANDROID_RESOURCES.contains(attr)) { + isResourcesOnly = false; + break; + } + } + RuleConfiguredTargetBuilder builder = new RuleConfiguredTargetBuilder(ruleContext); androidCommon.addTransitiveInfoProviders( builder, @@ -205,7 +257,8 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory { resourceApk, null, ImmutableList.<Artifact>of(), - NativeLibs.EMPTY); + NativeLibs.EMPTY, + isResourcesOnly); NestedSetBuilder<Artifact> transitiveResourcesJars = collectTransitiveResourceJars(ruleContext); if (androidCommon.getResourceClassJar() != null) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java index 0bc2391640..2c1a217ab7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java @@ -27,14 +27,13 @@ public abstract class AndroidResourcesProvider implements TransitiveInfoProvider public static AndroidResourcesProvider create( Label label, NestedSet<ResourceContainer> transitiveAndroidResources, - NestedSet<ResourceContainer> directAndroidResources) { + NestedSet<ResourceContainer> directAndroidResources, + boolean isResourcesOnly) { return new AutoValue_AndroidResourcesProvider( - label, transitiveAndroidResources, directAndroidResources); + label, transitiveAndroidResources, directAndroidResources, isResourcesOnly); } - /** - * Returns the label that is associated with this piece of information. - */ + /** Returns the label that is associated with this piece of information. */ public abstract Label getLabel(); /** Returns the transitive ResourceContainers for the label. */ @@ -43,5 +42,13 @@ public abstract class AndroidResourcesProvider implements TransitiveInfoProvider /** Returns the immediate ResourceContainers for the label. */ public abstract NestedSet<ResourceContainer> getDirectAndroidResources(); + /** + * Returns whether the targets contained within this provider only represent android resources or + * also contain other information. + * + * TODO(b/30307842): Remove this once android_resources is fully removed. + */ + public abstract boolean getIsResourcesOnly(); + AndroidResourcesProvider() {} } 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 136c404bf4..4f244bd683 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 @@ -112,10 +112,10 @@ public final class ResourceApk { * <p>If the ResourceApk was generated from local resources, that will be the direct dependencies and * the rest will be transitive. */ - public AndroidResourcesProvider toResourceProvider(Label label) { + public AndroidResourcesProvider toResourceProvider(Label label, boolean isResourcesOnly) { if (primaryResource == null) { - return resourceDeps.toProvider(label); + return resourceDeps.toProvider(label, isResourcesOnly); } - return resourceDeps.toProvider(label, primaryResource); + return resourceDeps.toProvider(label, primaryResource, isResourcesOnly); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java index 945fd1a603..92d4eebdbb 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java @@ -164,17 +164,21 @@ public final class ResourceDependencies { * Creates a new AndroidResourcesProvider with the supplied ResourceContainer as the direct dep. * * <p>When a library produces a new resource container the AndroidResourcesProvider should use - * that container as a the direct dependency for that library. This makes the consuming rule - * to identify the new container and merge appropriately. The previous direct dependencies are - * then added to the transitive dependencies. + * that container as a the direct dependency for that library. This makes the consuming rule to + * identify the new container and merge appropriately. The previous direct dependencies are then + * added to the transitive dependencies. * * @param label The label of the library exporting this provider. * @param newDirectResource The new direct dependency for AndroidResourcesProvider + * @param isResourcesOnly if the direct dependency is either an android_resources + * target or an android_library target with no fields that android_resources targets do not + * provide. * @return A provider with the current resources and label. */ - public AndroidResourcesProvider toProvider(Label label, ResourceContainer newDirectResource) { + public AndroidResourcesProvider toProvider( + Label label, ResourceContainer newDirectResource, boolean isResourcesOnly) { if (neverlink) { - return ResourceDependencies.empty().toProvider(label); + return ResourceDependencies.empty().toProvider(label, isResourcesOnly); } return AndroidResourcesProvider.create( label, @@ -182,7 +186,8 @@ public final class ResourceDependencies { .addTransitive(transitiveResources) .addTransitive(directResources) .build(), - NestedSetBuilder.<ResourceContainer>naiveLinkOrder().add(newDirectResource).build()); + NestedSetBuilder.<ResourceContainer>naiveLinkOrder().add(newDirectResource).build(), + isResourcesOnly); } /** @@ -193,13 +198,17 @@ public final class ResourceDependencies { * the resource merging as if this library didn't exist. * * @param label The label of the library exporting this provider. + * @param isResourcesOnly if the direct dependency is either an android_resources + * target or an android_library target with no fields that android_resources targets do not + * provide. * @return A provider with the current resources and label. */ - public AndroidResourcesProvider toProvider(Label label) { + public AndroidResourcesProvider toProvider(Label label, boolean isResourcesOnly) { if (neverlink) { - return ResourceDependencies.empty().toProvider(label); + return ResourceDependencies.empty().toProvider(label, isResourcesOnly); } - return AndroidResourcesProvider.create(label, transitiveResources, directResources); + return AndroidResourcesProvider.create( + label, transitiveResources, directResources, isResourcesOnly); } /** Provides an NestedSet of the direct and transitive resources. */ |