support hidden --java_optimization_mode flag in java_test rule.

--
MOS_MIGRATED_REVID=112290581

5 files changed, 23 insertions, 264 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 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<Artifact> apksUnderTest,
       Artifact proguardMapping) throws InterruptedException {
     ImmutableList<Artifact> 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<Artifact> failures =
         ImmutableList.<Artifact>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<String, Label> 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<BuildConfiguration> PROGUARD =
-      new LateBoundLabel<BuildConfiguration>(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<BuildConfiguration> ANDROID_SDK =
       new LateBoundLabel<BuildConfiguration>(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.
-   *
-   * <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 given specs or
-   * specs from dependencies.  {@link AndroidBinary#createAndroidBinary} relies on that behavior.
-   */
-  public static ImmutableList<Artifact> collectTransitiveProguardSpecs(RuleContext ruleContext,
-      Artifact... specsToInclude) {
-    JavaOptimizationMode optMode = getJavaOptimizationMode(ruleContext);
-    if (optMode == JavaOptimizationMode.NOOP) {
-      return ImmutableList.of();
-    }
-
-    ImmutableList<Artifact> proguardSpecs =
-        ruleContext.attributes().has(PROGUARD_SPECS, BuildType.LABEL_LIST)
-            ? ruleContext.getPrerequisiteArtifacts(PROGUARD_SPECS, Mode.TARGET).list()
-            : ImmutableList.<Artifact>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<Artifact> 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.
-   *
-   * <p>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<Artifact> proguardSpecs,
-      @Nullable Artifact proguardMapping,
-      NestedSet<Artifact> libraryJars,
-      Artifact proguardOutputJar,
-      boolean mappingRequested,
-      NestedSetBuilder<Artifact> 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();
-  }
-}