closure-compiler
closure-compiler copied to clipboard
Remove shaded content and restore dependency list to unshaded maven artifact
Uses rules_jvm_external's java_export rule to mark a given java_library as an artifact for export to maven. This rule will find any dependencies that were originally fetched from maven (by looking at the maven_coordinates tags attached to their java_library), and exclude them from the output jar, plus also populate a pom.xml file template with those dependencies.
For consistency, renamed the "unshaded" deploy jar to "uberjar", so as to be clear that the dependencies have been merged, but not relocated.
Both shaded and unshaded were built and installed locally (via mvn install:install-file) and tested with the sample project at https://gist.github.com/niloc132/1244ad7b72ec951800c5c1c12bfe08bc to confirm that a stage1 build would first write typedast content, and then a stage2+3 would read them and finish the job.
Fixes #3896
Build failed with
Run mkdir -p bazel-bin-unsymlink
cp: cannot stat 'bazel-bin/compiler_unshaded_deploy.jar': No such file or directory
I'll push a commit to fix the paths that are being referenced to "unshaded" content.
Added a commit that replaces nearly every "compiler_unshaded_deploy.jar" reference with "compiler_shaded.jar", as they almost certainly all expect to find the entire classpath in a single jar, rather than an actual unshaded jar.
I did notice that in package.json, the build
script is non-functional, it will try to call maven instead of bazel. That probably should be corrected, I can sneak it into this PR, or another followup?
> google-closure-compiler-linux
yarn run v1.22.18
$ node ./test.js
google-closure-compiler-linux
✓ compiler binary exists
Exception in thread "main" java.lang.IllegalArgumentException: class com.google.javascript.jscomp.jarjar.org.kohsuke.args4j.spi.BooleanOptionHandler does not have the proper constructor
at com.google.javascript.jscomp.jarjar.org.kohsuke.args4j.OptionHandlerRegistry.getConstructor(OptionHandlerRegistry.java:111)
at com.google.javascript.jscomp.jarjar.org.kohsuke.args4j.OptionHandlerRegistry.access$000(OptionHandlerRegistry.java:43)
at com.google.javascript.jscomp.jarjar.org.kohsuke.args4j.OptionHandlerRegistry$DefaultConstructorHandlerFactory.<init>(OptionHandlerRegistry.java:217)
at com.google.javascript.jscomp.jarjar.org.kohsuke.args4j.OptionHandlerRegistry.registerHandler(OptionHandlerRegistry.java:138)
at com.google.javascript.jscomp.jarjar.org.kohsuke.args4j.OptionHandlerRegistry.initHandlers(OptionHandlerRegistry.java:72)
at com.google.javascript.jscomp.jarjar.org.kohsuke.args4j.OptionHandlerRegistry.<init>(OptionHandlerRegistry.java:67)
at com.google.javascript.jscomp.jarjar.org.kohsuke.args4j.OptionHandlerRegistry.getRegistry(OptionHandlerRegistry.java:57)
at com.google.javascript.jscomp.jarjar.org.kohsuke.args4j.CmdLineParser.createOptionHandler(CmdLineParser.java:178)
at com.google.javascript.jscomp.jarjar.org.kohsuke.args4j.CmdLineParser.addOption(CmdLineParser.java:146)
at com.google.javascript.jscomp.jarjar.org.kohsuke.args4j.ClassParser.parse(ClassParser.java:34)
at com.google.javascript.jscomp.jarjar.org.kohsuke.args4j.CmdLineParser.<init>(CmdLineParser.java:96)
at com.google.javascript.jscomp.jarjar.org.kohsuke.args4j.CmdLineParser.<init>(CmdLineParser.java:71)
at com.google.javascript.jscomp.CommandLineRunner$Flags.<init>(CommandLineRunner.java:957)
at com.google.javascript.jscomp.CommandLineRunner.<init>(CommandLineRunner.java:1455)
at com.google.javascript.jscomp.CommandLineRunner.main(CommandLineRunner.java:2239)
From this latest failure and taking a quick look at the closure-compiler-npm repo, it appears that the graal build assumes an uberjar, but not shaded build. Does it seem likely that we would need three flavors, unshaded and shaded (as the terms meant prior to https://github.com/google/closure-compiler/commit/c9f3d327be6146a7d4de1f3d7c03feb36f49524a, and as they generally mean in the general java ecosystem), plus also either a single jar with the full runtime classpath contained within it, or an ordered list of jars to use, which we can somehow pass to the graal build?
My guess right now is that in google/closure-compiler-npm, the build-scripts/reflection-config.json needs to be updated to reflect the package to the shaded jars. I'm currently trying to set up the rest of the build so I can confirm that - the simplest fix seems to be to duplicate all org.kuhsuke.args4j.*
packages for a short time, until this release is out, then delete the unshaded packages.
Alternatively, I'm still looking for how the closure-compiler-npm build handled the unshaded jars pre-c9f3d327, but still had the full classpath. My best guess at the moment is that the pom file change is something of a red herring, and that the npm output was exclusively using bazel output (see https://github.com/google/closure-compiler/blob/c9f3d327be6146a7d4de1f3d7c03feb36f49524a^/.github/workflows/ci.yaml#L133 for example, the commit before the pom change, which explicitly uses the "unshaded" deploy jar). If npm wants non-jarjar'd uberjars jars, I can update these scripts - specifically, this diff would use compiler_uberjar_deploy.jar
where previously it used compiler_unshaded_deploy.jar
.
Haven't heard back in a while, so I tried the simple replacement of compiler_uberjar_deploy.jar in for compiler_unshaded_deploy.jar
that was used before this diff, since that seems to be what it actually wants. We'll see what CI says about this.
Sorry. This PR is on my to-do list, but other things keep pushing it down before I can get to it.
I've sent an internal change for review that will remove the bad "script" entries from package.json
and the corresponding yarn
commands from the README.md
file.
Thanks for pointing those out.
@ChadKillingsworth this change will need to be synchronized with a change to https://github.com/google/closure-compiler-npm since that repo builds and uses compiler_unshaded_deploy.jar
.
I think it should work to have it just use compiler_uberjar_deploy.jar
instead.
Do you have any concerns about that?
It looks like the uberjar just includes some additional .proto
files and META-INF/maven/protobuf-java/pom.(xml|properties)
files.
Here's a diff of the jar contents.
Just now seeing this. I have no concerns as long as a build target exists to produce the full jar. There will need to be path updates to the build made to the npm repo, but those changes would be pretty trivial.
And I just now saw Chad's response. :)
I've imported this PR and I'm sending it through internal review and testing.