Prototype migration of dependencies to a published Version Catalog #145

yevhenii-nadtochii · 2022-05-12T16:18:41Z

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

It addresses #359.

Information on this matter:

The PR prototypes migration of dependencies into a published version catalog. Initially, it was supposed to be a plugin for Settings, that should have declared a libs catalog on its own. But due to the problems with override, it took the form of just class which can operate upon Gradle-provided VersionCatalogBuilder.

In this PR, the catalog is represented by a separate version-catalog project that is published to Maven local in order to simplify prototyping. Also, it resides in time project, they are independent. time modules use this catalog, fetching it from Maven local in settings.gradle.kts file.

version-catalog has a facade upon VersionCatalogBuilder, which provides a declarative API for dependencies. The main goal was to hide the imperative nature of the builder. The spirit of the resulted API for dependencies declaration is very similar to the way we declare dependencies now. As a result, we get all the same Kotlin objects, only with a more strict, unified structure.

What have been done:

All our dependency objects have been moved out of buildSrc/io/spine/internal/dependency to the catalog and translated with the new API. For some of them, GitHub set an incorrect match in its Files changed section, but it is still possible to compare most of them.
Configured publishing of the catalog into Maven local for the local run. The same is done for GitHub workflow.
buildSrc and time modules have been adopted to use Version Catalog, which the module provides.
Added prons and cons section for this approach to the Miro board.

A showcase of the new API is a Dummy dependency. It covers all the available API in a single place. The comments show how a particular statements will be reflected in the generated type-safe accessors. Details on a local overriding of items, shipped by the catalog, can be found in README file.

The code of Dummy dependency, which showcases the API:

internal object Dummy : CatalogEntry() {

    private const val group = "org.dummy.company"
    override val module = "$group:dummy-lib" // libs.dummy
    override val version = "1.0.0"           // libs.versions.dummy

    val core by lib("$group:dummy-core")     // libs.dummy.core
    val runner by lib("$group:dummy-runner") // libs.dummy.runner
    val api by lib("$group:dummy-api")       // libs.dummy.api

    // In bundles, you can reference entries (which declare module), extra
    // libraries or declare them in-place.

    override val bundle = setOf( // libs.bundles.dummy
        this,
        core, runner, api,
        lib("params", "$group:dummy-params"), // libs.dummy.params
        lib("types", "$group:dummy-types"),   // libs.dummy.types
    )

    // "GradlePlugin" - is a special entry name. "gradlePlugin" suffix will not
    // be put for a final plugin alias. Note, that in an example below, we have
    // this suffix for the version and module, and does not have for ID.

    object GradlePlugin : CatalogEntry() {
        override val version = "0.0.8"                 // libs.versions.dummy.gradlePlugin
        override val module = "$group:my-dummy-plugin" // libs.dummy.gradlePlugin
        override val id = "my-dummy-plugin"            // libs.plugins.dummy
    }

    object Runtime : CatalogEntry() {

        // When an entry does not override the version, it will try to fetch it
        // from the closest parental entry, which has one. For example, in this case,
        // all libraries within "Runtime" entry will have version = "1.0.0".

        val win by lib("$group:runtime-win")     // libs.dummy.runtime.win
        val mac by lib("$group:runtime-mac")     // libs.dummy.runtime.mac
        val linux by lib("$group:runtime-linux") // libs.dummy.runtime.linux

        object Bom : CatalogEntry() {
            override val version = "2.0.0"           // libs.versions.dummy.runtime.bom
            override val module = "$group:dummy-bom" // libs.dummy.runtime.bom
        }
    }

    // It's also possible to declare an extra bundle by a property delegate.
    // Just like with extra modules.

    val runtime by bundle( // libs.bundles.dummy.runtime
        Runtime.Bom,
        Runtime.win,
        Runtime.mac,
        Runtime.linux,
    )
}

The issues below are relevant to this prototype:

1. Using of Version Catalog corrupts applying of standalone scripts.
2. Make version catalogs accessible from precompiled script plugins.
3. Typesafe accessors to version catalog do not work in the subprojects block.
4. Provide a way to use plugin definition from version catalog without version.
5. Accept plugin declarations from version catalog also as libraries.
6. False-positive "can't be called in this context by implicit receiver" with plugins in Gradle version catalogs as a TOML file.
7. Add possibility to overwrite items of already created version catalogs.
8. Provide means for testing Settings plugins.

