Hi,
this PR fixes #22036 by marking the starter jars with an additional attribute in their manifest which is later checked to exclude the starter JAR during packaging.
I opened the PR now to discuss the taken approach better.
Let me know what you think. Cheers, Christoph
Comment From: snicoll
I wonder if generalizing this slightly could help us fix https://github.com/spring-projects/spring-boot/issues/22025 as well.
Comment From: dreis2211
@snicoll You mean marking spring-boot-autoconfigure-processor
& spring-boot-configuration-processor
with a similar entry in its manifest (or with one that suits all)?
Comment From: snicoll
Yes. I should mention this was targeted to the team at large to see if we could come up with a manifest entry that makes sense for both this use case and the issue I've referenced.
Comment From: wilkinsona
I like that idea, @snicoll. A more general purpose manifest header that can be used to influence what's included sounds good to me. I'm struggling to think of a good name for it though. In the interests of consistency, I think it makes sense for the exclusion to apply more broadly than just building fat jars and wars. It should probably also affect the classpath used for spring-boot:run
, bootRun
, when using Gradle's application plugin, etc.
Comment From: dreis2211
What about Exclude-From-Runtime
as a name for the manifest entry?
@wilkinsona I would need a pointer which classes are involved when this should be applied on a broader level beyond packaging. At the moment I'm looking at AbstractRunMojo
for Maven and BootRun
for Gradle. But especially for Gradle I find it a bit difficult to apply the chosen mechanism to be honest. (I only have looked for a couple of minutes, though)
Comment From: wilkinsona
For Gradle, I'd try to do it by filtering the classpath that the BootRun
, BootJar
, and BootWar
plugins are configured with. I'd do that in JavaPluginAction
and WarPluginAction
when those tasks are being defined.
I've just reminded myself that the application distribution that Boot configures uses the fat jar or war so we don't need to worry about that one.
Comment From: dreis2211
For Gradle, this seems to conflict with AdditionalMetadataLocationsConfigurer
in JavaPluginAction
that seems to check for spring-boot-configuration-processor
being on the classpath.
Comment From: dreis2211
@snicoll I just tried adding the following to a test project's pom.xml
:
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-configuration-processor</artifactId>
<optional>true</optional>
</dependency>
When packaging this up I see no jar of the processor included in the build.
Comment From: wilkinsona
For Gradle, this seems to conflict with
AdditionalMetadataLocationsConfigurer
That's strange. That configurer checks the classpath of the JavaCompile
classpath which should be unaffected by any filtering that's being done to the classpath of BootJar
etc.
Comment From: dreis2211
When adding the following to a build.gradle
annotationProcessor "org.springframework.boot:spring-boot-autoconfigure-processor"
and packaging this up via bootJar
I see no jar included. So both in Maven & Gradle the processors don't seem to end up already in the fat JARs. Am I missing something here?
Comment From: wilkinsona
That's the behaviour that I would expect with Gradle. With Maven, my understanding was that the annotation processor would be included in the fat jar and #22025 was opened to improve that.
Comment From: dreis2211
Would that also render the classpath discussion obsolete? If it doesn't I would need help there, I'm afraid...
Comment From: dreis2211
Edit: I tried Maven again with a clean build again and it included the processors. Sorry for the noise. For Gradle, it seems to be not included as I wrote earlier.
Comment From: wilkinsona
Would that also render the classpath discussion obsolete?
No, I don't think so. I'd like to try to use a classpath-filtering-based mechanism for the starters.
If it doesn't I would need help there, I'm afraid...
Sure thing. Let us know when you're ready to pass the baton and we can take it from there.
Comment From: dreis2211
I've pushed a more general solution for the packaging aspect with a new PackageExcludeFilter
. Let me know what you think of that.
Other than that, this would be the point were you probably need to take over to cover the classpath filtering.
Comment From: dreis2211
I just renamed PackageExcludeFilter
to FileExcludeFilter
as this is a more fitting name imho.
Comment From: wilkinsona
Thanks, @dreis2211.
FWIW, I think Exclude-From-Runtime
that you mentioned above is preferable to Exclude-From-Packaging
that is currently proposed. That said, I've been wondering if we should go with something that's more declarative, describing what the dependency is rather than how it should be treated. Something like Spring-Boot-Dependency-Scope
or Spring-Boot-Dependency-Nature
with values of compileOnly
or dependencyMetadataOnly
respectively. I'm not sure yet though. Let's see what you and the rest of the team think.
Comment From: dreis2211
I wasn't really happy with either Exclude-From-Runtime
or Exclude-From-Packaging
, so I like the idea of introducing Spring-Boot-Dependency-Scope
, but I'm not sure about the values yet. The configuration processors would be marked with compileOnly
and the starters with dependencyMetadataOnly
, right? What about the other modules - maybe a simple default
value? I'm also thinking how the Gradle plugin structure would look like for that and how we would like to apply this to all modules.
Comment From: philwebb
We had a bit of a brainstorm today and we quite likeSpring-Boot-Jar-Type
for the key with values dependencies-starter
or annotation-processor
.
Comment From: dreis2211
And for the others? Leave it empty? I could at least do that before you can take it over...
Comment From: wilkinsona
Yeah, leave it empty for the others please.
Comment From: dreis2211
Okay, I will craft a new plugin then and let you know when I'm finished.
Comment From: dreis2211
I added a new AnnotationProcessorPlugin
similar to the starter plugin that applies the new jar type to the manifest. That seemed more pragmatic than introducing an additional DSL extension for the new jar type.
I finished the work that I wanted to do. I'm not happy here and there with some duplication (more specifically with the magic strings Spring-Boot-Jar-Type
and its values). Maybe you can find a good place for those constants somewhere.
Let me know if I should do something on this PR still.
Cheers
Comment From: wilkinsona
Thanks for the PR, @dreis2211. In reviewing this I came to the conclusion that we should take a different approach to filtering out the jars in the Maven and Gradle plugins so I reworked the changes here to add the Spring-Boot-Jar-Type
manifest entry to Boot's starters and annotation processors. The changes to the filtering were made in https://github.com/spring-projects/spring-boot/commit/143d19754bd7489889029afc85e109bc7115c6c3 (Gradle) and https://github.com/spring-projects/spring-boot/commit/e743d5fe66302feca910f81a7285ca0caf3e1b84 (Maven) and apply to both packaging and running an app.