diff options
6 files changed, 412 insertions, 18 deletions
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 new file mode 100644 index 0000000000..4ef1b5af75 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifest.java @@ -0,0 +1,183 @@ +// Copyright 2018 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.rules.android; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; +import com.google.devtools.build.lib.packages.ImplicitOutputsFunction; +import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction; +import com.google.devtools.build.lib.rules.java.JavaUtil; +import com.google.devtools.build.lib.vfs.PathFragment; +import javax.annotation.Nullable; + +/** Wraps an Android Manifest and provides utilities for working with it */ +@Immutable +public class AndroidManifest { + private static final SafeImplicitOutputsFunction MERGED_MANIFEST = + ImplicitOutputsFunction.fromTemplates("{name}_manifest/AndroidManifest.xml"); + private static final SafeImplicitOutputsFunction MERGE_LOG = + ImplicitOutputsFunction.fromTemplates("{name}_manifest/manifest_merger_log.txt"); + + private final RuleContext ruleContext; + /** + * The current manifest. May be null if this rule has no manifest and we have not yet generated + * one through merging or stamping. + */ + @Nullable private final Artifact manifest; + private final String pkg; + private final boolean isDummy; + + public static AndroidManifest empty(RuleContext ruleContext) { + return of(ruleContext, null, null); + } + + /** + * @param ruleContext the current context + * @param manifest this target's manifest. Can be null if this target has no manifest, in which + * case a dummy manifest will be generated. + * @param customPackage this target's custom package. If null, the default package, derived from + * BUILD file location, will be used. + * @return an AndroidManifest object wrapping the manifest and package + */ + public static AndroidManifest of( + RuleContext ruleContext, @Nullable Artifact manifest, @Nullable String customPackage) { + return new AndroidManifest( + ruleContext, + manifest, + customPackage == null ? getDefaultPackage(ruleContext) : customPackage, + manifest == null); + } + + AndroidManifest( + RuleContext ruleContext, @Nullable Artifact manifest, String pkg, boolean isDummy) { + this.ruleContext = ruleContext; + this.manifest = manifest; + this.pkg = pkg; + this.isDummy = isDummy; + } + + /** + * Merges the current manifest with manifests from the specified deps and stamps the result. + * + * <p>Manifests will not be merged if the dependencies do not provide {@link AndroidManifestInfo} + * or the provided manifests are generated dummies. + * + * <p>The resulting manifest will always be stamped if needed, even if no merging is done. + */ + public StampedAndroidManifest stampAndMergeWith(ImmutableList<ConfiguredTarget> deps) + throws InterruptedException { + ImmutableMap.Builder<Artifact, Label> mergeeBuilder = ImmutableMap.builder(); + for (ConfiguredTarget dep : deps) { + AndroidManifestInfo info = dep.get(AndroidManifestInfo.PROVIDER); + if (info == null || info.isDummy()) { + continue; + } + mergeeBuilder.put(info.getManifest(), dep.getLabel()); + } + + ImmutableMap<Artifact, Label> mergeeManifests = mergeeBuilder.build(); + + if (mergeeManifests.isEmpty()) { + return stamp(); + } + + Artifact merged = ruleContext.getImplicitOutputArtifact(MERGED_MANIFEST); + + // Since we're already invoking an action to merge, we may as well stamp here as well. + getActionBuilder(merged) + .setMergeeManifests(mergeeManifests) + .setLogOut(ruleContext.getImplicitOutputArtifact(MERGE_LOG)) + .build(ruleContext); + + return new StampedAndroidManifest(ruleContext, merged, pkg, false); + } + + /** If needed, stamps the manifest with the correct Java package */ + StampedAndroidManifest stamp() throws InterruptedException { + Artifact stamped = ruleContext.getImplicitOutputArtifact(MERGED_MANIFEST); + + getActionBuilder(stamped).build(ruleContext); + + return new StampedAndroidManifest(ruleContext, stamped, pkg, isDummy); + } + + /** + * Gets the manifest artifact wrapped by this object. May be null if the manifest is to be + * generated but has not been. + */ + @Nullable + Artifact getManifest() { + return manifest; + } + + String getPackage() { + return pkg; + } + + boolean isDummy() { + return isDummy; + } + + /** Gets a {@link ManifestMergerActionBuilder} with common settings always used by this object. */ + private ManifestMergerActionBuilder getActionBuilder(Artifact manifestOutput) { + return new ManifestMergerActionBuilder(ruleContext) + .setCustomPackage(pkg) + // The current manifest merger action uses the "custom_package" value when working on + // "library" targets, and ignores it and removes any tool annotations from the manifest + // otherwise. As this method is not intended to produce a final merged manifest, even when + // run on a binary, always use the "library" settings here. + .setLibrary(true) + .setManifest(manifest) + .setManifestOutput(manifestOutput); + } + + /** Gets the default Java package */ + static String getDefaultPackage(RuleContext ruleContext) { + PathFragment dummyJar = ruleContext.getPackageDirectory().getChild("Dummy.jar"); + return getJavaPackageFromPath(ruleContext, dummyJar); + } + + /** + * Gets the Java package of a JAR file based on it's path. + * + * <p>Bazel requires that all Java code (including Android code) be in a path prefixed with "java" + * or "javatests" followed by the Java package; this method validates and takes advantage of that + * requirement. + * + * @param ruleContext the current context + * @param jarPathFragment The path to a JAR file contained in the current BUILD file's directory. + * @return the Java package, as a String + */ + static String getJavaPackageFromPath(RuleContext ruleContext, PathFragment jarPathFragment) { + // TODO(bazel-team): JavaUtil.getJavaPackageName does not check to see if the path is valid. + // So we need to check for the JavaRoot. + if (JavaUtil.getJavaRoot(jarPathFragment) == null) { + ruleContext.ruleError( + "The location of your BUILD file determines the Java package used for " + + "Android resource processing. A directory named \"java\" or \"javatests\" will " + + "be used as your Java source root and the path of your BUILD file relative to " + + "the Java source root will be used as the package for Android resource " + + "processing. The Java source root could not be determined for \"" + + ruleContext.getPackageDirectory() + + "\". Move your BUILD file under a java or javatests directory, or set the " + + "'custom_package' attribute."); + } + return JavaUtil.getJavaPackageName(jarPathFragment); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ManifestMergerActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/ManifestMergerActionBuilder.java index f093925220..adff649444 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ManifestMergerActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ManifestMergerActionBuilder.java @@ -99,8 +99,10 @@ public class ManifestMergerActionBuilder { .getRunfilesSupport() .getRunfilesArtifacts()); - builder.addExecPath("--manifest", manifest); - inputs.add(manifest); + if (manifest != null) { + builder.addExecPath("--manifest", manifest); + inputs.add(manifest); + } if (mergeeManifests != null && !mergeeManifests.isEmpty()) { builder.add( 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 182167a6fc..7457da4eb0 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 @@ -23,7 +23,6 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; -import com.google.devtools.build.lib.rules.java.JavaUtil; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; @@ -390,21 +389,8 @@ public abstract class ResourceContainer { if (hasCustomPackage(ruleContext)) { return ruleContext.attributes().get("custom_package", Type.STRING); } - Artifact rJavaSrcJar = getJavaSourceJar(); - // TODO(bazel-team): JavaUtil.getJavaPackageName does not check to see if the path is valid. - // So we need to check for the JavaRoot. - if (JavaUtil.getJavaRoot(rJavaSrcJar.getExecPath()) == null) { - ruleContext.ruleError( - "The location of your BUILD file determines the Java package used for " - + "Android resource processing. A directory named \"java\" or \"javatests\" will " - + "be used as your Java source root and the path of your BUILD file relative to " - + "the Java source root will be used as the package for Android resource " - + "processing. The Java source root could not be determined for \"" - + ruleContext.getPackageDirectory() - + "\". Move your BUILD file under a java or javatests directory, or set the " - + "'custom_package' attribute."); - } - return JavaUtil.getJavaPackageName(rJavaSrcJar.getExecPath()); + + return AndroidManifest.getJavaPackageFromPath(ruleContext, getJavaSourceJar().getExecPath()); } private static boolean hasCustomPackage(RuleContext ruleContext) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/StampedAndroidManifest.java b/src/main/java/com/google/devtools/build/lib/rules/android/StampedAndroidManifest.java new file mode 100644 index 0000000000..2486151a38 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/android/StampedAndroidManifest.java @@ -0,0 +1,47 @@ +// Copyright 2018 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.rules.android; + +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; + +/** An {@link AndroidManifest} stamped with the correct package. */ +@Immutable +public class StampedAndroidManifest extends AndroidManifest { + + StampedAndroidManifest( + RuleContext ruleContext, Artifact manifest, String pkg, boolean isDummy) { + super(ruleContext, manifest, pkg, isDummy); + } + + @Override + StampedAndroidManifest stamp() { + // This manifest is already stamped + return this; + } + + /** + * Gets the manifest artifact wrapped by this object. Stamped manifests are guaranteed to have a + * non-null manifest artifact. + */ + @Override + Artifact getManifest() { + return super.getManifest(); + } + + public AndroidManifestInfo toProvider() { + return AndroidManifestInfo.of(getManifest(), getPackage(), isDummy()); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidManifestTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidManifestTest.java new file mode 100644 index 0000000000..eda59fd95b --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidManifestTest.java @@ -0,0 +1,158 @@ +// Copyright 2018 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.rules.android; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows; + + +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; +import com.google.devtools.build.lib.cmdline.LabelSyntaxException; +import com.google.devtools.build.lib.events.StoredEventHandler; +import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; +import java.io.IOException; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests the AndroidManifest class */ +@RunWith(JUnit4.class) +public class AndroidManifestTest extends BuildViewTestCase { + private static final String DEFAULT_PATH = "prefix/java/com/google/foo/bar"; + private static final String DEFAULT_PACKAGE = "com.google.foo.bar"; + + @Test + public void testGetDefaultPackage() throws Exception { + RuleContext ctx = makeContext(); + assertThat(AndroidManifest.getDefaultPackage(ctx)).isEqualTo(DEFAULT_PACKAGE); + ctx.assertNoErrors(); + } + + @Test + public void testGetDefaultPackage_NoJavaDir() throws Exception { + RuleContext ctx = makeContext("notjava/com/google/foo/bar"); + AndroidManifest.getDefaultPackage(ctx); + assertThrows(RuleErrorException.class, ctx::assertNoErrors); + } + + @Test + public void testIsDummy() throws Exception { + RuleContext ctx = makeContext(); + assertThat(AndroidManifest.of(ctx, /* manifest = */ null, DEFAULT_PACKAGE).isDummy()).isTrue(); + assertThat(AndroidManifest.of(ctx, ctx.createOutputArtifact(), DEFAULT_PACKAGE).isDummy()) + .isFalse(); + } + + @Test + public void testStampAndMergeWith_NoDeps() throws Exception { + AndroidManifest manifest = AndroidManifest.empty(makeContext()); + + StampedAndroidManifest stamped = manifest.stampAndMergeWith(ImmutableList.of()); + assertThat(stamped).isNotEqualTo(manifest); + assertThat(stamped.getPackage()).isEqualTo(DEFAULT_PACKAGE); + assertThat(stamped.getManifest()).isNotNull(); + assertThat(stamped.isDummy()).isTrue(); + + // The merge should still do a stamp, so future stamping should be a no-op. + assertThat(stamped).isSameAs(stamped.stamp()); + } + + @Test + public void testStampAndMergeWith_NoProviders() throws Exception { + AndroidManifest manifest = AndroidManifest.empty(makeContext()); + + AndroidManifest merged = manifest.stampAndMergeWith(getDeps("[]")); + + assertThat(merged.getManifest()).isNotNull(); + assertThat(merged.isDummy()).isTrue(); + assertThat(merged.getPackage()).isEqualTo(DEFAULT_PACKAGE); + + // The merge should still do a stamp, so future stamping should be a no-op. + assertThat(merged).isSameAs(merged.stamp()); + } + + @Test + public void testStampAndMergeWith_DummyProviders() throws Exception { + AndroidManifest manifest = AndroidManifest.empty(makeContext()); + + AndroidManifest merged = + manifest.stampAndMergeWith( + getDeps("[AndroidManifestInfo(manifest=manifest, package='com.pkg', is_dummy=True)]")); + + assertThat(merged.getManifest()).isNotNull(); + assertThat(merged.isDummy()).isTrue(); + assertThat(merged.getPackage()).isEqualTo(DEFAULT_PACKAGE); + + // The merge should still do a stamp, so future stamping should be a no-op. + assertThat(merged).isSameAs(merged.stamp()); + } + + @Test + public void testStampAndMergeWith() throws Exception { + AndroidManifest manifest = AndroidManifest.empty(makeContext()); + + AndroidManifest merged = + manifest.stampAndMergeWith( + getDeps("[AndroidManifestInfo(manifest=manifest, package='com.pkg', is_dummy=False)]")); + + // Merging results in a new, non-dummy manifest with the same package + assertThat(merged).isNotEqualTo(manifest); + assertThat(merged.getManifest()).isNotNull(); + assertThat(merged.getPackage()).isEqualTo(manifest.getPackage()); + assertThat(merged.isDummy()).isFalse(); + + // Merging should implicitly stamp + assertThat(merged).isSameAs(merged.stamp()); + } + + private ImmutableList<ConfiguredTarget> getDeps(String returnList) + throws IOException, LabelSyntaxException { + scratch.file( + "skylark/rule.bzl", + "def impl(ctx):", + " manifest = ctx.actions.declare_file(ctx.attr.name + 'AndroidManifest.xml')", + " ctx.actions.write(manifest, 'some values')", + " return " + returnList, + "test_rule = rule(implementation=impl)"); + + scratch.file( + "skylark/BUILD", + "load(':rule.bzl', 'test_rule')", + "test_rule(name='dep1')", + "test_rule(name='dep2')", + "test_rule(name='dep3')"); + + return ImmutableList.of( + getConfiguredTarget("//skylark:dep1"), + getConfiguredTarget("//skylark:dep2"), + getConfiguredTarget("//skylark:dep3")); + } + + private RuleContext makeContext() throws Exception { + return makeContext(DEFAULT_PATH); + } + + private RuleContext makeContext(String pkg) throws Exception { + // Use BuildView's getRuleContextForTesting method, rather than BuildViewTestCase's + // getRuleContext method, to avoid the StubEventHandler used by the latter, which prevents us + // from declaring new artifacts within code called from the test. + return view.getRuleContextForTesting( + scratchConfiguredTarget(pkg, "lib", "android_library(name = 'lib')"), + new StoredEventHandler(), + masterConfig); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/BUILD b/src/test/java/com/google/devtools/build/lib/rules/android/BUILD index 34fc1967fa..b2ad32d9ff 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/BUILD +++ b/src/test/java/com/google/devtools/build/lib/rules/android/BUILD @@ -342,6 +342,24 @@ java_test( ], ) +java_test( + name = "AndroidManifestTest", + srcs = ["AndroidManifestTest.java"], + runtime_deps = ["//src/test/java/com/google/devtools/build/lib:testutil"], + deps = [ + "//src/main/java/com/google/devtools/build/lib:android-rules", + "//src/main/java/com/google/devtools/build/lib:build-base", + "//src/main/java/com/google/devtools/build/lib:events", + "//src/main/java/com/google/devtools/build/lib:packages-internal", + "//src/main/java/com/google/devtools/build/lib/cmdline", + "//src/test/java/com/google/devtools/build/lib:analysis_testutil", + "//src/test/java/com/google/devtools/build/lib:testutil", + "//third_party:guava", + "//third_party:junit4", + "//third_party:truth", + ], +) + test_suite( name = "windows_tests", tags = [ |