Off-topic

Having to use output from buildSrc a lot for development, I've tried to get rid of the warnings it emits:

'compileJava' task (current target is 11) and 'compileKotlin' task (current target is 1.8) jvm target compatibility should be set to the same Java version. - nothing can be done. It's a bug in Gradle. They state, it is fixed in v7.5, which is the current Release Candidate.
Inconsistency with different Kotlin versions. I've forced the version in buildSrc to the one, used by Gradle itself. I've left a note about this in buildSrc/settings.gradle.kts file.
Dokka depends on libraries, which are indeed fat jars with Kotlin runtime in it. One more Kotlin version. I've excluded those libraries to get rid of annoying warnings. Interesting case, needs some investigation. I've commented this moment as well.

codecov · 2022-05-13T15:00:57Z

Codecov Report

Merging #145 (f5199ae) into master (edd9f99) will increase coverage by 0.11%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #145      +/-   ##
============================================
+ Coverage     94.61%   94.73%   +0.11%     
- Complexity      341      342       +1     
============================================
  Files            52       52              
  Lines           892      892              
  Branches         18       18              
============================================
+ Hits            844      845       +1     
  Misses           43       43              
+ Partials          5        4       -1

yevhenii-nadtochii · 2022-06-07T13:07:47Z

@armiol @alexander-yevsyukov Please, take a look.

alexander-yevsyukov

Got only one comment. But I did only a cursory review. In general it's LGTM. Unfortunately, I cannot put much efforts in this PR — busy with other urgent things now. So I rely on review from @armiol for a formal approval.

version-catalog/README.md

armiol

@yevhenii-nadtochii please see my comments. We need to discuss my comments vocally.

armiol · 2022-06-07T13:46:04Z

buildSrc/build.gradle.kts

- *     Dokka Releases</a>
- */
-val dokkaVersion = "1.6.20"
+// Let's get rid of warnings about different Kotlin version on the classpath.


Reading it again, I am thinking now, to which piece of code this comment relates? Perhaps, it is misplaced?

armiol · 2022-06-07T13:46:45Z

buildSrc/build.gradle.kts

