RFC: Pull prisms into own module, separate shaded generators#748
RFC: Pull prisms into own module, separate shaded generators#748cbarlin wants to merge 1 commit into
Conversation
Based on the discussion in avaje#743, there was mixed feelings on the shading being done for the generators. I figured since there is a major version on the horizon (based on discussions in avaje#730), what we could do is: * Isolate out the prism generation, to help ensure we don't end up with unintentional dependencies in the actual generators (since the generator would fail compilation in this repo if you tried) * Make the "main" versions of the processors no longer shaded * Publish separate "-shaded" versions of the generators that are shaded The idea being if you are using `proc=full` and/or having issues with the dependencies for the generator not coming through, you can use the `-shaded` versions (e.g. instead of `avaje-http-client-generator` you'd use `avaje-http-client-generator-shaded`). If (or when) shading becomes problematic, then we can easily remove those shaded versions from being published. There's not really a "solid reason" for doing this now rather than waiting to see what happens with `proc=full`. It's just something that was discussed, and could be added to the list of potentially breaking changes in that next major. During this though, I did notice a couple of things: * The test pipeline doesn't test on Java 11, despite that being the minimum (?) Java version * The main `http-api` has a dependency on the `http-inject-plugin` which has a dependency back on the `http-api` and also only builds on Java 21+ (while `http-api` will try to build on Java 11) * The `http-api` had a (`static`/`optional`) dependency on javalin, but there's a `http-api-javalin` that could house it and forward on the `http-api` dependency * Both Avaje SPI and Avaje Prisms publish their annotations depending on their processor, rather than the other way around, which seems incorrect. I'm not sure if Maven 4 will have a problem with the annotations being `jar` and the dependency being `processor`. You can't have the annotations as both since duplicate G/A/V is not permitted, but even if it doesn't it "feels like" it should be the reverse (processor depends on annotations) I've marked this as draft/rfc because I want to know: * Is the separate shaded versions an OK compromise? * Is there a preferred path for dealing with the circular dependency? I also haven't finished: * all the shaded versions, * patching the tests so they use a mix of non-shaded and shaded generators (so we can be sure we don't break the shaded versions) * testing the Java 11 addition to the build process But I figured I'd wait for the discussion before doing too much more.
|
93 files and not done yet either 😅 |
Not sure I follow
At that point might as well lose shading entirely and use the compiler plugin. My thing is that I want to reduce the times the user has to change as much as possible. A change that may require pom changes without a concrete use case describing an issue/enhancement doesn't do it for me.
helidon/jex has a 21 baseline requirement.
petition the JDK team once more to allow optional SPI . Ideally we would make it part of the http-api module itself, but the JVM would throw an an exception for non-avaje projects.
It works both ways. Even using the compiler plugin approach it doesn't matter |
|
It's easter, so time to go on holiday and take a break (well, I am but yeah you do whatever) ... but at a quick look this PR isn't focused and is trying to do too much and so rather unlikely to progress as I see it. To me, for a PR to get merged it needs to stay focused on one thing. If you have a list of questions then those are likely better to be raised as questions via an issue and not really good material for a PR with multiple changes. |
Based on the discussion in #743, there was mixed feelings on the shading being done for the generators. I figured since there is a major version on the horizon (based on discussions in #730), what we could do is:
The idea being if you are using
proc=fulland/or having issues with the dependencies for the generator not coming through, you can use the-shadedversions (e.g. instead ofavaje-http-client-generatoryou'd useavaje-http-client-generator-shaded). If (or when) shading becomes problematic, then the bulk of the "clean-up" is done and we can just delete theshadedmaven module.There's not really a "solid reason" for doing this now rather than waiting to see what happens with
proc=full. It's just something that was discussed, and could be added to the list of potentially breaking changes in that next major. Phase it out (if it ever is fully removed) kinda thing.During this though, I did notice a couple of things:
http-apihas a dependency on thehttp-inject-pluginwhich has a dependency back on thehttp-apiand also only builds on Java 21+ (whilehttp-apiwill try to build on Java 11)http-apihad a (static/optional) dependency on javalin, but there's ahttp-api-javalinthat could house it and forward on thehttp-apidependencyjarand the dependency beingprocessor. You can't have the annotations as both since duplicate G/A/V is not permitted, but even if it doesn't it "feels like" it should be the reverse (processor depends on annotations)I've marked this as draft/rfc because I want to know:
I also haven't finished:
But I figured I'd wait for the discussion before doing too much more.