Add bazel flag to control how proguard runs

--
MOS_MIGRATED_REVID=107989708

3 files changed, 90 insertions, 15 deletions

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 e17201f940..9d9be90ac2 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
@@ -55,6 +55,8 @@ import com.google.devtools.build.lib.rules.cpp.CppHelper;
 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.JavaCompilationArgsProvider;
+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.JavaSemantics;
 import com.google.devtools.build.lib.rules.java.JavaSourceInfoProvider;
 import com.google.devtools.build.lib.rules.java.JavaTargetAttributes;
@@ -327,12 +329,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
       JavaTargetAttributes resourceClasses,
       ImmutableList<Artifact> apksUnderTest,
       Artifact proguardMapping) throws InterruptedException {
-
-    ImmutableList<Artifact> proguardSpecs =
-        getTransitiveProguardSpecs(
-            ruleContext,
-            resourceApk,
-            ruleContext.getPrerequisiteArtifacts(PROGUARD_SPECS, Mode.TARGET).list());
+    ImmutableList<Artifact> proguardSpecs = getTransitiveProguardSpecs(ruleContext, resourceApk);
 
     ProguardOutput proguardOutput =
         applyProguard(
@@ -727,15 +724,25 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
   /**
    * Retrieves the full set of proguard specs that should be applied to this binary.
    *
-   * <p>If an empty list is passed (i.e., there are no proguardSpecs on this rule), an empty list
-   * will be returned, regardless of any specs from dependencies or the resourceApk.
+   * <p>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 specs from
+   * dependencies or the resourceApk.
    */
   private static ImmutableList<Artifact> getTransitiveProguardSpecs(
-      RuleContext ruleContext, ResourceApk resourceApk, ImmutableList<Artifact> proguardSpecs) {
-    if (proguardSpecs.isEmpty()) {
-      return proguardSpecs;
+      RuleContext ruleContext, ResourceApk resourceApk) {
+    JavaOptimizationMode optMode = getJavaOptimizationMode(ruleContext);
+    if (optMode == JavaOptimizationMode.NOOP) {
+      return ImmutableList.of();
+    }
+
+    ImmutableList<Artifact> proguardSpecs =
+        ruleContext.getPrerequisiteArtifacts(PROGUARD_SPECS, Mode.TARGET).list();
+    if (optMode == JavaOptimizationMode.LEGACY && proguardSpecs.isEmpty()) {
+      return ImmutableList.of();
     }
 
+    // TODO(kmb): In modes other than LEGACY verify that proguard specs don't include -dont... flags
+    // since those flags would override the desired optMode (b/25621573)
     ImmutableSortedSet.Builder<Artifact> builder =
         ImmutableSortedSet.<Artifact>orderedBy(Artifact.EXEC_PATH_COMPARATOR).addAll(proguardSpecs);
     for (ProguardSpecProvider dep :
@@ -747,6 +754,11 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
     return builder.build().asList();
   }
 
+  private static JavaOptimizationMode getJavaOptimizationMode(RuleContext ruleContext) {
+    return ruleContext.getConfiguration().getFragment(JavaConfiguration.class)
+        .getJavaOptimizationMode();
+  }
+
   /** Applies the proguard specifications, and creates a ProguardedJar. */
   private static ProguardOutput applyProguard(
       RuleContext ruleContext,
@@ -781,11 +793,15 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
       failures.add(
           ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_BINARY_PROGUARD_MAP));
     }
+    JavaOptimizationMode optMode = getJavaOptimizationMode(ruleContext);
     ruleContext.registerAction(
         new FailAction(
             ruleContext.getActionOwner(),
             failures.build(),
-            "Can't generate Proguard jar or mapping without proguard_specs."));
+            String.format("Can't generate Proguard jar or mapping %s.",
+                optMode == JavaOptimizationMode.LEGACY
+                    ? "without proguard_specs"
+                    : "in optimization mode " + optMode)));
     return new ProguardOutput(deployJarArtifact, null);
   }
 
@@ -831,8 +847,12 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
     }
 
     Artifact proguardOutputMap = null;
