Skip to content

Kotlin Gradle scripts. Part 2 of 2#534

Merged
dmdashenkov merged 43 commits intomasterfrom
more-kotlin-gradle
May 26, 2020
Merged

Kotlin Gradle scripts. Part 2 of 2#534
dmdashenkov merged 43 commits intomasterfrom
more-kotlin-gradle

Conversation

@dmdashenkov
Copy link
Contributor

@dmdashenkov dmdashenkov commented May 19, 2020

This PR finalizes the transition to Kotlin Gradle scripts for the base repository. Here we translate smoke-tests and base-validating-builders into Kotlin.

Custom pull scripts

Kotlin is a strictly-typed language which does not allow unresolved references at compile time. One of the ways to add classes into a Kotlin build script compile classpath is declaring those classes in the buildSrc project.

The buildSrc project is a conventionally-named Gradle plugin, which applied to all the projects in the hierarchy implicitly. We use this feature for extracting common dependencies and configuration routines via deps.kt. The whole buildSrc project is copied from the config Git submodule into the root project dir, where it is found by Gradle by convention.

However, the smoke-tests and base-validating-builders, being separate builds, do not receive the buildSrc from our main build. Furthermore, it is impossible to configure the buildSrc location.

In order for the separate builds to receive buildSrc in their script compile classpath, we copy the buildSrc directory into the root project directories of those builds: ./tools/smoke-tests and ./base-validating-builders. To automate this, we introduce custom scripts: ./pull and .\pull.bat. Those scrips do whatever their config counterparts do and also copy the buildSrc dir from the root project into the included builds.

Those scripts now should be used instead of ./config/pull and .\config\pull.bat.

Fixes in the test lib

In the plugin-testlib we've got tools for creating temporary Gradle projects to test our plugins on. For that, some of the project's own files were copied into a temporary directory. To find those files, we need to find the root project. That is done by looking for a version.gradle file. However, while renaming that file to version.gradle.kts, we did not leave a room for using old name and so if a project using a version.gradle used that tool, an obscure exception would pop up.

In this PR we allow projects to use either version.gradle or version.gradle.kts.

@dmdashenkov dmdashenkov self-assigned this May 19, 2020
@codecov
Copy link

codecov bot commented May 19, 2020

Codecov Report

Merging #534 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #534   +/-   ##
=========================================
  Coverage     73.71%   73.71%           
  Complexity     2936     2936           
=========================================
  Files           503      503           
  Lines         11904    11904           
  Branches        670      670           
=========================================
  Hits           8775     8775           
  Misses         2905     2905           
  Partials        224      224           

@dmdashenkov dmdashenkov requested a review from armiol May 19, 2020 18:55
@dmdashenkov dmdashenkov marked this pull request as ready for review May 19, 2020 18:55
@dmdashenkov
Copy link
Contributor Author

@armiol, PTAL.

Copy link
Collaborator

@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.

@dmdashenkov please see my comments and suggestions.

testAnnotationProcessor deps.build.autoService.processor
testCompileOnly deps.build.autoService.annotations
plugins {
kotlin("jvm").version("1.3.72")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please consider commenting on the string literal here (e.g. https://kotlinlang.org/docs/reference/using-gradle.html#targeting-the-jvm).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please see the PR in config.

@@ -0,0 +1,32 @@
#!/usr/bin/env bash
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would rename this file to sameDirectory.sh.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have it same-dir.sh, please? To avoid the case issues under different OS.


/**
* Creates {@link #FILE_NAME} file in the root of the test project, copying it from resources.
* Creates {@link #BUILD_GRADLE build.gradle} or {@link #BUILD_GRADLE_KTS build.gradle.kts} file in
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please also say in which case either of these two is supposed to be created/copied.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a paragraph on the priority order of build.gradle and build.gradle.kts.

testEnvGradle.createFile();
}

private void whiteBuildSrc() throws IOException {
Copy link
Collaborator

Choose a reason for hiding this comment

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

write


Some Gradle build files in this project rely on `modelCompiler` heavily. We cannot get a type-safe
accessor for the extension. In those build files we use Groovy instead of Kotlin. At alternative
would be using Kotlin and `withGroovyBuilder(..)`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if this is still the case. I am not able to locate any withGroovyBuilder(..) usages.

modelCompiler {

interfaces {
mark(messages().inFiles(mapOf("suffix" to "documents.proto")), asType("io.spine.tools.protoc.DocumentMessage"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Previously it was a much clearer code piece:

mark messages().inFiles(suffix: 'documents.proto'), asType('io.spine.tools.protoc.DocumentMessage')

Maybe we could wrap mapOf("suffix" to "documents.proto") with some method to avoid those repeated literals and mapOfs? E.g.

mark(messages().inFiles(withSuffix("documents.proto"))), asType("io.spine.tools.protoc.DocumentMessage"))

@dmdashenkov dmdashenkov requested a review from armiol May 25, 2020 11:10
@dmdashenkov
Copy link
Contributor Author

@armiol, PTAL again.

@dmdashenkov dmdashenkov merged commit 1b3ff90 into master May 26, 2020
@dmdashenkov dmdashenkov deleted the more-kotlin-gradle branch May 26, 2020 17:38
@dmitrykuzmin dmitrykuzmin mentioned this pull request Sep 8, 2020
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.

3 participants