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

refreshVersionsCleanup cleans up the wrong files #726

Open
Vampire opened this issue Sep 25, 2024 · 0 comments
Open

refreshVersionsCleanup cleans up the wrong files #726

Vampire opened this issue Sep 25, 2024 · 0 comments

Comments

@Vampire
Copy link
Contributor

Vampire commented Sep 25, 2024

The refreshVersionsCleanup task cleans up different files than the refreshVersions task changed.
For example it uses hard-coded File("gradle/libs.versions.toml") (just with the path from a constant) to clean up the version catalog file.
But using the File constructor in Gradle logic is at best flaky, as is in almost any JVM program.
The File constructor resolves relative paths relative to the current working directory.
In a Gradle run this often is the root project directory, but this is not guaranteed and not always the case.
It could also be the IDE installation directory, or the daemon log directory, ....
The only case I'm aware of where File constructor with a relative path is a good idea is, when developing a commandline utility where the relative path is coming from a commandline argument as there you can usually safely assume that the path is menat realtive to the current working directory.

In my case I'm developing a custom convention plugin that also integrates this plugin.
I added a functional test for refreshVersionsCleanup as I'm doing some tidying to work-around #661 and #663.
Unfortunately, this then "cleaned up" the libs.versions.toml of the plugin project, not the one of the functional test SUT project as the current working directory obviously is not the one of the functional test SUT project.

Unfortunately, this is also not configurable on the refreshVersionsCleanup task, so there is no way to work-around this bug besides avoiding to use the task or disabling it to be on the safe side.


Other similar problematic places currently I have seen are:

  • Releasing.main.kts where you should use __FILE__.parentFile instead of File(".")
  • probably VersionsCatalogUpdaterTest#rulesDir
  • probably TestUtilsKt#testResources
  • probably ResourcesKt#testResources
  • probably ResourcesKt#mainResources
  • probably MigrationTest#Search for files that may contain dependency notations#expected
  • probably MigrationTest#testResources
  • probably ResourcesKt#mainResources
  • probably PluginsVersionKt#pluginsVersion
  • probably DependencyNotationsWebpageUpdateTest#update the list of dependency notations on the website#destination
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

1 participant