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

Support lazy property evaluation (appengine.stage.setArtifact() doesn't carry task dependencies) #999

Open
martinbonnin opened this issue Sep 4, 2022 · 5 comments

Comments

@martinbonnin
Copy link
Contributor

The follow Gradle configuration:

val myTask = tasks.register("myTask", Jar::class.java) {
  archiveBaseName.set("myJar")
}
appengine {
  stage {
    setArtifact(myTask.flatMap { it.archiveFile })
  }
}

Fails with the below:

$ ./gradlew appengineDeploy
> Task :backend:service-import:appengineStage FAILED

FAILURE: Build failed with an exception.

* What went wrong:
A problem was found with the configuration of task ':appengineStage' (type 'StageAppYamlTask').
  - In plugin 'com.google.cloud.tools.gradle.appengine.appyaml.AppEngineAppYamlPlugin' type 'com.google.cloud.tools.gradle.appengine.appyaml.StageAppYamlTask' property 'stagingExtension.artifact' specifies file '/Users/mbonnin/git/[...]/build/libs/myJar.jar' which doesn't exist.
    
    Reason: An input file was expected to be present but it doesn't exist.

I would have expected the provider to call myTask as a dependency automatically as outlined in https://docs.gradle.org/current/userguide/lazy_configuration.html#lazy_properties.

Without this, one has to rely on manual dependency wiring

@elefeint
Copy link
Contributor

elefeint commented Sep 6, 2022

Lazy property evaluation is not supported in this plugin. Leaving it open as a feature request.

@elefeint elefeint changed the title appengine.stage.setArtifact() doesn't carry task dependencies Support lazy property evaluation (appengine.stage.setArtifact() doesn't carry task dependencies) Sep 6, 2022
@yigit
Copy link

yigit commented Nov 7, 2022

Note that this is happening for other properties too.
e.g. this also doesn't work:

setAppEngineDirectory(buildAppEngineDirectoryTask.map {
  it.outputDirectory
})

The problem seems to be in these lines where the inputs are being normalized into a file immediately.

If you change StageAppYamlExtension's properties to RegularFileProperty, that should fix the issue but not sure if you would need to break API to do that.

@elefeint
Copy link
Contributor

elefeint commented Nov 7, 2022

We would accept a user contribution for the feature.

Backwards compatibility would be ideal, although not always possible (e.g. the user programmatically accessing the value will have to call .get() on the object, which is not needed when the value is a string).

@TWiStErRob
Copy link
Contributor

TWiStErRob commented Nov 23, 2022

Wanted to open a similar issue. Laziness is missing overall. The problem goes as far as NPE in some cases because a task is not realized yet, so a workaround like this is needed:

// Eagerly configure Jar task, because AppEngineAppYamlPlugin uses Groovy properties APIs to access it.
tasks.named("jar").get()

Based on best practices from Google's very own @liutikas, here are some easily fixable red flags from AppEngineAppYamlPlugin:

  • project.getTasks().create(). -> .getTasks().register
  • project.afterEvaluate {} -> Provider.map and Gradle org.gradle.api.provider.Property
  • project.getTasks().getByName().... -> getTasks().named().configure { }
  • project.getProperties().get("jar") -> getTasks().named("jar") + provider

They all have solutions, some are breaking as you described above. Would anyone be willing to collab on having a stab at this with me? And would anyone from @GoogleCloudPlatform promise to review the changes we make to land it, because it looks like @chanseokoh tried something similar 1.5 years ago, but it stalled.

@martinbonnin
Copy link
Contributor Author

Still interested by this. Haven't looked at my GCP projects for a while but I can try to do a small PR at some point to get the ball rolling. Happy to review/test anything too.

@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

4 participants