Bump checkerframework to 2.2.2 and stop building from source #4795

davido · 2018-03-08T06:57:02Z

Fixes: #4709.

This change stops building checker framework from the source and consumes
the artifacts from Central. As the consequence, the sources are removed
from the Bazel tree and we discontinue packaging the sources in Bazel
source artifact.

GPL v2 with Classpath exception sources have to be still included in
third_party:gpl-srcs rule. The source jars are included in this rule,
that is used in embeded_tools archive:

$ unzip -t bazel-genfiles/src/embedded_tools.zip|grep checker_framework
testing: third_party/checker_framework_dataflow/dataflow-2.2.2-sources.jar
testing: third_party/checker_framework_javacutil/javacutil-2.2.2-sources.jar

philwo · 2018-03-09T10:43:54Z

Thanks a lot for figuring this out! If @cushon approves, I'll handle the merge.

philwo · 2018-03-09T11:27:05Z

@davido Maybe I'm missing something, but where does the dataflow-2.2.2.jar come from? Doesn't seem to be part of this PR?

davido · 2018-03-09T11:51:32Z

@philwo Oh, my bad, sorry, probably I missed to add it to the commit? I will re-add it later today and push a new version of the PR. (The source is just Maven Central, the location that was referenced by @cushon).

Also I'm not sure you really want to merge the 2 Java 9 related commits in this series. They are more or less a RFC.

philwo · 2018-03-09T12:34:28Z

src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/BUILD

