-
-
Notifications
You must be signed in to change notification settings - Fork 120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for Gradle configuration cache #111
Comments
I think we might have to drop support for older versions of gradle for this.
|
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. |
Thank you so much for the information @eskatos! The dance performed in I'll see if I can get a branch going this week |
Started working on the branch Everything worked fine up until I switched from |
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. |
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 |
What's up with |
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 See: NodeSetupTask.kt |
Hi @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. |
Thanks for your quick answer. Sounds good Hope your dog is fine now! 🐶 |
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 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. |
@deepy I managed to fix the unit tests. I also tried to mock the 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! |
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 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! |
With the state of things I think we should be able to merge #128 to master and then fix the |
I could remove many usages of We use a repository to download a file from a Ivy repository. For that we currently use @eskatos would you have any tip to do that from a task without using the |
@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. |
@deepy it sounds like we can close this issue, right? |
Yeah, no reports of any issues so far either :-) |
|
gradle/gradle#13490
Gradle 6.6 is introducing configuration caching
Helpful resources
The text was updated successfully, but these errors were encountered: