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

Configuration cache and org.ajoberstar.grgit:grgit-core #23

Closed
JavierSegoviaCordoba opened this issue Jan 19, 2022 · 31 comments
Closed

Comments

@JavierSegoviaCordoba
Copy link

That dependency is not configuration cache friendly, JGit is already in the project, can grgit be replaced by JGit?

@xvik
Copy link
Owner

xvik commented Jan 20, 2022

That dependency is not configuration cache friendly

Why?

JGit is already in the project, can grgit be replaced by JGit?

In theory, yes, but I use gradle-git-publish plugin (from the author of grgit) and to replace it I will need to re-implement it. That's a lot of work and I can't afford to lose so much time now on something already working.

I'm sure there should be a simpler solution for the configuration cache problem (just need to understand what is the problem)

@JavierSegoviaCordoba
Copy link
Author

ajoberstar/grgit#322

@JavierSegoviaCordoba
Copy link
Author

The problem is this issue is very old (2 years), there are more issues related to it, and the author doesn't reply to them.

I was using reckon too but the same author just disappeared and doesn't reply to any issue in that repo too.

@xvik
Copy link
Owner

xvik commented Jan 20, 2022

Thank you for the link. I will look

the author doesn't reply to them

He clearly stated that he has no time to maintain his projects. Still, he resolved the urgent jgit version problem recently so he didn't disappear.

@JavierSegoviaCordoba
Copy link
Author

JavierSegoviaCordoba commented Jan 20, 2022

Looks like he only appeared to solve a problem it has had in his own projects because he uses his plugins too.

I saw unreviewed open PRs and one reviewed by Eskatos about Configuration Cache, but almost 1 year ago, I don't see movements on it and on the other PRs.

Indeed, I created a PR for Reckon and I just dropped, getting no reply in months and I ended up creating my own Reckon after using it for a year. Based on this one, I am not going to lose my time creating more PRs or forking plus fixing the current one.

So I don't see this can be fixed soon if nothing changes.

@JavierSegoviaCordoba
Copy link
Author

Looks like grgit is being updated and the latest prerelease has fixed it, when 5.0.0 is being released it should fix the config cache problem.

@xvik
Copy link
Owner

xvik commented Feb 11, 2022

5.0 was released, but it drops support for java 8 and gradle 4-6.
I'll look it closer a bit later (I did not expect such radical changes), but will release a new version anyway (just maybe will have to support two branches because I can't lose java 8 support)

@JavierSegoviaCordoba
Copy link
Author

jgit 5 needs java 11, that is the reason grgit needs java 11.

Why you need java 8?

@xvik
Copy link
Owner

xvik commented Feb 11, 2022

Because java 8 is still the most popular version and so I need to target libraries on this version.
Of course, I can build docs with java 11 or above, but I need to make sure the plugin itself will not break builds on java 8 (CI builds).

@JavierSegoviaCordoba
Copy link
Author

But libraries can be java 8 compatible but built with Java 11. I don't see as a problem that a Gradle plugin targets Java 11, consumers can be Java 8 which is the important thing IMO.

@xvik
Copy link
Owner

xvik commented Feb 11, 2022

Yes, of course, it could be built with java 11, but to make sure it is better to build with java 8 (API compatibility sometimes is not enough: there could be different behaviors (I already faced several cases)). And so gradle should work on java 8 too.

In any case, as I said, I will release a new version with the released grgit plugin.

@xvik
Copy link
Owner

xvik commented Apr 2, 2022

Status update: the issue still on hold becasuse there are problems reported for the latest grgit regarding multi-module projects (affected by gradle bug, promised to be fixed in 7.5).

I'm also can't add newer jdk to CI becuase it requires gradle 7, which I can't use now becuase it would break backwards compatibility for the plugin (groovy plugin compiled with gradle 7 would not work on previous gradle versions).
Eventually, I'll have to move forward and drop older gradle versions support and at that time it would be ideal moment to update grgit (all issues should be fixed by that time).

@JavierSegoviaCordoba
Copy link
Author

JavierSegoviaCordoba commented May 12, 2022

