From e3ce21fab041c6c8e6da2811f5dce75615ccb1f4 Mon Sep 17 00:00:00 2001 From: Googler Date: Fri, 15 Jan 2016 23:40:14 +0000 Subject: support hidden --java_optimization_mode flag in java_test rule. -- MOS_MIGRATED_REVID=112290581 --- .../lib/bazel/rules/java/BazelJavaTestRule.java | 6 + .../build/lib/rules/android/AndroidBinary.java | 27 +- .../lib/rules/android/AndroidConfiguration.java | 19 -- .../lib/rules/android/AndroidRuleClasses.java | 22 +- .../build/lib/rules/android/AndroidSdk.java | 3 +- .../build/lib/rules/android/ProguardHelper.java | 216 -------------- .../devtools/build/lib/rules/java/JavaBinary.java | 114 ++++++-- .../build/lib/rules/java/JavaConfiguration.java | 17 +- .../devtools/build/lib/rules/java/JavaOptions.java | 8 + .../build/lib/rules/java/JavaSemantics.java | 16 +- .../build/lib/rules/java/ProguardHelper.java | 315 +++++++++++++++++++++ 11 files changed, 476 insertions(+), 287 deletions(-) delete mode 100644 src/main/java/com/google/devtools/build/lib/rules/android/ProguardHelper.java create mode 100644 src/main/java/com/google/devtools/build/lib/rules/java/ProguardHelper.java diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaTestRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaTestRule.java index c3c5b7558c..82c0012a60 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaTestRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaTestRule.java @@ -14,8 +14,10 @@ package com.google.devtools.build.lib.bazel.rules.java; +import static com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition.HOST; import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL; +import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; import static com.google.devtools.build.lib.packages.BuildType.TRISTATE; import static com.google.devtools.build.lib.syntax.Type.BOOLEAN; import static com.google.devtools.build.lib.syntax.Type.STRING; @@ -51,6 +53,10 @@ public final class BazelJavaTestRule implements RuleDefinition { return builder .requiresConfigurationFragments(JavaConfiguration.class, Jvm.class) .setImplicitOutputsFunction(BazelJavaRuleClasses.JAVA_BINARY_IMPLICIT_OUTPUTS) + // Proguard can be run over java_test targets using the --java_optimization_mode flag. + // Primarily this is intended to help test changes to Proguard. + .add(attr(":proguard", LABEL).cfg(HOST).value(JavaSemantics.PROGUARD).exec()) + .add(attr(":extra_proguard_specs", LABEL_LIST).value(JavaSemantics.EXTRA_PROGUARD_SPECS)) .override(attr("stamp", TRISTATE).value(TriState.NO)) .override(attr("use_testrunner", BOOLEAN).value(true)) .override(attr(":java_launcher", LABEL).value(JavaSemantics.JAVA_LAUNCHER)) 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 de84cec192..2979e4e0db 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 @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.rules.android; import static com.google.common.base.Strings.isNullOrEmpty; -import static com.google.devtools.build.lib.rules.android.ProguardHelper.PROGUARD_SPECS; import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; @@ -48,7 +47,6 @@ import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.TriState; import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory; import com.google.devtools.build.lib.rules.android.AndroidRuleClasses.MultidexMode; -import com.google.devtools.build.lib.rules.android.ProguardHelper.ProguardOutput; import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider; import com.google.devtools.build.lib.rules.cpp.CppHelper; import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder; @@ -59,6 +57,8 @@ import com.google.devtools.build.lib.rules.java.JavaConfiguration.JavaOptimizati import com.google.devtools.build.lib.rules.java.JavaSemantics; import com.google.devtools.build.lib.rules.java.JavaSourceInfoProvider; import com.google.devtools.build.lib.rules.java.JavaTargetAttributes; +import com.google.devtools.build.lib.rules.java.ProguardHelper; +import com.google.devtools.build.lib.rules.java.ProguardHelper.ProguardOutput; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.PathFragment; @@ -131,7 +131,9 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { } if (ruleContext.attributes().isAttributeValueExplicitlySpecified("proguard_apply_mapping") - && ruleContext.attributes().get(PROGUARD_SPECS, BuildType.LABEL_LIST).isEmpty()) { + && ruleContext.attributes() + .get(ProguardHelper.PROGUARD_SPECS, BuildType.LABEL_LIST) + .isEmpty()) { ruleContext.attributeError("proguard_apply_mapping", "'proguard_apply_mapping' can only be used when 'proguard_specs' is also set"); return null; @@ -342,7 +344,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { ImmutableList apksUnderTest, Artifact proguardMapping) throws InterruptedException { ImmutableList proguardSpecs = ProguardHelper.collectTransitiveProguardSpecs( - ruleContext, resourceApk.getResourceProguardConfig()); + ruleContext, ImmutableList.of(resourceApk.getResourceProguardConfig())); ProguardOutput proguardOutput = applyProguard( @@ -731,7 +733,10 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { .getJavaOptimizationMode(); } - /** Applies the proguard specifications, and creates a ProguardedJar. */ + /** + * Applies the proguard specifications, and creates a ProguardedJar. Proguard's output artifacts + * are added to the given {@code filesBuilder}. + */ private static ProguardOutput applyProguard( RuleContext ruleContext, AndroidCommon common, @@ -757,9 +762,12 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { .add(sdk.getAndroidJar()) .addTransitive(common.getTransitiveNeverLinkLibraries()) .build(); - return ProguardHelper.createProguardAction(ruleContext, sdk.getProguard(), deployJarArtifact, - proguardSpecs, proguardMapping, libraryJars, proguardOutputJar, - ruleContext.attributes().get("proguard_generate_mapping", Type.BOOLEAN), filesBuilder); + ProguardOutput result = ProguardHelper.createProguardAction(ruleContext, sdk.getProguard(), + deployJarArtifact, proguardSpecs, proguardMapping, libraryJars, proguardOutputJar, + ruleContext.attributes().get("proguard_generate_mapping", Type.BOOLEAN)); + // Since Proguard is being run, add its output artifacts to the given filesBuilder + result.addAllToSet(filesBuilder); + return result; } private static ProguardOutput createEmptyProguardAction(RuleContext ruleContext, @@ -767,8 +775,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { ImmutableList.Builder failures = ImmutableList.builder().add(proguardOutputJar); if (ruleContext.attributes().get("proguard_generate_mapping", Type.BOOLEAN)) { - failures.add( - ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_BINARY_PROGUARD_MAP)); + failures.add(ruleContext.getImplicitOutputArtifact(JavaSemantics.JAVA_BINARY_PROGUARD_MAP)); } JavaOptimizationMode optMode = getJavaOptimizationMode(ruleContext); ruleContext.registerAction( diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java index 05f5e2ceb5..564a344325 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java @@ -21,7 +21,6 @@ import com.google.devtools.build.lib.Constants; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildConfiguration.DefaultLabelConverter; import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration.LabelConverter; import com.google.devtools.build.lib.analysis.config.BuildConfiguration.StrictDepsConverter; import com.google.devtools.build.lib.analysis.config.BuildConfiguration.StrictDepsMode; import com.google.devtools.build.lib.analysis.config.BuildOptions; @@ -146,14 +145,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { help = "Specifies Android SDK/platform that is used to build Android applications.") public Label sdk; - @Option(name = "proguard_top", - defaultValue = "null", - category = "version", - converter = LabelConverter.class, - help = "Specifies which version of ProGuard to use for code removal when building an " - + "Android binary.") - public Label proguard; - @Option(name = "legacy_android_native_support", defaultValue = "true", category = "semantics", @@ -188,10 +179,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { @Override public void addAllLabels(Multimap labelMap) { - if (proguard != null) { - labelMap.put("android_proguard", proguard); - } - if (androidCrosstoolTop != null) { labelMap.put("android_crosstool_top", androidCrosstoolTop); } @@ -249,7 +236,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { private final boolean incrementalNativeLibs; private final boolean fatApk; private final ConfigurationDistinguisher configurationDistinguisher; - private final Label proguard; private final boolean useJackForDexing; private final boolean jackSanityChecks; @@ -261,7 +247,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { this.cpu = options.cpu; this.fatApk = !options.realFatApkCpus().isEmpty(); this.configurationDistinguisher = options.configurationDistinguisher; - this.proguard = options.proguard; this.useJackForDexing = options.useJackForDexing; this.jackSanityChecks = options.jackSanityChecks; } @@ -314,8 +299,4 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { public String getOutputDirectoryName() { return configurationDistinguisher.suffix; } - - public Label getProguardLabel() { - return proguard; - } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java index 09702fcf93..f4704d329b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java @@ -47,6 +47,7 @@ import com.google.devtools.build.lib.packages.TriState; import com.google.devtools.build.lib.rules.android.AndroidConfiguration.ConfigurationDistinguisher; import com.google.devtools.build.lib.rules.cpp.CppOptions; import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider; +import com.google.devtools.build.lib.rules.java.JavaConfiguration; import com.google.devtools.build.lib.rules.java.JavaSemantics; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.FileType; @@ -92,8 +93,6 @@ public final class AndroidRuleClasses { fromTemplates("%{name}_deploy.jar"); public static final SafeImplicitOutputsFunction ANDROID_BINARY_PROGUARD_JAR = fromTemplates("%{name}_proguard.jar"); - public static final SafeImplicitOutputsFunction ANDROID_BINARY_PROGUARD_MAP = - fromTemplates("%{name}_proguard.map"); public static final SafeImplicitOutputsFunction ANDROID_BINARY_INSTRUMENTED_JAR = fromTemplates("%{name}_instrumented.jar"); public static final SafeImplicitOutputsFunction ANDROID_TEST_FILTERED_JAR = @@ -150,19 +149,6 @@ public final class AndroidRuleClasses { public static final Label DEFAULT_AAR_GENERATOR = Label.parseAbsoluteUnchecked(Constants.TOOLS_REPOSITORY + "//tools/android:aar_generator"); - /** - * Implementation for the :proguard attribute. - */ - static final LateBoundLabel PROGUARD = - new LateBoundLabel(AndroidConfiguration.class) { - @Override - public Label getDefault(Rule rule, BuildConfiguration configuration) { - // If --proguard_top is not specified, null is returned. AndroidSdk will take care of using - // android_sdk.proguard then. - return configuration.getFragment(AndroidConfiguration.class).getProguardLabel(); - } - }; - public static final LateBoundLabel ANDROID_SDK = new LateBoundLabel(DEFAULT_ANDROID_SDK, AndroidConfiguration.class) { @Override @@ -276,7 +262,7 @@ public final class AndroidRuleClasses { if (hasProguardSpecs) { functions.add(AndroidRuleClasses.ANDROID_BINARY_PROGUARD_JAR); if (mapping) { - functions.add(AndroidRuleClasses.ANDROID_BINARY_PROGUARD_MAP); + functions.add(JavaSemantics.JAVA_BINARY_PROGUARD_MAP); } } return fromFunctions(functions).getImplicitOutputs(rule); @@ -315,10 +301,10 @@ public final class AndroidRuleClasses { @Override public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) { return builder - .requiresConfigurationFragments(AndroidConfiguration.class) + .requiresConfigurationFragments(JavaConfiguration.class, AndroidConfiguration.class) .setUndocumented() // This is the Proguard that comes from the --proguard_top attribute. - .add(attr(":proguard", LABEL).cfg(HOST).value(PROGUARD).exec()) + .add(attr(":proguard", LABEL).cfg(HOST).value(JavaSemantics.PROGUARD).exec()) // This is the Proguard in the BUILD file that contains the android_sdk rule. Used when // --proguard_top is not specified. .add(attr("proguard", LABEL).mandatory().cfg(HOST).allowedFileTypes(ANY_FILE).exec()) diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSdk.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSdk.java index f00b415aae..8904b22e68 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSdk.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSdk.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.analysis.RunfilesProvider; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory; +import com.google.devtools.build.lib.rules.java.JavaConfiguration; /** * Implementation of the {@code android_sdk} rule. @@ -33,7 +34,7 @@ public class AndroidSdk implements RuleConfiguredTargetFactory { // If the user didn't specify --proguard_top, go with the proguard attribute in the android_sdk // rule. Otherwise, use what she told us to. FilesToRunProvider proguard = - ruleContext.getFragment(AndroidConfiguration.class).getProguardLabel() == null + ruleContext.getFragment(JavaConfiguration.class).getProguardBinary() == null ? ruleContext.getExecutablePrerequisite("proguard", Mode.HOST) : ruleContext.getExecutablePrerequisite(":proguard", Mode.HOST); diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ProguardHelper.java b/src/main/java/com/google/devtools/build/lib/rules/android/ProguardHelper.java deleted file mode 100644 index f752e182a5..0000000000 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ProguardHelper.java +++ /dev/null @@ -1,216 +0,0 @@ -// Copyright 2015 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.base.Joiner; -import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSortedSet; -import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.analysis.FilesToRunProvider; -import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; -import com.google.devtools.build.lib.analysis.RuleContext; -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.SpawnAction.Builder; -import com.google.devtools.build.lib.collect.nestedset.NestedSet; -import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; -import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; -import com.google.devtools.build.lib.packages.BuildType; -import com.google.devtools.build.lib.rules.java.JavaConfiguration; -import com.google.devtools.build.lib.rules.java.JavaConfiguration.JavaOptimizationMode; -import com.google.devtools.build.lib.rules.java.ProguardSpecProvider; - -import javax.annotation.Nullable; - -/** - * Common code for proguarding Android binaries. - */ -public class ProguardHelper { - - static final String PROGUARD_SPECS = "proguard_specs"; - - @Immutable - static final class ProguardOutput { - private final Artifact outputJar; - @Nullable private final Artifact mapping; - - ProguardOutput(Artifact outputJar, @Nullable Artifact mapping) { - this.outputJar = outputJar; - this.mapping = mapping; - } - - public Artifact getOutputJar() { - return outputJar; - } - - @Nullable - public Artifact getMapping() { - return mapping; - } - - } - - private ProguardHelper() {} - - /** - * Retrieves the full set of proguard specs that should be applied to this binary, including the - * specs passed in, if Proguard should run on the given rule. {@link #createProguardAction} - * relies on this method returning an empty list if the given rule doesn't declare specs in - * --java_optimization_mode=legacy. - * - *

If Proguard shouldn't be applied, or the legacy link mode is used and there are no - * proguard_specs on this rule, an empty list will be returned, regardless of any given specs or - * specs from dependencies. {@link AndroidBinary#createAndroidBinary} relies on that behavior. - */ - public static ImmutableList collectTransitiveProguardSpecs(RuleContext ruleContext, - Artifact... specsToInclude) { - JavaOptimizationMode optMode = getJavaOptimizationMode(ruleContext); - if (optMode == JavaOptimizationMode.NOOP) { - return ImmutableList.of(); - } - - ImmutableList proguardSpecs = - ruleContext.attributes().has(PROGUARD_SPECS, BuildType.LABEL_LIST) - ? ruleContext.getPrerequisiteArtifacts(PROGUARD_SPECS, Mode.TARGET).list() - : ImmutableList.of(); - if (optMode == JavaOptimizationMode.LEGACY && proguardSpecs.isEmpty()) { - return ImmutableList.of(); - } - - // TODO(bazel-team): In modes except LEGACY verify that proguard specs don't include -dont... - // flags since those flags would override the desired optMode - ImmutableSortedSet.Builder builder = - ImmutableSortedSet.orderedBy(Artifact.EXEC_PATH_COMPARATOR) - .addAll(proguardSpecs) - .add(specsToInclude) - .addAll(ruleContext - .getPrerequisiteArtifacts(":extra_proguard_specs", Mode.TARGET) - .list()); - for (ProguardSpecProvider dep : - ruleContext.getPrerequisites("deps", Mode.TARGET, ProguardSpecProvider.class)) { - builder.addAll(dep.getTransitiveProguardSpecs()); - } - - // Generate and include implicit Proguard spec for requested mode. - if (!optMode.getImplicitProguardDirectives().isEmpty()) { - Artifact implicitDirectives = - getProguardConfigArtifact(ruleContext, optMode.name().toLowerCase()); - ruleContext.registerAction( - new FileWriteAction( - ruleContext.getActionOwner(), - implicitDirectives, - optMode.getImplicitProguardDirectives(), - /*executable*/ false)); - builder.add(implicitDirectives); - } - - return builder.build().asList(); - } - - /** - * Creates an action to run Proguard over the given {@code programJar} with various other given - * inputs to produce {@code proguardOutputJar}. If requested explicitly, or implicitly with - * --java_optimization_mode, the action also produces a mapping file (which shows what methods and - * classes in the output Jar correspond to which methods and classes in the input). The "pair" - * returned by this method indicates whether a mapping is being produced. - * - *

See the Proguard manual for the meaning of the various artifacts in play. - * - * @param proguard Proguard executable to use - * @param proguardSpecs Proguard specification files to pass to Proguard - * @param proguardMapping optional mapping file for Proguard to apply - * @param libraryJars any other Jar files that the {@code programJar} will run against - * @param mappingRequested whether to ask Proguard to output a mapping file (a mapping will be - * produced anyway if --java_optimization_mode includes obfuscation) - * @param filesBuilder all artifacts produced by this rule will be added to this builder - */ - public static ProguardOutput createProguardAction(RuleContext ruleContext, - FilesToRunProvider proguard, - Artifact programJar, - ImmutableList proguardSpecs, - @Nullable Artifact proguardMapping, - NestedSet libraryJars, - Artifact proguardOutputJar, - boolean mappingRequested, - NestedSetBuilder filesBuilder) throws InterruptedException { - JavaOptimizationMode optMode = getJavaOptimizationMode(ruleContext); - Preconditions.checkArgument(optMode != JavaOptimizationMode.NOOP); - Preconditions.checkArgument(optMode != JavaOptimizationMode.LEGACY || !proguardSpecs.isEmpty()); - - Builder builder = new SpawnAction.Builder() - .addInput(programJar) - .addInputs(libraryJars) - .addInputs(proguardSpecs) - .addOutput(proguardOutputJar) - .setExecutable(proguard) - .setProgressMessage("Trimming binary with Proguard") - .setMnemonic("Proguard") - .addArgument("-injars") - .addArgument(programJar.getExecPathString()); - - for (Artifact libraryJar : libraryJars) { - builder.addArgument("-libraryjars") - .addArgument(libraryJar.getExecPathString()); - } - - filesBuilder.add(proguardOutputJar); - - if (proguardMapping != null) { - builder.addInput(proguardMapping) - .addArgument("-applymapping") - .addArgument(proguardMapping.getExecPathString()); - } - - builder.addArgument("-outjars") - .addArgument(proguardOutputJar.getExecPathString()); - - for (Artifact proguardSpec : proguardSpecs) { - builder.addArgument("@" + proguardSpec.getExecPathString()); - } - - Artifact proguardOutputMap = null; - if (mappingRequested || optMode.alwaysGenerateOutputMapping()) { - // TODO(bazel-team): Verify that proguard spec files don't contain -printmapping directions - // which this -printmapping command line flag will override. - proguardOutputMap = ruleContext.getImplicitOutputArtifact( - AndroidRuleClasses.ANDROID_BINARY_PROGUARD_MAP); - - builder.addOutput(proguardOutputMap) - .addArgument("-printmapping") - .addArgument(proguardOutputMap.getExecPathString()); - filesBuilder.add(proguardOutputMap); - } - - ruleContext.registerAction(builder.build(ruleContext)); - return new ProguardOutput(proguardOutputJar, proguardOutputMap); - } - - /** - * Returns an intermediate artifact used to run Proguard. - */ - public static Artifact getProguardConfigArtifact(RuleContext ruleContext, String prefix) { - // TODO(bazel-team): Remove the redundant inclusion of the rule name, as getUniqueDirectory - // includes the rulename as well. - return Preconditions.checkNotNull(ruleContext.getUniqueDirectoryArtifact( - "proguard", - Joiner.on("_").join(prefix, ruleContext.getLabel().getName(), "proguard.cfg"), - ruleContext.getBinOrGenfilesDirectory())); - } - - private static JavaOptimizationMode getJavaOptimizationMode(RuleContext ruleContext) { - return ruleContext.getConfiguration().getFragment(JavaConfiguration.class) - .getJavaOptimizationMode(); - } -} diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java index 87d45b9c92..67ba2c3dff 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.rules.java; import static com.google.devtools.build.lib.rules.java.DeployArchiveBuilder.Compression.COMPRESSED; +import static com.google.devtools.build.lib.rules.java.DeployArchiveBuilder.Compression.UNCOMPRESSED; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -22,6 +23,7 @@ import com.google.common.collect.Lists; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.FileProvider; +import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; import com.google.devtools.build.lib.analysis.RuleContext; @@ -46,6 +48,8 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; +import javax.annotation.Nullable; + /** * An implementation of java_binary. */ @@ -213,21 +217,45 @@ public class JavaBinary implements RuleConfiguredTargetFactory { RuleConfiguredTargetBuilder builder = new RuleConfiguredTargetBuilder(ruleContext); - semantics.addProviders(ruleContext, common, jvmFlags, classJar, srcJar, - genClassJar, genSourceJar, ImmutableMap.of(), + semantics.addProviders(ruleContext, common, jvmFlags, classJar, srcJar, + genClassJar, genSourceJar, ImmutableMap.of(), helper, filesBuilder, builder); + Artifact deployJar = + ruleContext.getImplicitOutputArtifact(JavaSemantics.JAVA_BINARY_DEPLOY_JAR); + boolean runProguard = applyProguardIfRequested( + ruleContext, deployJar, common.getBootClasspath(), mainClass, filesBuilder); + NestedSet filesToBuild = filesBuilder.build(); + // Need not include normal runtime classpath in runfiles if Proguard is used because _deploy.jar + // is used as classpath instead. Keeping runfiles unchanged has however the advantage that + // manually running executable without --singlejar works (although it won't depend on Proguard). collectDefaultRunfiles(runfilesBuilder, ruleContext, common, filesToBuild, launcher, dynamicRuntimeActionInputs); Runfiles defaultRunfiles = runfilesBuilder.build(); - RunfilesSupport runfilesSupport = createExecutable - ? runfilesSupport = RunfilesSupport.withExecutable( - ruleContext, defaultRunfiles, executable, - semantics.getExtraArguments(ruleContext, common)) - : null; + RunfilesSupport runfilesSupport = null; + if (createExecutable) { + List extraArgs = new ArrayList<>(semantics.getExtraArguments(ruleContext, common)); + if (runProguard) { + // Instead of changing the classpath written into the wrapper script, pass --singlejar when + // running the script (which causes the deploy.jar written by Proguard to be used instead of + // the normal classpath). It's a bit odd to do this b/c manually running the script wouldn't + // use Proguard's output unless --singlejar is explicitly supplied. On the other hand the + // behavior of the script is more consistent: the (proguarded) deploy.jar is only used with + // --singlejar. Moreover, people will almost always run tests using blaze test, which does + // use Proguard's output thanks to this extra arg when enabled. Also, it's actually hard to + // get the classpath changed in the wrapper script (would require calling + // JavaCommon.setClasspathFragment with a new fragment at the *end* of this method because + // the classpath is evaluated lazily when generating the wrapper script) and the wrapper + // script would essentially have an if (--singlejar was set), set classpath to deploy jar, + // otherwise, set classpath to deploy jar. + extraArgs.add("--wrapper_script_flag=--singlejar"); + } + runfilesSupport = + RunfilesSupport.withExecutable(ruleContext, defaultRunfiles, executable, extraArgs); + } RunfilesProvider runfilesProvider = RunfilesProvider.withData( defaultRunfiles, @@ -236,26 +264,30 @@ public class JavaBinary implements RuleConfiguredTargetFactory { ImmutableList deployManifestLines = getDeployManifestLines(ruleContext, originalMainClass); - Artifact deployJar = - ruleContext.getImplicitOutputArtifact(JavaSemantics.JAVA_BINARY_DEPLOY_JAR); - - Artifact unstrippedDeployJar = - ruleContext.getImplicitOutputArtifact(JavaSemantics.JAVA_UNSTRIPPED_BINARY_DEPLOY_JAR); - + // When running Proguard: + // (1) write single jar to intermediate destination; Proguard will write _deploy.jar file + // (2) Don't depend on runfiles to avoid circular dependency, since _deploy.jar is itself part + // of runfiles when Proguard runs (because executable then needs it) and _deploy.jar depends + // on this single jar. + // (3) Don't bother with compression since Proguard will write the final jar anyways deployArchiveBuilder - .setOutputJar(deployJar) + .setOutputJar( + runProguard + ? ruleContext.getImplicitOutputArtifact(JavaSemantics.JAVA_BINARY_MERGED_JAR) + : deployJar) .setJavaStartClass(mainClass) .setDeployManifestLines(deployManifestLines) .setAttributes(attributes) .addRuntimeJars(common.getJavaCompilationArtifacts().getRuntimeJars()) .setIncludeBuildData(true) .setRunfilesMiddleman( - runfilesSupport == null ? null : runfilesSupport.getRunfilesMiddleman()) - .setCompression(COMPRESSED) - .setLauncher(launcher); - - deployArchiveBuilder.build(); + runProguard || runfilesSupport == null ? null : runfilesSupport.getRunfilesMiddleman()) + .setCompression(runProguard ? UNCOMPRESSED : COMPRESSED) + .setLauncher(launcher) + .build(); + Artifact unstrippedDeployJar = + ruleContext.getImplicitOutputArtifact(JavaSemantics.JAVA_UNSTRIPPED_BINARY_DEPLOY_JAR); if (stripAsDefault) { unstrippedDeployArchiveBuilder .setOutputJar(unstrippedDeployJar) @@ -413,4 +445,48 @@ public class JavaBinary implements RuleConfiguredTargetFactory { return result.build(); } + + /** + * This method uses {@link ProguardHelper#applyProguardIfRequested} to create a proguard action + * if necessary and adds any artifacts created by proguard to the given {@code filesBuilder}. + * This is convenience to make sure the proguarded Jar is included in the files to build, which is + * necessary because the Jar written by proguard is used at runtime. + * If this method returns {@code true} the Proguard is being used and we need to use a + * {@link DeployArchiveBuilder} to write the input artifact assumed by + * {@link ProguardHelper#applyProguardIfRequested}. + */ + private static boolean applyProguardIfRequested(RuleContext ruleContext, Artifact deployJar, + ImmutableList bootclasspath, String mainClassName, + NestedSetBuilder filesBuilder) throws InterruptedException { + // We only support proguarding tests so Proguard doesn't try to proguard itself. + if (!ruleContext.getRule().getRuleClass().endsWith("_test")) { + return false; + } + ProguardHelper.ProguardOutput output = + JavaBinaryProguardHelper.INSTANCE.applyProguardIfRequested( + ruleContext, deployJar, bootclasspath, mainClassName); + if (output == null) { + return false; + } + output.addAllToSet(filesBuilder); + return true; + } + + private static class JavaBinaryProguardHelper extends ProguardHelper { + + static final JavaBinaryProguardHelper INSTANCE = new JavaBinaryProguardHelper(); + + @Override + @Nullable + protected FilesToRunProvider findProguard(RuleContext ruleContext) { + // TODO(bazel-team): Find a way to use Proguard specified in android_sdk rules + return ruleContext.getExecutablePrerequisite(":proguard", Mode.HOST); + } + + @Override + protected ImmutableList collectProguardSpecsForRule(RuleContext ruleContext, + String mainClassName) { + return ImmutableList.of(generateSpecForJavaBinary(ruleContext, mainClassName)); + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java index 315cf67797..2983140fd0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java @@ -32,6 +32,8 @@ import com.google.devtools.common.options.TriState; import java.util.List; +import javax.annotation.Nullable; + /** * A java compiler configuration containing the flags required for compilation. */ @@ -137,17 +139,17 @@ public final class JavaConfiguration extends Fragment { private final Label javacBootclasspath; private final Label javacExtdir; private final ImmutableList javacOpts; + private final Label proguardBinary; private final ImmutableList

If this method returns artifacts then {@link DeployArchiveBuilder} needs to write the + * assumed input artifact (instead of the conventional deploy.jar, which now Proguard writes). + * Do not use this method for binary rules that themselves declare {@link #PROGUARD_SPECS} + * attributes, which includes of 1/2016 {@code android_binary} and {@code android_test}. + */ + @Nullable + public ProguardOutput applyProguardIfRequested(RuleContext ruleContext, Artifact deployJar, + ImmutableList bootclasspath, String mainClassName) throws InterruptedException { + JavaOptimizationMode optMode = getJavaOptimizationMode(ruleContext); + if (optMode == JavaOptimizationMode.NOOP || optMode == JavaOptimizationMode.LEGACY) { + // For simplicity do nothing in LEGACY mode + return null; + } + + Preconditions.checkArgument(bootclasspath.isEmpty(), + "Bootclasspath should be empty b/c not compiling for Android device: %s", bootclasspath); + FilesToRunProvider proguard = findProguard(ruleContext); + if (proguard == null) { + ruleContext.ruleError("--proguard_top required for --java_optimization_mode=" + optMode); + return null; + } + + ImmutableList proguardSpecs = collectProguardSpecs(ruleContext, mainClassName); + Artifact singleJar = + ruleContext.getImplicitOutputArtifact(JavaSemantics.JAVA_BINARY_MERGED_JAR); + return createProguardAction(ruleContext, proguard, singleJar, proguardSpecs, (Artifact) null, + NestedSetBuilder.emptySet(NAIVE_LINK_ORDER), deployJar, + /* mappingRequested */ false); + } + + private ImmutableList collectProguardSpecs( + RuleContext ruleContext, String mainClassName) { + return ProguardHelper.collectTransitiveProguardSpecs(ruleContext, + collectProguardSpecsForRule(ruleContext, mainClassName)); + } + + /** + * Returns the Proguard binary to invoke when using {@link #applyProguardIfRequested}. Returning + * {@code null} from this method will generate an error in that method. + * + * @return Proguard binary or {@code null} if none is available + */ + @Nullable + protected abstract FilesToRunProvider findProguard(RuleContext ruleContext); + + /** + * Returns rule-specific proguard specs not captured by {@link #PROGUARD_SPECS} attributes when + * using {@link #applyProguardIfRequested}. Typically these are generated artifacts such as specs + * generated for android resources. This method is only called if Proguard will definitely used, + * so it's ok to generate files here. + */ + protected abstract ImmutableList collectProguardSpecsForRule( + RuleContext ruleContext, String mainClassName); + + /** + * Retrieves the full set of proguard specs that should be applied to this binary, including the + * specs passed in, if Proguard should run on the given rule. {@link #createProguardAction} + * relies on this method returning an empty list if the given rule doesn't declare specs in + * --java_optimization_mode=legacy. + * + *

If Proguard shouldn't be applied, or the legacy link mode is used and there are no + * proguard_specs on this rule, an empty list will be returned, regardless of any given specs or + * specs from dependencies. + * {@link com.google.devtools.build.lib.rules.android.AndroidBinary#createAndroidBinary} relies on + * that behavior. + */ + public static ImmutableList collectTransitiveProguardSpecs(RuleContext ruleContext, + Iterable specsToInclude) { + JavaOptimizationMode optMode = getJavaOptimizationMode(ruleContext); + if (optMode == JavaOptimizationMode.NOOP) { + return ImmutableList.of(); + } + + ImmutableList proguardSpecs = + ruleContext.attributes().has(PROGUARD_SPECS, BuildType.LABEL_LIST) + ? ruleContext.getPrerequisiteArtifacts(PROGUARD_SPECS, Mode.TARGET).list() + : ImmutableList.of(); + if (optMode == JavaOptimizationMode.LEGACY && proguardSpecs.isEmpty()) { + return ImmutableList.of(); + } + + // TODO(bazel-team): In modes except LEGACY verify that proguard specs don't include -dont... + // flags since those flags would override the desired optMode + ImmutableSortedSet.Builder builder = + ImmutableSortedSet.orderedBy(Artifact.EXEC_PATH_COMPARATOR) + .addAll(proguardSpecs) + .addAll(specsToInclude) + .addAll(ruleContext + .getPrerequisiteArtifacts(":extra_proguard_specs", Mode.TARGET) + .list()); + for (ProguardSpecProvider dep : + ruleContext.getPrerequisites("deps", Mode.TARGET, ProguardSpecProvider.class)) { + builder.addAll(dep.getTransitiveProguardSpecs()); + } + + // Generate and include implicit Proguard spec for requested mode. + if (!optMode.getImplicitProguardDirectives().isEmpty()) { + Artifact implicitDirectives = + getProguardConfigArtifact(ruleContext, optMode.name().toLowerCase()); + ruleContext.registerAction( + new FileWriteAction( + ruleContext.getActionOwner(), + implicitDirectives, + optMode.getImplicitProguardDirectives(), + /*executable*/ false)); + builder.add(implicitDirectives); + } + + return builder.build().asList(); + } + + /** + * Creates a proguard spec that tells proguard to use the JDK's rt.jar as a library jar, similar + * to how android_binary would give Android SDK's android.jar to Proguard as library jar, and + * to keep the binary's entry point, ie., the main() method to be invoked. + */ + protected static Artifact generateSpecForJavaBinary(RuleContext ruleContext, + String mainClassName) { + // Add -libraryjars /lib/rt.jar so Proguard uses JDK bootclasspath, which JavaCommon + // doesn't expose when building for JDK (see checkArgument in applyProguardIfRequested). + // Note /lib/rt.jar refers to rt.jar that comes with JVM running Proguard, which + // should be identical to the JVM that will run the binary. + Artifact result = ProguardHelper.getProguardConfigArtifact(ruleContext, "jvm"); + ruleContext.registerAction( + new FileWriteAction( + ruleContext.getActionOwner(), + result, + String.format("-libraryjars /lib/rt.jar%n" + + "-keep class %s {%n" + + " public static void main(java.lang.String[]);%n" + + "}", + mainClassName), + /*executable*/ false)); + return result; + } + + /** + * Creates an action to run Proguard over the given {@code programJar} with various other given + * inputs to produce {@code proguardOutputJar}. If requested explicitly, or implicitly with + * --java_optimization_mode, the action also produces a mapping file (which shows what methods and + * classes in the output Jar correspond to which methods and classes in the input). The "pair" + * returned by this method indicates whether a mapping is being produced. + * + *

See the Proguard manual for the meaning of the various artifacts in play. + * + * @param proguard Proguard executable to use + * @param proguardSpecs Proguard specification files to pass to Proguard + * @param proguardMapping optional mapping file for Proguard to apply + * @param libraryJars any other Jar files that the {@code programJar} will run against + * @param mappingRequested whether to ask Proguard to output a mapping file (a mapping will be + * produced anyway if --java_optimization_mode includes obfuscation) + */ + public static ProguardOutput createProguardAction(RuleContext ruleContext, + FilesToRunProvider proguard, + Artifact programJar, + ImmutableList proguardSpecs, + @Nullable Artifact proguardMapping, + NestedSet libraryJars, + Artifact proguardOutputJar, + boolean mappingRequested) throws InterruptedException { + JavaOptimizationMode optMode = getJavaOptimizationMode(ruleContext); + Preconditions.checkArgument(optMode != JavaOptimizationMode.NOOP); + Preconditions.checkArgument(optMode != JavaOptimizationMode.LEGACY || !proguardSpecs.isEmpty()); + + Builder builder = new SpawnAction.Builder() + .addInput(programJar) + .addInputs(libraryJars) + .addInputs(proguardSpecs) + .addOutput(proguardOutputJar) + .setExecutable(proguard) + .setProgressMessage("Trimming binary with Proguard") + .setMnemonic("Proguard") + .addArgument("-injars") + .addArgument(programJar.getExecPathString()); + + for (Artifact libraryJar : libraryJars) { + builder.addArgument("-libraryjars") + .addArgument(libraryJar.getExecPathString()); + } + + if (proguardMapping != null) { + builder.addInput(proguardMapping) + .addArgument("-applymapping") + .addArgument(proguardMapping.getExecPathString()); + } + + builder.addArgument("-outjars") + .addArgument(proguardOutputJar.getExecPathString()); + + for (Artifact proguardSpec : proguardSpecs) { + builder.addArgument("@" + proguardSpec.getExecPathString()); + } + + Artifact proguardOutputMap = null; + if (mappingRequested || optMode.alwaysGenerateOutputMapping()) { + // TODO(bazel-team): Verify that proguard spec files don't contain -printmapping directions + // which this -printmapping command line flag will override. + proguardOutputMap = + ruleContext.getImplicitOutputArtifact(JavaSemantics.JAVA_BINARY_PROGUARD_MAP); + + builder.addOutput(proguardOutputMap) + .addArgument("-printmapping") + .addArgument(proguardOutputMap.getExecPathString()); + } + + ruleContext.registerAction(builder.build(ruleContext)); + return new ProguardOutput(proguardOutputJar, proguardOutputMap); + } + + /** + * Returns an intermediate artifact used to run Proguard. + */ + public static Artifact getProguardConfigArtifact(RuleContext ruleContext, String prefix) { + // TODO(bazel-team): Remove the redundant inclusion of the rule name, as getUniqueDirectory + // includes the rulename as well. + return Preconditions.checkNotNull(ruleContext.getUniqueDirectoryArtifact( + "proguard", + Joiner.on("_").join(prefix, ruleContext.getLabel().getName(), "proguard.cfg"), + ruleContext.getBinOrGenfilesDirectory())); + } + + /** + * Returns {@link JavaConfiguration#getJavaOptimizationMode()}. + */ + public static JavaOptimizationMode getJavaOptimizationMode(RuleContext ruleContext) { + return ruleContext.getConfiguration().getFragment(JavaConfiguration.class) + .getJavaOptimizationMode(); + } +} -- cgit v1.2.3