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

Look for atomicfu-gradle-plugin version in the buildscript of the current project as well. #405

Merged
merged 7 commits into from
Feb 29, 2024

Conversation

mvicsokolova
Copy link
Collaborator

We only looked for atomicfu-gradle-plugin version in the buildscript of the rootProject, though atomicfu may be applied for one module only, in this case we failed to resolve it's version.

An example of the project: integration-testing/examples/multi-module-test.

Fixes #403

" }\n" +
"\n" +
" dependencies {\n" +
" classpath(\"org.jetbrains.kotlinx:atomicfu-gradle-plugin:0.23.2\")\n" +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we take a version from project settings when building the plugin or something like that? The amount of places where we have to manually update the version string before a release continue growing and one day someone will definitely forget to change it.

Copy link

@whyoleg whyoleg Feb 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we take a version from project settings when building the plugin or something like that?

If you are gonna to decide to do this, then all this hacks to fetch an atomicfu version from a build script will be not needed at all, as you will already know a version of the plugin :)

And IMO, this could be a better approach, than introducing this good error message, because in that case, there will be no need for it as there will be no need to add atomicfu to build classpath in user projects, and so just applying plugin will be enough :)
It will also resolve issues like #210, #137 or any other issues when users just apply plugin in some way (f.e via version catalogs) and not via buildscript.classpath (which is, AFAIK, is Gradle legacy and is not suggested approach)

There are multiple ways to do it, from manual ones to some existing Gradle plugins like this (which works pretty fine at my side)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! That's actually a much better approach.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, the comment should be left here: #405 (comment)

val pluginVersion = rootProject.buildscript.configurations.findByName("classpath")
?.allDependencies?.find { it.name == "atomicfu-gradle-plugin" }?.version
val classpath = project.buildscript.configurations.getByName("classpath")
val pluginVersion = classpath.resolvedConfiguration.resolvedArtifacts
Copy link

@whyoleg whyoleg Feb 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, looks like this could not work in all cases.
From last comment in a thread:

However, it fails for a multi-module gradle project where the plugin is added in the root projects as apply false, then applied without a version in the subproject.

Sorry, that I've sent a link to the specific message instead of just a thread :)

I was more thinking of the approach with a .properties file with version in it (mentioned in second comment) if we don't want to bring external plugin for such simple thing (which is fine).

Also, I've found more recent thread (Sep 2023) where there is even better suggestion without a need for a file generation, as it's the safest and conventional way.

And BTW KGP does the same!

  • build setup is here, though most likely the task name here will be just processResources
  • template project.properties file is here
  • function to read version is here
  • example of usage is here
  • And additionally, just may be it will also help, here is the commit in KGP which adds additional property to this file, so it touches all affected places mentioned earlier (though, it's from 2019, so some code could be moved already)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the great navigation 🔥!) Seems, that this approach worked for me.
I've pushed a commit with atomicfu.properties file generation.

Copy link

@whyoleg whyoleg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Base automatically changed from metadata-lost to develop February 29, 2024 12:55
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

Successfully merging this pull request may close these issues.

None yet

3 participants