From a9cdc3dd2355d1de382e40f7b4692e345ab310d0 Mon Sep 17 00:00:00 2001 From: cushon Date: Tue, 30 Jul 2019 17:03:10 -0700 Subject: [PATCH] Move ProguardHelper to android and remove support for unused --java_optimization_mode flag PiperOrigin-RevId: 260826546 --- .../rules/android/BazelAndroidSemantics.java | 2 +- .../bazel/rules/java/BazelJavaTestRule.java | 10 -- .../lib/rules/android/AndroidBinary.java | 12 +- .../lib/rules/android/AndroidRuleClasses.java | 1 - .../lib/rules/android/AndroidSemantics.java | 2 +- .../rules/android/ProcessedAndroidData.java | 1 - .../{java => android}/ProguardHelper.java | 169 ++---------------- .../devtools/build/lib/rules/java/BUILD | 1 - .../build/lib/rules/java/JavaBinary.java | 88 +-------- .../lib/rules/java/JavaConfiguration.java | 82 --------- .../build/lib/rules/java/JavaOptions.java | 15 +- .../build/lib/rules/java/JavaSemantics.java | 5 +- .../lib/rules/android/AndroidBinaryTest.java | 95 ---------- 13 files changed, 26 insertions(+), 457 deletions(-) rename src/main/java/com/google/devtools/build/lib/rules/{java => android}/ProguardHelper.java (77%) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidSemantics.java index f8a7509115d995..05419552e2f504 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidSemantics.java @@ -25,10 +25,10 @@ import com.google.devtools.build.lib.rules.android.AndroidConfiguration; import com.google.devtools.build.lib.rules.android.AndroidDataContext; import com.google.devtools.build.lib.rules.android.AndroidSemantics; +import com.google.devtools.build.lib.rules.android.ProguardHelper.ProguardOutput; import com.google.devtools.build.lib.rules.java.JavaCompilationArtifacts; import com.google.devtools.build.lib.rules.java.JavaSemantics; import com.google.devtools.build.lib.rules.java.JavaTargetAttributes; -import com.google.devtools.build.lib.rules.java.ProguardHelper.ProguardOutput; /** * Implementation of Bazel-specific behavior in Android rules. 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 a749cffa796361..549a05849d2d83 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 @@ -16,7 +16,6 @@ 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; @@ -24,7 +23,6 @@ import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; -import com.google.devtools.build.lib.analysis.config.HostTransition; import com.google.devtools.build.lib.bazel.rules.java.BazelJavaRuleClasses.BaseJavaBinaryRule; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; @@ -53,14 +51,6 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) return builder .requiresConfigurationFragments(JavaConfiguration.class, CppConfiguration.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(HostTransition.createFactory()) - .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 7edd27013ec43a..973f30d5688fca 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 @@ -67,13 +67,13 @@ import com.google.devtools.build.lib.rules.android.AndroidBinaryMobileInstall.MobileInstallResourceApks; import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion; 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.android.ZipFilterBuilder.CheckHashMismatchMode; import com.google.devtools.build.lib.rules.android.databinding.DataBinding; import com.google.devtools.build.lib.rules.cpp.CppSemantics; import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder; import com.google.devtools.build.lib.rules.java.JavaCommon; 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.JavaConfiguration.OneVersionEnforcementLevel; import com.google.devtools.build.lib.rules.java.JavaRuntimeInfo; import com.google.devtools.build.lib.rules.java.JavaSemantics; @@ -81,8 +81,6 @@ import com.google.devtools.build.lib.rules.java.JavaTargetAttributes; import com.google.devtools.build.lib.rules.java.JavaToolchainProvider; import com.google.devtools.build.lib.rules.java.OneVersionCheckActionBuilder; -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.rules.java.ProguardSpecProvider; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.vfs.PathFragment; @@ -423,7 +421,6 @@ public static RuleConfiguredTargetBuilder createAndroidBinary( // which this -printmapping command line flag will override. Artifact proguardOutputMap = null; if (ProguardHelper.genProguardMapping(ruleContext.attributes()) - || ProguardHelper.getJavaOptimizationMode(ruleContext).alwaysGenerateOutputMapping() || shrinkResources) { proguardOutputMap = androidSemantics.getProguardOutputMap(ruleContext); } @@ -818,16 +815,11 @@ private static ProguardOutput createEmptyProguardAction( semantics, proguardOutputMap); outputs.addAllToSet(failures); - JavaOptimizationMode optMode = ProguardHelper.getJavaOptimizationMode(ruleContext); ruleContext.registerAction( new FailAction( ruleContext.getActionOwner(), failures.build(), - String.format( - "Can't run Proguard %s", - optMode == JavaOptimizationMode.LEGACY - ? "without proguard_specs" - : "in optimization mode " + optMode))); + String.format("Can't run Proguard without proguard_specs"))); return new ProguardOutput(deployJarArtifact, null, null, null, null, null, null); } 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 982909951cb372..99a8b59e735834 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 @@ -59,7 +59,6 @@ import com.google.devtools.build.lib.rules.java.JavaInfo; import com.google.devtools.build.lib.rules.java.JavaRuleClasses; import com.google.devtools.build.lib.rules.java.JavaSemantics; -import com.google.devtools.build.lib.rules.java.ProguardHelper; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.skylarkbuildapi.android.AndroidSplitTransititionApi; import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter; 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 2f7470c987d7a0..f638577460cee8 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 @@ -21,10 +21,10 @@ import com.google.devtools.build.lib.cmdline.Label; 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.android.ProguardHelper.ProguardOutput; import com.google.devtools.build.lib.rules.java.JavaCompilationArtifacts; import com.google.devtools.build.lib.rules.java.JavaSemantics; import com.google.devtools.build.lib.rules.java.JavaTargetAttributes; -import com.google.devtools.build.lib.rules.java.ProguardHelper.ProguardOutput; import com.google.devtools.build.lib.syntax.Type; import java.util.Map; import java.util.Optional; diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ProcessedAndroidData.java b/src/main/java/com/google/devtools/build/lib/rules/android/ProcessedAndroidData.java index d89f019aa4d396..bb22a8eea4ba50 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ProcessedAndroidData.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ProcessedAndroidData.java @@ -21,7 +21,6 @@ import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion; import com.google.devtools.build.lib.rules.android.databinding.DataBinding; import com.google.devtools.build.lib.rules.android.databinding.DataBindingContext; -import com.google.devtools.build.lib.rules.java.ProguardHelper; import com.google.devtools.build.lib.syntax.Type; import java.util.List; import java.util.Map; diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/ProguardHelper.java b/src/main/java/com/google/devtools/build/lib/rules/android/ProguardHelper.java similarity index 77% rename from src/main/java/com/google/devtools/build/lib/rules/java/ProguardHelper.java rename to src/main/java/com/google/devtools/build/lib/rules/android/ProguardHelper.java index 27233a134fbc82..12bebf591f05c5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/ProguardHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ProguardHelper.java @@ -11,12 +11,11 @@ // 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.java; +package com.google.devtools.build.lib.rules.android; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; -import com.google.common.base.Ascii; import com.google.common.base.Joiner; import com.google.common.base.Optional; import com.google.common.base.Preconditions; @@ -29,7 +28,6 @@ import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext; 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.configuredtargets.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.cmdline.Label; @@ -37,13 +35,17 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.BuildType; -import com.google.devtools.build.lib.rules.java.JavaConfiguration.JavaOptimizationMode; +import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder; +import com.google.devtools.build.lib.rules.java.JavaConfiguration; +import com.google.devtools.build.lib.rules.java.JavaSemantics; +import com.google.devtools.build.lib.rules.java.JavaTargetAttributes; +import com.google.devtools.build.lib.rules.java.ProguardSpecProvider; import com.google.devtools.build.lib.syntax.Type; import java.util.Map; import javax.annotation.Nullable; -/** Common code for proguarding Java binaries. */ -public abstract class ProguardHelper { +/** Common code for proguarding. */ +public final class ProguardHelper { /** * Attribute for attaching proguard specs explicitly to a rule, if such an attribute is desired. @@ -145,93 +147,6 @@ public void addAllToSet(NestedSetBuilder filesBuilder, Artifact finalP } } - protected ProguardHelper() {} - - /** - * Creates an action to run Proguard to output the given {@code deployJar} artifact if - * --java_optimization_mode calls for it from an assumed input artifact {@link - * JavaSemantics#JAVA_BINARY_MERGED_JAR}. Returns the artifacts that Proguard will generate or - * {@code null} if Proguard isn't used. - * - *

If this method returns artifacts then {@link - * com.google.devtools.build.lib.rules.java.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 as of - * includes 1/2016 {@code android_binary} and {@code android_test}. - */ - @Nullable - public ProguardOutput applyProguardIfRequested( - RuleContext ruleContext, - Artifact deployJar, - ImmutableList bootclasspath, - String mainClassName, - JavaSemantics semantics) - 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 not be empty"); - FilesToRunProvider proguard = findProguard(ruleContext); - if (proguard == null) { - ruleContext.ruleError("--proguard_top required for --java_optimization_mode=" + optMode); - return null; - } - - ImmutableList proguardSpecs = - collectProguardSpecs(ruleContext, bootclasspath, mainClassName); - Artifact singleJar = - ruleContext.getImplicitOutputArtifact(JavaSemantics.JAVA_BINARY_MERGED_JAR); - - // TODO(bazel-team): Verify that proguard spec files don't contain -printmapping directions - // which this -printmapping command line flag will override. - Artifact proguardOutputMap = null; - if (genProguardMapping(ruleContext.attributes()) || optMode.alwaysGenerateOutputMapping()) { - proguardOutputMap = - ruleContext.getImplicitOutputArtifact(JavaSemantics.JAVA_BINARY_PROGUARD_MAP); - } - return createOptimizationActions( - ruleContext, - proguard, - singleJar, - proguardSpecs, - /* proguardSeeds */ (Artifact) null, - /* proguardUsage */ (Artifact) null, - /* proguardMapping */ (Artifact) null, - /* proguardDictionary */ (Artifact) null, - bootclasspath, - deployJar, - semantics, - /* optimizationPases */ 3, - proguardOutputMap); - } - - private ImmutableList collectProguardSpecs( - RuleContext ruleContext, ImmutableList bootclasspath, String mainClassName) { - return ProguardHelper.collectTransitiveProguardSpecs( - ruleContext, collectProguardSpecsForRule(ruleContext, bootclasspath, 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, ImmutableList bootclasspath, 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 @@ -297,17 +212,10 @@ public static ImmutableList collectTransitiveProguardSpecs( Iterable specsToInclude, ImmutableList localProguardSpecs, Iterable proguardDeps) { - JavaOptimizationMode optMode = getJavaOptimizationMode(context); - if (optMode == JavaOptimizationMode.NOOP) { - return ImmutableList.of(); - } - - if (optMode == JavaOptimizationMode.LEGACY && localProguardSpecs.isEmpty()) { + if (localProguardSpecs.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(localProguardSpecs) @@ -316,40 +224,9 @@ public static ImmutableList collectTransitiveProguardSpecs( builder.addAll(dep.getTransitiveProguardSpecs()); } - // Generate and include implicit Proguard spec for requested mode. - if (!optMode.getImplicitProguardDirectives().isEmpty()) { - Artifact implicitDirectives = - getProguardConfigArtifact(label, context, Ascii.toLowerCase(optMode.name())); - context.registerAction( - FileWriteAction.create( - context, - implicitDirectives, - optMode.getImplicitProguardDirectives(), - /*makeExecutable=*/ false)); - builder.add(implicitDirectives); - } - return builder.build().asList(); } - /** - * Creates a proguard spec that tells proguard to keep the binary's entry point, ie., the {@code - * main()} method to be invoked. - */ - protected static Artifact generateSpecForJavaBinary( - RuleContext ruleContext, String mainClassName) { - Artifact result = ProguardHelper.getProguardConfigArtifact(ruleContext, "jvm"); - ruleContext.registerAction( - FileWriteAction.create( - ruleContext, - result, - String.format( - "-keep class %s {%n public static void main(java.lang.String[]);%n}", - mainClassName), - /*makeExecutable=*/ false)); - return result; - } - /** @return true if proguard_generate_mapping is specified. */ public static final boolean genProguardMapping(AttributeMap rule) { return rule.has("proguard_generate_mapping", Type.BOOLEAN) @@ -369,13 +246,12 @@ public static ProguardOutput getProguardOutputs( JavaSemantics semantics, @Nullable Artifact proguardOutputMap) throws InterruptedException { - JavaOptimizationMode optMode = getJavaOptimizationMode(ruleContext); boolean mappingRequested = genProguardMapping(ruleContext.attributes()); Artifact proguardOutputProtoMap = null; Artifact proguardConstantStringMap = null; - if (mappingRequested || optMode.alwaysGenerateOutputMapping()) { + if (mappingRequested) { // TODO(bazel-team): if rex is enabled, the proguard map will change and then will no // longer correspond to the proto map proguardOutputProtoMap = semantics.getProtoMapping(ruleContext); @@ -431,9 +307,7 @@ public static ProguardOutput createOptimizationActions( @Nullable Integer optimizationPasses, @Nullable Artifact proguardOutputMap) throws InterruptedException { - JavaOptimizationMode optMode = getJavaOptimizationMode(ruleContext); - Preconditions.checkArgument(optMode != JavaOptimizationMode.NOOP); - Preconditions.checkArgument(optMode != JavaOptimizationMode.LEGACY || !proguardSpecs.isEmpty()); + Preconditions.checkArgument(!proguardSpecs.isEmpty()); ProguardOutput output = getProguardOutputs( @@ -447,8 +321,7 @@ public static ProguardOutput createOptimizationActions( if (Iterables.size(libraryJars) > 1) { JavaTargetAttributes attributes = new JavaTargetAttributes.Builder(semantics).build(); Artifact combinedLibraryJar = - getProguardTempArtifact( - ruleContext, Ascii.toLowerCase(optMode.name()), "combined_library_jars.jar"); + getProguardTempArtifact(ruleContext, "combined_library_jars.jar"); new DeployArchiveBuilder(semantics, ruleContext) .setOutputJar(combinedLibraryJar) .setAttributes(attributes) @@ -485,8 +358,7 @@ public static ProguardOutput createOptimizationActions( } else { // Optimization passes have been specified, so run proguard in multiple phases. Artifact lastStageOutput = - getProguardTempArtifact( - ruleContext, Ascii.toLowerCase(optMode.name()), "proguard_preoptimization.jar"); + getProguardTempArtifact(ruleContext, "proguard_preoptimization.jar"); SpawnAction.Builder initialAction = new SpawnAction.Builder(); CustomCommandLine.Builder initialCommandLine = CustomCommandLine.builder(); defaultAction( @@ -534,7 +406,6 @@ public static ProguardOutput createOptimizationActions( Artifact optimizationOutput = getProguardTempArtifact( ruleContext, - Ascii.toLowerCase(optMode.name()), mnemonic + "_optimization_" + i + ".jar"); SpawnAction.Builder optimizationAction = new SpawnAction.Builder(); CustomCommandLine.Builder optimizationCommandLine = CustomCommandLine.builder(); @@ -688,9 +559,8 @@ private static void defaultAction( } /** Returns an intermediate artifact used to run Proguard. */ - public static Artifact getProguardTempArtifact( - RuleContext ruleContext, String prefix, String name) { - return getProguardTempArtifact(ruleContext.getLabel(), ruleContext, prefix, name); + public static Artifact getProguardTempArtifact(RuleContext ruleContext, String name) { + return getProguardTempArtifact(ruleContext.getLabel(), ruleContext, "legacy", name); } /** Returns an intermediate artifact used to run Proguard. */ @@ -710,15 +580,6 @@ public static Artifact getProguardConfigArtifact( Label label, ActionConstructionContext context, String prefix) { return getProguardTempArtifact(label, context, prefix, "proguard.cfg"); } - - /** Returns {@link JavaConfiguration#getJavaOptimizationMode()}. */ - public static JavaOptimizationMode getJavaOptimizationMode(ActionConstructionContext context) { - return context - .getConfiguration() - .getFragment(JavaConfiguration.class) - .getJavaOptimizationMode(); - } - private static Map> getBytecodeOptimizers(RuleContext ruleContext) { return ruleContext .getConfiguration() diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/BUILD b/src/main/java/com/google/devtools/build/lib/rules/java/BUILD index af87385e83d2d9..7f85a71c37f950 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/java/BUILD @@ -30,7 +30,6 @@ java_library( "JavaToolchain.java", "JavaToolchainAliasRule.java", "JavaToolchainRule.java", - "ProguardHelper.java", "ProguardLibrary.java", "ProguardLibraryRule.java", "ProguardSpecProvider.java", 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 4b0c3b3c234d35..de9adbfe42ab5a 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 @@ -15,7 +15,6 @@ import static com.google.devtools.build.lib.rules.cpp.CppRuleClasses.STATIC_LINKING_MODE; 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.base.Optional; import com.google.common.collect.ImmutableList; @@ -25,7 +24,6 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; import com.google.devtools.build.lib.analysis.ConfiguredTarget; -import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.OutputGroupInfo; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory; @@ -51,7 +49,6 @@ import com.google.devtools.build.lib.rules.cpp.LibraryToLink; import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider.ClasspathType; import com.google.devtools.build.lib.rules.java.JavaConfiguration.OneVersionEnforcementLevel; -import com.google.devtools.build.lib.rules.java.ProguardHelper.ProguardOutput; import com.google.devtools.build.lib.rules.java.proto.GeneratedExtensionRegistryProvider; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.OS; @@ -60,7 +57,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; -import javax.annotation.Nullable; /** An implementation of java_binary. */ public class JavaBinary implements RuleConfiguredTargetFactory { @@ -318,9 +314,6 @@ public ConfiguredTarget create(RuleContext ruleContext) Artifact deployJar = ruleContext.getImplicitOutputArtifact(JavaSemantics.JAVA_BINARY_DEPLOY_JAR); - boolean runProguard = - applyProguardIfRequested( - ruleContext, deployJar, common.getBootClasspath(), mainClass, semantics, filesBuilder); if (javaConfig.oneVersionEnforcementLevel() != OneVersionEnforcementLevel.OFF) { // This JavaBinary class is also the implementation for java_test targets (via the @@ -344,9 +337,6 @@ public ConfiguredTarget create(RuleContext ruleContext) } 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, @@ -362,21 +352,6 @@ public ConfiguredTarget create(RuleContext ruleContext) if (createExecutable) { List extraArgs = new ArrayList<>(semantics.getExtraArguments(ruleContext, common.getSrcsArtifacts())); - 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"); - } // The executable we pass here will be used when creating the runfiles directory. E.g. for the // stub script called bazel-bin/foo/bar_bin, the runfiles directory will be created under // bazel-bin/foo/bar_bin.runfiles . On platforms where there's an extra stub script (Windows) @@ -400,25 +375,16 @@ public ConfiguredTarget create(RuleContext ruleContext) ImmutableList deployManifestLines = getDeployManifestLines(ruleContext, originalMainClass); - // 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( - runProguard - ? ruleContext.getImplicitOutputArtifact(JavaSemantics.JAVA_BINARY_MERGED_JAR) - : deployJar) + .setOutputJar(deployJar) .setJavaStartClass(mainClass) .setDeployManifestLines(deployManifestLines) .setAttributes(attributes) .addRuntimeJars(javaArtifacts.getRuntimeJars()) .setIncludeBuildData(true) .setRunfilesMiddleman( - runProguard || runfilesSupport == null ? null : runfilesSupport.getRunfilesMiddleman()) - .setCompression(runProguard ? UNCOMPRESSED : COMPRESSED) + runfilesSupport == null ? null : runfilesSupport.getRunfilesMiddleman()) + .setCompression(COMPRESSED) .setLauncher(launcher) .setOneVersionEnforcementLevel( javaConfig.oneVersionEnforcementLevel(), @@ -634,55 +600,7 @@ public static Collection collectNativeLibraries( return LibraryToLink.getDynamicLibrariesForLinking(linkerInputs); } - /** - * 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, - JavaSemantics semantics, - NestedSetBuilder filesBuilder) - throws InterruptedException { - // We only support proguarding tests so Proguard doesn't try to proguard itself. - if (!isJavaTestRule(ruleContext)) { - return false; - } - ProguardOutput output = - JavaBinaryProguardHelper.INSTANCE.applyProguardIfRequested( - ruleContext, deployJar, bootclasspath, mainClassName, semantics); - if (output == null) { - return false; - } - output.addAllToSet(filesBuilder); - return true; - } - private static boolean isJavaTestRule(RuleContext ruleContext) { return ruleContext.getRule().getRuleClass().endsWith("_test"); } - - 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, ImmutableList bootclasspath, 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 37598fe1dcf400..b36cf133b09d31 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 @@ -13,7 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.rules.java; -import static com.google.common.base.Preconditions.checkArgument; import com.google.common.base.Ascii; import com.google.common.base.Optional; @@ -75,76 +74,6 @@ public enum ImportDepsCheckingLevel { ERROR } - /** - * Values for the --java_optimization_mode option, which controls how Proguard is run over binary - * and test targets. Note that for the moment this has no effect when building library targets. - */ - public enum JavaOptimizationMode { - /** Proguard is used iff top-level target has {@code proguard_specs} attribute. */ - LEGACY, - /** - * No link-time optimizations are applied, regardless of the top-level target's attributes. In - * practice this mode skips Proguard completely, rather than invoking Proguard as a no-op. - */ - NOOP("-dontshrink", "-dontoptimize", "-dontobfuscate"), - /** - * Symbols have different names except where configured not to rename. This mode is primarily - * intended to aid in identifying missing configuration directives that prevent symbols accessed - * reflectively etc. from being renamed or removed. - */ - RENAME("-dontshrink", "-dontoptimize"), - /** - * "Quickly" produce small binary typically without changing code structure. In practice this - * mode removes unreachable code and uses short symbol names except where configured not to - * rename or remove. This mode should build faster than {@link #OPTIMIZE_MINIFY} and may hence - * be preferable during development. - */ - FAST_MINIFY("-dontoptimize"), - /** - * Produce fully optimized binary with short symbol names and unreachable code removed. Unlike - * {@link #FAST_MINIFY}, this mode may apply code transformations, in addition to removing and - * renaming code as the configuration allows, to produce a more compact binary. This mode should - * be preferable for producing and testing release binaries. - */ - OPTIMIZE_MINIFY; - - private final String proguardDirectives; - - JavaOptimizationMode(String... donts) { - StringBuilder proguardDirectives = new StringBuilder(); - for (String dont : donts) { - checkArgument(dont.startsWith("-dont"), "invalid Proguard directive: %s", dont); - proguardDirectives.append(dont).append('\n'); - } - this.proguardDirectives = proguardDirectives.toString(); - } - - /** Returns additional Proguard directives necessary for this mode (can be empty). */ - public String getImplicitProguardDirectives() { - return proguardDirectives; - } - - /** - * Returns true if all affected targets should produce mappings from original to renamed symbol - * names, regardless of the proguard_generate_mapping attribute. This should be the case for all - * modes that force symbols to be renamed. By contrast, the {@link #NOOP} mode will never - * produce a mapping file since no symbols are ever renamed. - */ - public boolean alwaysGenerateOutputMapping() { - switch (this) { - case LEGACY: - case NOOP: - return false; - case RENAME: - case FAST_MINIFY: - case OPTIMIZE_MINIFY: - return true; - default: - throw new AssertionError("Unexpected mode: " + this); - } - } - } - private final ImmutableList commandLineJavacFlags; private final Label javaLauncherLabel; private final boolean useIjars; @@ -166,7 +95,6 @@ public boolean alwaysGenerateOutputMapping() { private final ImmutableList