Support Up-to-date checks (AKA Incremental Build) in Gradle plugin #1133

anatoliy-balakirev · 2019-07-08T16:33:20Z

Enhancement

Gradle plugin doesn't seem to support incremental build => executing generateContractTests task every time, which is slowing down execution of completely not relevant unit tests significantly.

Here is some sample project, I created to simulate our setup, where we have a lot of contract tests (we are using Pact jsons as an input):
https://github.com/anatoliy-balakirev/cloud.contract.demo

Those contract tests in the sample project are not really useful, I just wanted to show the case, where there is a project with a lot of them (in real project we have much more, actually).

So if you get into that project and execute following command a couple of times (it's just a unit test):
./gradlew :test --tests "com.example.demo.service.UserServiceImplTest" --scan

You'll see that generateContractTests is executed every time and takes a couple of seconds even though it's not needed for this particular unit test and its input / output is not changed between runs.
Here is an example of such scan:
https://scans.gradle.com/s/qy3gtncjmopdo/performance/execution
https://scans.gradle.com/s/l2em6bwohe54c/performance/execution

As you can see, there it took almost 2.5 seconds to do something, which is not needed anyway (in our real project it takes up to 10 seconds).

Would be great if plugin could utilise incremental build (check here: https://docs.gradle.org/current/userguide/more_about_tasks.html#sec:up_to_date_checks) and skip tasks if they are up to date.

The text was updated successfully, but these errors were encountered:

marcingrzejszczak · 2019-07-12T10:12:10Z

Great idea @anatoliy-balakirev ! Care for a PR? :)

fixes gh-1133

marcingrzejszczak · 2019-08-01T12:03:12Z

Sorry, I seem too stupid to work with Gradle. What I thought that is working, is not. I will not re-attempt to implement this feature in the upcoming time. If anyone is a Gradle expert, please feel free to try to tackle this problem

shanman190 · 2019-08-02T00:17:03Z

@marcingrzejszczak, what's the minimum Gradle version you all are supporting/building against with Hoxton?

marcingrzejszczak · 2019-08-05T06:15:15Z

We're building against 5.5

shanman190 · 2019-08-05T17:50:16Z

Also maintaining a 5.0 compatibility like the Spring Boot Gradle plugin?

marcingrzejszczak · 2019-08-05T18:30:30Z

I mean assuming that gradle 5.5 is compatible with gradle 5.0 everything should be fine.

shanman190 · 2019-08-05T22:13:34Z

To a point, yes. Prime example of where this matters is in the 4.x line. As of 4.8, if memory serves, is where Gradle introduces task configuration avoidance. So in those terms, if we were base lining against a version before that point then the task configuration avoidance API would be off limits. I haven't had a chance to look in depth through the Gradle 5.x API yet, so I'm not sure if Gradle 5.x has any of these types of features that have been introduced during it's lifecycle, but wanted to make sure of the baseline version to ensure feature compatibility.

anatoliy-balakirev · 2019-08-07T15:31:50Z

Hi. Sorry, was quite busy on something else and then was on vacation.
I'm also not so familiar with gradle plugins, but I checked what @marcingrzejszczak did here:
3197241

And it seems to be fine, except for one small thing, which is mentioned here:
https://docs.gradle.org/current/userguide/more_about_tasks.html#sec:task_input_output_annotations

Annotations must be placed on getters or on Groovy properties. Annotations placed on setters, or on a Java field without a corresponding annotated getter are ignored.

So when I adjusted ContractsCopyTask to move @InputDirectory from File input to:

	@InputDirectory
	private File getInput() {

and output to:

	@CompileDynamic
	@OutputDirectory
	private File getOutputContractsFolder() {

It all started to work as expected. I've also modified my sample project to make it easier to use snapshot builds there, so you can use it to check this on your own. So executing my original command with snapshot plugin now gives me this result:
https://scans.gradle.com/s/777u3uwijnjro/performance/execution

11 tasks in 1 project
:compileTestJavaUP-TO-DATE	0.574s	 	 
:compileJavaUP-TO-DATE	0.537s	 	 
:testUP-TO-DATE	0.076s	 	 
:processTestResourcesUP-TO-DATE	0.017s	 	 
:copyContractsUP-TO-DATE	0.005s	 	 
:compileTestGroovyNO-SOURCE	0.002s	 	 
:processResourcesUP-TO-DATE	0.002s	 	 
:compileGroovyNO-SOURCE	0.001s	 	 
:generateContractTestsUP-TO-DATE	0.001s	 	 
:classesUP-TO-DATE	0.000s	 	 
:testClassesUP-TO-DATE	0.000s

Which is exactly what we needed! :)

@marcingrzejszczak do you want to proceed with your commit, applying this small fix?

marcingrzejszczak · 2019-08-07T15:42:34Z

Yeah please do!

I've completely haven't noticed that info about the annotations

anatoliy-balakirev · 2019-08-07T15:49:46Z

Actually, playing with that further it looks like generateContractTests is now always UP-TO-DATE even if I change input for copyContracts one (which is correctly restarted in that case). Will have to check it further.

marcingrzejszczak · 2019-08-07T15:50:23Z

I mean please feel free to apply a pr, revert my commit that reverted the fix and apply your changes :)

anatoliy-balakirev · 2019-08-08T19:12:27Z

Update on this: I worked on it today and it turned out to be not so easy. There is this global valiable ContractVerifierConfigProperties, which is passed around between tasks, mutated here and there and used as an input for one and output for another. Quite some refactoring is required there, to clean it up and make it more clear what is an input and what is an output to each task. I'm on it.

marcingrzejszczak · 2019-08-08T19:16:29Z

Yeah, that was exactly the problem :)

