Skip to content

Commit

Permalink
Unconditionally run the SJD machinery, which is necessary for jdeps o…
Browse files Browse the repository at this point in the history
…utput

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
  • Loading branch information
cushon authored and Copybara-Service committed Jul 27, 2018
1 parent 1d96c7c commit 1ac3597
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -169,11 +158,6 @@ Dependencies buildDependenciesProto(ImmutableList<Path> classpath, boolean succe
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<Path> directJars() {
return directJars;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
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;
Expand Down Expand Up @@ -172,10 +171,14 @@ public void finish() {
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);
Expand Down Expand Up @@ -214,9 +217,6 @@ private static class CheckingTreeScanner extends TreeScanner {
/** Strict deps diagnostics. */
private final List<SjdDiagnostic> diagnostics;

/** The strict_java_deps mode */
private final StrictJavaDeps strictJavaDepsMode;

/** Missing targets */
private final Set<JarOwner> missingTargets;

Expand Down Expand Up @@ -245,7 +245,6 @@ public CheckingTreeScanner(
Set<JarOwner> missingTargets,
Set<Path> platformJars) {
this.directJars = dependencyModule.directJars();
this.strictJavaDepsMode = dependencyModule.getStrictJavaDeps();
this.diagnostics = diagnostics;
this.missingTargets = missingTargets;
this.directDependenciesMap = dependencyModule.getExplicitDependenciesMap();
Expand Down Expand Up @@ -275,7 +274,7 @@ private void checkTypeLiteral(JCTree node, Symbol sym) {
* 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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
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;
Expand Down Expand Up @@ -115,8 +114,7 @@ Result compile() throws IOException {
// To avoid having to apply the same exemptions here, we just ignore strict deps errors
// and leave enforcement to JavaBuilder.
ImmutableSet<Path> 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
Expand Down Expand Up @@ -261,15 +259,13 @@ static ImmutableList<String> processJavacopts(TurbineOptions turbineOptions) {

private static DependencyModule buildDependencyModule(
TurbineOptions turbineOptions,
StrictJavaDeps strictDepsMode,
ImmutableSet<Path> 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<Path> directJars = ImmutableSet.builder();
for (String path : turbineOptions.directJars()) {
directJars.add(Paths.get(path));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1207,6 +1207,37 @@ public void ignoreStrictDepsErrors() throws Exception {
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(
Expand Down

0 comments on commit 1ac3597

Please sign in to comment.