Generate pseudo-compile jars for bazel java compilation #518
Conversation
|
I feel like just deleting the entire manifest.mf file will have other follow on problems for the jars that might set things like What about just deleting the |
|
I'd not thought about the case where we'd be dealing with a multi-release jar. Good catch. I've reworked the PR to use |
|
Looks good. I wonder if we should just combine the part where we add the The other condition we check for in |
|
The I may modify |
shs96c
force-pushed
the
compile-jar
branch
2 times, most recently
from
da73035
to
c74550f
Compare
private/rules/jvm_import.bzl
Outdated
| outputs = [compilejar], | ||
| ) | ||
|
|
||
| compilejar = ctx.actions.declare_file("header_" + injar.basename, sibling = injar) |
There was a problem hiding this comment.
I'm guessing this wasn't meant to be duplicated?
| inputs = [injar] + ctx.files._add_jar_manifest_entry, | ||
| args = ctx.actions.args() | ||
| args.add_all(["--source", injar, "--output", outjar]) | ||
| args.add_all(["--manifest-entry", "Target-Label:{target_label}".format(target_label = ctx.label)]) |
There was a problem hiding this comment.
Can this be combined with the call below so we only call add_jar_manifest_entry once?
There was a problem hiding this comment.
Desirable as that may be, the stamped jar and the compile jar are used for different purposes. The output jar will be supplied as a transitive dependency and is included in the list of runtime jars.
The compile jar is just used by javac for compilation. Normally, bazel uses ijar to generate this, and that strips out the bodies of methods, leaving only the ABI in place. For this case, we could remove everything that wasn't the class files, and reduce the manifest down to indicate whether or not this is a multi-release jar. In previous reviews, you've asked for the manifest to be left intact, which is done, other than stripping the Class-Path, since that's the thing that causes bazel to the bad path element warnings this PR was designed to remove :)
| # the lint check will emit a `bad path element` warning. We get quiet and | ||
| # correct builds if we remove the `Class-Path` manifest entry entirely. | ||
| args.add_all(["--remove-entry", "Class-Path"]) | ||
| ctx.actions.run( |
There was a problem hiding this comment.
Could you also add a mnemonic and progress message?
Normally, bazel uses `ijar` to prepare a jar containing just ABI-entries for classfiles. These are stable when non-API breaking changes are made, and so allow bazel to compile code faster, and are known as "compile jars" Because of bazelbuild/bazel#4549 `rules_jvm_external` doesn't use `ijar`, and instead uses the downloaded jar as the "compile jar". Normally, this is fine, but Bazel 4.0.0 now sets `-Xlint:path` for javac invocations, and this means that if there's a `Class-Path` manifest entry in a compile jar, the jars within _that_ will be checked. This can lead to noisy builds: bazelbuild/bazel#12968 This change generates a "pseudo-compile jar" for `jvm_import` targets. All it does is repack the input jar, excluding the `META-INF/MANIFEST.MF` file. This means that we should avoid compilation problems whilst still working well with Kotlin projects.
Normally, bazel uses
ijarto prepare a jar containing justABI-entries for classfiles. These are stable when non-API breaking
changes are made, and so allow bazel to compile code faster, and are
known as "compile jars"
Because of bazelbuild/bazel#4549
rules_jvm_externaldoesn't useijar, and instead uses thedownloaded jar as the "compile jar". Normally, this is fine, but Bazel
4.0.0 now sets
-Xlint:pathfor javac invocations, and this meansthat if there's a
Class-Pathmanifest entry in a compile jar, thejars within that will be checked. This can lead to noisy builds:
bazelbuild/bazel#12968
This change generates a "pseudo-compile jar" for
jvm_importtargets. All it does is repack the input jar, excluding the
META-INF/MANIFEST.MFfile. This means that we should avoidcompilation problems whilst still working well with Kotlin projects.