rules_jvm_external icon indicating copy to clipboard operation
rules_jvm_external copied to clipboard

Buildozer suggestions broken in rules_jvm_external?

Open dhalperi opened this issue 4 years ago • 7 comments

Bazel 1.1.0 started warning about sha128 uses, which has pushed me to switch to rules_jvm_external from rules_maven.

Am I doing something wrong, or is Bazel now unable to give useful buildozer suggestions?

For example, in this case A and B both require Guava.

java_library(
    name = "A",
    srcs = ["A.java"],
    deps = ["@maven//:com_google_guava_guava"],
)

java_library(
    name = "B",
    srcs = ["B.java"],
    deps = [
        ":A",
        "@maven//:com_google_guava_guava",
    ],
)

java_library(
    name = "B_no_dep",
    srcs = ["B.java"],
    deps = [
        ":A",
    ],
)

The error I get is not especially helpful at identifying the missing target that needs to be added to B_no_dep:

INFO: Writing tracer profile to '/private/var/tmp/_bazel_dan/55aaa51ea5620bd26651800f3bc9bddd/command.profile.gz'
INFO: Invocation ID: 57a6ccc4-bb94-4891-b8d4-6bdb1badfabd
INFO: Analyzed 3 targets (0 packages loaded, 0 targets configured).
INFO: Found 3 targets...
ERROR: /Users/dan/git-workspace/rules_jvm_demo/rules_jvm/BUILD:16:1: Building libB_no_dep.jar (1 source file) failed (Exit 1)
B.java:6: error: [strict] Using type com.google.common.collect.Lists from an indirect dependency (TOOL_INFO: "external/maven/v1/https/repo1.maven.org/maven2/com/google/guava/guava/26.0-jre/guava-26.0-jre.jar").
        System.err.println(Lists.reverse(A.INTEGERS));
                           ^
INFO: Elapsed time: 0.438s, Critical Path: 0.26s
INFO: 0 processes.
FAILED: Build did NOT complete successfully

With rules_maven, I got a buildozer command that worked:

INFO: Analyzed 3 targets (1 packages loaded, 5 targets configured).
INFO: Found 3 targets...
ERROR: /Users/dan/git-workspace/rules_jvm_demo/rules_maven/BUILD:16:1: Building libB_no_dep.jar (1 source file) failed (Exit 1)
B.java:6: error: [strict] Using type com.google.common.collect.Lists from an indirect dependency (TOOL_INFO: "@com_google_guava_guava//jar"). See command below **
        System.err.println(Lists.reverse(A.INTEGERS));
                           ^
 ** Please add the following dependencies: 
  @com_google_guava_guava//jar to //:B_no_dep 
 ** You can use the following buildozer command: 
buildozer 'add deps @com_google_guava_guava//jar' //:B_no_dep 

INFO: Elapsed time: 1.469s, Critical Path: 1.19s
INFO: 3 processes: 1 darwin-sandbox, 2 worker.
FAILED: Build did NOT complete successfully

Am I doing something wrong in setting this up so that Bazel is not able to give me useful suggestions? Code: https://github.com/dhalperi/rules_jvm_demo

dhalperi avatar Nov 03 '19 07:11 dhalperi

Looks like this is probably highly related to https://github.com/bazelbuild/bazel/issues/4584

dhalperi avatar Nov 04 '19 17:11 dhalperi

Thanks for the helpful repro. This is definitely related to our custom jvm_import.bzl and https://github.com/bazelbuild/bazel/issues/4584.

I dug into StrictJavaDepsPlugin.java for Bazel and found this snippet: https://github.com/bazelbuild/bazel/blob/3c341016a15d34f8e7b8f4527e679120c5d15c07/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/StrictJavaDepsPlugin.java#L326-L343

It looks like we'll need to inject the Target-Label attribute into every jar's manifest for the buildozer recommendation to show up.

jin avatar Nov 05 '19 05:11 jin

Will be fixed by https://github.com/bazelbuild/rules_jvm_external/pull/285

jin avatar Nov 05 '19 05:11 jin

😮 🎊 Thank you!

I tried it out with the following (irreproducible) patch:

http_archive(
     name = "rules_jvm_external",
-    sha256 = RULES_JVM_EXTERNAL_SHA,
-    strip_prefix = "rules_jvm_external-%s" % RULES_JVM_EXTERNAL_TAG,
-    url = "https://github.com/bazelbuild/rules_jvm_external/archive/%s.zip" % RULES_JVM_EXTERNAL_TAG,
+    strip_prefix = "rules_jvm_external-stamp_manifest",
+    url = "https://github.com/jin/rules_jvm_external/archive/stamp_manifest.zip",
 )

and it worked great. Looking forward to it and let me know if you could use more testing.

dhalperi avatar Nov 05 '19 06:11 dhalperi

I've received reports that the current implementation with the jar manifest stamping is not deterministic, and causes reproducibility issues.

See https://github.com/bazelbuild/rules_jvm_external/pull/376

https://github.com/bazelbuild/rules_jvm_external/pull/355 adds a --@rules_jvm_external//settings:stamp_manifest flag to trigger this behavior, which I'll default to false for now because of the reproducibility issue.

jin avatar Feb 16 '20 22:02 jin

@JaredNeil kindly pointed out that the Bazel source tree has a tool to reproducibly produce modified JARs: https://github.com/bazelbuild/bazel/blob/master/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/jarhelper/JarHelper.java

jin avatar Apr 27 '20 02:04 jin

Any update on this? Would be great to be able to set this setting globally and not have to rely on a command line flag or .bazelrc option. According to the 4.0 release notes, it looks like the timestamp issue may be fixed?

virusdave avatar Feb 11 '21 23:02 virusdave