google / closure-compiler

A JavaScript checker and optimizer.
https://developers.google.com/closure/compiler/
Apache License 2.0
7.36k stars 1.14k forks source link

Versions after and including v20220803 include module-info.class for org.jspecify #3992

Open locke-chappel opened 2 years ago

locke-chappel commented 2 years ago

Starting in v20220803 the release jar now contains an erroneous module-info.class file. Pre-v20220803 releases contain no such file and allow Java to use automatic modules. Starting in v20220803 a module-info.class file in included, seemingly from org.jspecify causing java to recognize the jar as containing only the org.jspecify.nullness package rendering closure-compile unusable in module enabled projects as the downstream project is now unable to specify the appropriate requires closure.compiler.v20220803; in it's module-info.java file.

Affected Versions

v20220803 and later

Module Description for v20220803 and later

jar --file=closure-compiler-v20220803.jar --describe-module

org.jspecify@0.2.0 jar:file:/closure-compiler-v20220803.jar!/module-info.class
exports org.jspecify.nullness
requires java.base mandated

Module Description for v20220719 and prior

jar --file=closure-compiler-v20220719.jar --describe-module

No module descriptor found. Derived automatic module.

closure.compiler.v20220719 automatic
requires java.base mandated
contains com.google.debugging.sourcemap
contains com.google.debugging.sourcemap.proto
contains com.google.javascript.jscomp
contains com.google.javascript.jscomp.ant
contains com.google.javascript.jscomp.base
contains com.google.javascript.jscomp.bundle
contains com.google.javascript.jscomp.colors
contains com.google.javascript.jscomp.deps
contains com.google.javascript.jscomp.diagnostic
contains com.google.javascript.jscomp.disambiguate
contains com.google.javascript.jscomp.graph
contains com.google.javascript.jscomp.ijs
contains com.google.javascript.jscomp.instrumentation
contains com.google.javascript.jscomp.instrumentation.reporter
contains com.google.javascript.jscomp.instrumentation.reporter.proto
contains com.google.javascript.jscomp.jarjar.com.google.auto.value
contains com.google.javascript.jscomp.jarjar.com.google.auto.value.extension.memoized
contains com.google.javascript.jscomp.jarjar.com.google.common.annotations
contains com.google.javascript.jscomp.jarjar.com.google.common.base
contains com.google.javascript.jscomp.jarjar.com.google.common.base.internal
contains com.google.javascript.jscomp.jarjar.com.google.common.cache
contains com.google.javascript.jscomp.jarjar.com.google.common.collect
contains com.google.javascript.jscomp.jarjar.com.google.common.escape
contains com.google.javascript.jscomp.jarjar.com.google.common.eventbus
contains com.google.javascript.jscomp.jarjar.com.google.common.graph
contains com.google.javascript.jscomp.jarjar.com.google.common.hash
contains com.google.javascript.jscomp.jarjar.com.google.common.html
contains com.google.javascript.jscomp.jarjar.com.google.common.io
contains com.google.javascript.jscomp.jarjar.com.google.common.math
contains com.google.javascript.jscomp.jarjar.com.google.common.net
contains com.google.javascript.jscomp.jarjar.com.google.common.primitives
contains com.google.javascript.jscomp.jarjar.com.google.common.reflect
contains com.google.javascript.jscomp.jarjar.com.google.common.util.concurrent
contains com.google.javascript.jscomp.jarjar.com.google.common.util.concurrent.internal
contains com.google.javascript.jscomp.jarjar.com.google.common.xml
contains com.google.javascript.jscomp.jarjar.com.google.errorprone.annotations
contains com.google.javascript.jscomp.jarjar.com.google.errorprone.annotations.concurrent
contains com.google.javascript.jscomp.jarjar.com.google.gson
contains com.google.javascript.jscomp.jarjar.com.google.gson.annotations
contains com.google.javascript.jscomp.jarjar.com.google.gson.internal
contains com.google.javascript.jscomp.jarjar.com.google.gson.internal.bind
contains com.google.javascript.jscomp.jarjar.com.google.gson.internal.bind.util
contains com.google.javascript.jscomp.jarjar.com.google.gson.reflect
contains com.google.javascript.jscomp.jarjar.com.google.gson.stream
contains com.google.javascript.jscomp.jarjar.com.google.protobuf
contains com.google.javascript.jscomp.jarjar.com.google.protobuf.compiler
contains com.google.javascript.jscomp.jarjar.com.google.re2j
contains com.google.javascript.jscomp.jarjar.com.google.thirdparty.publicsuffix
contains com.google.javascript.jscomp.jarjar.javax.annotation
contains com.google.javascript.jscomp.jarjar.javax.annotation.concurrent
contains com.google.javascript.jscomp.jarjar.javax.annotation.meta
contains com.google.javascript.jscomp.jarjar.javax.annotation.security
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.attribute
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.dispatch
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.filters
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.filters.util
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.helper
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.input
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.listener
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.loader
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.property
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.compilers
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.condition
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.cvslib
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.email
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.launcher
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.modules
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.ccm
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.clearcase
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.depend
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.depend.constantpool
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.ejb
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.extension
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.extension.resolvers
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.i18n
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.j2ee
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.javacc
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.javah
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.jlink
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.jsp
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.jsp.compilers
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.native2ascii
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.net
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.pvcs
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.script
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.sos
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.testing
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.unix
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.vss
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.optional.windows
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.taskdefs.rmic
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.types
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.types.mappers
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.types.optional
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.types.optional.depend
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.types.resources
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.types.resources.comparators
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.types.resources.selectors
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.types.selectors
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.types.selectors.modifiedselector
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.types.spi
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.util
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.util.depend
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.util.facade
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.util.java15
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.util.optional
contains com.google.javascript.jscomp.jarjar.org.apache.tools.ant.util.regexp
contains com.google.javascript.jscomp.jarjar.org.apache.tools.bzip2
contains com.google.javascript.jscomp.jarjar.org.apache.tools.mail
contains com.google.javascript.jscomp.jarjar.org.apache.tools.tar
contains com.google.javascript.jscomp.jarjar.org.apache.tools.zip
contains com.google.javascript.jscomp.jarjar.org.kohsuke.args4j
contains com.google.javascript.jscomp.jarjar.org.kohsuke.args4j.spi
contains com.google.javascript.jscomp.lint
contains com.google.javascript.jscomp.modules
contains com.google.javascript.jscomp.parsing
contains com.google.javascript.jscomp.parsing.parser
contains com.google.javascript.jscomp.parsing.parser.trees
contains com.google.javascript.jscomp.parsing.parser.util
contains com.google.javascript.jscomp.parsing.parser.util.format
contains com.google.javascript.jscomp.regex
contains com.google.javascript.jscomp.resources
contains com.google.javascript.jscomp.serialization
contains com.google.javascript.jscomp.transpile
contains com.google.javascript.jscomp.type
contains com.google.javascript.refactoring
contains com.google.javascript.refactoring.examples
contains com.google.javascript.rhino
contains com.google.javascript.rhino.dtoa
contains com.google.javascript.rhino.jstype
main-class com.google.javascript.jscomp.CommandLineRunner