anatoliy-balakirev · 2019-08-26T15:26:10Z

@marcingrzejszczak I guess they will be downloaded / used the first time and then for all subsequent runs described problem will be the case, right? Do you have some sample config to reproduce this? I was trying to get that use case working, but looks like made a mistake there (never had that set up, so never tested).

marcingrzejszczak · 2019-08-26T15:28:55Z

Here is the sample https://github.com/spring-cloud-samples/spring-cloud-contract-samples/tree/2.2.x/producer_with_git

Unfotunately there are the following issues

the initTask is never called since it's always up to date
even if I make it not always uptodate, what happens is that

if (downloaded != null) {
            logger.info("Downloaded contracts to [" + downloaded + "]")
            config.includedContracts.set(downloaded.inclusionProperties.includedContracts)
            config.includedRootFolderAntPattern.set(downloaded.inclusionProperties.includedRootFolderAntPattern)
            config.initialisedContractsDirectory.set(downloaded.downloadedContracts)
        }
        logger.info("For project [{}] will use contracts provided in the folder [{}]", project.name, config.initialisedContractsDirectory.get())

running this ^^ results in

Downloaded contracts to [org.springframework.cloud.contract.verifier.plugin.GradleContractsDownloader$DownloadedData(/tmp/git-contracts-1566818849546-0/META-INF/com.example/beer-api-producer-git/0.0.1.BUILD-SNAPSHOT/contracts, org.springframework.cloud.contract.stubrunner.ContractDownloader$InclusionProperties@751f17e7)]
For project [beer-api-producer-git] will use contracts provided in the folder [/home/marcin/repo/spring-cloud-contract-samples/producer_with_git/src/test/resources/contracts]

as you can see, even though we set a configuration element, the get() method returns sth else. I fixed that, but there's no option to make one configuration change the output and pass it to another task.

anatoliy-balakirev · 2019-08-26T15:42:46Z

as you can see, even though we set a configuration element, the get() method returns sth else.

That's quite weird...

I fixed that,

Just out of curiosity, how exactly did you fix that? Just wonder if that fix actually caused this:

but there's no option to make one configuration change the output and pass it to another task.

Updating the property should be propagated everywhere, in theory... That output property is used as an input to other tasks.

marcingrzejszczak · 2019-08-26T15:45:45Z

