Replace HostPlatform with Gradle BuildPlatform service#11815
Replace HostPlatform with Gradle BuildPlatform service#11815AlexeyKuznetsov-DD wants to merge 2 commits into
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
BuildPlatform relies on Gradle's native services, which are unavailable when configuring tasks under ProjectBuilder in unit tests. Evaluating project.isLinuxArm64() eagerly during test-task configuration made TestJvmConstraintsPluginTest fail with NativeIntegrationUnavailableException. Defer the check to execution time via the existing lazy additionalCondition on ProvideJvmArgsOnJvmLauncherVersion, keeping it out of the configuration path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bric3
left a comment
There was a problem hiding this comment.
I'm not quite convinced this gives any useful benefit. It appears to trade a simple property check in HostPlatform with a Gradle service, which means it needs to be queried from a project service.
Also, I would avoid making Gradle Groovy script import BuildPlatformExtensionsKt.
@bric3
Since you have a better Gradle expertise, I will let you decide on this PR. |
But this functionality is incubating, and currently accessed through an internal service. If we didn't had any other option that would be OK, but we do.
Clearly. Adn probably fine in Kotlin as well, but I feel they are weird in groovy dsl, but that's mostly csometic. And as you said at some point it will be migrated to kotlin. One thing that comes to mind is I need to check is whether accessing the
In the end that's precisely my point, I don't see any useful benefit. That said I don't see any strong point against either (maybe the possible issue with configuration cache, e.g. if the method is used at the wrong place and happens to inject the project instance, but I'm unsure). |
|
Closed PR, as there no clear benefit from the change. We can reconsider later. |
What Does This Do
Replaces the custom
HostPlatformhelper (which reados.name/os.archsystem properties) with Gradle'sBuildPlatform, obtained from the injectableCurrentBuildPlatformservice viaserviceOf.Motivation
Follow-up to a review note on #11364: prefer Gradle's platform detection over hand-rolled system-property string matching.
Additional Notes
Helper functions make this PR looks better.