Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reorganize Gradle build #23282

Closed
13 tasks done
bclozel opened this issue Jul 12, 2019 · 12 comments
Closed
13 tasks done

Reorganize Gradle build #23282

bclozel opened this issue Jul 12, 2019 · 12 comments
Assignees
Labels
type: task A general task
Milestone

Comments

@bclozel
Copy link
Member

bclozel commented Jul 12, 2019

This is a much needed follow up issue for #20440.

Currently the Spring Framework build is mixing scripts, conventions and dependency information in .gradle files, some of them externalized in a gradle folder. The goal of this issue is to refactor the current build to scripts and conventions into build plugins. Each project/subproject should then selectively apply those plugins and only describe the specific dependencies in their build file.

Since our last efforts, Gradle had significant improvements and lots of new features that could be useful to Spring Framework.

Here is the list of the tasks for this issue:

  • Rewrite the "test source sets" plugin from Groovy to Java
  • Do not use the propdeps plugin and replace provided/optional with compileOnly and a custom optional configuration (see spring-core-5.2.0.M3.pom missing netty dependencies #23234)
  • Do not publish optional/provided dependencies in published POMs
  • Move the Java compilation configuration to a convention. The Kotlin one is short and comes with the Kotlin plugin declaration, so we're letting that part in the main build file for now.
  • Remove Gradle wrapper customization as script and move it to gradle.properties if necessary
  • Move the JDiff to a separate plugin and use Japicmp instead
  • Move integration tests to a separate module
  • Reorganize tasks and scripts to selectively apply them to one of: root project, framework module, internal module (BOM, coroutines, integration-tests)
  • Download the "spring-docs-resources" zip as an URL and do not resolve it as a dependency (see Stop using libs-* repo in favour of consistent use of Maven Central #23124)
  • Use "spring-*" as a project name only for Spring Framework published modules. Rename internal modules accordingly ("spring-framework-bom", "spring-core-coroutines"...)
  • Use Gradle Platform features for publishing the Spring Framework BOM
  • Use the standard Gradle "Maven Publish plugin"
  • Apply and configure the artifactory publish plugin directly (instead of delegating that to the CI server). This allows us to really control which artifacts are published, without workarounds.
@bclozel bclozel added the type: task A general task label Jul 12, 2019
@bclozel bclozel added this to the 5.2 RC1 milestone Jul 12, 2019
@bclozel bclozel self-assigned this Jul 12, 2019
@sbrannen
Copy link
Member

Remove outdated Eclipse support (replaced by Buildship)

We need to be cautious here. Buildship is not perfect, and the last time I checked... we need our custom configuration for the Eclipse classpath, etc.

Maybe you can perform that work in a dedicated PR for closer review?

@bclozel
Copy link
Member Author

bclozel commented Jul 12, 2019

@sbrannen We don't use that support in our own documentation for contributors: https://github.com/spring-projects/spring-framework/blob/master/import-into-eclipse.md

I thought it wasn't relevant anymore?

@sbrannen
Copy link
Member

sbrannen commented Jul 14, 2019

@bclozel, whenever one uses Buildship in Eclipse, the eclipse customization in the Gradle build is honored.

I don't think it's well documented, but it is at least officially mentioned here and here.

And the "Project synchronization" section of this article demonstrates advanced features like those that we use in ide.gradle.

In summary, if we remove all of our eclipse customization in the Gradle build I fear that our projects will not work properly with the standard Buildship features. Hence the need to be cautious before totally removing all of that.

@bclozel bclozel modified the milestones: 5.2 RC1, 5.2 RC2 Jul 22, 2019
bclozel added a commit that referenced this issue Aug 13, 2019
This commit moves the existing "test sources" Gradle plugin from Groovy
to Java and updates the "buildSrc" build file to prepare for additional
plugins in the Spring Framework build.

The plugin itself looks, for a given Spring Framework module, at all the
project dependencies for the following scopes: "compile", "testCompile",
"api", "implementation" and "optional" (to be supported by a different
plugin).

See gh-23282
bclozel added a commit that referenced this issue Aug 13, 2019
Prior to this commit, the Spring Framework build would be using the
propdeps Gradle plugin to introduce two new configurations to the build:
"optional" and "provided". This would also configure related conventions
for IDEs, adding those configurations to published POMs.

This commit removes the need for this plugin and creates instead a
custom plugin for an "optional" configuration. While the Eclipse IDE
support is still supported, there is no need for specific conventions
for IntelliJ IDEA anymore.
This new plugin does not introduce the "provided" scope, as
"compileOnly" and "testCompileOnly" are here for that.

Also as of this commit, optional/provided dependencies are not published
with the Spring Framework modules POMs annymore.
Generally, these dependencies do not provide actionable information to
the developers reading / tools consuming the published POMs.

Optional/Provided dependencies are **not**:
* dependencies you can add to enable some supported feature
* dependencies versions that you can use to figure out CVEs or bugs
* dependencies that might be missing in existing Spring applications

In the context of Spring Framework, optional dependencies are just
libraries are Spring is compiling against for various technical reasons.
With that in mind, we are not publishing that information anymore.

See gh-23282
@rwinch
Copy link
Member

rwinch commented Aug 13, 2019

@bclozel Is there a reason we want to remove the provided/optional dependencies? As a consumer of projects, I find it useful to look in the pom to see what version of optional dependencies they use so I know which versions are going to work. Granted I could look in the Gradle build for this information, but Gradle's flexibility in this manner makes it harder to figure out than looking in a pom.xml file.

@bclozel
Copy link
Member Author

bclozel commented Aug 14, 2019

@rwinch, I'm elaborating on that in #23234
Also, this issue will move all the dependency management in the main build.gradle file, so things should be easier to read there (versions, BOMs, exclusions, everything!).

@rwinch
Copy link
Member

rwinch commented Aug 14, 2019

@bclozel Thanks for the additional information.

I don't agree with the assessment. Even if things are easier to read by moving them to the main build.gradle, this is something specific to Spring Framework's Gradle build. This simplification still requires knowledge of Gradle and Spring Frameworks build (other projects could just as easily place the information in the build.gradle for each project). This means a Maven user must have knowledge of Gradle in order to figure out the optional dependencies.

User's could look at Spring Boot for guidance on dependencies, but this is also confusing. I've never been told to look for dependencies I should use by leveraging a separate project. How is a user to know that they should look at Spring Boot for the dependencies? Spring Boot can help with versions, but how does it indicate what groupId/artifactId users should be using for each Spring project?

I understand that mapping in Maven is far from perfect. However, defining optional and provided dependencies is what is expected in Maven pom.xml files. Deviating from this convention in a Maven pom.xml is confusing to Maven users even if it is not a direct mapping from our Gradle build. Asking Maven users to learn another build system or look in another project for dependency information is not ideal either (it deviates from Maven definition of a pom.xml).

Please consider adding optional and provided dependencies back to Spring's pom.xml.

@bclozel
Copy link
Member Author

bclozel commented Aug 14, 2019

@rwinch my last assessment was only directed to you, when looking for information about the Spring Framework build as a project maintainer.

I think there are two issues here:

  1. We're somehow considering that shipping optional/provided dependencies in our generated POM is a legitimate way to convey information about Spring's support for libraries. The maven metadata model is not tailored for that as it is declaring things for your build but the intent is not to communicate additional metadata to consumers. That's my personal opinion, and I think that's where Gradle is coming from with the feature variants.
  2. Even if I disagree with that model, developers are indeed looking at the generated POM for guidance or building automation (see spring-core-5.2.0.M3.pom missing netty dependencies #23234). As explained in this comment, I've actually tried to create feature variants for Spring Framework and let those translate to optional dependencies in the generated POM. It turns out most dependencies indicate how we want to build Spring rather than what we can do with it or how we should use it.

In the end, removing that information from the generated POM is more about avoiding to communicate misleading information. The Spring Boot BOM fills that role in a much better way: it provides information about dependencies (coordinates, version) that are fully tested by the Spring Boot build with many Spring projects. This BOM can be used by non-Spring Boot applications as well. I for some reason we don't want to involve Spring Boot here, we should provide this information in a different way and not try to derive it from our build since it seems we can't extract meaningful information from it for this use case.

@rwinch
Copy link
Member

rwinch commented Aug 14, 2019

The problem with these approaches is that it deviates from Maven's definition of a pom.xml and yet we are publishing a Maven pom.xml Deviating from the Maven definition of a pom is even more confusing (because we are trying to define it ourselves). The confusion around what the optional / provided mean is expected in a Maven pom and developers using Maven have become accustom to that. If we are going to publish a pom, it should follow Maven conventions which include provided and optional dependencies.

@bclozel
Copy link
Member Author

bclozel commented Aug 14, 2019

We're still publishing a perfectly valid POM, and generating that information from our Gradle build. We do have many custom and non-custom configurations, including compileOnly, which don't map to maven concepts. We're not exporting those.

Even in a pure Maven setup, you can filter/flatten your published POMs, and I'd argue we're doing that now to avoid misleading our users. Unless we find a way to provide accurate and meaningful information in our metadata, I don't see a good reason to bring that back into the published POM.

Let's take a step back. What are the use cases for getting the information we were providing so far and why?

@rwinch
Copy link
Member

rwinch commented Aug 14, 2019

What are the use cases for getting the information we were providing so far and why?

As a user of pom.xml this is where I know to go to get dependency information. Putting the information anywhere else puts Spring on an island which means extra work for users to discover where that information is.

I believe all comments about why we don't want to do it because it is confusing is something that is expected in Maven projects. Removing the information is not helpful because it is different than what users are expecting. A few responses to the examples:

has a couple of log4j and slf4j dependencies, but those are not the actual dependencies you should have to enable logging with a logging framework. Those are dependencies that we use to build the specific bridge implementations, but using a logging framework requires more than that.

I don't understand why this is a problem. User's aren't going to bring the dependencies in if they don't need them. They are optional/provided...so a user isn't going to add them.

depends on netty-buffer, but a Spring application should not depend on Netty directly, but rather on Reactor Netty. This is used for managing DataBuffer instances with Reactor Netty as a server. So this tells us more about a particular spring-webflux use case, rather than something that is actually linked with spring-core itself

Correct, but the information for reactor-netty is in the pom which they do need information about. If the dependency is listed as optional or provided users are use to sorting through this.

While the Framework itself is compatible with Servlet 3.1+, some modules might build specific support for Servlet 4 features. Reading the versions of those optional dependencies is just misleading.

This is something that users are use to. Place the best version as an optional dependency to convey what is preferred but don't remove the information all together. This is not any different than a required dependency where multiple versions are supported. Boot takes a preference for JUnit 4 and provides this in the pom, but JUnit 5 is still supported.

bclozel added a commit that referenced this issue Aug 15, 2019
This commit moves the compile configuration from the Gradle DSL to a
convention. This configuration is not changing often, and we're using
that opportunity to make the Java source compatibility a project
property so as to easily recent JDKs this on the command line.

See gh-23282
bclozel added a commit that referenced this issue Aug 16, 2019
bclozel added a commit that referenced this issue Aug 16, 2019
This commit removes unused parts of the Gradle build:
* Gradle wrapper customization which should not be needed in recent
versions of Gradle (or can be replaced with options in the
gradle.properties file)
* the branch strategy configuration

See gh-23282
bclozel added a commit that referenced this issue Aug 19, 2019
This commit removes JDiff from the Spring Framework build and instead,
adds a Gradle plugin that configure JApiCmp tasks on the framework
modules.

Fixes gh-22942
See gh-23282
bclozel added a commit that referenced this issue Aug 19, 2019
This commit moves the dependency management and test source files
related to integration tests to a dedicated module.
This allows us to focus the root project on building the Spring
Framework.

See gh-23282
bclozel added a commit that referenced this issue Aug 20, 2019
This commit reorganizes tasks and scripts in the build to only apply
them where they're needed. We're considering here 3 "types" of projects
in our build:
* the root project, handling documentation, publishing, etc
* framework modules (a project that's published as a spring artifact)
* internal modules, such as the BOM, our coroutines support and our
integration-tests

With this change, we're strealining the project configuration for all
spring modules and only applying plugins when needed (typically our
kotlin support).

See gh-23282
bclozel added a commit that referenced this issue Aug 20, 2019
This commit ensures that the spring-framework-bom project is ignored
from the global configuration block. If not, many conventions and
dependencies are added to it and add noise to the published BOM.

See gh-23282
bclozel added a commit that referenced this issue Aug 20, 2019
Prior to this commit, the reference documentation build with asciidoctor
would get the common "spring-docs-resources" as a dependency and then
use it when generating the docs.

As seen in #23124, this can cause problems since we'd like to
consistently resolve our dependencies. In this case, the
"spring-doc-resources" archive is not published on maven central since
it's not officially supported by the Spring team as an open source
project.

This commit updates the reference documentation build to get this
archive as a simple download task and avoid resolving it as a
dependency.

See gh-23282
bclozel added a commit that referenced this issue Aug 21, 2019
Prior to this commit, the Spring Framework build would mix proper
framework modules (spring-* modules published to maven central) and
internal modules such as:
* "spring-framework-bom" (which publishes the Framework BOM with all
modules)
* "spring-core-coroutines" which is an internal modules for Kotlin
compilation only

This commit renames these modules so that they don't start with
"spring-*"; we're also moving the "kotlin-coroutines" module under
"spring-core", since it's merged in the resulting JAR.

See gh-23282
bclozel added a commit that referenced this issue Aug 21, 2019
This commit configures the Gradle Download plugin that's used a build
step when generating the reference documentation. Here we're making sure
that the task is caching and reusing the resource if it's been
downloaded already.

See gh-23282
bclozel added a commit that referenced this issue Aug 21, 2019
Prior to this commit, the build would use a custom task to create a BOM
and manually include/exclude/customize dependencies. It would also use
the "maven" plugin to customize the POM before publication.

This commit now uses a Gradle Java Platform for publishing the Spring
Framework BOM. We're also now using the "maven-publish" plugin to
prepare and customize publications.

This commit also tells the artifactory plugin (which is currently
applied only on the CI) not to publish internal modules.

See gh-23282
bclozel added a commit that referenced this issue Aug 21, 2019
This commit switches to the default publication name considered by the
artifactory plugin when it comes to publishing artifacts to the
artifactory repository.

See gh-23282
bclozel added a commit that referenced this issue Aug 21, 2019
Instead of relying on the CI server to apply and configure this plugin,
this commit does it directly in the Spring Framework build.
This allows us to take full control over which projects are published
and how.

See gh-23282
bclozel added a commit that referenced this issue Aug 22, 2019
This commit ensures that Gradle publications are using resolved
dependency versions for Maven publications (i.e. POMs). This is useful
since we're using the Spring dependency management plugin and we can't
rely on declared dependency versions only.

See gh-23282
larsgrefer added a commit to larsgrefer/spring-framework that referenced this issue Aug 22, 2019
@bclozel
Copy link
Member Author

bclozel commented Aug 23, 2019

Our Gradle build has been significantly refactored and published artifacts are stable.
We'll create new issues for other build changes.

@bclozel bclozel closed this as completed Aug 23, 2019
bclozel added a commit that referenced this issue Aug 27, 2019
Since gh-23282, our CI does not apply automatically the Artifactory
Gradle plugin during our build and we're now applying it "manually".
This commit backports this change since the build configuration is
shared between branches.

See gh-23282
@sbrannen
Copy link
Member

Related to #23550

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
Development

No branches or pull requests

3 participants