Just out of curiosity, how exactly did you fix that? Just wonder if that fix actually caused this:

afair I've removed some @Optional annotations.

Unfortunately, due to the amount of time I have to put into Gradle, and Gradle's lack of being developer friendly in any way, I will timebox it to a day or two and most likely will be forced to remove this feature or add some hacks to make the tests pass again.

anatoliy-balakirev · 2019-08-26T15:53:11Z

I can take a look into this. Maybe whole idea to try to make that init part incremental was wrong because there is already caching in the GradleContractsDownloader...

marcingrzejszczak · 2019-08-26T15:59:26Z

Yeah looks like it

anatoliy-balakirev · 2019-08-26T16:00:31Z

By when do you need this fixed?

marcingrzejszczak · 2019-08-26T16:00:50Z

The sooner, the better ;)

anatoliy-balakirev · 2019-08-26T16:13:46Z

Trying to run that project locally and getting:

Caused by: org.eclipse.jgit.errors.NoRemoteRepositoryException: file:///Users/anatolii.balakiriev/Developer/spring-cloud-contract-samples/producer_with_git/build/contract_git/.git: not found.

Seems to be caused by this config:

	contractRepository {
		repositoryUrl = "git://file://${project.buildDir}/contract_git/"
	}

Any suggestions on how to make it working? Can't understand how it can work with that config in place...

marcingrzejszczak · 2019-08-26T16:15:45Z

It works on my machine ™️

just copy the contract_git folder from root of the project to build/contract_git and mv build/contract_git/git -> build/contract_git/.git. A Gradle task there should do that.

$ mkdir build
$ cp -r ../contract_git build/contract_git
$ mv build/contract_git/git build/contract_git/.git

anatoliy-balakirev · 2019-08-27T08:25:26Z

Ok, thx. There actually seems to be a gradle task to do that, but it has a wrong dependency on:

copyContracts.dependsOn("prepareGit")

and must be this instead:

initContracts.dependsOn("prepareGit")

So after applying that and disabling incremental build for initContracts task like this:

		// TODO: Enable it when all caching from `GradleContractsDownloader` is replaced with Gradle's one here 
		// @OutputDirectory
		DirectoryProperty initialisedContractsDirectory

I got another problem in the generateContractTests task. It looks like there was a bug there, which was ignoring inclusion pattern, generated by the GradleContractsDownloader, so that task was always including everything. Here is an old log (the last line is an interesting one here):

> Task :generateContractTests
Task ':generateContractTests' is not up-to-date because:
  Task has not declared any outputs despite executing actions.
Generated test sources dir [/Users/anatolii.balakiriev/Developer/spring-cloud-contract-samples/producer_with_git/build/generated-test-sources/contracts]
Spring Cloud Contract Verifier Plugin: Invoking test sources generation
Contracts are unpacked to [/Users/anatolii.balakiriev/Developer/spring-cloud-contract-samples/producer_with_git/build/stubs/META-INF/com.example/beer-api-producer-git/0.0.1.BUILD-SNAPSHOT/contracts]
Included contracts are [.*]

And after I started propagating that properly - we have this log:

> Task :generateContractTests
Caching disabled for task ':generateContractTests' because:
  Build cache is disabled
Task ':generateContractTests' is not up-to-date because:
  Output property 'config.generatedTestResourcesDir' file /Users/anatolii.balakiriev/Developer/spring-cloud-contract-samples/producer_with_git/build/generated-test-resources/contracts has been removed.
  Output property 'config.generatedTestSourcesDir' file /Users/anatolii.balakiriev/Developer/spring-cloud-contract-samples/producer_with_git/build/generated-test-sources/contracts has been removed.
