Skip to content

Conversation

@yevhenii-nadtochii
Copy link
Contributor

@yevhenii-nadtochii yevhenii-nadtochii commented May 11, 2022

This PR is a prototype. It is not going to be merged.


It addresses #359.

Information on this matter:

  1. Documentation on Version Catalog.
  2. Miro board.

The PR prototypes a straightforward approach of migration to Version Catalog. This approach suggests declaration of all dependencies in a single TOML file. The original file can "live" in config repository and be propagated to other repos by pull script. The version override is done just in this file.

This PR unveiled some pluses and minuses of brand-new Version Catalog. And in particular, of a plain TOML file. But more importantly, it confirmed the viability of this solution for us.

The issues below are relevant to this prototype:

  1. Typesafe accessors to version catalog do not work in the subprojects block.
  2. Make version catalogs accessible from precompiled script plugins.
  3. Provide a way to use plugin definition from version catalog without version.
  4. False-positive "can't be called in this context by implicit receiver" with plugins in Gradle version catalogs as a TOML file.
  5. Using of Version Catalog corrupts applying of standalone scripts.

Programmatically-declared dependencies are still preferable for us, and we struggle for this. But, it was much easier and simpler to "try out" version catalog, using the simplest approach firstly.

The next prototype to implement is a programmatically-declared catalog in a form of a published plugin for Settings.

@yevhenii-nadtochii yevhenii-nadtochii self-assigned this May 11, 2022
@codecov
Copy link

codecov bot commented May 11, 2022

Codecov Report

Merging #144 (52173b2) into master (edd9f99) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #144   +/-   ##
=========================================
  Coverage     94.61%   94.61%           
  Complexity      341      341           
=========================================
  Files            52       52           
  Lines           892      892           
  Branches         18       18           
=========================================
  Hits            844      844           
  Misses           43       43           
  Partials          5        5           

Copy link
Contributor

@alexander-yevsyukov alexander-yevsyukov left a comment

Choose a reason for hiding this comment

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

It looks as a significant improvement to me. Please also see a couple of comments and questions.

Copy link
Contributor

@alexander-yevsyukov alexander-yevsyukov left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@armiol armiol left a comment

Choose a reason for hiding this comment

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

I would like to understand three things:

  1. How does this work in a multi-module project?
  2. How this is shared between repositories?
  3. How does one override the version?

Honestly, I don't like the idea of having the fragile .toml text file. There will be no auto-completion, for instance. And it's a bit repetitive inside.

Also, I don't like the idea of broken apply(from = ""). It's not only the version.gradle.kts that would be great to have as a simple place to hold the important versions. It's about the whole mechanism becoming inavailable to us.

Looking forward to the next step of prototyping.

@yevhenii-nadtochii
Copy link
Contributor Author

  1. How does this work in a multi-module project?
  2. How this is shared between repositories?
  3. How does one override the version?
  1. time itself is multi-module. It consists of two modules. When a TOML file is put under gradle directory in the root, it is picked up by build files automatically. Including root's build file and build files of its subprojects. Just put a file with deps in gradle directory and refresh Gradle in IDEA. libs extension will become available. For buildSrc, this file should be imported in settings.gradle.kts. A dedicated frame on the board.
  2. I think it could be done similarly to our current sharing. Original file can reside in config/gradle/libs.versions.toml. And it can be picked up by concrete repositories along with buildSrc directory using the same config/pull script.
  3. By modification of a local TOML file.

@armiol
Copy link
Contributor

armiol commented May 13, 2022

@yevhenii-nadtochii

Please consider this use-case. This is a project which has two different sets of modules — as it's the most feasible way to launch the truly-integration tests.

Also, if we aren't able to use the version.gradle.kts everywhere, I suggest to remove this file. Then this is going to be a valid prototyping. Right now it is still used.

@yevhenii-nadtochii
Copy link
Contributor Author

@armiol

A case, when we have a nested, independent Gradle build, is very similar to buildSrc. We can import a TOML file from settings by file location, just like we did in buildSrc. An individual copy of this file in ProtoData/tests is not needed. We just link to ProtoData's one.

In ProtoData/tests/settings.gradle.kts:

dependencyResolutionManagement {
    versionCatalogs {
        create("libs") {
            from(files("../gradle/libs.versions.toml"))
        }
    }
}

I've removed version.gradle.kts from this PR.

Also, opting out of this file can lead to manual syncing between versions declared in buildscript section of the build file and the rest file. Since this buildscript block is special when it comes to sharing code (constants) within build.gradle.kts.

@yevhenii-nadtochii yevhenii-nadtochii changed the title Prototype migration of dependancies to a TOML file Prototype migration of dependencies to a TOML file May 24, 2022
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.

4 participants