Re: [PR] Allow easier verification of the Panama Vectorization provider with newer Java versions [lucene]
ChrisHegarty merged PR #13986: URL: https://github.com/apache/lucene/pull/13986 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
Re: [PR] Allow easier verification of the Panama Vectorization provider with newer Java versions [lucene]
ChrisHegarty commented on PR #13986: URL: https://github.com/apache/lucene/pull/13986#issuecomment-2473764740 > > Personally I would prefer a less if/else/default handling using Optional like done in the previous sysprops. > > I'll make that change before merging. Well... I tried, and it was awful! The reason being the need to parseInt the value and catch the exception, which, while possible, just lead to more convoluted code. Easy to refactor later, if at all. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
Re: [PR] Allow easier verification of the Panama Vectorization provider with newer Java versions [lucene]
uschindler commented on PR #13986: URL: https://github.com/apache/lucene/pull/13986#issuecomment-2473792705 > > > Personally I would prefer a less if/else/default handling using Optional like done in the previous sysprops. > > > > > > I'll make that change before merging. > > Well... I tried, and it was awful! The reason being the need to parseInt the value and catch the exception, which, while possible, just lead to more convoluted code. Easy to refactor later, if at all. I would have left the code to bail out if the Sysprop is empty or no valid integer, so not catch exception at all. Anyways, it's good as is. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
Re: [PR] Allow easier verification of the Panama Vectorization provider with newer Java versions [lucene]
ChrisHegarty commented on PR #13986: URL: https://github.com/apache/lucene/pull/13986#issuecomment-2471543719 > Personally I would prefer a less if/else/default handling using Optional like done in the previous sysprops. I'll make that change before merging. > Greetings from Costa Rica! Enjoy!!! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
Re: [PR] Allow easier verification of the Panama Vectorization provider with newer Java versions [lucene]
ChrisHegarty commented on code in PR #13986: URL: https://github.com/apache/lucene/pull/13986#discussion_r1836935485 ## gradle/testing/defaults-tests.gradle: ## @@ -128,7 +128,13 @@ allprojects { jvmArgs '--add-modules', 'jdk.management' // Enable the vector incubator module on supported Java versions: - if (rootProject.vectorIncubatorJavaVersions.contains(rootProject.runtimeJavaVersion)) { + def v = JavaVersion.VERSION_1_1 + def prop = providers.systemProperty("org.apache.lucene.vectorization.upperJavaFeatureVersion") Review Comment: thanks @dweiss, that's a bit cleaner. Done. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
Re: [PR] Allow easier verification of the Panama Vectorization provider with newer Java versions [lucene]
dweiss commented on code in PR #13986: URL: https://github.com/apache/lucene/pull/13986#discussion_r1836911525 ## gradle/testing/defaults-tests.gradle: ## @@ -128,7 +128,13 @@ allprojects { jvmArgs '--add-modules', 'jdk.management' // Enable the vector incubator module on supported Java versions: - if (rootProject.vectorIncubatorJavaVersions.contains(rootProject.runtimeJavaVersion)) { + def v = JavaVersion.VERSION_1_1 + def prop = providers.systemProperty("org.apache.lucene.vectorization.upperJavaFeatureVersion") Review Comment: It'd be probably more consistent to use the propertyOrDefault "function" that we defined globally to allow passing such properties via -P (gradle's project properties) or -D (system properties). You can provide the default as the second argument - look at any existing call of that function. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[PR] Allow easier verification of the Panama Vectorization provider with newer Java versions [lucene]
ChrisHegarty opened a new pull request, #13986: URL: https://github.com/apache/lucene/pull/13986 This commit allows easier verification of the Panama Vectorization provider with newer Java versions. The upper bound Java version of the Vectorization provider is hardcoded to the version that has been tested and is known to work. This is a bit inflexible when experimenting with and verifying newer JDK versions. This change proposes to add a new system property that allows to set the upper bound of the range of Java versions supported. With this change, and the accompanying small gradle change, then one can verify newer JDKs as follows: ``` CI=true; RUNTIME_JAVA_HOME=/Users/chegar/binaries/jdk-24.jdk-ea-b23/Contents/Home ./gradlew :lucene:core:test -Dorg.apache.lucene.vectorization.upperJavaFeatureVersion=24 ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org