Generated test sources dir [/Users/anatolii.balakiriev/Developer/spring-cloud-contract-samples/producer_with_git/build/generated-test-sources/contracts]
Generated test resources dir [/Users/anatolii.balakiriev/Developer/spring-cloud-contract-samples/producer_with_git/build/generated-test-resources/contracts]
Spring Cloud Contract Verifier Plugin: Invoking test sources generation
Contracts are unpacked to [/Users/anatolii.balakiriev/Developer/spring-cloud-contract-samples/producer_with_git/build/stubs/META-INF/com.example/beer-api-producer-git/0.0.1.BUILD-SNAPSHOT/contracts]
Included contracts are [^/var/folders/54/71d7df0530ggpmhmbp_0_dv40000gn/T/git-contracts-1566891881312-0/META-INF/com.example/beer-api-producer-git/0.0.1.BUILD-SNAPSHOT/contracts.*$]

So as you can see, now we have correct includedContracts, but in fact it doesn't make sense here, as those should already be copied and that temp directory doesn't matter...

anatoliy-balakirev · 2019-08-27T08:28:06Z

Here is an old code, which I actually remember we even discussed:

		if (getConfigProperties().getContractDependency()) {
			project.logger.debug("Updating the stubs locations for the case where we have a JAR with contracts")
			props.contractsDslDir = contractsDslDir
			props.includedContracts = ".*"
		}

So it was always setting it to ".*" for some reason (as getConfigProperties().getContractDependency() could never be null). I'm not sure what was the logic around it, but will just restore that behaviour for now.

marcingrzejszczak · 2019-08-27T08:30:03Z

and must be this instead:

we also check for backward compatibility in those samples, so most likely it will fail cause Finchley doesn't have the initContracts task. Don't worry about it, I'll fix it later.

Here is an old code, which I actually remember we even discussed:

Ah yeah, you're right, now I remember.

So it was always setting it to ".*" for some reason (as getConfigProperties().getContractDependency() could never be null). I'm not sure what was the logic around it, but will just restore that behaviour for now.

Yeah, restore it for now. We'll deal with that later.

With current implementation of the `GradleContractsDownloader`, where we have custom caching, `InitContractsTask` doesn't really need to be incremental. Moreover, there is an issue in current implementation of the `InitContractsTask`: it doesn't mark itself as out of date when dynamic dependency is used (`+` or `SNAPSHOT` version). So for now just disabling incremental build for that task. Also restored old behaviour, where we used to always use `".*"` pattern to define included contracts for `GenerateServerTests` task. Also fixed minor logging issue. Fixes spring-cloudgh-1133

anatoliy-balakirev · 2019-08-27T09:08:42Z

Created PR. Once that one is merged - I can also try to take a look into replacing custom caching in the GradleContractsDownloader with Gradle's incremental build. If we do that - we won't even need InitContractsTask any more (was created to be kind of a bridge between that custom caching and Gradle's incremental build).

anatoliy-balakirev · 2019-08-27T09:30:33Z

I just checked that further and today caching of downloaded artifacts is done in a way that they are cached per groupId / artifactId. I.e. if caching is enabled and I run plugin a couple of times - we'll get following behaviour:

groupId:1, artifactId:1 -> downloaded
groupId:1, artifactId:1 -> using cached from step Add a Gitter chat badge to README.md #1
groupId:2, artifactId:2 -> downloaded
groupId:1, artifactId:1 -> using cached from step Add a Gitter chat badge to README.md #1
groupId:2, artifactId:2 -> using cached from step Migrate most important issues from Accurest #3

With Gradle's caching, steps #4 and #5 will trigger new download, as input was changed (i.e. it won't remember history of executions, only the previous one). So will be:

groupId:1, artifactId:1 -> downloaded
groupId:1, artifactId:1 -> using cached from step Add a Gitter chat badge to README.md #1
groupId:2, artifactId:2 -> downloaded
groupId:1, artifactId:1 -> downloaded
groupId:2, artifactId:2 -> downloaded

Is that ok?

marcingrzejszczak · 2019-08-27T09:37:01Z

I guess so. However, with the history of working with Gradle I really feel scared to rely on anything that Gradle provides. We have no certainty that with Gradle 25.0 that with their current pace might be released in a month this feature will not be removed. OTOH it's not a gigantic change I guess so yeah, go ahead with using Gradle caching.

