Add the option for warning-level enforcement of One Version violations. This updates --experimental_one_version_enforcement from a boolean flag to a 3-state enum (OFF, WARNING, ERROR).

PiperOrigin-RevId: 154978203

4 files changed, 67 insertions, 27 deletions

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 60aa02855c..df9fd7cc3e 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
@@ -45,6 +45,7 @@ import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CppHelper;
 import com.google.devtools.build.lib.rules.cpp.LinkerInput;
 import com.google.devtools.build.lib.rules.java.JavaCompilationArgs.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;
@@ -286,7 +287,7 @@ public class JavaBinary implements RuleConfiguredTargetFactory {
     boolean runProguard = applyProguardIfRequested(
         ruleContext, deployJar, common.getBootClasspath(), mainClass, semantics, filesBuilder);
 
-    if (javaConfig.isEnforceOneVersion()) {
+    if (javaConfig.oneVersionEnforcementLevel() != OneVersionEnforcementLevel.OFF) {
       Artifact oneVersionOutput =
           ruleContext
               .getAnalysisEnvironment()
@@ -300,7 +301,8 @@ public class JavaBinary implements RuleConfiguredTargetFactory {
       OneVersionCheckActionBuilder.build(
           ruleContext,
           transitiveDependencies,
-          oneVersionOutput);
+          oneVersionOutput,
+          javaConfig.oneVersionEnforcementLevel());
     }
     NestedSet<Artifact> filesToBuild = filesBuilder.build();
 
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 31f9fefbae..2bdfafbfb5 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
@@ -54,6 +54,22 @@ public final class JavaConfiguration extends Fragment {
     BLAZE
   }
 
+  /** Values for the --experimental_one_version_enforcement option */
+  public enum OneVersionEnforcementLevel {
+    /** Don't attempt to check for one version violations (the default) */
+    OFF,
+    /**
+     * Check for one version violations, emit warnings to stderr if any are found, but don't break
+     * the binary.
+     */
+    WARNING,
+    /**
+     * Check for one version violations, emit warnings to stderr if any are found, and break the
+     * rule if it's found.
+     */
+    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.
@@ -133,7 +149,7 @@ public final class JavaConfiguration extends Fragment {
   private final boolean headerCompilationDirectClasspath;
   private final boolean generateJavaDeps;
   private final boolean strictDepsJavaProtos;
-  private final boolean enforceOneVersion;
+  private final OneVersionEnforcementLevel enforceOneVersion;
   private final JavaClasspathMode javaClasspath;
   private final ImmutableList<String> defaultJvmFlags;
   private final ImmutableList<String> checkedConstraints;
@@ -373,12 +389,13 @@ public final class JavaConfiguration extends Fragment {
   }
 
   /**
-   * Returns true if Bazel should attempt to enforce one-version correctness on java_binary rules
-   * using the 'oneversion' tool in the java_toolchain. One-version correctness will inspect for
-   * multiple non-identical versions of java classes in the transitive dependencies for a
-   * java_binary.
+   * Returns an enum representing whether or not Bazel should attempt to enforce one-version
+   * correctness on java_binary rules using the 'oneversion' tool in the java_toolchain.
+   *
+   * One-version correctness will inspect for multiple non-identical versions of java classes in the
+   * transitive dependencies for a java_binary.
    */
-  public boolean isEnforceOneVersion() {
+  public OneVersionEnforcementLevel oneVersionEnforcementLevel() {
     return enforceOneVersion;
   }
 
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 1380128516..552dc9b3d6 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.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.build.lib.rules.java.JavaConfiguration.OneVersionEnforcementLevel;
 import com.google.devtools.common.options.EnumConverter;
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionsParser.OptionUsageRestrictions;
@@ -58,6 +59,14 @@ public class JavaOptions extends FragmentOptions {
     }
   }
 
+  /** Converter for the --java_optimization_mode option. */
+  public static class OneVersionEnforcementLevelConverter
+      extends EnumConverter<OneVersionEnforcementLevel> {
+    public OneVersionEnforcementLevelConverter() {
+      super(OneVersionEnforcementLevel.class, "Enforcement level for Java One Version violations");
+    }
+  }
+
   @Option(
     name = "javabase",
     defaultValue = "@bazel_tools//tools/jdk:jdk",
@@ -417,13 +426,15 @@ public class JavaOptions extends FragmentOptions {
 
   @Option(
     name = "experimental_one_version_enforcement",
-    defaultValue = "false",
+    defaultValue = "OFF",
+    converter = OneVersionEnforcementLevelConverter.class,
     optionUsageRestrictions = OptionUsageRestrictions.UNDOCUMENTED,
     help =
         "When enabled, enforce that a java_binary rule can't contain more than one version "
-            + "of the same class file on the classpath"
+            + "of the same class file on the classpath. This enforcement can break the build, or "
+            + "can just result in warnings."
   )
-  public boolean enforceOneVersion;
+  public OneVersionEnforcementLevel enforceOneVersion;
 
   @Override
   public FragmentOptions getHost(boolean fallback) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/OneVersionCheckActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/java/OneVersionCheckActionBuilder.java
index 92db2eb983..aab1f12d5c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/OneVersionCheckActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/OneVersionCheckActionBuilder.java
@@ -22,16 +22,21 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
+import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.Builder;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.CustomMultiArgv;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.rules.java.JavaConfiguration.OneVersionEnforcementLevel;
 
 /** Utility for generating a call to the one-version binary. */
 public class OneVersionCheckActionBuilder {
 
   public static void build(
-      RuleContext ruleContext, NestedSet<Artifact> jarsToCheck, Artifact oneVersionOutput) {
+      RuleContext ruleContext,
+      NestedSet<Artifact> jarsToCheck,
+      Artifact oneVersionOutput,
+      OneVersionEnforcementLevel enforcementLevel) {
     JavaToolchainProvider javaToolchain = JavaToolchainProvider.fromRuleContext(ruleContext);
     Artifact oneVersionTool = javaToolchain.getOneVersionBinary();
     Artifact oneVersionWhitelist = javaToolchain.getOneVersionWhitelist();
@@ -45,21 +50,26 @@ public class OneVersionCheckActionBuilder {
       return;
     }
 
-    ruleContext.registerAction(new SpawnAction.Builder()
-        .addOutput(oneVersionOutput)
-        .addInput(oneVersionWhitelist)
-        .addTransitiveInputs(jarsToCheck)
-        .setExecutable(oneVersionTool)
-        .setCommandLine(
-            CustomCommandLine.builder()
-                .addExecPath("--output", oneVersionOutput)
-                .addExecPath("--whitelist", oneVersionWhitelist)
-                .add(new OneVersionJarMapArgv(jarsToCheck))
-                .build())
-        .alwaysUseParameterFile(ParameterFileType.SHELL_QUOTED)
-        .setMnemonic("JavaOneVersion")
-        .setProgressMessage("Checking for one-version violations in " + ruleContext.getLabel())
-        .build(ruleContext));
+    Builder oneVersionArgsBuilder =
+        CustomCommandLine.builder()
+            .addExecPath("--output", oneVersionOutput)
+            .addExecPath("--whitelist", oneVersionWhitelist);
+    if (enforcementLevel == OneVersionEnforcementLevel.WARNING) {
+      oneVersionArgsBuilder.add("--succeed_on_found_violations");
+    }
+    oneVersionArgsBuilder.add(new OneVersionJarMapArgv(jarsToCheck));
+    CustomCommandLine oneVersionArgs = oneVersionArgsBuilder.build();
+    ruleContext.registerAction(
+        new SpawnAction.Builder()
+            .addOutput(oneVersionOutput)
+            .addInput(oneVersionWhitelist)
+            .addTransitiveInputs(jarsToCheck)
+            .setExecutable(oneVersionTool)
+            .setCommandLine(oneVersionArgs)
+            .alwaysUseParameterFile(ParameterFileType.SHELL_QUOTED)
+            .setMnemonic("JavaOneVersion")
+            .setProgressMessage("Checking for one-version violations in " + ruleContext.getLabel())
+            .build(ruleContext));
   }
 
   private static class OneVersionJarMapArgv extends CustomMultiArgv {