SOLR-17321: Bump minimum required Java version to 21
https://issues.apache.org/jira/browse/SOLR-17321
We are upgrading the minimum Java version for Solr main branch to 21. However, at the same, It has been suggested to be not so aggressive with SolrJ (and thus solr-api, a dependency) Java version – setting it to 17.
Depreciated API
- AccessController [Deprecated since 17 and marked for removal]
- AccessControlContext [Deprecated since 17 and marked for removal]
- java.net.URL ctors [Deprecated since 20]
- SecurityManager [Deprecated since 17 and marked for removal]
- Thread#getId() [Deprecated in Java 19]
To not make any extra code changes below annotation were used:
@SuppressForbidden(reason = "<custom message>") //Still need to decide what the message should be for all. Thinking of adding "Deprecated in Java <version>"
@SuppressWarnings("removal")
However, at few places, for demonstration purpose, URL has been created via new suggested way instead of using the ctors:
url = new URL(urlStr);
url = URI.create(urlStr).toURL();
Or,
URL solrURL = new URI(solrUrl).toURL();
Also, Bump up the ecj version to 3.37 and ca.cutterslade.analyze to 1.9.2.
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)
- [x] I have developed this patch against the
mainbranch. - [x] I have run
./gradlew check. - [] I have added tests for my changes.
- [ ] I have added documentation for the Reference Guide
For now two things pending
- Set different Java level for SolrJ and Solr. Currently both are 21.
- And, another one is to change the test-via-crave.yml file to run the build with Java 21.
Feel free to push changes to this branch.
@SuppressForbidden(reason = "
") //Still need to decide what the message should be for all. Thinking of adding "Deprecated in Java "
Ideally those are addressed for real, otherwise they will continue to linger. Could be a separate JIRA/PR.
RE URL: If you have IntelliJ Ultimate edition (not sure if Community), you could do "Structural Replace" to find & replace systematically everywhere.
Like always, Thanks for the air support. "Structural Replace" is a Game Changer!
RE URL: If you have IntelliJ Ultimate edition (not sure if Community), you could do "Structural Replace" to find & replace systematically everywhere.
Like always, Thanks for the air support. "Structural Replace" is a Game Changer!
I need to learn that! Looks amazing!
Would it be too much to ask to separate all this URL conversion stuff to a separate PR, ultimately being its own commit focused on exactly that? Needn't be another JIRA; needn't touch CHANGES.txt in that commit. If I had to do this, I'd start with the trick of taking a GitHub PR URL and adding the ".patch" (or maybe it's ".diff" for a different format) and then copy-paste into IntellIJ "Apply patch from Clipboard" and selectively exclude stuff unrelated at that point (or after).
So SuppressForbidden is okay for now, and later on (Just after this one!) deal with removing it in a different PR?
I suggest flipping the order. Prep the code for Java 21 compatibility first so that we don't go adding SupressForbidden to many classes to only then fix the issue later. i.e. visit these classes once not twice. You know exactly how to fix it and have even starting doing so; it's not a tricky matter. Again, IntelliJ will do much of the work.
On Wed, Jun 5, 2024 at 8:55 AM Sanjay Dutt @.***> wrote:
Would it be too much to ask to separate all this URL conversion stuff to a separate PR, ultimately being its own commit focused on exactly that? Needn't be another JIRA; needn't touch CHANGES.txt in that commit. If I had to do this, I'd start with the trick of taking a GitHub PR URL and adding the ".patch" (or maybe it's ".diff" for a different format) and then copy-paste into IntellIJ "Apply patch from Clipboard" and selectively exclude stuff unrelated at that point (or after).
So SuppressForbidden is okay for now, and later on (Just after this one!) deal with removing it in a different PR?
— Reply to this email directly, view it on GitHub https://github.com/apache/solr/pull/2497#issuecomment-2149791390, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC4DTZJTS2ICJN2TM6AJZLZF4C77AVCNFSM6AAAAABIY26TRCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBZG44TCMZZGA . You are receiving this because your review was requested.Message ID: @.***>
Test case failing, on main branch as well!
./gradlew :solr:core:test --tests "org.apache.solr.search.function.TestDenseVectorFunctionQuery.floatFieldVectors_shouldReturnFloatSimilarity" -Ptests.jvms=96 "-Ptests.jvmargs=-XX:TieredStopAtLevel=1 -XX:+UseParallelGC -XX:ActiveProcessorCount=1 -XX:ReservedCodeCacheSize=120m" -Ptests.seed=8F0CED8E85B304BD -Ptests.timeoutSuite=600000! -Ptests.file.encoding=US-ASCII
References to JavaVersion (a Gradle class) like >= JavaVersion.VERSION_16 should be reviewed to see if we can make changes to remove conditions that will no longer vary.
Searching for "Java 17" or "JDK 17" or similar searches or other versions can reveal stuff where maybe a change is needed.
Setting different Java version for main and SolrJ resulting in error. Currently, :solr:api:ecjLintTest task is failing and below is the error.
> Task :solr:api:ecjLintTest
----------
1. ERROR in /Users/sanjaydutt/Documents/Solr/solr-main/solr/solr/api/src/test/org/apache/solr/util/TestSolrVersion.java (at line 19)
import org.apache.solr.SolrTestCase;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The import org.apache.solr.SolrTestCase cannot be resolved
----------
2. ERROR in /Users/sanjaydutt/Documents/Solr/solr-main/solr/solr/api/src/test/org/apache/solr/util/TestSolrVersion.java (at line 23)
public class TestSolrVersion extends SolrTestCase {
^^^^^^^^^^^^
SolrTestCase cannot be resolved to a type
The api test cases not able to access the class, SolrTestClass, from the main branch. Further, I logged the java version that has been used to configure SolrJ via adding println statements.
ecj-lint.gradle
allprojects {
plugins.withType(JavaPlugin) {
// Create a [sourceSetName]EcjLint task for each source set
// with a non-empty java.srcDirs. These tasks are then
// attached to project's "ecjLint" task.
println "The project name is " + project + " and the sourceCompatibility is " + project.sourceCompatibility
def lintTasks = sourceSets.collect { sourceSet ->
def srcDirs = sourceSet.java.srcDirs.findAll { dir -> dir.exists() }
tasks.create(sourceSet.getTaskName("ecjLint", null), JavaExec, {task ->
....
...
Output
> Configure project :solr:server
The project name is project ':solr:server' and the sourceCompatibility is 21
> Configure project :solr:core
The project name is project ':solr:core' and the sourceCompatibility is 21
> Configure project :solr:api
The project name is project ':solr:api' and the sourceCompatibility is 17
> Configure project :solr:solrj
The project name is project ':solr:solrj' and the sourceCompatibility is 17
The version has changed for SolrJ and for api as well. However, as @dsmiley pointed out before that test classes for both of these project should point to 21. Still looking for a way How we can achieve that.