Maintaining reference to System.getProperties() in PlatformHelper breaks configuration cache in Gradle 7.4-rc1

[nhoughto]

I get this error when trying to run nodeSetup with --configuration-cache enabled:

org.gradle.api.GradleException: Could not load the value of field `props` of `com.github.gradle.node.util.PlatformHelper` bean found in field `platformHelper` of `com.github.gradle.node.variant.VariantComputer` bean found in field `variantComputer` of task `:g

Based on https://docs.gradle.org/7.4-rc-1/userguide/configuration_cache.html#config_cache:troubleshooting it appears to be related to maintaining a reference to System.getProperties() that is different across gradle executions, and also has some special wrapping in 7.4+ to auto-detect system property usages.

Not keeping a reference to it and looking it up each time might be the answer? 🤔

[nhoughto]

Happy to open simple PR to fix if ok with it

[deepy]

Oh this one is a little tricky, we're using PlatformHelper as a shared place where we look into a bunch of things related to platform.

The majority of which I guess would generally be safe, os.arch for example isn't likely to change and neither is the result of uname (except there's a discussion in a PR where exactly that is happening)

But judging by the error I think we might need to remove the platformHelper from the VariantComputer class and make sure it's only queried during configuration
(pretty sure we're looking it up during both during configuration and during execution)

And I think NpmProxy.kt might have the same issue

[nhoughto]

ah ok I won't bother with a PR then, more complex than it appears (as always 😬).

My understanding of the problem is that configuration caching essentially serialises objects created during configuration phase and rehydrates them later to skip re-running configuration phase. So any object that is serialised must support certain behaviours and work within some contraints. One of those constraints is not holding references to special objects / objects that will change after rehydration, in this case that is System.getProperties(). You can't hold a reference (and thus serialise) system properties, you have to access it directly otherwise configuration caching will fail like this.

You could of course look up the values you need out of system props and store those values directly (and flag them as task and configuration inputs), the problem is the Properties object itself.

[deepy]

Yeah, though it might not be that difficult to rework it so that we query all these values during configuration but it would involve touching quite a few places (but luckily just a few lines of test code)

If you (or anyone else) wants to give it a shot I think YarnTask might be the easiest place to start (though the one with the least amount of test coverage)
And this is one of those features where breaking changes are probably ok

[nhoughto]

I'll try a PR, not much else on atm.

[nhoughto]

Had a crack, more changes than I wanted but can refine
#211

[deepy]

3.2.0 is released and this is fixed, thank you for the PR! 🎉