The issue is not going to be fixed with 7.5 because it was removed from the milestone (gradle/gradle#17559).

Is grgit used extensively here? Maybe a custom git implementation can be feasible.

I am not even using the publish task so git confit is useless for me, but I understand the rest of the users uses it. Is there a way to add this task as an independent artifact or something so?

@xvik
Copy link
Owner

xvik commented May 12, 2022

Removing gradle-git-publish plugin would require complete re-implementation of this part from scratch (with pure jgit). I don't have so much time now.
I'll look if I can split the plugin into complete and lightweight versions (without publication). But some time later.

@JavierSegoviaCordoba
Copy link
Author

If you want to keep the compatibility with old Gradle versions, then maybe it is easier to just rid of grgit.

Why do you think it can be hard to reimplement the push part with jgit? Is it doing something special or only pushing to a remote repo?

@xvik
Copy link
Owner

xvik commented May 12, 2022

It would mean complete re-implementation of gradle-git-publish plugin (which is using grgit): tasks for repository checkout, content synchronization (with content declaration dsl to allow addition of custom files, build by other gradle tasks) and publication itself.

I quickly searched for an alternative plugin based on pure jgit, but there is no one.

I will review the possibilities later when come back to it. Of course, for me, it would be ideal to just update to the fixed git plugin. I'll think what could be done.

@JavierSegoviaCordoba
Copy link
Author

@xvik looks like 7.5 didn't get the fix and the issue is not in a milestone anymore.

I think the only alternative is using JGit directly if there is no other alternative :/

@xvik
Copy link
Owner

xvik commented Nov 1, 2022

I have a bad news: just released version 3.0.0 will add more warnings because python plugin now have to use gradle build listener (in order to properly close started docker containers).

Maybe a good news would be that there is now a lightweight plugin version without publication tasks: ru.vyarus.mkdocs-build.

Next, if gradle would finally introduce proper ignorance api, I'll use it in both plugins.
Another alternative would be using build services (like grgit did), but this would require dropping gradle 5 support (minimum required gradle would be 6.2) - that was the reason why I did not use it already.

So solution again postponed for future releases

@JavierSegoviaCordoba
Copy link
Author

JavierSegoviaCordoba commented Nov 1, 2022

For me I think it is a good new as I only using only this task in my custom task:

val mkdocsBuildTask = project.tasks.named<MkdocsBuildTask>("mkdocsBuild")

Do I only need to replace the previous plugin with ru.vyarus.mkdocs-build in order to get it working, right?

About Gradle 5, personally, I don't care a lot, I am using 7.5.1 in all my personal projects or in the company projects.

Not sure how many people is still stuck on that version, maybe you can try to ask for that data in the Gradle official Slack.

@xvik
Copy link
Owner

xvik commented Nov 1, 2022

Do I only need to replace the previous plugin with ru.vyarus.mkdocs-build in order to get it working, right?

yes (it is the same plugin, just without grgit and publish tasks). By the way, there is now an incremental versions file update supported, if you need it

Not sure how many people is still stuck on that version, maybe you can try to ask for that data in the Gradle official Slack.

I have to deal with old projects quite often and know how great it is sometimes to be able to simply update one plugin instead of entire build (ofc. the latter is better, but, sometimes, other plugins could hold it).

Usually I'm trying to keep compatibility as much as possible. With this release I want to present docker and requirements support for a wider gradle versions range. On next release I could drop gradle 5 entirely (current plugin version should be good enough to use for a long time). Most likely, will come back to it next year

@JavierSegoviaCordoba
Copy link
Author

I got a different issue:

1 problem was found storing the configuration cache.
- Plugin class 'ru.vyarus.gradle.plugin.python.PythonPlugin': registration of listener on 'Gradle.buildFinished' is unsupported
  See https://docs.gradle.org/7.5.1/userguide/configuration_cache.html#config_cache:requirements:build_listeners

...

* What went wrong:
Configuration cache state could not be cached: field 'memoizedMethodClosure$buildModulesList' from type 'ru.vyarus.gradle.plugin.python.task.CheckPythonTask': error writing value of type 'org.codehaus.groovy.runtime.memoize.Memoize$MemoizeFunction'
> Configuration cache state could not be cached: field 'cache' from type 'org.codehaus.groovy.runtime.memoize.Memoize$MemoizeFunction': error writing value of type 'org.codehaus.groovy.runtime.memoize.ConcurrentCommonCache'
   > Configuration cache state could not be cached: field 'readLock' from type 'org.codehaus.groovy.runtime.memoize.ConcurrentCommonCache': error writing value of type 'java.util.concurrent.locks.ReentrantReadWriteLock$ReadLock'
      > Unable to make field private final java.util.concurrent.locks.ReentrantReadWriteLock$Sync java.util.concurrent.locks.ReentrantReadWriteLock$ReadLock.sync accessible: module java.base does not "opens java.util.concurrent.locks" to unnamed module @6dc8f8f6

I can create this issue in your other plugin repo if you want.

@xvik
Copy link
Owner

xvik commented Nov 9, 2022

Reproduced. This appears only with java 16+.

The more I "deal" with configuration cache the less I like it. I belive the only way to workaround this is to write "memoization" manually in a simpler way. But even if I do it, configuration cache would still not work.

I found several similar (and related) issues. The one in spotless (987) with current mechanism critics was quite interesting (you participated there). I honestly not sure configuration cache support worth all these efforts

Overall, fixing cache requires minimal gradle increase. I don't think it makes sence to fix the exception only.
So, as before, I'll either wait for configuration cache avoidance api or will try to support it next year (with rising minimal gradle version).

@JavierSegoviaCordoba
Copy link
Author

@xvik this can be interesting: gradle/gradle#23222

@xvik
Copy link
Owner

xvik commented Jan 28, 2023

Yes, probably, a way to go. Will look better after gradle release. Strange that it targets 8.1 while 8.0 wasn't released yet.

@JavierSegoviaCordoba
Copy link
Author

First 8.1 RC which includes the FlowScope API, https://docs.gradle.org/8.1-rc-1/release-notes.html

@JavierSegoviaCordoba
Copy link
Author

Any update? This is the only missing piece I am getting to get the configuration cache working which is not a failing task.

@xvik
Copy link
Owner

xvik commented Jul 24, 2023

I will look this next month

@JavierSegoviaCordoba
Copy link
Author

Sorry for pinging again @xvik. I am especially interested in this as Gradle 8.6 will return the configuration cache feature to CI.

@xvik
Copy link
Owner

xvik commented Dec 4, 2023

It's completely ok to ping me! I could forget someting.

Sorry for delay, I was disturbed by other things (while I was already started working on python plugin update). Now I'm back to python plugin. Can't promse release in December, more likely in January.

@xvik
Copy link
Owner

xvik commented Mar 20, 2024

Status update.
Sorry for long delay. I started with simpler plugins to learn how to deal with configuration cache.

Just pushed configuration cache support for python plugin. It was hard to do, but finally it's working.
Few more things need to be done before final release. After that I would work on mkdocs plugin (should be much simpler).

Overall, if no blocking issues appear, mkdocs plugin should be released within a month.

@xvik
Copy link
Owner

xvik commented Apr 15, 2024

released 4.0.0 with configuration cache support (today 4.0.1 would be released with non-strict mode fix)

@xvik xvik closed this as completed Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants