Crude output of jdeps from vanilla java builder for intellij plugin #6587

wcurrie · 2018-11-03T20:46:48Z

The intellij plugin relies on jdeps to build the list of External Libraries. bazelbuild/intellij#207 (comment)

The vanilla java builder writes empty jdeps files. As a result, when using the vanilla builder, the list of External Libraries that show up after any given "Sync project with BUILD files" run depends on the working set (git status).

one exception to this: for targets in the 'working set' (list of modified files), we add libraries for all dependencies, not just those required at compile time (as determined by the jdeps data)

Is there a reason not to output dependencies to jdeps files from the vanilla builder?

The attempt in this PR is likely broken in at least one way.

wcurrie · 2018-11-08T19:31:52Z

Why is this interesting? This thread mentions the plan to support compiling to target --release 10 involves the vanilla builder

We make it so that if a JDK is used that's incompatible with all the smartness in the regular JavaBuilder, we revert to the vanilla one at the cost of e.g. losing strict dependency checks and Error Prone

The intellij plugin depends on external libraries being listed in the .jdeps files output by the code that drives the "strict dependency checks"

bazel/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/StrictJavaDepsPlugin.java

Line 67 in bc9b35b

public final class StrictJavaDepsPlugin extends BlazeJavaCompilerPlugin {

Alternatives:

adding whitespace to every BUILD file in a workspace so the working set in the IJ plugin detects includes every target
change the IJ plugin to not depend on .jdeps files

cushon · 2018-11-08T19:37:35Z

src/java_tools/buildjar/java/com/google/devtools/build/buildjar/VanillaJavaBuilder.java

+    ImmutableList.Builder<Deps.Dependency> builder = ImmutableList.builder();
+    for (File file : fileManager.getLocation(StandardLocation.CLASS_PATH)) {
+      String path = file.toString();
+      builder.add(Deps.Dependency.newBuilder().setKind(Deps.Dependency.Kind.EXPLICIT).setPath(path).build());


This is an over-approximation of the deps that were needed, and defeats the point of the optimization IntelliJ is doing. It might make more sense to just provide a way to disable the Intellij plugins' jdeps handling and use the full classpath.

A better solution for VanillaJavaBuilder might be to have it use the bytecode-based
strict deps / jdeps tool:

bazel/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/Main.java

Line 48 in 423fe65

public class Main {

cushon · 2018-11-08T19:39:29Z

@lberki @iirina

This thread mentions to the plan support compiling to target --release 10 involves the vanilla builder

The thinking has changed a bit since that thread. There's a more recent but very long thread here [1], and the new plan involves #6316.

VanillaJavaBuilder is not currently a supported toolchain:

bazel/tools/jdk/BUILD

Line 210 in 423fe65

# The 'vanilla' toolchain is an unsupported alternative to the default.

.

[1] https://groups.google.com/d/topic/bazel-sig-jvm/wA9f-B6Tl5w/discussion

wcurrie · 2018-11-08T20:59:33Z

Thanks for the update. Really just fishing for knowledge 😄. We've currently got an internal build of the intellij plugin. Maybe we'll try to upstream some changes there. Using the full classpath in the plugin handles #5723 (comment) too.

gergelyfabian · 2020-05-08T12:37:27Z

Getting back to this topic... @wcurrie are you still maintaining this forked plugin?
What is the current solution for this problem?

uri-canva · 2020-05-08T17:23:47Z

We currently don't use the vanilla compiler, so we don't need the patch, but if we're going to use java 14 before bazel supports it we might go back to it.

gergelyfabian · 2020-05-08T18:39:15Z

I managed to reproduce this problem with IntelliJ 2019.3.4, Bazel 3.0.0 and the latest IntelliJ plugin (2020.04.13). I don't have a minimal reproduction though.

@uri-canva what would you suggest what is the most optimal way to work it around currently? I managed to take canva's patch (to always scan all external dependencies: Canva/bazelbuild-intellij#1) and apply it on top of the latest plugin version and that solves the problem.

Is there any better fix? Am I using the vanilla compiler if I have this issue?

uri-canva · 2020-05-08T23:52:56Z

Yes, this issue only affects the vanilla compiler. If you have this issue with the default bazel compiler it's a different issue.

gergelyfabian · 2020-05-11T07:52:54Z

I believe this is a general issue with Bazel 3.0.0. I'm not using the vanilla compiler, but the default remote jdk.
As the release notes say (https://blog.bazel.build/2020/03/31/bazel-3.0.html):
"Using JDK 9 or 10 as a --host_javabase is no longer officially supported. As always, you can use the @bazel_tools//tools/jdk:toolchain_vanilla Java toolchain to use older or newer JDKs than what Bazel currently supports."

Indeed, also setting Java 8 as a host_javabase fails. I could try setting it with a vanilla java toolchain, but that is not a solution as vanilla toolchain does not support strict deps that would be necessary to provide .jdeps output for the IntelliJ plugin. So I'd believe that this is now a general bug with latest Bazel versions.

The only option I see for a workaround is to use a forked plugin.

Btw, who should be fixing this bug? Bazel or the IntelliJ plugin?

There is a bug in Bazel, that it does not produce proper jdeps files for the IntelliJ plugin, thus breaking the existing API. The only workaround seems to be to force the plugin to treat all files to be part of the working set, and load external dependencies for them. More info at: bazelbuild#490 bazelbuild/bazel#5723 bazelbuild/bazel#6587

uri-canva · 2020-05-11T17:20:27Z

Not sure, but if you're not using the vanilla toolchain then it's not the same problem this PR is talking about, but a different issue with the same symptom. You should open a new issue for it.

gergelyfabian · 2020-05-12T14:20:44Z

Not sure, but if you're not using the vanilla toolchain then it's not the same problem this PR is talking about, but a different issue with the same symptom. You should open a new issue for it.

Thanks for the hint. Indeed you are right. This is a different issue with the same symptom.

The issue is that there is a bug in the cooperation between intellij plugin and rules_scala.
If I use the pattern documented by rules_scala to define a scala target using Scalafmt (https://github.com/bazelbuild/rules_scala/blob/master/docs/phase_scalafmt.md), then intellij plugin won't detect it as a Scala target, but rather a Java one (because intellij plugin has hard-coded the targets it expects from rules_scala).

The Scala targets generated by rules_scala do not expose a jdeps interface, hence intellij BlazeJavaWorkspaceImporter cannot find the dependencies for them (assuming they are not in the working set).

At the same time I checked, that both proper Java targets get proper dependencies with BlazeJavaWorkspaceImporter and (detected) Scala targets get proper dependencies with intellij BlazeScalaSyncPlugin (Java and Scala use separate mechanisms for import, but both work on their own).

This means this is not a bug on Bazel but on the intellij plugin, that'll report.

gergelyfabian · 2020-05-13T10:05:09Z

I created a new issue on the intelliJ plugin for my case of missing external dependencies bazelbuild/intellij#1824

Crude output of jdeps for intellij plugin

45a2549

wcurrie requested a review from a team as a code owner November 3, 2018 20:46

googlebot added the cla: yes label Nov 3, 2018

jin added the team-Rules-Java Issues for Java rules label Nov 4, 2018

cushon reviewed Nov 8, 2018

View reviewed changes

wcurrie closed this Nov 8, 2018

wcurrie deleted the will/vanilla-build-jdeps branch November 8, 2018 20:59

This was referenced Nov 15, 2018

Import all external libraries on every sync Canva/bazelbuild-intellij#1

Closed

Option to sync ignoring .jdeps files and instead add all external libraries to IJ bazelbuild/intellij#447

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Crude output of jdeps from vanilla java builder for intellij plugin #6587

Crude output of jdeps from vanilla java builder for intellij plugin #6587

wcurrie commented Nov 3, 2018

wcurrie commented Nov 8, 2018 •

edited

Loading

cushon Nov 8, 2018 •

edited

Loading

cushon commented Nov 8, 2018

wcurrie commented Nov 8, 2018

gergelyfabian commented May 8, 2020

uri-canva commented May 8, 2020

gergelyfabian commented May 8, 2020

uri-canva commented May 8, 2020

gergelyfabian commented May 11, 2020

uri-canva commented May 11, 2020

gergelyfabian commented May 12, 2020

gergelyfabian commented May 13, 2020

Crude output of jdeps from vanilla java builder for intellij plugin #6587

Crude output of jdeps from vanilla java builder for intellij plugin #6587

Conversation

wcurrie commented Nov 3, 2018

wcurrie commented Nov 8, 2018 • edited Loading

cushon Nov 8, 2018 • edited Loading

Choose a reason for hiding this comment

cushon commented Nov 8, 2018

wcurrie commented Nov 8, 2018

gergelyfabian commented May 8, 2020

uri-canva commented May 8, 2020

gergelyfabian commented May 8, 2020

uri-canva commented May 8, 2020

gergelyfabian commented May 11, 2020

uri-canva commented May 11, 2020

gergelyfabian commented May 12, 2020

gergelyfabian commented May 13, 2020

wcurrie commented Nov 8, 2018 •

edited

Loading

cushon Nov 8, 2018 •

edited

Loading