Add support for Gradle configuration cache

[deepy]

gradle/gradle#13490

Gradle 6.6 is introducing configuration caching

Helpful resources

• https://medium.com/androiddevelopers/configuration-caching-deep-dive-bcb304698070
• https://docs.gradle.org/nightly/userguide/configuration_cache.html
• https://docs.gradle.org/nightly/userguide/configuration_cache.html#config_cache:requirements:disallowed_types
• https://docs.gradle.org/nightly/userguide/custom_gradle_types.html#service_injection

[deepy]

I think we might have to drop support for older versions of gradle for this.
There's ~630 public repos on GitHub with at least mentions of this plugin, having done a very quick analysis of the gradle wrappers in the projects we've got the following numbers:

  34 6.5.1
 241 6.5
  85 6.4.1
  19 6.4
 113 6.3
  20 6.2.2
   4 6.2.1
  12 6.2
  60 6.1.1
 234 6.1
 160 6.0.1
  16 6.0
  13 5.6.4
   5 5.6.3
 113 5.6.2
   2 5.6.1
   5 5.6
  15 5.5.1
   4 5.5
  18 5.4.1
   6 5.4
   8 5.3
  24 5.2.1
 106 5.1.1
   2 5.1
   6 5.0
   2 4.8.1
   2 4.8
   1 4.4.1
   3 4.4
  12 4.10.3
   2 4.10.2
   5 4.10
   2 4.0
   6 3.5.1
  16 3.5
   4 3.1
   4 2.6

Version	Users	Percentage
Gradle 6	998	72.11%
Gradle 5	327	23.63%
Gradle 4	29	2.10%
Gradle 3	26	1.88%
Gradle 2	4	0.29%
Total	1,384	100%

[eskatos]

It depends (famous answer :) ). With the configuration cache, some things are unsupported, but you can still branch depending on the current Gradle version to provide support.

Looking at the numbers above, targeting Gradle >= 5 .0 could be considered but even with that there are APIs that are only available in >6.x and branching per Gradle version would be mandatory here and there anyway 🤷

The plugin is quite different but this PR michel-kraemer/gradle-download-task#157 made the Download task compatible with the configuration cache without losing compatibility with Gradle >=2.0.

[deepy]

Thank you so much for the information @eskatos!

The dance performed in ProjectApiHelper is pretty much exactly what we need :-)

I'll see if I can get a branch going this week

[deepy]

Started working on the branch 2.x-configuration-cache

Everything worked fine up until I switched from project.exec, I think everything is working but we just need to change mockExec in the tests.
https://github.com/node-gradle/gradle-node-plugin/blob/2.2.4/src/test/groovy/com/moowork/gradle/node/task/AbstractTaskTest.groovy#L33

[eskatos]

Sounds good!

To make sure the support is complete I would make sure that there's at least tests that runs builds with the configuration cache enabled, reusing the cached state, for a clean build, i.e. running all tasks after skipping the configuration phase. In other words, with build cache disabled and configuration cache enabled, run once to store the cache then clean and then run again reusing the cache.

[deepy]

Having looked some at this and tested on an example project I think we can make this work pretty easily on 2.x for the download = false cases.

[eskatos]

What's up with download = true? If you point me to the related part of the code I could take a look

[deepy]

We're adding the repository for downloading node/npm/yarn in the tasks' action, something which would probably be pretty easy to fix but moving it would change functionality enough to be a major change, and the next major is planned to be the kotlin rewrite so my idea is to see how far I can take the 2.x branch, then do the same on master and then finalise the support on master

See: NodeSetupTask.kt

[eskatos]

Hi @deepy,
Any news on a first cut of the support?

[deepy]

Hi @eskatos, unfortunately there were some more things that needed to be fixed in 2.x before this could be added and they're already done on master, so we'll have to do it after 3.x is released.
The release of which was delayed a bit by my dog getting sick and my laptop being in the wrong town, but when 3.x is released this is the top priority.

[eskatos]

Thanks for your quick answer. Sounds good

Hope your dog is fine now! 🐶

[deepy]

I've begun working on this and while I get the integration tests to work all the unit tests fails to capture the invocation of execSpec: NpmInstallTaskTest.groovy#L15

The mock and spy is setup like this: AbstractTaskTest.groovy#L31-L45 and while this works fine with project I can't get this same pattern to work with ExecOperations.
I tried the same pattern but can't find anything to hook onto, I tried retrieving the DefaultProjectApiHelper and changing the execOperations through metaprogramming but with no results.

On the bright side, the only integration tests are failing (so far) are the proxy related ones and I think that might be this laptop.

[bsautel]

@deepy I managed to fix the unit tests.

I also tried to mock the ExecOperations object, but did not success. But I then had another idea that finally worked. I mock the ProjectApiHelper instance that is used by the task. For that, I have to change the field reference, I replace its original value by the mocked one using reflection (the field is read-only in the Kotlin code).

That's not very beautiful, but what we did previously was not beautiful either. That's the same mechanism, but this time it is more visible in each test code since we need to do the trick on the task instance instead of the project one.

I'll try to write some integration tests using the configuration cache in the week-end. I'll let you know.

Have a nice day!

[bsautel]

I enabled the configuration cache in the Kotlin DSL integration test. This test can be seen as a global test since it uses all the available options and all the available task types. But it does not provide a 100% coverage of the plugin, for instance it does not download and install Node.js.

I also added the warning-mode=fail option to all the integration tests to ensure we don't miss any execution warnings.

I tried to enable the configuration cache on all unit tests and some tests failed. I don't know why but I need to investigate that before the release. I'll let you know!

[deepy]

With the state of things I think we should be able to merge #128 to master and then fix the download = true case there, I got sidetracked this weekend but during the week I should have enough time to work on things

[bsautel]

I could remove many usages of project from the nodeSetup task, but there are two usages that I don't know how to remove.

We use a repository to download a file from a Ivy repository. For that we currently use project.dependencies and project.configurations. It sounds like there is no available service to do that. The code is here.

@eskatos would you have any tip to do that from a task without using the project?

[bsautel]

@deepy suggested to move the file download from the task to the plugin code. I did that and it seems to be satisfying. It sounds like we are very close to close this issue.

[bsautel]

@deepy it sounds like we can close this issue, right?

[deepy]

Yeah, no reports of any issues so far either :-)

[nbrugger-tgm]

It seems that configuration cache support broke. I am using 7.1.0 and get the issues mentioned in #326
Layer 8 problem