Edited for better readability

blickly commented 2 years ago

Thanks for the report. We just added the dependency on jspecify in https://github.com/google/closure-compiler/commit/33bf62ad769c4915c734bcf31387e4fbf972ea8a on July 27, so that seems a likely candidate for when this started.

Since we are one of the early users of the jspecify annotations, I'm not clear if this is an issue with how we are pulling them in or if the annotations themselves are set-up overlooking this particular use-case.

@cpovirk, do you have any idea what the issue here might be?

cpovirk commented 2 years ago

Hmm, thanks. I'll poke around with your build configuration to see if I can figure out how it's getting pulled in. It would be a little surprising for a built-in rule like java_binary to be including it, but that's a possibility, or there may be something in the extra Starlark you use to generate your final artifacts.

cpovirk commented 2 years ago

(The java_binary target definitely has it. I haven't looked at what might contribute to that, and I likely won't get to the bottom of it before I'm done for the week shortly. But I'll come back to this.)

blickly commented 2 years ago

Thanks Chris!

blickly commented 1 year ago

The java_binary target definitely has it.

@cpovirk, by this do you mean that java_binary already includes a dependency on org.jspecify and our extra dependency is unnecessary, or that using the java_binary rule automatically generates these module-info.java files that are causing issues?

cpovirk commented 1 year ago

Sorry, the latter, or at least something like that? Building a java_binary with jspecify in the deps results in getting its module-info.class in the deploy jar.

But I'd tried Monday to reproduce that with a smaller example, and it didn't reproduce. So something is going on; I just haven't gotten back to it :(

cpovirk commented 1 year ago

Sorry again. Finally back. But not having much luck.

It looks to me like module-info.class shows up in a deploy jar under Bazel (5.3.0) but not under its Google-internal variant. I thought maybe I just needed to supply the right flags to Bazel, but my attempts so far (--java_language_version=11 --java_runtime_version=remotejdk_17 --tool_java_runtime_version=remotejdk_17) haven't helped.

I'm additionally confused that https://bazel.build/docs/user-manual#java-language-version suggests that the default is Java 11, but I see errors if I try to build a module-info.java unless I pass --java_language_version=11 or explicitly set javacopts = ["-source 11 -target 11"] on the target... I think? Updating to 5.3.1 doesn't change anything, and rules_java appears to already be on the newest version.... But -source 8 -target 8 is getting in as a default from somewhere. I see that even in a fresh workspace I created from scratch. Could the docs simply have gotten ahead of the actual behavior?

Getting back to module-info: It doesn't appear to matter whether the module-info comes from sources in the repo, from a jar imported from within the repo, or from a jar from another repo (maybe my terminology is wrong—@org_jspecify_jspecify//:jspecify-0.2.0.jar, that kind of thing).

cpovirk commented 1 year ago

Anyway...

It's possible that Bazel could be made to automatically omit module-info files from deploy jars. (As noted above, I found that it's apparently doing so already inside Google, and I see a similar claim in the description of internal CL 325114973.) However, it's also possible that Bazel won't want to make changes to its existing behavior (since any change could break someone) until they figure out the long-term plan in https://github.com/bazelbuild/bazel/issues/11930.

My suggestion would be to use a genrule or similar to produce a new jar with the module-info.class file removed (roughly cp old.jar new.jar && zip -d module-info.class new.jar).

blickly commented 1 year ago

Thanks for looking into this. I'll also follow along on on https://github.com/bazelbuild/bazel/issues/11930

Since we tend to use bazel and the Google-internal variant interchangeably, any improvements to make them behave the same way for the deploy jars would be a nice improvement.

leobut commented 10 months ago

Hello, has there been any update on this or an ETA?

And thank you very much for looking into it 😄 👍