@@ -11,6 +11,10 @@ filegroup(
 java_library(
    name = "autocodec",
    exported_plugins = [":autocodec-plugin"],
+    javacopts = select({


@janakdr What's your thought on building autocodec with --add-modules=jdk.unsupported for Java 9? Do you think there's a better way?

I think the Java 9 changes should be made separately from the "bump checkerframework to 2.2.2 and stop building from source" change. The latter is big enough for one commit, and the details former may need more discussion.

Done. Note that I was adding it here only to be able to build Java 9. Coming from Gerrit, what is the right way in GitHub to add dependent pull requests?

cushon · 2018-03-09T16:02:05Z

third_party/BUILD

@@ -46,8 +44,6 @@ filegroup(
 filegroup(


@philwo @janakdr this change probably isn't OK? We still want to include the sources.

@cushon Can you elaborate? Is that because of LGPL license?
The sources must be in tree? Can't we merge the from Source-JAR downloaded from Central?

I don't see anything now. Are these changes reverted?

Yes, I uploaded a new version of the PR. I restored the GPL source packaging. But now the jars are included in embedded_tools:

$ bazel build src:embedded_tools INFO: Analysed target //src:embedded_tools (1 packages loaded). INFO: Found 1 target... Target //src:embedded_tools up-to-date: bazel-genfiles/src/embedded_tools.zip INFO: Elapsed time: 0.279s, Critical Path: 0.00s $ unzip -t bazel-genfiles/src/embedded_tools.zip | grep checker_framework testing: third_party/checker_framework_dataflow/dataflow-2.2.2-sources.jar OK testing: third_party/checker_framework_javacutil/javacutil-2.2.2-sources.jar OK

cushon · 2018-03-09T16:04:03Z

src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/BUILD

@@ -11,6 +11,10 @@ filegroup(
 java_library(
    name = "autocodec",
    exported_plugins = [":autocodec-plugin"],
+    javacopts = select({


I think the Java 9 changes should be made separately from the "bump checkerframework to 2.2.2 and stop building from source" change. The latter is big enough for one commit, and the details former may need more discussion.

cushon · 2018-03-09T16:04:24Z

third_party/checker_framework_dataflow/BUILD

-    javacopts = ["-Xep:MissingCasesInEnumSwitch:OFF"],
+    jars = [
+        "dataflow-2.2.2.jar",
+    ],


Please add the source jars too.

cushon · 2018-03-09T16:04:59Z

third_party/grpc/compiler/src/java_plugin/cpp/java_generator.cpp

-      "    value = \"by gRPC proto compiler$grpc_version$\",\n"
-      "    comments = \"Source: $file_name$\")\n"
+      "// Generated by gRPC proto compiler$grpc_version$\n"
+      "// Source: $file_name$\n"


This doesn't belong in this PR, it should be a separate change.

cushon · 2018-03-13T16:37:28Z

third_party/checker_framework_dataflow/BUILD

@@ -4,23 +4,16 @@ licenses(["restricted"])  # GNU GPL v2 with Classpath exception

 filegroup(
    name = "srcs",
-    srcs = glob(["**"]),
+    srcs = ["dataflow-2.2.2-sources.jar"],


These sources get collected into a tar archive by //:bazel-srcs. Currently the tar contains the .java sources, this changes causes the tar to contain the source jar. I think we want the preserve the current behaviour.

Currently the tar contains the .java sources, this changes causes the tar to contain the source jar.

This PR is a principle change in handling of checker framework. We stop building from the source and start to consume the pre-built artifacts (downloaded from Maven Central).

If we do not build from the source, and do not have sources in our tree, why should we supply sources in :bazel-srcs at all? We don't include sources neither for guava, nor for error prone and nor for custom java compiler. How is checker framework is different from those and other dozens of Bazel dependencies without supplied sources in :bazel-srcs artifact?

In fact, before this change, the //:bazel-srcs tar already contains some source JARs:

$ davido@wizball:~/projects/bazel (master %=)$ tar -tf bazel-bin/bazel-srcs.tar | grep --regexp\="third_party/.*jar$" | grep source ./third_party/asm/asm-6.0-sources.jar ./third_party/asm/asm-analysis-6.0-sources.jar ./third_party/asm/asm-commons-6.0-sources.jar ./third_party/asm/asm-tree-6.0-sources.jar ./third_party/asm/asm-util-6.0-sources.jar

After this change we have two more:

[...] ./third_party/checker_framework_dataflow/dataflow-2.2.2-sources.jar ./third_party/checker_framework_javacutil/javacutil-2.2.2-sources.jar

So, if you would argue, that we should stop include source JARs for checker framework in //:bazel-srcs artifact I think would agree with this point. Should we rather remove checker framework source JARs from //:bazel-srcs entirely?

A different point is third-party:gpl-srcs. There we also include source JARs for the checker-framework that is part of embeded zip. But there we also have other JARs.

Moreover, I also think, that a follow-up change could be to stop building jFormatString from the source (another LGPL licensed library) and consume pre-built binary from the Central:

<dependency> <groupId>com.google.code.findbugs</groupId> <artifactId>jFormatString</artifactId> <version>3.0.0</version> </dependency>

Should we rather remove checker framework source JARs from //:bazel-srcs entirely?

I discontinued packaging checker framework source jars in //:bazel-srcs in the updated PR version, and extended the commit message:

This change stops building checker framework from the source and consumes
the artifacts from Central. As the consequence, the sources are removed
from the Bazel tree and we discontinue packaging the sources in Bazel
source artifact.

GPL v2 with Classpath exception sources have to be still included in
third_party:gpl-srcs rule. The source jars are included in this rule,
that is used in embeded_tools archive:

$ unzip -t bazel-genfiles/src/embedded_tools.zip|grep checker_framework testing: third_party/checker_framework_dataflow/dataflow-2.2.2-sources.jar testing: third_party/checker_framework_javacutil/javacutil-2.2.2-sources.jar

Moreover, I also think, that a follow-up change could be to stop building jFormatString from the source (another LGPL licensed library) and consume pre-built binary from the Central

Done in #4843

Fixes: bazelbuild#4709. This change stops building checker framework from the source and consumes the artifacts from Central. As the consequence, the sources are removed from the Bazel tree and we discontinue packaging the sources in Bazel source artifact. GPL v2 with Classpath exception sources have to be still included in third_party:gpl-srcs rule. The source jars are included in this rule, that is used in embeded_tools archive: $ unzip -t bazel-genfiles/src/embedded_tools.zip|grep checker_framework testing: third_party/checker_framework_dataflow/dataflow-2.2.2-sources.jar testing: third_party/checker_framework_javacutil/javacutil-2.2.2-sources.jar

lfpino · 2018-03-27T13:44:23Z

Hi @davido, Bazel sheriff here, what's the status of this pull request? It hasn't been updated for more than a week so it'd be good to understand if it's still ongoing or safe to close. Thanks.

googlebot added the cla: yes label Mar 8, 2018

davido force-pushed the fix-checkerframework-fork-mess branch 2 times, most recently from d8ddc24 to 5d62f6a Compare March 9, 2018 05:21

davido mentioned this pull request Mar 9, 2018

Java 9: Missing strict dependency is erroneously reported #4812

Closed

laszlocsomor requested review from cushon and philwo March 9, 2018 10:18

laszlocsomor assigned philwo Mar 9, 2018

philwo reviewed Mar 9, 2018

View reviewed changes

cushon requested changes Mar 9, 2018

View reviewed changes

davido force-pushed the fix-checkerframework-fork-mess branch 2 times, most recently from 85b552a to 5cb1731 Compare March 9, 2018 22:55

cushon requested changes Mar 13, 2018

View reviewed changes

davido force-pushed the fix-checkerframework-fork-mess branch 2 times, most recently from 117bc0c to 4c96cfb Compare March 14, 2018 19:37

davido force-pushed the fix-checkerframework-fork-mess branch from 4c96cfb to 5a405cc Compare March 16, 2018 20:31

cushon mentioned this pull request Mar 27, 2018

Fix bazel build with java 9 #4820

Closed

davido closed this Mar 27, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump checkerframework to 2.2.2 and stop building from source #4795

Bump checkerframework to 2.2.2 and stop building from source #4795

davido commented Mar 8, 2018 •

edited

Loading

philwo commented Mar 9, 2018

philwo commented Mar 9, 2018

davido commented Mar 9, 2018 •

edited

Loading

philwo Mar 9, 2018

cushon Mar 9, 2018

davido Mar 9, 2018 •

edited

Loading

cushon Mar 9, 2018

davido Mar 9, 2018 •

edited

Loading

janakdr Mar 9, 2018

davido Mar 9, 2018 •

edited

Loading

cushon Mar 9, 2018

cushon Mar 9, 2018

davido Mar 9, 2018

cushon Mar 9, 2018

davido Mar 9, 2018

cushon Mar 13, 2018

davido Mar 13, 2018 •

edited

Loading

davido Mar 14, 2018 •

edited

Loading

davido Mar 14, 2018 •

edited

Loading

lfpino commented Mar 27, 2018

Bump checkerframework to 2.2.2 and stop building from source #4795

Bump checkerframework to 2.2.2 and stop building from source #4795

Conversation

davido commented Mar 8, 2018 • edited Loading

philwo commented Mar 9, 2018

philwo commented Mar 9, 2018

davido commented Mar 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davido Mar 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davido Mar 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davido Mar 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davido Mar 13, 2018 • edited Loading

Choose a reason for hiding this comment

davido Mar 14, 2018 • edited Loading

Choose a reason for hiding this comment

davido Mar 14, 2018 • edited Loading

Choose a reason for hiding this comment

lfpino commented Mar 27, 2018

davido commented Mar 8, 2018 •

edited

Loading

davido commented Mar 9, 2018 •

edited

Loading

davido Mar 9, 2018 •

edited

Loading

davido Mar 9, 2018 •

edited

Loading

davido Mar 9, 2018 •

edited

Loading

davido Mar 13, 2018 •

edited

Loading

davido Mar 14, 2018 •

edited

Loading

davido Mar 14, 2018 •

edited

Loading