Skip to content
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

Make app-gradle-plugin configuration cache compatible #997

Open
liutikas opened this issue Jan 23, 2023 · 6 comments
Open

Make app-gradle-plugin configuration cache compatible #997

liutikas opened this issue Jan 23, 2023 · 6 comments

Comments

@liutikas
Copy link

Make app-gradle-plugin configuration cache compatible. Configuration cache is about to ship stable. Enabling it currently causes GCP gradle plugin to fail in multiple ways

3 problems were found reusing the configuration cache.
- Task `:appengineDeploy` of type `com.google.cloud.tools.gradle.appengine.core.DeployTask`: cannot deserialize object of type 'org.gradle.api.Project' as these are not supported with the configuration cache.
  See https://docs.gradle.org/7.6/userguide/configuration_cache.html#config_cache:requirements:disallowed_types
- Task `:appengineStage` of type `com.google.cloud.tools.gradle.appengine.appyaml.StageAppYamlTask`: cannot deserialize object of type 'org.gradle.api.Project' as these are not supported with the configuration cache.
  See https://docs.gradle.org/7.6/userguide/configuration_cache.html#config_cache:requirements:disallowed_types
- Task `:downloadCloudSdk` of type `com.google.cloud.tools.gradle.appengine.core.DownloadCloudSdkTask`: invocation of 'Task.project' at execution time is unsupported.
  See https://docs.gradle.org/7.6/userguide/configuration_cache.html#config_cache:requirements:use_project_during_execution
@diegomarquezp
Copy link
Contributor

Hi @liutikas. What would it take to make the plugin configuration-cache compatible?

liutikas referenced this issue in liutikas/app-gradle-plugin Mar 1, 2023
- Replace project with injected FileOperations object
- Replace project with logger as that's the only thing used

Fixes part of #454 issue

Test: existing tests pass
@liutikas
Copy link
Author

liutikas commented Mar 1, 2023

Partial fix here GoogleCloudPlatform/app-gradle-plugin#455

@TWiStErRob
Copy link
Contributor

TWiStErRob commented Aug 4, 2023

@diegomarquezp I'm keen to move this forward, we need maintainer involvement to help make decisions.

GoogleCloudPlatform/app-gradle-plugin#455 takes use ⅔ the way there. It fixes

- Task `:appengineStage` of type `StageAppYamlTask`: invocation of 'Task.project' at execution time is unsupported.
- Task `:downloadCloudSdk` of type `DownloadCloudSdkTask`: invocation of 'Task.project' at execution time is unsupported.

There is 1 bigger problem, which is the following from many sources:

cannot deserialize object of type 'Project' as these are not supported with the configuration cache.

Each Extension is referenced from Tasks, which means that those Extensions cannot have Project typed fields. I changed all those Project fields to FileOperations, similar to #455, and I was able to do a GAE deployment while reusing configuration cache. 🎉

⚠ One thing I don't know how to make work with this is projectAsService. Since this.project is essentially removed, there's no way to make the code execute.


To finalize the work we need to discuss what to do with the internal API usage:

  • We can accept it and just live with it until this plugin drops old Gradle versions.
    (I support this, given the low Gradle requirement.)
  • We can increase the minimum supported Gradle version from 4.0 to 6.0 (at least, hopefully that's enough) where public replacement APIs are available.

See https://github.com/GoogleCloudPlatform/app-gradle-plugin/pull/455/files#r1148827989 for more.

To make the extension migration production ready, we'll have to add one reflective call to support < Gradle 5.3, because at that point they removed a method that we would use. (There's a replacement on the same interface with a different name from Gradle 4.10.)

@TWiStErRob
Copy link
Contributor

I also found more usages of Task.getProject(): TWiStErRob/gcp-gradle-plugin@7dc366e

btw, the branch is https://github.com/TWiStErRob/gcp-gradle-plugin/commits/cc if you want to see all the commits at once.

@TWiStErRob
Copy link
Contributor

TWiStErRob commented Aug 4, 2023

In case someone is looking for a workaround:

org.gradle.configuration-cache=true
org.gradle.configuration-cache-problems=fail

+

kts
listOf(
	// com.google.cloud.tools.gradle.appengine.core.CheckCloudSdkTask::class, // OK
	com.google.cloud.tools.gradle.appengine.core.DownloadCloudSdkTask::class, // getProject()
	com.google.cloud.tools.gradle.appengine.core.GcloudTask::class, // DeployExtension.gradleProject (except CloudSdkLoginTask)
	com.google.cloud.tools.gradle.appengine.core.ShowConfigurationTask::class, // getProject()
	com.google.cloud.tools.gradle.appengine.appyaml.StageAppYamlTask::class, // StageAppYamlExtension.project + getProject()
	com.google.cloud.tools.gradle.appengine.standard.StageStandardTask::class, // StageStandardExtension.project + getProject()
	com.google.cloud.tools.gradle.appengine.standard.DevAppServerRunTask::class, // RunExtension.project
	com.google.cloud.tools.gradle.appengine.standard.DevAppServerStartTask::class, // RunExtension.project
	com.google.cloud.tools.gradle.appengine.standard.DevAppServerStopTask::class, // RunExtension.project
	com.google.cloud.tools.gradle.appengine.sourcecontext.GenRepoInfoFileTask::class, // GenRepoInfoFileExtension.project
).forEach {
	tasks.withType(it).configureEach {
		notCompatibleWithConfigurationCache("https://github.com/GoogleCloudPlatform/appengine-plugins/issues/997")
	}
}
or
Groovy
//noinspection UnnecessaryQualifiedReference
[
		// com.google.cloud.tools.gradle.appengine.core.CheckCloudSdkTask.class, // OK
		com.google.cloud.tools.gradle.appengine.core.DownloadCloudSdkTask.class, // getProject()
		com.google.cloud.tools.gradle.appengine.core.GcloudTask.class, // DeployExtension.gradleProject (except CloudSdkLoginTask)
		com.google.cloud.tools.gradle.appengine.core.ShowConfigurationTask.class, // getProject()
		com.google.cloud.tools.gradle.appengine.appyaml.StageAppYamlTask.class, // StageAppYamlExtension.project + getProject()
		com.google.cloud.tools.gradle.appengine.standard.StageStandardTask.class, // StageStandardExtension.project + getProject()
		com.google.cloud.tools.gradle.appengine.standard.DevAppServerRunTask.class, // RunExtension.project
		com.google.cloud.tools.gradle.appengine.standard.DevAppServerStartTask.class, // RunExtension.project
		com.google.cloud.tools.gradle.appengine.standard.DevAppServerStopTask.class, // RunExtension.project
		com.google.cloud.tools.gradle.appengine.sourcecontext.GenRepoInfoFileTask.class, // GenRepoInfoFileExtension.project
].each {
	tasks.withType(it).configureEach {
		notCompatibleWithConfigurationCache("https://github.com/GoogleCloudPlatform/appengine-plugins/issues/997")
	}
}
=

3 problems were found storing the configuration cache

but no build failure.

This means we can use CC for running test and other checks, but CC is skipped for when these tasks are configured, which could slow down local development for example (DevAppServer*Task).

@TWiStErRob
Copy link
Contributor

Can we get some reply? @diegomarquezp cc @suztomo

@JoeWang1127 JoeWang1127 transferred this issue from GoogleCloudPlatform/app-gradle-plugin Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants