From c180c9292a45a000c197d7215a05da315aaf604b Mon Sep 17 00:00:00 2001 From: asteinb Date: Wed, 9 May 2018 10:38:16 -0700 Subject: Properly rename manifests Make AndroidSkylarkData abstract and add a getAndroidSemantics() method The Skylark stamp_manifest rule should properly use AndroidSemantics to rename the manifest as well, in keeping with the android_library rule. AndroidLocalTest should do the same manifest renaming regardless of which AndroidSemantics it is run under. Have it use a seperate method to do so. RELNOTES: none PiperOrigin-RevId: 195992300 --- .../bazel/rules/android/BazelAndroidLocalTest.java | 6 ---- .../lib/rules/android/AndroidLocalTestBase.java | 17 +++------ .../build/lib/rules/android/AndroidManifest.java | 31 ++++++++++++++++- .../build/lib/rules/android/AndroidSemantics.java | 29 +++++----------- .../lib/rules/android/AndroidSkylarkData.java | 13 +++++-- .../lib/rules/android/ApplicationManifest.java | 40 ++++++++++++++++++++-- 6 files changed, 91 insertions(+), 45 deletions(-) (limited to 'src/main') diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTest.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTest.java index 685562d738..2f66c3f8d5 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTest.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTest.java @@ -24,7 +24,6 @@ import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTa import com.google.devtools.build.lib.bazel.rules.java.BazelJavaSemantics; import com.google.devtools.build.lib.rules.android.AndroidLocalTestBase; import com.google.devtools.build.lib.rules.android.AndroidMigrationSemantics; -import com.google.devtools.build.lib.rules.android.AndroidSemantics; import com.google.devtools.build.lib.rules.java.JavaCommon; import com.google.devtools.build.lib.rules.java.JavaCompilationArtifacts; import com.google.devtools.build.lib.rules.java.JavaCompilationHelper; @@ -37,11 +36,6 @@ public class BazelAndroidLocalTest extends AndroidLocalTestBase { Artifact androidAllJarsPropFile; - @Override - protected AndroidSemantics createAndroidSemantics() { - return BazelAndroidSemantics.INSTANCE; - } - @Override protected AndroidMigrationSemantics createAndroidMigrationSemantics() { return BazelAndroidMigrationSemantics.INSTANCE; 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 05a9cc32e1..d9f1feeaeb 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 @@ -77,7 +77,6 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor ruleContext.checkSrcsSamePackage(true); JavaSemantics javaSemantics = createJavaSemantics(); - AndroidSemantics androidSemantics = createAndroidSemantics(); createAndroidMigrationSemantics().validateRuleContext(ruleContext); AndroidLocalTestConfiguration androidLocalTestConfiguration = ruleContext.getFragment(AndroidLocalTestConfiguration.class); @@ -92,7 +91,7 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor if (AndroidResources.decoupleDataProcessing(ruleContext)) { StampedAndroidManifest manifest = - StampedAndroidManifest.from(ruleContext, androidSemantics).mergeWithDeps(ruleContext); + AndroidManifest.from(ruleContext).mergeWithDeps(ruleContext); resourceApk = ProcessedAndroidData.processLocalTestDataFrom(ruleContext, manifest) @@ -103,7 +102,7 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor ResourceDependencies.fromRuleDeps(ruleContext, /* neverlink= */ false); ApplicationManifest applicationManifest = - getApplicationManifest(ruleContext, androidSemantics, resourceDependencies); + getApplicationManifest(ruleContext, resourceDependencies); // Create the final merged R class resourceApk = @@ -403,15 +402,13 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor * @throws RuleErrorException */ private ApplicationManifest getApplicationManifest( - RuleContext ruleContext, - AndroidSemantics androidSemantics, - ResourceDependencies resourceDependencies) + RuleContext ruleContext, ResourceDependencies resourceDependencies) throws InterruptedException, RuleErrorException { ApplicationManifest applicationManifest; if (AndroidResources.definesAndroidResources(ruleContext.attributes())) { AndroidResources.validateRuleContext(ruleContext); - ApplicationManifest ruleManifest = androidSemantics.getManifestForRule(ruleContext); + ApplicationManifest ruleManifest = ApplicationManifest.renamedFromRule(ruleContext); applicationManifest = ruleManifest.mergeWith(ruleContext, resourceDependencies); } else { // we don't have a manifest, merge like android_library with a stub manifest @@ -510,8 +507,7 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor // The dep may be a simple JAR and not a java rule, hence we can't simply do // dep.getProvider(JavaCompilationArgsProvider.class).getRecursiveJavaCompilationArgs(), // so we reuse the logic within JavaCompilationArgs to handle both scenarios. - return JavaCompilationArgsProvider.legacyFromTargets(ImmutableList.of(deps)) - .getRuntimeJars(); + return JavaCompilationArgsProvider.legacyFromTargets(ImmutableList.of(deps)).getRuntimeJars(); } private static String getAndCheckTestClass( @@ -551,9 +547,6 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor /** Get JavaSemantics */ protected abstract JavaSemantics createJavaSemantics(); - /** Get AndroidSemantics */ - protected abstract AndroidSemantics createAndroidSemantics(); - /** Get AndroidMigrationSemantics */ protected abstract AndroidMigrationSemantics createAndroidMigrationSemantics(); 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 02b741dd1d..dfe7a014d0 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 @@ -45,6 +45,26 @@ public class AndroidManifest { */ public static AndroidManifest from(RuleContext ruleContext, AndroidSemantics androidSemantics) throws RuleErrorException, InterruptedException { + return innerFrom(ruleContext, androidSemantics); + } + + /** + * Gets the manifest for this rule. + * + *

If no manifest is specified in the rule's attributes, an empty manifest will be generated. + * + *

Unlike {@link #from(RuleContext, AndroidSemantics)}, the AndroidSemantics-specific manifest + * processing methods will not be applied in this method. The manifest returned by this method + * will be the same regardless of the AndroidSemantics being used. + */ + public static AndroidManifest from(RuleContext ruleContext) + throws InterruptedException, RuleErrorException { + return innerFrom(ruleContext, null); + } + + private static AndroidManifest innerFrom( + RuleContext ruleContext, @Nullable AndroidSemantics androidSemantics) + throws RuleErrorException, InterruptedException { if (!AndroidResources.definesAndroidResources(ruleContext.attributes())) { // Generate a dummy manifest return StampedAndroidManifest.createEmpty(ruleContext, /* exported = */ false); @@ -52,8 +72,17 @@ public class AndroidManifest { AndroidResources.validateRuleContext(ruleContext); + Artifact rawManifest = ApplicationManifest.getManifestFromAttributes(ruleContext); + + Artifact renamedManifest; + if (androidSemantics != null) { + renamedManifest = androidSemantics.renameManifest(ruleContext, rawManifest); + } else { + renamedManifest = ApplicationManifest.renameManifestIfNeeded(ruleContext, rawManifest); + } + return new AndroidManifest( - androidSemantics.getManifestForRule(ruleContext).getManifest(), + renamedManifest, getAndroidPackage(ruleContext), AndroidCommon.getExportsManifest(ruleContext)); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSemantics.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSemantics.java index 3726808cb5..1620f3bc30 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSemantics.java @@ -18,7 +18,6 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; import com.google.devtools.build.lib.analysis.actions.SpawnAction; -import com.google.devtools.build.lib.analysis.actions.SymlinkAction; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.rules.java.JavaCompilationArtifacts; @@ -40,26 +39,14 @@ public interface AndroidSemantics { */ default ApplicationManifest getManifestForRule(RuleContext ruleContext) throws InterruptedException, RuleErrorException { - ApplicationManifest result = ApplicationManifest.fromRule(ruleContext); - Artifact manifest = result.getManifest(); - if (manifest.getFilename().equals("AndroidManifest.xml")) { - return result; - } else { - /* - * If the manifest file is not named AndroidManifest.xml, we create a symlink named - * AndroidManifest.xml to it. aapt requires the manifest to be named as such. - */ - Artifact manifestSymlink = - ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_SYMLINKED_MANIFEST); - SymlinkAction symlinkAction = - new SymlinkAction( - ruleContext.getActionOwner(), - manifest, - manifestSymlink, - "Renaming Android manifest for " + ruleContext.getLabel()); - ruleContext.registerAction(symlinkAction); - return ApplicationManifest.fromExplicitManifest(ruleContext, manifestSymlink); - } + Artifact rawManifest = ApplicationManifest.getManifestFromAttributes(ruleContext); + return ApplicationManifest.fromExplicitManifest( + ruleContext, renameManifest(ruleContext, rawManifest)); + } + + default Artifact renameManifest(RuleContext ruleContext, Artifact rawManifest) + throws InterruptedException { + return ApplicationManifest.renameManifestIfNeeded(ruleContext, rawManifest); } /** Returns the name of the file in which the file names of native dependencies are listed. */ 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 5fa56a3be0..cb13f6a559 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 @@ -47,7 +47,9 @@ import javax.annotation.Nullable; doc = "Utilities for working with Android data (manifests, resources, and assets). " + "This API is non-final and subject to change without warning; do not rely on it.") -public class AndroidSkylarkData { +public abstract class AndroidSkylarkData { + + public abstract AndroidSemantics getAndroidSemantics(); /** * Skylark API for getting a asset provider for android_library targets that don't specify assets. @@ -201,7 +203,8 @@ public class AndroidSkylarkData { }, doc = "Stamps a manifest with package information.") public AndroidManifestInfo stampAndroidManifest( - SkylarkRuleContext ctx, Object manifest, Object customPackage, boolean exported) { + SkylarkRuleContext ctx, Object manifest, Object customPackage, boolean exported) + throws InterruptedException { String pkg = fromNoneable(customPackage, String.class); if (pkg == null) { pkg = AndroidManifest.getDefaultPackage(ctx.getRuleContext()); @@ -212,7 +215,11 @@ public class AndroidSkylarkData { return StampedAndroidManifest.createEmpty(ctx.getRuleContext(), pkg, exported).toProvider(); } - return new AndroidManifest(primaryManifest, pkg, exported) + // If needed, rename the manifest to "AndroidManifest.xml", which aapt expects. + Artifact renamedManifest = + getAndroidSemantics().renameManifest(ctx.getRuleContext(), primaryManifest); + + return new AndroidManifest(renamedManifest, pkg, exported) .stamp(ctx.getRuleContext()) .toProvider(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java index 8ecb28d045..783f0f284c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java @@ -25,6 +25,7 @@ import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; import com.google.devtools.build.lib.analysis.actions.FileWriteAction; import com.google.devtools.build.lib.analysis.actions.SpawnAction; +import com.google.devtools.build.lib.analysis.actions.SymlinkAction; 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.cmdline.Label; @@ -126,9 +127,44 @@ public final class ApplicationManifest { return stubManifest; } - public static ApplicationManifest fromRule(RuleContext ruleContext) throws RuleErrorException { + public static Artifact getManifestFromAttributes(RuleContext ruleContext) { + return ruleContext.getPrerequisiteArtifact("manifest", Mode.TARGET); + } + + /** + * Gets the manifest specified in the "manifest" attribute, renaming it if needed. + * + *

Unlike {@link AndroidSemantics#getManifestForRule(RuleContext)}, this method will not + * perform AndroidSemantics-specific manifest processing. This method will do the same work + * regardless of the AndroidSemantics implementation being used; that method may do different work + * depending on the implementation. + */ + public static ApplicationManifest renamedFromRule(RuleContext ruleContext) + throws InterruptedException, RuleErrorException { return fromExplicitManifest( - ruleContext, ruleContext.getPrerequisiteArtifact("manifest", Mode.TARGET)); + ruleContext, renameManifestIfNeeded(ruleContext, getManifestFromAttributes(ruleContext))); + } + + static Artifact renameManifestIfNeeded(RuleContext ruleContext, Artifact manifest) + throws InterruptedException { + if (manifest.getFilename().equals("AndroidManifest.xml")) { + return manifest; + } else { + /* + * If the manifest file is not named AndroidManifest.xml, we create a symlink named + * AndroidManifest.xml to it. aapt requires the manifest to be named as such. + */ + Artifact manifestSymlink = + ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_SYMLINKED_MANIFEST); + SymlinkAction symlinkAction = + new SymlinkAction( + ruleContext.getActionOwner(), + manifest, + manifestSymlink, + "Renaming Android manifest for " + ruleContext.getLabel()); + ruleContext.registerAction(symlinkAction); + return manifestSymlink; + } } public static ApplicationManifest fromExplicitManifest(RuleContext ruleContext, Artifact manifest) -- cgit v1.2.3