Jakarta EE 9 transition #6085
Jakarta EE 9 transition #6085
Conversation
basil
added
the
rfe
| private static void transform(String... args) { | ||
| Transformer jTrans = new Transformer(System.out, System.err); | ||
| jTrans.setOptionDefaults(ClassicPluginStrategy.class, getOptionDefaults()); | ||
| jTrans.setArgs(args); | ||
|
|
||
| @SuppressWarnings("unused") | ||
| int rc = jTrans.run(); | ||
| if (rc != 0) { | ||
| throw new RuntimeException("Transformer failed with exit code: " + rc); | ||
| } | ||
| } | ||
|
|
||
| private static Map<AppOption, String> getOptionDefaults() { | ||
| Map<AppOption, String> optionDefaults = new HashMap<>(); | ||
| optionDefaults.put(AppOption.RULES_RENAMES, "jakarta-renames.properties"); | ||
| optionDefaults.put(AppOption.RULES_VERSIONS, "jakarta-versions.properties"); | ||
| optionDefaults.put(AppOption.RULES_BUNDLES, "jakarta-bundles.properties"); | ||
| optionDefaults.put(AppOption.RULES_DIRECT, "jakarta-direct.properties"); | ||
| optionDefaults.put(AppOption.RULES_MASTER_TEXT, "jakarta-text-master.properties"); | ||
| optionDefaults.put(AppOption.RULES_PER_CLASS_CONSTANT, "jakarta-per-class-constant-master.properties"); | ||
| return Collections.unmodifiableMap(optionDefaults); | ||
| } |
There was a problem hiding this comment.
Beginnings of a compatibility layer for plugins. This initial prototype uses Eclipse Transformer to transform the extracted plugin to a Jakarta EE 9 compatible version after exploding the plugin JPI. Some possible ideas for how to productize this:
- Perhaps we might update
plugin-pom/maven-hpi-pluginto apply this transformation when the plugin is built and then mark the plugin as having been transformed inMANIFEST.MF. Then we can skip over the transformation for that plugin at runtime. - Perhaps it's too heavyweight to rewrite all the plugins' contents at startup. The Eclipse Transformer
READMElists a "possible secondary use" of dynamic rewriting at class loading time. Perhaps we want to lazily do the transformation at class loading time rather than eagerly transforming all plugins when Jenkins starts up.
There was a problem hiding this comment.
Perhaps we might update
plugin-pom/maven-hpi-pluginto apply this transformation when the plugin is built and then mark the plugin as having been transformed inMANIFEST.MF. Then we can skip over the transformation for that plugin at runtime.
Is the Servlet API available to plugins not tied to the core version the plugin depends on? If it is, I imagine at some point a plugin maintainer raises the Jenkins core dependency to whichever release includes this change, and then the plugin should no longer build unless imports are fixed. Do you want maintainers to not have to do that? Because declaring a lesser Jenkins core dependency, and then unconditionally rewriting imports would be incompatible with that core.
| // TODO needs triage | ||
| //return new Hudson(home, createWebServer(), pluginManager); | ||
| return new Hudson(home, null, pluginManager); |
There was a problem hiding this comment.
I haven't even begun to try and get the test harness up and running, but needless to say there will be a lot of changes required there as well.
|
Another problem is that Guice currently doesn't support Jakarta EE 9. |
basil
force-pushed
the
jakarta
branch
2 times, most recently
from
1b49351
to
4065205
Compare
Good start though :P |
|
Even though I called |
|
Looks like despite jenkins-infra/docker-inbound-agents#7 there is no |
basil
force-pushed
the
jakarta
branch
9 times, most recently
from
2a69788
to
f17d65f
Compare
|
First CI build: 849 tests failed, 28261 tests passed. It can only get better from here! |
basil
force-pushed
the
jakarta
branch
2 times, most recently
from
3d4dcba
to
8342b47
Compare
core/src/main/java/hudson/diagnosis/HudsonHomeDiskUsageMonitor.java
Outdated
Show resolved
Hide resolved
|
Did a big update on this PR today. I updated the Transformer logic so that it transforms the classes dynamically at class loading time. The resulting design is much like the old Bytecode Compatibility Transformer that I got rid of a while back. :-) You can see the result in The result is that Tests still don't work, because they don't go through the same class loader hierarchy as production code does. I haven't yet thought about how we can apply the compatibility transformation in test context. |
Not something we want to do if we can possibly avoid it IMO. Is it fair to say that while many plugins would be broken by a change of Servlet API package, the number using other stuff like JavaMail are pretty small and could be reasonably updated manually, as I think you have already started to do? If so, we should be able to avoid any bytecode / class loader tricks. Just include |
"Just" is an understatement. This isn't as easy as it sounds. |
|
I would not expect it to be trivial, but probably a lot easier than the Acegi Security → Spring Security stubs from JEP-227? |
Probably as hard if not harder I think. |
|
Why would you say that? It is mostly about package renames, right? And most code deals with |
Try it and you'll find out. 😄 |
|
OK…just trying to understand if this is something you have already tried to prototype, and ran into difficulties, or if you have only attempted the bytecode manipulation approach. |
I spent a couple hours on it before realizing I could get a lot further a lot faster with the bytecode manipulation approach. |
|
Right, makes sense. This is a long way out (right?) so we should have time to make a serious attempt to do a compatibility shim in plain old Java code, which would be easier to debug for regular mortals and should not require changes to tooling. The shims in #4848 were somewhat tedious to write but I learned some tricks along the way; much of the time spent was actually setting up infrastructure to get adequate test coverage. |
Please speak for yourself (e.g. "I think") rather than for others ("we should"). I have a different opinion. Writing the shims you describe is time-consuming and difficult. I think that time would be better spent migrating callers to the new APIs. Once most callers have been migrated to the new APIs, we can go back and reëvaluate the bytecode manipulation approach. By that point, writing shims for the small number of remaining callers would likely be much easier, and it should be possible to rip out the bytecode manipulation. True, the transition period would be messier, but on the other hand the end result would be cleaner with fewer shims (and it would take less time to get there). |
|
Perhaps, but ever shipping the bytecode library will probably force a fair amount of tricky tooling work, and I find this sort of work much harder and riskier than just writing Java code to delegate interfaces, which can be understood and tested in an IDE. I would rather avoid ever putting bytecode manipulation or class loader tricks in trunk. Definitely it is a good idea to be looking for outlier plugins which use exotic parts of the API surface and simplify them in advance so that we do not waste time working on shims for difficult cases that are rarely used to begin with. In the case of #4848 I did just that—only wrote a shim after reviewing usages of the deprecated API and concluding that there were too many to reasonably rewrite in every plugin before shipping the core change. |
|
I have edited the PR description to read "do not review" to express several things:
This is an experimental PR purely for the purposes of getting incremental builds and running tests on CI. |
|
I ripped out the bytecode rewriting logic and implemented some basic Java compatibility shims in Stapler: nothing exhaustive, but enough to get test runs off the ground. With that we've gone 849 failing tests to only 8 failing tests. This I call progress. Interactive testing shows that many things are working, but more plugin compatibility is needed. and |
|
https://github.com/jenkinsci/timestamper-plugin/blob/1f98c5153e4742e1926674664294126052ec4ba2/src/main/java/hudson/plugins/timestamper/format/TimestampFormatProvider.java#L68 the type could just be changed to
https://github.com/jenkinsci/pipeline-stage-view-plugin/blob/35de9f668c0aec5bd4560884b61f94f335579987/rest-api/src/main/java/com/cloudbees/workflow/util/JsonResponse.java#L12-L16 is a bit unusual—normally you would just return Will be interesting to see the extent of breakage in |
|
Please refer to the PR description:
|
|
Changes have been requested when I specifically wrote:
Therefore, I am closing this PR. |
|
Hmm? I did not “request changes”. I did not review at all. You mentioned some test failures and I was offering some tips based on prior experience doing something analogous which may or may not be applicable. |
|
I am not going to do this project alone. Keeping this PR open invites people to leave GitHub comments with advice rather than writing code, which I am not going to encourage. |
Do not review. This is an experimental PR purely for the purposes of getting incremental builds and running tests on CI.