-    if (ruleContext.attributes().get("proguard_generate_mapping", Type.BOOLEAN)) {
-       proguardOutputMap = ruleContext.getImplicitOutputArtifact(
+    JavaOptimizationMode optMode = getJavaOptimizationMode(ruleContext);
+    if (ruleContext.attributes().get("proguard_generate_mapping", Type.BOOLEAN)
+        || optMode.alwaysGenerateOutputMapping()) {
+      // TODO(kmb): 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)
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 69e6c5e6af..ecc71af308 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
@@ -48,6 +48,37 @@ public final class JavaConfiguration extends Fragment {
     BLAZE
   }
 
+  /**
+   * 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 static 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. */
+    NOOP,
+    /** Produce fully optimized binary with short symbol names and unreachable code removed. */
+    OPTIMIZE_MINIMIZE;
+
+    /**
+     * 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 OPTIMIZE_MINIMIZE:
+          return true;
+        default:
+          throw new AssertionError("Unexpected mode: " + this);
+      }
+    }
+  }
+
   private final ImmutableList<String> commandLineJavacFlags;
   private final Label javaLauncherLabel;
   private final Label javaBuilderTop;
@@ -67,8 +98,9 @@ public final class JavaConfiguration extends Fragment {
   private final ImmutableList<Label> translationTargets;
   private final String javaCpu;
   private final boolean allowPrecompiledJarsInSrcs;
+  private final JavaOptimizationMode javaOptimizationMode;
 
-  private Label javaToolchain;
+  private final Label javaToolchain;
 
   JavaConfiguration(boolean generateJavaDeps,
       List<String> defaultJvmFlags, JavaOptions javaOptions, Label javaToolchain, String javaCpu,
@@ -93,6 +125,7 @@ public final class JavaConfiguration extends Fragment {
     this.javaCpu = javaCpu;
     this.javaToolchain = javaToolchain;
     this.allowPrecompiledJarsInSrcs = javaOptions.allowPrecompiledJarsInSrcs;
+    this.javaOptimizationMode = javaOptions.javaOptimizationMode;
 
     ImmutableList.Builder<Label> translationsBuilder = ImmutableList.builder();
     for (String s : javaOptions.translationTargets) {
@@ -249,4 +282,9 @@ public final class JavaConfiguration extends Fragment {
   public boolean allowPrecompiledJarsInSrcs() {
     return allowPrecompiledJarsInSrcs;
   }
+
+  /** Returns the --java_optimization_mode flag setting. */
+  public JavaOptimizationMode getJavaOptimizationMode() {
+    return javaOptimizationMode;
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java
index d41d2ed97f..324ae14c74 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java
@@ -27,6 +27,7 @@ import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.rules.java.JavaConfiguration.JavaClasspathMode;
+import com.google.devtools.build.lib.rules.java.JavaConfiguration.JavaOptimizationMode;
 import com.google.devtools.common.options.Converter;
 import com.google.devtools.common.options.Converters.StringSetConverter;
 import com.google.devtools.common.options.EnumConverter;
@@ -162,6 +163,15 @@ public class JavaOptions extends FragmentOptions {
     }
   }
 
+  /**
+   * Converter for the --java_optimization_mode option.
+   */
+  public static class JavaOptimizationModeConverter extends EnumConverter<JavaOptimizationMode> {
+    public JavaOptimizationModeConverter() {
+      super(JavaOptimizationMode.class, "Java optimization strategy");
+    }
+  }
+
   @Option(name = "javabase",
       defaultValue = "",
       converter = JavabaseConverter.class,
@@ -369,6 +379,13 @@ public class JavaOptions extends FragmentOptions {
       help = "No-op. Kept here for backwards compatibility.")
   public boolean allowPrecompiledJarsInSrcs;
 
+  @Option(name = "java_optimization_mode",
+      defaultValue = "legacy",
+      converter = JavaOptimizationModeConverter.class,
+      category = "undocumented",
+      help = "Applies desired link-time optimizations to Java binaries and tests.")
+  public JavaOptimizationMode javaOptimizationMode;
+
   @Override
   public FragmentOptions getHost(boolean fallback) {
     JavaOptions host = (JavaOptions) getDefault();