anatoliy-balakirev · 2019-08-27T09:39:12Z

:) hopefully won't be removed. That incremental build stuff is quite a fundamental thing in Gradle, making it so fast.
Ok, will try to do that.

With current implementation of the `GradleContractsDownloader`, where we have custom caching, `InitContractsTask` doesn't really need to be incremental. Moreover, there is an issue in current implementation of the `InitContractsTask`: it doesn't mark itself as out of date when dynamic dependency is used (`+` or `SNAPSHOT` version). So for now just disabling incremental build for that task. Also restored old behaviour, where we used to always use `".*"` pattern to define included contracts for `GenerateServerTests` task. Also fixed minor logging issue. Fixes spring-cloudgh-1133

With current implementation of the `GradleContractsDownloader`, where we have custom caching, `InitContractsTask` doesn't really need to be incremental. Moreover, there is an issue in current implementation of the `InitContractsTask`: it doesn't mark itself as out of date when dynamic dependency is used (`+` or `SNAPSHOT` version). So for now just disabling incremental build for that task. Also restored old behaviour, where we used to always use `".*"` pattern to define included contracts for `GenerateServerTests` task. Also fixed minor logging issue. Fixes gh-1133

Replaced custom caching of the downloaded contracts with Gradle's incremental build. After that `InitContractsTask` is not needed any more, as its main purpose was to be a bridge between custom caching and Gradle's one. Simplified code to download contracts is now part of the `ContractsCopyTask`. Fixes spring-cloudgh-1133

Replaced custom caching of the downloaded contracts with Gradle's incremental build. After that `InitContractsTask` is not needed any more, as its main purpose was to be a bridge between custom caching and Gradle's one. Simplified code to download contracts is now part of the `ContractsCopyTask`. Also fixed `PublishStubsToScmTask` and removed incremental stuff from there for now. Fixes spring-cloudgh-1133

Replaced custom caching of the downloaded contracts with Gradle's incremental build. After that `InitContractsTask` is not needed any more, as its main purpose was to be a bridge between custom caching and Gradle's one. Simplified code to download contracts is now part of the `ContractsCopyTask`. Also fixed `PublishStubsToScmTask` and removed incremental stuff from there for now. Fixes gh-1133

anatoliy-balakirev changed the title ~~Support Up-to-date checks (AKA Incremental Build)~~ Support Up-to-date checks (AKA Incremental Build) in Gradle plugin Jul 8, 2019

spring-projects-issues added the waiting-for-triage label Jul 8, 2019

marcingrzejszczak added enhancement and removed waiting-for-triage labels Jul 12, 2019

marcingrzejszczak added the in progress label Jul 29, 2019

marcingrzejszczak added this to the 2.2.0.M2 milestone Jul 31, 2019

marcingrzejszczak added a commit that referenced this issue Jul 31, 2019

Added support for incremental Gradle builds

b9f3af3

fixes gh-1133

marcingrzejszczak mentioned this issue Jul 31, 2019

Added support for incremental Gradle builds #1144

Closed

marcingrzejszczak added a commit that referenced this issue Jul 31, 2019

Added support for incremental Gradle builds

8b3a303

fixes gh-1133

marcingrzejszczak closed this as completed in 3197241 Jul 31, 2019

marcingrzejszczak reopened this Aug 1, 2019

marcingrzejszczak removed the in progress label Aug 1, 2019

marcingrzejszczak removed this from the 2.2.0.M2 milestone Aug 1, 2019

marcingrzejszczak reopened this Aug 26, 2019

anatoliy-balakirev mentioned this issue Aug 27, 2019

Disable broken incremental build for InitContractsTask #1186

Merged

marcingrzejszczak closed this as completed in #1186 Aug 27, 2019

anatoliy-balakirev mentioned this issue Aug 27, 2019

Replace custom caching with Gradle's incremental build #1187

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Up-to-date checks (AKA Incremental Build) in Gradle plugin #1133

