bazelbuild
feat(singlejar): Add Log4j plugins cache combiner
In singlejar, add support for combining Log4j2 plugins cache file.
Log4j2 plugins are Java annotations collected by a compiler plugin into .dat files for Log4j2 runtime to find them fast. With correct dependency on a java_plugin, Bazel already runs the java plugin compiler correctly. The behavior is correct on bazel run JAR, but not on _deploy.jar fat jars.
The silent clobbering of files in general, and the example of these Log4j2 .dat files in particular, is discussed in https://github.com/bazelbuild/bazel/issues/7330.
- File structure of Log4j2Plugins.dat files
- the test data was generated by a python script, happy to commit it as well
- How to merge Log4j2Plugins.dat?
I tested this fix in my project using Bazel 7.0.2, compiling //src:java_tools_prebuilt.zip, and overriding @remote_java_tools_darwin_arm64 and @remote_java_tools_linux.
@sgowroji what question do I need to answer for "awaiting-user-response" to get removed?
@softprops @jwilliams-ocient from https://github.com/bazelbuild/bazel/issues/7330; does this help the issue for you?
@hvadehra @meteorcloudy for Blaze we don't need to support log4j, so supporting this isn't a priority for me. I also don't want to stand in the way of progress if you want to support this in Bazel. I left a note in #7330 about the idea of trying to make this kind of thing more pluggable, but there may not be great alternatives to doing it directly in singlejar. If that's something you want to pursue, perhaps the new 'combiner' here could be factored into a separate file and only wired up for Bazel.
@cushon I would prefer to author this in Java to reuse the log4j merger logic from existing implementation. Ideally I agree with you if we could provide a set of custom combiners as configuration instead of having to add complexity/features in singlejar this would be very nice.
Where could we start to pry open singlejar and allow external combiners to be configured?
In the short term, if I address the comments, is this something that could live in singlejar until we rollout a configurability mechanism?
@shs96c if this ends up being authorable in Java, would https://github.com/bazel-contrib/rules_jvm be a good home for this log4j jar combiner extension?
contrib_rules_jvm is too high a level. You might want this in rules_jvm_external so maven artifacts we create are correct, but the only way for this to work with a deploy jar is to have this in rules_Java or wherever singlejar lives.
Friendly ping @cushon. Ideally I would like this to make it into Bazel 8 and a 7 point release.
Sorry for the delay.
Thanks for refactoring the combiner into a separate file, I'm fine with the current approach. (Internally we use a separate singlejar entry point with additional combiners, we may not wire up the new combiner there, which is fine.)
@hvadehra do you want to review for Bazel?
@hvadehra I added the script in https://github.com/bazelbuild/bazel/pull/22581/commits/6021715bf15a3edf6fa4b057df8d78de1e36fc19. Ideally I would prefer to keep the files in tree as "golden" files.
Ideally I would prefer to keep the files in tree as "golden" files.
That makes sense for the expected output being checked in the test(s), but for the inputs it would be nicer to have them in a readily accessible form. This is particularly useful when iterating on changes / debugging test failures.
I am not sure I understand the ask here.
If the ask is for the 2 input jar to we generated by a genrule: these jar files are not stable because of zip data IIUC and they would be cached. Having them on disk circumvents this.
If the ask is Bazel does not want to suffer a zx kind of scenario, we can audit the python code and generate those jars in a trusted context (or directly inspect the JARs, they are a single file).
Ping @cushon; IIUC we got approval from the Bazel and C++ side for this change.
It appears that importing this will need a bit of work, I'll do that myself.
Can this be merged into 7.5.0 ? (or whatever a 7.X release it can go into)
Can this be merged into 7.5.0 ? (or whatever a 7.X release it can go into)
That shouldn't be necessary. Once we have a java_tools / rules_java release with this change, one can just update those deps and stay on Bazel 7.
@hvadehra I'm looking at java_tools but it's not clear how to map the release back to a specific Bazel commit? I"m trying to prove to myself what version of rules_java has this fix.
The java_tools releases have their provenance information embedded in the archives. For eg: if you look at https://github.com/bazelbuild/java_tools/releases/tag/java_v13.9 , download any one of the zip archives and look at the top-level README file inside.
The fix from this PR has not made it into a java_tools / rules_java release just yet. You can check the state at https://github.com/bazelbuild/bazel/pull/24696 / https://github.com/bazelbuild/java_tools/issues/93
Cool thanks; I saw there are tags for the releases -- maybe I'l open a suggestion if it's possible to commit to the repo the provenance so that it's browsable in the GitHub repo.