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

Optimize dependency resolution via platform module #2915

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

malliaridis
Copy link
Contributor

Description

After the upgrade to version catalogs there were changes made in the way our project resolves the dependencies. The new "flow introduced has proven to be quite confusing and problematic.

Solution

This PR aims to solve the issues by introducing a platform module that holds all the constraints with the libraries included in our version catalog. This platform module is added to the root modules solr:api and solr.core as via the api configuration, so that it is inherited across all the other modules.

This way, it is no longer necessary to add constraints to the gradle/validation/dependencies.gradle, but it is sufficient to include the conflicting dependency in the version catalog.

Tests

These changes can be manually tested by comparing the libraries included in the output of gradlew assemble. There should not be any differences.

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Reference Guide

The platform module includes all dependencies from the
version catalog as constraints and is added to the root
modules as API for transitive inheritance of the constraints.
Copy link
Contributor

@dsmiley dsmiley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR removes many lines, and adds barely little. Wow! Yay? But AFAICT the solution is to effectively increase the scope of dependencies across the project, which was previously of lesser scopes. SolrJ depends on "api" module too; so does this mean we unwittingly add dependencies for SolrJ users which they previously didn't have?

An aside: I've wished we had some build validation on the output of JARs in the final assembly. I've expressed this before and finally wrote a couple JIRA tickets: https://issues.apache.org/jira/browse/SOLR-17602 https://issues.apache.org/jira/browse/SOLR-17603

@malliaridis
Copy link
Contributor Author

SolrJ depends on "api" module too; so does this mean we unwittingly add dependencies for SolrJ users which they previously didn't have?

We have to distinguish between constraints and actual dependencies. This new platform module adds constraints to all modules that depend on it, either directly or via transitive dependencies. And the constraints currently applied are all the dependency versions currently present in the version catalog. So SolrJ only inherits the constraints, "forcing it" to use the same dependency versions as the rest of the project for the dependencies it has.

What I am not sure and need to verify is, do we set this way the constraints for any consumer of SolrJ as well, even for dependencies SolrJ does not include? If so, this would probably not be optimal.

If that is the case, this could probably be resolved by including a different version catalog for SolrJ, as Gradle allows having multiple version catalogs.

It is also worth noting, that the previous solution (if I am correct) was applying the constraints to all module configurations as well, but only if there was any conflict in any of the modules and we had to add explicitly a rule. So SolrJ inherited constraints that were not necessarily part of it anyway. So this solution should not have a large impact, but it is still under investigation.

@dsmiley
Copy link
Contributor

dsmiley commented Dec 28, 2024

Ah; thanks for making the important distinction between constraints and dependencies. Your Gradle knowledge is impressive!

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

Successfully merging this pull request may close these issues.

2 participants