Skip to content
This repository has been archived by the owner on Aug 19, 2020. It is now read-only.

Use of project property as opt-in toggle for build caching of scripts is cumbersome in practice #1032

Closed
ldaley opened this issue Aug 12, 2018 · 8 comments

Comments

@ldaley
Copy link

ldaley commented Aug 12, 2018

Expected Behavior

A convenient and idiomatic way to opt-in to this functionality on a per user or per project basis.

Current Behavior

I can either explicitly add the project property to the invocation, which is cumbersome, or use some obtuse ORG_GRADLE_PROJECT_org.gradle.kotlin.dsl.caching.buildcache=true env var.

I should have the same options for enabling this as I do for build cache (probably minus the first class invocation option). That is, I should be able to put something in my «GRADLE_USER_HOME»/gradle.properties or the same file in the project root, as I can with org.gradle.caching.

@eskatos
Copy link
Member

eskatos commented Aug 13, 2018

The following in a gradle.properties file should work already:

org.gradle.project.org.gradle.kotlin.dsl.caching.buildcache=true

@ldaley
Copy link
Author

ldaley commented Aug 13, 2018

@eskatos if you can turn that should into does, then we can close this.

It's a bit cumbersome in its length, but the mechanics are right.

@eskatos
Copy link
Member

eskatos commented Aug 13, 2018

@ldaley it doesn't work because the property is checked on StartParameter which doesn't resolve properties from gradle.properties

https://github.com/gradle/kotlin-dsl/blob/b9cb04969dab25fdf549de5b32231fc6326883f4/subprojects/provider/src/main/kotlin/org/gradle/kotlin/dsl/cache/BuildServices.kt#L49-L50

This needs to be fixed.

@eskatos
Copy link
Member

eskatos commented Aug 13, 2018

We should not make that flag a public build option. Its purpose is to test drive script compilation caching during the RC phase. The plan is to get rid of it and simply tie enabling script compilation caching to enabling the build cache.

Making it a project property means one could enable/disable it per project, which is not the desired behavior. Moreover, the script compilation build cache service is build scoped and doesn't have enough context to honor the flag on each invocation.

I think that using a system property instead would better capture the intent.

I confirmed that it can also be set in a gradle.properties file, e.g.

systemProp.org.gradle.kotlin.dsl.caching.buildcache=true

and is then available when we need it to setup the script compilation cache.

@bamboo wdyt?

@bamboo
Copy link
Member

bamboo commented Aug 13, 2018

Making it a project property means one could enable/disable it per project, which is not the desired behavior.

The property was modelled after the org.gradle.caching project property but I missed that reading the property from StartParameter wouldn't honour gradle.properties.

ScriptCache could read the property from the target and then it would.

@eskatos It seems more consistent with org.gradle.caching to keep it as a project property and fix the behaviour by reading the property from the target project.

@eskatos
Copy link
Member

eskatos commented Aug 13, 2018

IIUC org.gradle.caching is a Gradle property bound to a build option, not a Project property.

@eskatos
Copy link
Member

eskatos commented Aug 14, 2018

Follow up PR gradle/gradle#6378 has been merged
Closing

@eskatos eskatos closed this as completed Aug 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants