littlerobots / version-catalog-update-plugin

Gradle plugin for updating a project version catalog
Apache License 2.0
565 stars 23 forks source link

Update Version Catalog updates version ref #36

Closed victorkifer closed 2 years ago

victorkifer commented 2 years ago

./gradlew versionCatalogUpdate causes version.ref to be updated instead of version. I'm not sure if it's a bug or expected behaviour.

The issue happens with room in my case. Here is the git changelog

Screenshot 2022-04-15 at 16 50 52 Screenshot 2022-04-15 at 16 51 03

I would expect versions.room to be updated instead of replacing version.ref = room with version.ref = roomPaging

hvisser commented 2 years ago

It's not entirely clear what's going on from your example, if you have a small project to reproduce that would help...Or if that's not possible could you share the report.json file and your toml file?

Did the room version also get removed? There are some heuristics to try to update the correct version for dependencies like Room that update everything in the group to the same version and it looks that is what going on, also removing any duplicates.

victorkifer commented 2 years ago

Here's the report.json file report.json.txt

victorkifer commented 2 years ago

room wasn't removed. Just references to room version where updated with roomPaging in libraries section

hvisser commented 2 years ago

OK looking at the report, the reason why room wasn't updated is probably because the room-compiler did not update versions while the other ones did. So what's happening here is that multiple dependencies in androidx.room updates to 2.5.0-alpha01 and you already had a 2.5.0-alpha01 reference for paging, it switched the affected dependencies to the roomPaging common version. The heuristic is that if there are multiple dependencies in the same group using the same version, the version ref must be for the whole group, which is where it breaks down a bit now.

I can understand this is maybe unexpected, but since the room version was also still valid updating the room version ref would also be incorrect (room-compiler is still on 2.4.2).

I guess multiple things could happen here:

Happy to hear your thoughts here too, I don't have an immediate solution other than renaming the version reference to something more appropriate because in this case it appears that some of the androidx.room artifacts are on 2.5.0-alpha01 while others aren't.

michaelcarrano commented 2 years ago

I am working on automating updates to our dependencies and from testing, I noticed I am running into this issue myself.

Simplified libs.versions.toml:

[versions]
airbnb-mavericks = "2.5.1"

[libraries]
airbnb-lottie = "com.airbnb.android:lottie:3.7.0"
airbnb-lottie-compose = "com.airbnb.android:lottie-compose:4.2.2"
airbnb-mavericks = { module = "com.airbnb.android:mavericks", version.ref = "airbnb-mavericks" }
airbnb-mavericks-mocking = { module = "com.airbnb.android:mavericks-mocking", version.ref = "airbnb-mavericks" }
airbnb-mavericks-testing = { module = "com.airbnb.android:mavericks-testing", version.ref = "airbnb-mavericks" }

After running versionCatalogUpdate this is what I see:

[versions]
airbnb-mavericks = "5.0.3"

[libraries]
airbnb-lottie = { module = "com.airbnb.android:lottie", version.ref = "airbnb-mavericks" }
airbnb-lottie-compose = { module = "com.airbnb.android:lottie-compose", version.ref = "airbnb-mavericks" }
airbnb-mavericks = { module = "com.airbnb.android:mavericks", version.ref = "airbnb-mavericks" }
airbnb-mavericks-mocking = { module = "com.airbnb.android:mavericks-mocking", version.ref = "airbnb-mavericks" }
airbnb-mavericks-testing = { module = "com.airbnb.android:mavericks-testing", version.ref = "airbnb-mavericks" }

It updated so that lottie is now referencing the version from airbnb-mavericks which isn't what I want or expected. It now has a lottie version which won't work for mavericks since that mavicks version doesn't exist.

Ideally, I would like to keep the airbnb group. I have things setup where I would use the same naming from version as part of my library.

So with the example above, I have version airbnb-mavericks and then I have libraries airbnb-mavericks + airbnb-mavericks-*

Right now, my thoughts are that if I didn't specify a version ref, then I wouldn't expect the update to add one or to update a library to use a version ref.

hvisser commented 2 years ago

Thanks for sharing that snippet!

I'm still not sure what the best fix is. The use case for adding version refs is simple: you can just dump in your dependencies and if they are part of a group, you get a common version that you can bump or have bumped by the plugin. Even if you start out with no version references and later add more dependencies from that group. That also prevents you from mixing and matching "wrong" versions too. And at the same time keep the amount of version references reasonable and optimised.

But as you point out, if the group is too common, like with airbnb here (IMHO Lottie and Mavericks shouldn't be in the same group) then things start to break down. I'm not sure if it works currently if you manually set up those refs. I guess if you manually correct it once and it would stick that could be OK (but not sure how to deal with libs without a version reference; there would be no way to tell if you want that to be without a version on purpose or not).

Switching the ref on mavericks for a non-existent version is wrong in any case, it shouldn't be doing that. So this might be a slightly different scenario vs the initial Room one. I think ideally it should add a version ref if it can (unique version for a set of libraries within the same group for example) or otherwise leave it.

Open to other suggestions and other edge cases, this probably will need some good tests around when working towards a fix.

If anyone else sees something like this, please share the "before" relevant TOML snippet and the report.json if you have it, that will help me to setup some tests...I'll try to spend some time on this soonish.