+    implementation(libs.protobuf.gradlePlugin)
+
+    /*
+     These guys below use a fat jar with Kotlin runtime inside. One more


Let's be more polite and format here. They aren't "these guys".

armiol · 2022-06-07T15:02:41Z

version-catalog/README.md

+
+`spine-version-catalog` consists of several modules:
+
+1. `api` – represents a facade upon Gradle's provided `VersionCatalogBuilder`. 


We have discussed this previously. I would not have this module as a standalone entity.

armiol · 2022-06-07T15:07:38Z

version-catalog/api/src/main/kotlin/io/spine/internal/catalog/CatalogNotations.kt

+package io.spine.internal.catalog
+
+/**
+ * Defines what information is necessary to create one or another version


Did you mean this?

"Defines the information necessary to create a versioned catalog-compatible item."

armiol · 2022-06-07T15:10:53Z

version-catalog/api/src/main/kotlin/io/spine/internal/catalog/CatalogNotations.kt

+ * Defines what information is necessary to create one or another version
+ * catalog-compatible item.
+ *
+ * Notations are citizens of a declaration site. They form DSL and say what is


We need to discuss these notations vocally. So far, I have very little understanding of what they are (especially in this DSL part), and why there's still some parallel hierarchies with "Catalog-Version-Library-..." terms.

armiol · 2022-06-07T15:29:52Z

version-catalog/api/src/main/kotlin/io/spine/internal/catalog/entry/VersionInheritingEntry.kt

+ *
+ * See direct implementations: [PluginEntry], [DependencyEntry].
+ */
+open class VersionInheritingEntry : CatalogEntry(), VersionNotation {


Let's discuss this vocally. From our last round, you said CatalogEntry is a counter-example for VersionInheritingEntry, as CatalogEntry takes the properties from its children, while VersionInheritingEntry descendants — at the moment — just take their versions from their parent.

Two points:

We might need to inherit more than just a version. Hence the name is still questionable.

CatalogEntry is not a counter-example, but a direct parent type. Which makes your previous comments confusing to me.

armiol · 2022-06-07T15:32:47Z

...ion-catalog/func-test/dummy-catalog/src/main/kotlin/io/spine/internal/catalog/entry/Dummy.kt

+
+    // "GradlePlugin" - is a special entry name for `PluginEntry`.
+    // For plugin entries with this name, the facade will not put "gradlePlugin"
+    // suffix for a plugin's id. Note, that we have this suffix for the version


armiol · 2022-06-07T15:41:00Z

...ion-catalog/func-test/dummy-catalog/src/main/kotlin/io/spine/internal/catalog/entry/Dummy.kt

+package io.spine.internal.catalog.entry
+
+/**
+ * This dependency describes an imaginary library.


Here it is again. See, you say that it describes a library, while you aren't using a LibraryEntry on top, but a DependencyEntry.

I suggest to pick just one of them and kill another one.

...ion-catalog/func-test/dummy-catalog/src/main/kotlin/io/spine/internal/catalog/entry/Dummy.kt

armiol · 2022-06-07T15:45:18Z

...ion-catalog/func-test/dummy-catalog/src/main/kotlin/io/spine/internal/catalog/entry/Dummy.kt

+    object Tools : VersionEntry() {
+        override val version = "3.0.0" // libs.versions.dummy.tools
+    }
+}


It would also be nice to see how one overrides a version of some Dummy part (such as Gradle plugin, for instance) in a Gradle project which originally imports the version catalog declaring Dummy.

yevhenii-nadtochii · 2022-06-13T14:28:26Z

@armiol Please take another look.

alexander-yevsyukov · 2022-06-13T14:50:50Z

version-catalog/README.md

+ 1. Go to `catalog` module.
+ 2. Open `io.spine.internal.catalog.entry` package.
+ 3. Create a new file there, which contains an object declaration, named after 
+a dependency, that is being added.


Please adjust the text wrapping so that vertical line of item numbers is not broken by wrapped text.

armiol

@yevhenii-nadtochii LGTM with a number of minor documentation-related comments. Please make sure to address those.

armiol · 2022-06-14T10:10:56Z

version-catalog/README.md

+   val api by lib("$group:dummy-api")       // libs.dummy.api
+
+   // In bundles, you can reference entries (which declare module), extra
+   // libraries or declare them in-place.


", or declare"

armiol · 2022-06-14T10:11:28Z

version-catalog/README.md

+
+## Overriding of items shipped by `SpineVersionCatalog`
+
+Sometimes, it happens that a projects needs to override items, shipped by the catalog.


"that a project"

armiol · 2022-06-14T10:14:25Z

version-catalog/README.md

+
+When overriding libraries and plugins, a version should be specified in-place.
+
+For example:


In this example, there should be more comments which literally point to the rules which you describe in the sentences above.

armiol · 2022-06-14T10:15:11Z

version-catalog/README.md

+      which exposes the declarative facade.
+   2. Makes `dummy-project` use `dummy-catalog` from Maven local. 
+   3. Builds `dummy-project`. It has assertions in its build file. Those assertions 
+      verify the generated type-safe accessors to `Dummy` dependency. When any 


yevhenii-nadtochii added 2 commits May 12, 2022 18:52

Implement a plugin skeleton

f13912a

Link a skeleton to the project

40de0cd

yevhenii-nadtochii self-assigned this May 12, 2022

yevhenii-nadtochii added 4 commits May 13, 2022 17:38

[temporarily] Make workflow publish dependencies plugin to mavenLocal

876e753

Apply plugin in settings files

cdacc66

Migrate first portion of dependencies

f73d91a

Update reports

6223f80

yevhenii-nadtochii marked this pull request as draft May 13, 2022 14:55

yevhenii-nadtochii added 21 commits May 13, 2022 18:01

[temporarily] Make workflow publish settings plugin to mavenLocal

7bff623

Migrate second portion of dependencies

82fd9e8

Resolve a provider

3e2cf05

Fetch contributors automatically

e8658f8

Rename a method

e580c66

Implement SpineVersionCatalogBuilder.kt

14227a0

Simplify SpineVersionCatalogBuilder.kt

ac3713d

Don't use a hyphen

e60dc17

Annotate unused where needed

90769f9

Migrate third portion of dependencies

a9dbff9

Implement a primitive test

52f19ee

Implement a full-fledged functional test

213fbbf

Better DSL

5339b08

Shortage library() to lib()

54c8691

Migrate fourth portion of dependencies

1636d63

Implement VersionCatalogEntry

8d7a569

Merge all Auto* into GoogleAuto dependency

b062f39

Get rid of Kotlin runtime warnings of buildSrc

b6b9b97

Try out VersionCatalogEntry

63fafc9

Substitute catalog contributor with entry

3e9190e

Refactor VersionCatalogEntry

671127c

Write a README.md

7d3d397

alexander-yevsyukov reviewed Jun 7, 2022

View reviewed changes

version-catalog/README.md Outdated Show resolved Hide resolved

Use a full name of Gradle's settings file in README.md

94235bd

armiol requested changes Jun 7, 2022

View reviewed changes

yevhenii-nadtochii added 5 commits June 8, 2022 11:22

Place a comment closer to its target

9486741

Split a comment into two in build.gradle.kts

6c074b1

Rename AlwaysReturnDelegate to MemoizingDelegate

58c9884

Merge api module into catalog

afa5519

Leave only a single CatalogEntry

3ca8a35

yevhenii-nadtochii mentioned this pull request Jun 8, 2022

Better implementation of a published Version Catalog #147

Merged

yevhenii-nadtochii added 11 commits June 9, 2022 11:52

Don't expose concrete records

c9601de

Update documentation to Alias and CatalogRecords

4619471

Write top-level docs for CatalogEntry

5b1698d

Drop a special treatment for extra libs named after the entry

54c1d3f

Adjust tests to a single CatalogEntry

66608d5

Extract Alias-related code to Alias class

e2c6cbc

Document public properties and methods of CatalogEntry

e21c29b

Polish code and docs

7786049

Add examples of override to README

391abf9

Update modules structure in README

77c8585

Don't use too wide code term

17aaf48

yevhenii-nadtochii force-pushed the published-settings-plugin branch from 9a6f115 to 17aaf48 Compare June 13, 2022 14:14

yevhenii-nadtochii requested a review from armiol June 13, 2022 14:28

alexander-yevsyukov reviewed Jun 13, 2022

View reviewed changes

Adjust text wrapping in numerated lists

35eb30f

armiol approved these changes Jun 14, 2022

View reviewed changes

yevhenii-nadtochii added 2 commits June 14, 2022 18:06

Minor improvements to README.md

b9e00ae

Better describe rules of override

f5199ae

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prototype migration of dependencies to a published Version Catalog #145

Prototype migration of dependencies to a published Version Catalog #145

yevhenii-nadtochii commented May 12, 2022 •

edited

Loading

codecov bot commented May 13, 2022 •

edited

Loading

yevhenii-nadtochii commented Jun 7, 2022

alexander-yevsyukov left a comment

armiol left a comment

armiol Jun 7, 2022

armiol Jun 7, 2022

armiol Jun 7, 2022

armiol Jun 7, 2022

armiol Jun 7, 2022

armiol Jun 7, 2022

armiol Jun 7, 2022

armiol Jun 7, 2022

armiol Jun 7, 2022

yevhenii-nadtochii commented Jun 13, 2022

alexander-yevsyukov Jun 13, 2022

armiol left a comment

armiol Jun 14, 2022

armiol Jun 14, 2022

armiol Jun 14, 2022

armiol Jun 14, 2022


		`spine-version-catalog` consists of several modules:

		1. `api` – represents a facade upon Gradle's provided `VersionCatalogBuilder`.


		## Overriding of items shipped by `SpineVersionCatalog`

		Sometimes, it happens that a projects needs to override items, shipped by the catalog.


		When overriding libraries and plugins, a version should be specified in-place.

		For example:

Prototype migration of dependencies to a published Version Catalog #145

Are you sure you want to change the base?

Prototype migration of dependencies to a published Version Catalog #145

Conversation

yevhenii-nadtochii commented May 12, 2022 • edited Loading

Off-topic

codecov bot commented May 13, 2022 • edited Loading

Codecov Report

yevhenii-nadtochii commented Jun 7, 2022

alexander-yevsyukov left a comment

Choose a reason for hiding this comment

armiol left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yevhenii-nadtochii commented Jun 13, 2022

Choose a reason for hiding this comment

armiol left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yevhenii-nadtochii commented May 12, 2022 •

edited

Loading

codecov bot commented May 13, 2022 •

edited

Loading