stevesaliman / gradle-properties-plugin

Gradle plugin to simplify loading project properties from external environment specific files
Apache License 2.0
192 stars 28 forks source link

Unable to use Configuration Cache on Gradle 6.8.3 #39

Open mochadwi opened 3 years ago

mochadwi commented 3 years ago
Screen Shot 2021-03-01 at 9 59 20 PM

Fixes/solutions:

  1. https://docs.gradle.org/6.8.3/userguide/configuration_cache.html#config_cache:troubleshooting
  2. https://docs.gradle.org/6.8.3/userguide/configuration_cache.html#config_cache:requirements:undeclared_sys_prop_read
Vampire commented 3 years ago

I fear there are way bigger problems. If you use configuration cache you also have to declare the properties you use by using a provider. But at least with the current version this does not work. If you have a gradle-local.properties with content foo=bar and then do

val foo: String? by project
println(foo)
println(providers.gradleProperty("foo").forUseAtConfigurationTime().orNull)

the output is

bar
null

Currently just using a Gradle property does not yet throw a formal error, but this will hopefully change and the docs already forbid the plain usage. And if you do not stick to it, you will not get a cache miss if you change the variable.

stevesaliman commented 3 years ago

I agree that there are going to be bigger problems getting this plugin to work with configuration caching.

I started looking into the specific issue mentioned in the OP. Gradle documentation states that instead of getting a system property directly, we should get it through providers.systemProperty(someName) instead. The problem is, this plugin doesn't fetch any particular property - it iterates through all of them to see what was set via system properties. Gradle doesn't seem to currently have an exposed method do the same thing.

This is a challenge because the way this plugin works is by attempting to re-create what Gradle does during the configuration phase, but with a couple of extra steps to accommodate the environment or user specific files.

I would imagine that Gradle needs to be able to load system properties in a way that is safe for the configuration cache, but it will take some work to figure out what that way is.

The plugin will almost certainly have the same problem with environment variables.

Vampire commented 2 years ago

Starting with 7.4, no special syntax is needed anymore for system property, environment variable, and gradle property reads. The accesses are simply instrumented by bytecode manipulation of the build script class path.

So the actual error in the OP is void starting with 7.4-rc-1.

But you maybe need to declare the plugin-specific files as input to the configuration cache, by using constructs like providers.fileContents(layout.projectDirectory.file("gradle-local.properties")).asText.get() to get their contents, then they are tracked as input and the configuration cache is invalidated if anything in the file changes.

The problem that providers.gradleProperty("...") will not get the properties set by this plugin remains though while the extra properties are used to overlay the Gradle properties.

Vampire commented 2 years ago

And now you also have the possibility to request only system properties and environment variables with a given prefix in a configuration cache safe way so that not all system properties and environment variables are considered configuration cache inputs.

vlad20012 commented 1 year ago

Configuration Cache seems to work out-of-the box without any warnings in Gradle 8.3 now?

Vampire commented 1 year ago

Without warning, maybe. But still not usable, as it does not use the special methods to access system properties and so on with a specific prefix, now all system properties and environment variables are recognized as input due to the automatic input recognition, so just applying this plugin makes the cofiguration cache never reuse a cache entry and thus only waste time and never save any.