solr icon indicating copy to clipboard operation
solr copied to clipboard

SOLR-17406: Introduce Version Catalogs

Open malliaridis opened this issue 1 year ago • 10 comments

https://issues.apache.org/jira/browse/SOLR-17406

Description

With the version catalogs all versions and references to them are managed in a single file and updated from there.

Solution

This PR introduces version catalogs and removes the palantir consistent versions plugin, since it is no longer needed. Additionally, the build module(s) are restructured and further changes from https://github.com/apache/lucene/pull/13484 were applied to reflect the same behavior.

For syncing versions a new gradle file (dependencies.gradle) is introduced, where the version of a dependency (either transitive or non-transitive) is set via constraints.

This PR contains the changes from https://github.com/apache/solr/pull/2646 without the dependency updates made, that will be handled in separate PR(s).

Closes #2646.

Tests

Minor adjustments have been made to some test files that fix some references (due to restructuring of build files), and besides that spotless and tidy are added to run on build files too.

Checklist

Please review the following and check all that apply:

  • [X] I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • [X] I have created a Jira issue and added the issue ID to my pull request title.
  • [X] 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)
  • [X] I have developed this patch against the main branch.
  • [X] I have run ./gradlew check.
  • [ ] I have added tests for my changes.
  • [X] I have added documentation for the Reference Guide

malliaridis avatar Sep 11 '24 14:09 malliaridis

spotless and tidy are added to run on build files too

Awesome!

dsmiley avatar Sep 25 '24 05:09 dsmiley

I checked out this branch and did gw assemble then renamed solr/packaging/build/solr-10.0.0-SNAPSHOT/ to have an "-after" suffix. Then I checked out a recent main (before cross-DC addition) and did gw assemble. Then compared: diff -qr solr-10.0.0-SNAPSHOT/ solr-10.0.0-SNAPSHOT-after/

There were many differences. Just one example: the opentelemetry module didn't have netty libs but now it does. Surely some differences are because main has moved along since when you started.

Can you please re-sync with main and repeat this exercise so we can hopefully get to no differences, or at least those differences that exist needs to be spelled out specifically and why. Then lets merge this; I'll approve.

dsmiley avatar Sep 28 '24 20:09 dsmiley

@dsmiley thanks for the instructions, I didn't know before how to check the builds for consistency.

I am using now IntelliJ for a better overview and I see that the configurations that were included in gradle/validation/dependencies.gradle are not sufficient. I have now extended the list locally to the following:

def consolidatedConfigurations = project.configurations.matching {
    it.name in [
            "annotationProcessor",
            "api",
            "apiElements",
            "compileClasspath",
            "compileOnly",
            "libExt",
            "implementation",
            "runtimeClasspath",
            "runtimeElements",
            "runtimeOnly",
            "runtimeLibs",
            "serverLib",
            "testCompileClasspath",
            "testCompileOnly",
            "testImplementation",
            "testRuntimeClasspath",
            "testRuntimeOnly"
    ]
}

@dweiss any recommendations how to constellate this list of configurations?

Now, besides a few new libraries included in some modules that I need to figure out where they are coming from, the main differences are in netty libraries (consistently now 2.0.65.Final), and google-api-grpc-proto libraries (consistently now 2.42.0).

The affected modules by these upgrades are:

  • modules/gcs-modules
  • modules/opentelemetry
  • modules/s3-repository
  • server/solr-webapp

I will pick up work again and try to solve this before the next review.

malliaridis avatar Sep 30 '24 15:09 malliaridis

spotless and tidy are added to run on build files too

Awesome!

It's cool, but I really would have preferred to have those in a separate PR. It clutters this PR quite a lot.

henrik242 avatar Oct 01 '24 12:10 henrik242

It's cool, but I really would have preferred to have those in a separate PR. It clutters this PR quite a lot.

I totally agree with you. I should have tried to separate these two changes. The reason I ended up doing it in this PR is the reference I used, which was https://github.com/apache/lucene/pull/13484. I couldn't separate one from the other, so I decided to do it in a similar way.

About the configurations and dependencies: I am still not very sure about the configurations that we should sync the dependencies / include in the consolidated configurations list, so I ended up with the below state.

Updated dependencies

  • hamcrest:hamcrest-core is replaced with hamcrest:hamcrest - module was renamed and included twice with different names before
  • netty-tcnative-broingssl-static was updated through version syncing from 2.0.61.Final to 2.0.65.Final
  • netty-tcnative-classes was updated through version syncing from 2.0.61.Final to 2.0.65.Final
  • proto-google-common-protos was updated through version syncing from 2.41.0 to 2.42.0

module: gcs-repository

  • uses the updated proto-google-common-protos library (version 2.42.0)

module: opentelemetry

  • uses the updated proto-google-common-protos library (version 2.42.0)

server/solr-webapp

  • netty-tcnative-broingssl-static was updated through version syncing from 2.0.61.Final to 2.0.65.Final
  • netty-tcnative-classes was updated through version syncing from 2.0.61.Final to 2.0.65.Final

@dsmiley the diff output should now show only the above changes if I have done everything correct.

malliaridis avatar Oct 01 '24 16:10 malliaridis

Thanks Christos.

I suggest a separate PR for the version bumping listed. It'd also be nice to backport that to 9x.

May I suggest that we have a separate PR that merely formats our gradle files? Could be just a quick automated thing. It'll make this PR much easier to read and moreover, over time, we'll see what this change here did, not obscured by formatting. A sweeping formatting change like this would not go to 9x.

Finally, this PR here is then just version catalogs refactoring, and only happens in v10.

dsmiley avatar Oct 01 '24 18:10 dsmiley

BTW if you are worried about how to update this PR if those other things merge; I could figure that out. The end-state will be no different, so it should be possible to do a bit of git magic on the merge from main into this to say that this side (the PR) "wins" for all differing files.

dsmiley avatar Oct 01 '24 19:10 dsmiley

Thanks for the input and considering handling non-relevant changes in separate PRs. I feel more at ease if we do so, so I will continue with that and create these PRs as suggested before continuing work on this.

Changing this to a draft PR for now.

malliaridis avatar Oct 02 '24 11:10 malliaridis

I think the next step is just some dependency version alignment?

dsmiley avatar Oct 14 '24 21:10 dsmiley

I think it would be a good idea to first resolve the current solrbot PRs before aligning the versions. I am already looking into some of the PRs to see how they can be resolved.

malliaridis avatar Oct 16 '24 19:10 malliaridis

The current state should differ now only in hamcrest-core, which is being merged with hamcrest in the dependencies.gradle.

malliaridis avatar Nov 09 '24 17:11 malliaridis

Thanks @gus-asf for your review and input. Your questions are pointing out some of our upcoming TODOs. I'll leave some comments about the current state on each of your comments, hopefully clarifying and answering your questions.

FYI the documentation will be updated periodically in the near future as we work to clean up and improve specific processes.

malliaridis avatar Nov 14 '24 05:11 malliaridis

I have updated the documentation to hopefully be clearer, but it is still not perfect yet. I have created https://issues.apache.org/jira/browse/SOLR-17563 to address outdated documentation.

malliaridis avatar Nov 15 '24 17:11 malliaridis