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.

Sign up for GitHub

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

Jump to bottom

Gradle plugin not compatible with Gradle configuration cache #1527

Closed
jjohannes opened this issue Feb 1, 2024 · 12 comments · Fixed by #1542
Closed

Gradle plugin not compatible with Gradle configuration cache #1527

jjohannes opened this issue Feb 1, 2024 · 12 comments · Fixed by #1542
Assignees
@l-1squared
Labels
bug under development

Comments

@jjohannes
Copy link

The Configuration Cache feature of Gradle requires a strict separation between configuration time and execution time state.

The JGiven Gradle plugin is violating this right now by postponing the access to some configuration state to execution time by using Java Callables in JGivenPlugin to delay access to certain configuration values. For this kind of "lazy access" Gradle's Properties and Providers would be the better solution.

To reproduce the problem, run gradle jgivenTestReport --configuraion-cache on a Gradle project that applies the com.tngtech.jgiven.gradle-plugin plugin.

Trace from the configuration cache problem report:

org.gradle.api.InvalidUserCodeException: Execution of task ':entsendung.biz.qs:jgivenTestReport' caused invocation of 'Task.extensions' by task ':entsendung.biz.qs:test' at execution time which is unsupported.
	at org.gradle.configurationcache.problems.DefaultProblemFactory$problem$1$build$diagnostics$1.get(DefaultProblemFactory.kt:86)
	at org.gradle.configurationcache.problems.DefaultProblemFactory$problem$1$build$diagnostics$1.get(DefaultProblemFactory.kt:86)
	at org.gradle.internal.problems.DefaultProblemDiagnosticsFactory$DefaultProblemStream.getImplicitThrowable(DefaultProblemDiagnosticsFactory.java:111)
	at org.gradle.internal.problems.DefaultProblemDiagnosticsFactory$DefaultProblemStream.forCurrentCaller(DefaultProblemDiagnosticsFactory.java:100)
	at org.gradle.configurationcache.problems.DefaultProblemFactory$problem$1.build(DefaultProblemFactory.kt:86)
	at org.gradle.configurationcache.initialization.DefaultConfigurationCacheProblemsListener.onTaskExecutionAccessProblem(ConfigurationCacheProblemsListener.kt:134)
	at org.gradle.configurationcache.initialization.DefaultConfigurationCacheProblemsListener.onConventionAccess(ConfigurationCacheProblemsListener.kt:81)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.event.DefaultListenerManager$ListenerDetails.dispatch(DefaultListenerManager.java:472)
	at org.gradle.internal.event.DefaultListenerManager$ListenerDetails.dispatch(DefaultListenerManager.java:454)
	at org.gradle.internal.event.AbstractBroadcastDispatch.dispatch(AbstractBroadcastDispatch.java:83)
	at org.gradle.internal.event.AbstractBroadcastDispatch.dispatch(AbstractBroadcastDispatch.java:69)
	at org.gradle.internal.event.DefaultListenerManager$EventBroadcast$ListenerDispatch.dispatch(DefaultListenerManager.java:443)
	at org.gradle.internal.event.DefaultListenerManager$EventBroadcast$ListenerDispatch.dispatch(DefaultListenerManager.java:431)
	at org.gradle.internal.event.AbstractBroadcastDispatch.dispatch(AbstractBroadcastDispatch.java:43)
	at org.gradle.internal.event.AbstractBroadcastDispatch.dispatch(AbstractBroadcastDispatch.java:66)
	at org.gradle.internal.event.DefaultListenerManager$EventBroadcast$ListenerDispatch.dispatch(DefaultListenerManager.java:443)
	at org.gradle.internal.event.DefaultListenerManager$EventBroadcast.dispatch(DefaultListenerManager.java:232)
	at org.gradle.internal.event.DefaultListenerManager$EventBroadcast.dispatch(DefaultListenerManager.java:203)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94)
	at com.sun.proxy.$Proxy88.onConventionAccess(Unknown Source)
	at org.gradle.configurationcache.AbstractTaskProjectAccessChecker.notifyConventionAccess(TaskExecutionAccessCheckers.kt:45)
	at org.gradle.api.internal.AbstractTask.notifyConventionAccess(AbstractTask.java:1073)
	at org.gradle.api.internal.AbstractTask.getConventionVia(AbstractTask.java:604)
	at org.gradle.api.internal.AbstractTask.getExtensions(AbstractTask.java:600)
	at org.gradle.api.DefaultTask.getExtensions(DefaultTask.java:324)
	at com.tngtech.jgiven.gradle.JGivenPlugin.lambda$configureDefaultReportTask$5(JGivenPlugin.java:85)
	at org.gradle.util.internal.GUtil.uncheckedCall(GUtil.java:437)
@l-1squared
Copy link
Collaborator

Thanks for the report and the hints at how to investigate the issue. Ill tackle this with priority

@l-1squared l-1squared self-assigned this Feb 1, 2024
@jjohannes
Copy link
Author

Thank you @l-1squared. If you have any questions, let me know. I can also review a PR.

@l-1squared
Copy link
Collaborator