Support Up-to-date checks (AKA Incremental Build) in Gradle plugin #1133

anatoliy-balakirev commented Jul 8, 2019

marcingrzejszczak commented Jul 12, 2019

marcingrzejszczak commented Aug 1, 2019

shanman190 commented Aug 2, 2019

marcingrzejszczak commented Aug 5, 2019

shanman190 commented Aug 5, 2019

marcingrzejszczak commented Aug 5, 2019

shanman190 commented Aug 5, 2019

anatoliy-balakirev commented Aug 7, 2019 •

edited

Loading

marcingrzejszczak commented Aug 7, 2019

anatoliy-balakirev commented Aug 7, 2019

marcingrzejszczak commented Aug 7, 2019

anatoliy-balakirev commented Aug 8, 2019

marcingrzejszczak commented Aug 8, 2019

anatoliy-balakirev commented Aug 26, 2019 •

edited

Loading

marcingrzejszczak commented Aug 26, 2019

anatoliy-balakirev commented Aug 26, 2019 •

edited

Loading

marcingrzejszczak commented Aug 26, 2019

anatoliy-balakirev commented Aug 26, 2019

marcingrzejszczak commented Aug 26, 2019

anatoliy-balakirev commented Aug 26, 2019

marcingrzejszczak commented Aug 26, 2019

anatoliy-balakirev commented Aug 26, 2019 •

edited

Loading

marcingrzejszczak commented Aug 26, 2019

anatoliy-balakirev commented Aug 27, 2019

anatoliy-balakirev commented Aug 27, 2019

marcingrzejszczak commented Aug 27, 2019

anatoliy-balakirev commented Aug 27, 2019

anatoliy-balakirev commented Aug 27, 2019

marcingrzejszczak commented Aug 27, 2019

anatoliy-balakirev commented Aug 27, 2019

Support Up-to-date checks (AKA Incremental Build) in Gradle plugin #1133

Support Up-to-date checks (AKA Incremental Build) in Gradle plugin #1133

Comments

anatoliy-balakirev commented Jul 8, 2019

marcingrzejszczak commented Jul 12, 2019

marcingrzejszczak commented Aug 1, 2019

shanman190 commented Aug 2, 2019

marcingrzejszczak commented Aug 5, 2019

shanman190 commented Aug 5, 2019

marcingrzejszczak commented Aug 5, 2019

shanman190 commented Aug 5, 2019

anatoliy-balakirev commented Aug 7, 2019 • edited Loading

marcingrzejszczak commented Aug 7, 2019

anatoliy-balakirev commented Aug 7, 2019

marcingrzejszczak commented Aug 7, 2019

anatoliy-balakirev commented Aug 8, 2019

marcingrzejszczak commented Aug 8, 2019

anatoliy-balakirev commented Aug 26, 2019 • edited Loading

marcingrzejszczak commented Aug 26, 2019

anatoliy-balakirev commented Aug 26, 2019 • edited Loading

marcingrzejszczak commented Aug 26, 2019

anatoliy-balakirev commented Aug 26, 2019

marcingrzejszczak commented Aug 26, 2019

anatoliy-balakirev commented Aug 26, 2019

marcingrzejszczak commented Aug 26, 2019

anatoliy-balakirev commented Aug 26, 2019 • edited Loading

marcingrzejszczak commented Aug 26, 2019

anatoliy-balakirev commented Aug 27, 2019

anatoliy-balakirev commented Aug 27, 2019

marcingrzejszczak commented Aug 27, 2019

anatoliy-balakirev commented Aug 27, 2019

anatoliy-balakirev commented Aug 27, 2019

marcingrzejszczak commented Aug 27, 2019

anatoliy-balakirev commented Aug 27, 2019

anatoliy-balakirev commented Aug 7, 2019 •

edited

Loading

anatoliy-balakirev commented Aug 26, 2019 •

edited

Loading

anatoliy-balakirev commented Aug 26, 2019 •

edited

Loading

anatoliy-balakirev commented Aug 26, 2019 •

edited

Loading