From 1ac359743176e659e9c7472645e3142f3c44b9e8 Mon Sep 17 00:00:00 2001 From: cushon Date: Thu, 26 Jul 2018 18:48:21 -0700 Subject: Unconditionally run the SJD machinery, which is necessary for jdeps output and just skip the diagnostics at the end if strict deps errors are disabled. This is necessary to javac-turbine, where we don't report strict deps errors (we rely on JavaBuilder for enforcement) but where we still want to emit jdeps. See also bazelbuild/rules_scala#559 PiperOrigin-RevId: 206257304 --- .../javac/plugins/dependency/DependencyModule.java | 22 +++------------ .../plugins/dependency/StrictJavaDepsPlugin.java | 19 +++++++------ .../build/java/turbine/javac/JavacTurbine.java | 8 ++---- .../build/java/turbine/javac/JavacTurbineTest.java | 31 ++++++++++++++++++++++ 4 files changed, 45 insertions(+), 35 deletions(-) (limited to 'src/java_tools') diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/DependencyModule.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/DependencyModule.java index f3e7cc7a56..6319df0c28 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/DependencyModule.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/DependencyModule.java @@ -57,22 +57,11 @@ public final class DependencyModule { public static enum StrictJavaDeps { /** Legacy behavior: Silently allow referencing transitive dependencies. */ - OFF(false), + OFF, /** Warn about transitive dependencies being used directly. */ - WARN(true), + WARN, /** Fail the build when transitive dependencies are used directly. */ - ERROR(true); - - private final boolean enabled; - - StrictJavaDeps(boolean enabled) { - this.enabled = enabled; - } - - /** Convenience method for just checking if it's not OFF */ - public boolean isEnabled() { - return enabled; - } + ERROR } private final StrictJavaDeps strictJavaDeps; @@ -169,11 +158,6 @@ public final class DependencyModule { return deps.build(); } - /** Returns whether strict dependency checks (strictJavaDeps) are enabled. */ - public boolean isStrictDepsEnabled() { - return strictJavaDeps.isEnabled(); - } - /** Returns the paths of direct dependencies. */ public ImmutableSet directJars() { return directJars; diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/StrictJavaDepsPlugin.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/StrictJavaDepsPlugin.java index 5fcb35d606..ca8d007065 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/StrictJavaDepsPlugin.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/StrictJavaDepsPlugin.java @@ -22,7 +22,6 @@ import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.buildjar.JarOwner; import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin; -import com.google.devtools.build.buildjar.javac.plugins.dependency.DependencyModule.StrictJavaDeps; import com.google.devtools.build.lib.view.proto.Deps; import com.google.devtools.build.lib.view.proto.Deps.Dependency; import com.sun.tools.javac.code.Flags; @@ -172,10 +171,14 @@ public final class StrictJavaDepsPlugin extends BlazeJavaCompilerPlugin { for (SjdDiagnostic diagnostic : diagnostics) { JavaFileObject prev = log.useSource(diagnostic.source()); try { - if (dependencyModule.getStrictJavaDeps() == ERROR) { - log.error(diagnostic.pos(), "proc.messager", diagnostic.message()); - } else { - log.warning(diagnostic.pos(), "proc.messager", diagnostic.message()); + switch (dependencyModule.getStrictJavaDeps()) { + case ERROR: + log.error(diagnostic.pos(), "proc.messager", diagnostic.message()); + break; + case WARN: + log.warning(diagnostic.pos(), "proc.messager", diagnostic.message()); + break; + case OFF: // continue below } } finally { log.useSource(prev); @@ -214,9 +217,6 @@ public final class StrictJavaDepsPlugin extends BlazeJavaCompilerPlugin { /** Strict deps diagnostics. */ private final List diagnostics; - /** The strict_java_deps mode */ - private final StrictJavaDeps strictJavaDepsMode; - /** Missing targets */ private final Set missingTargets; @@ -245,7 +245,6 @@ public final class StrictJavaDepsPlugin extends BlazeJavaCompilerPlugin { Set missingTargets, Set platformJars) { this.directJars = dependencyModule.directJars(); - this.strictJavaDepsMode = dependencyModule.getStrictJavaDeps(); this.diagnostics = diagnostics; this.missingTargets = missingTargets; this.directDependenciesMap = dependencyModule.getExplicitDependenciesMap(); @@ -275,7 +274,7 @@ public final class StrictJavaDepsPlugin extends BlazeJavaCompilerPlugin { * strict_java_deps is enabled, it emits a [strict] compiler warning/error. */ private void collectExplicitDependency(Path jarPath, JCTree node, Symbol sym) { - if (strictJavaDepsMode.isEnabled() && !isStrictDepsExempt) { + if (!isStrictDepsExempt) { // Does it make sense to emit a warning/error for this pair of (type, owner)? // We want to emit only one error/warning per owner. if (!directJars.contains(jarPath) && seenStrictDepsViolatingJars.add(jarPath)) { diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java b/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java index 1cc9391ca3..dd61582bd2 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java @@ -23,7 +23,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.buildjar.javac.JavacOptions; import com.google.devtools.build.buildjar.javac.plugins.dependency.DependencyModule; -import com.google.devtools.build.buildjar.javac.plugins.dependency.DependencyModule.StrictJavaDeps; import com.google.devtools.build.buildjar.javac.plugins.dependency.StrictJavaDepsPlugin; import com.google.turbine.binder.ClassPathBinder; import com.google.turbine.options.TurbineOptions; @@ -115,8 +114,7 @@ public class JavacTurbine implements AutoCloseable { // To avoid having to apply the same exemptions here, we just ignore strict deps errors // and leave enforcement to JavaBuilder. ImmutableSet platformJars = ImmutableSet.copyOf(asPaths(turbineOptions.bootClassPath())); - DependencyModule dependencyModule = - buildDependencyModule(turbineOptions, StrictJavaDeps.WARN, platformJars); + DependencyModule dependencyModule = buildDependencyModule(turbineOptions, platformJars); if (sources.isEmpty()) { // accept compilations with an empty source list for compatibility with JavaBuilder @@ -261,15 +259,13 @@ public class JavacTurbine implements AutoCloseable { private static DependencyModule buildDependencyModule( TurbineOptions turbineOptions, - StrictJavaDeps strictDepsMode, ImmutableSet platformJars) { DependencyModule.Builder dependencyModuleBuilder = new DependencyModule.Builder() .setReduceClasspath() .setTargetLabel(turbineOptions.targetLabel().orNull()) .addDepsArtifacts(asPaths(turbineOptions.depsArtifacts())) - .setPlatformJars(platformJars) - .setStrictJavaDeps(strictDepsMode.toString()); + .setPlatformJars(platformJars); ImmutableSet.Builder directJars = ImmutableSet.builder(); for (String path : turbineOptions.directJars()) { directJars.add(Paths.get(path)); diff --git a/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java b/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java index b7b8250f17..c598edf538 100644 --- a/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java +++ b/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java @@ -1207,6 +1207,37 @@ public class JavacTurbineTest extends AbstractJavacTurbineCompilationTest { assertThat(result).isNotEqualTo(Result.OK_WITH_REDUCED_CLASSPATH); } + @Test + public void ignoreStrictDepsErrorsForFailingCompilations() throws Exception { + + Path lib = + createClassJar( + "deps.jar", + AbstractJavacTurbineCompilationTest.class, + JavacTurbineTest.class, + Lib.class); + + addSourceLines( + "Hello.java", + "import " + Lib.class.getCanonicalName() + ";", + "import no.such.Class;", + "class Hello extends Lib {", + "}"); + + optionsBuilder.addClassPathEntries(ImmutableList.of(lib.toString())); + + optionsBuilder.addSources(ImmutableList.copyOf(Iterables.transform(sources, TO_STRING))); + + StringWriter errOutput = new StringWriter(); + Result result; + try (JavacTurbine turbine = + new JavacTurbine(new PrintWriter(errOutput, true), optionsBuilder.build())) { + result = turbine.compile(); + } + assertThat(errOutput.toString()).doesNotContain("[strict]"); + assertThat(result).isEqualTo(Result.ERROR); + } + @Test public void clinit() throws Exception { addSourceLines( -- cgit v1.2.3