l-1squared commented Feb 9, 2024 •
edited
Loading

Hi,
I finally got around to tackling this a second time (after my failure at the beginning of the week). I think I understood now what went wrong in my previous attempt, but I lack the knowledge to find the solution. And here I could use your help.

So, the gradle documentation suggests that you use properties and providers for "everything", which prompted me to alter the JGivenTaskExtension in the following way:

public interface JGivenTaskExtension {
    Property<File> getResultsDir();

}

Unfortunately, upon testing, this produces an UnknownServiceException no service of type object factory available in default services.

I tried to debugv this and the error seems to happen, because we create the extensions in JGivenPlugin.java:38 on a Task, and not on the Project.

Thing is, that I want to create them on the task, which puts me at an impasse, because I don't know how to supply an object factory to my extension.

@jjohannes
Copy link
Author

That sounds like you are on the right track. The error is unexpected though. Maybe it's just some detail that is not right. It's unfortunately often too easy to get something wrong with these APIs.

If you could push what you have to a branch, I am happy to take a look.

@l-1squared
Copy link
Collaborator

sure: https://github.com/TNG/JGiven/tree/feature/separate-plugin-config-and-execution

@l-1squared
Copy link
Collaborator

Ah, I finally happened upon this: gradle/gradle#16936. I'll try it

@l-1squared
Copy link
Collaborator

hmm, maybe not. But it gave me an idea: I could create the extension via the object factory and then simply add to the task. (Did so in my WIP commit) I might be on to something...

@jjohannes
Copy link
Author

jjohannes commented Feb 12, 2024 •
edited
Loading

Okay it looks like you are right. Unfortunately task extensions do not seem to be handled that easily as "normal" (project) extensions. But the workaround suggested in gradle/gradle#16936 (comment) looks slightly wrong. That syntax does not work for me.

I adjusted everything I would in this commit:
https://github.com/jjohannes/JGiven/commit/cc3f67a7b4b39f9f78599c2cea18c42c95870c73

Feel free to pick parts you need from there.

Unfortunately, now the configuration cache is still failing with issues with the TaskReportContainer which looks somethign Gradle internal. But other reporting task also use that. So it should be usable somehow.

@l-1squared
Copy link
Collaborator

Okay it looks like you are right. Unfortunately task extensions do not seem to be handled that easily as "normal" (project) extensions. But the workaround suggested in gradle/gradle#16936 (comment) looks slightly wrong. That syntax does not work for me.

I adjusted everything I would in this commit: jjohannes@cc3f67a

Feel free to pick parts you need from there.

Unfortunately, now the configuration cache is still failing with issues with the TaskReportContainer which looks somethign Gradle internal. But other reporting task also use that. So it should be usable somehow.

Thanks for the refactoring which helped me out a great deal. Aside from the issue you already mentioned, I also removed a null checked which seemed to be important. Hope I'll get both sorted out quickly

l-1squared added a commit that referenced this issue Feb 13, 2024
@l-1squared
Issue-1527: implement changes from jjohannes
ae3bea9 
these have been detailed https://github.com/jjohannes/JGiven/commit/cc3f67a7b4b39f9f78599c2cea18c42c95870c73
and authority to use these has been explicitly granted #1527 (comment)

Signed-off-by: l-1squared <30831153+l-1squared@users.noreply.github.com>
@l-1squared
Copy link
Collaborator

Unfortunately, this will still take a while.
The Report and Container both use task based internal classes and especially the public api of the reports container has A TON of methods to implement :(

Also there is another test failure about the test task running, when it shouldn't that I don't understand yet.

@jjohannes
Copy link
Author

jjohannes commented Feb 16, 2024 •
edited
Loading

I am not sure if extending the (internal) ReportContainer implementation is the issue here. I also don't know about an alternative solution. Implementing all "DomainObjectContainer" methods yourself can't be it. It feels to me like public API is missing and what you are doing is OK.

I have looked at this again and TBH I don't understand the error. I can't finde the reference in the code the error is complaining about. For reference, this is the trace from the CC report:

CC

I'll see if I can get someone from Gradle to have a look.

@jjohannes
Copy link
Author

jjohannes commented Feb 16, 2024 •
edited
Loading

Good news @l-1squared.

@mlopatkin helped me to track down the problem. It is totally unrelated to the "internal" container implementation. You do not need to change anything there.

This is the fix: 37c736e

l-1squared added a commit that referenced this issue Feb 23, 2024
@l-1squared
Issue-1527: implement changes from jjohannes
da07bf2 
these have been detailed https://github.com/jjohannes/JGiven/commit/cc3f67a7b4b39f9f78599c2cea18c42c95870c73
and authority to use these has been explicitly granted #1527 (comment)

Signed-off-by: l-1squared <30831153+l-1squared@users.noreply.github.com>
@l-1squared l-1squared linked a pull request Feb 29, 2024 that will close this issue
Feature/separate plugin config and execution #1542
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@l-1squared l-1squared

Labels
bug under development
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Feature/separate plugin config and execution TNG/JGiven
2 participants
@jjohannes @l-1squared