Write GitHub Actions workflow files in Kotlin instead of YAML #1630

Vampire · 2023-04-12T02:39:30Z

For now a 1:1 translation from previous Yaml.

codecov · 2023-04-12T02:51:03Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.82%. Comparing base (2c7db77) to head (daa6a19).
Report is 96 commits behind head on master.

❗ Current head daa6a19 differs from pull request most recent head 90dc92d

Please upload reports for the commit 90dc92d to get more accurate results.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1630      +/-   ##
============================================
+ Coverage     80.44%   81.82%   +1.37%     
- Complexity     4337     4569     +232     
============================================
  Files           441      446       +5     
  Lines         13534    14322     +788     
  Branches       1707     1813     +106     
============================================
+ Hits          10888    11719     +831     
+ Misses         2008     1936      -72     
- Partials        638      667      +29

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Vampire · 2023-04-12T07:33:53Z

Well, almost 1:1, I added some names where only IDs were present. Due to that the required checks now do not match anymore as they are considering the name, not the ID.

leonard84

Not seeing the advantages yet.

leonard84 · 2023-04-12T17:00:57Z

build.gradle

+configurations {
+ kotlinCompilerClasspath {
+ canBeConsumed = false
+ }
+ kotlinScriptClasspath {
+ canBeConsumed = false
+ }
+}
+
+dependencies {
+ kotlinCompilerClasspath 'org.jetbrains.kotlin:kotlin-compiler:1.8.20'
+ kotlinCompilerClasspath 'org.jetbrains.kotlin:kotlin-scripting-compiler:1.8.20'
+ kotlinScriptClasspath('org.jetbrains.kotlin:kotlin-main-kts:1.8.20') { transitive = false }
+}
+
+def preprocessWorkflows = tasks.register('preprocessWorkflows')
+file('.github/workflows').eachFileMatch(~/.*\.main\.kts$/) { workflowScript ->
+ def workflowName = workflowScript.name - ~/\.main\.kts$/
+ def pascalCasedWorkflowName = workflowName
+ .replaceAll(/-\w/) { it[1].toUpperCase() }
+ .replaceFirst(/^\w/) { it[0].toUpperCase() }
+ def preprocessWorkflow = tasks.register("preprocess${pascalCasedWorkflowName}Workflow", JavaExec) {
+ inputs
+ .file(workflowScript)
+ .withPropertyName('workflowScript')
+ outputs
+ .file(new File(workflowScript.parent, "${workflowName}.yaml"))
+ .withPropertyName('workflowFile')
+
+ javaLauncher = javaToolchains.launcherFor {
+ languageVersion = JavaLanguageVersion.of(17)
+ }
+ classpath(configurations.kotlinCompilerClasspath)
+ mainClass = 'org.jetbrains.kotlin.cli.jvm.K2JVMCompiler'
+ args('-no-stdlib', '-no-reflect')
+ args('-classpath', configurations.kotlinScriptClasspath.asPath)
+ args('-script', workflowScript.absolutePath)
+ }
+ preprocessWorkflows.configure {
+ dependsOn(preprocessWorkflow)
+ }


This whole thing should be extracted into build-logic this files is already too bloated.

Also can we add the validation here and just have on job that validates all workflows instead of having it inline of each job? Changes to the release job wouldn't be validated in the PR otherwise.

This whole thing should be extracted into build-logic this files is already too bloated.

Yep, no problem, moved to a build-logic subproject.

Also can we add the validation here and just have on job that validates all workflows instead of having it inline of each job?

Well, yes and no.
Indeed we should have a check that verifies all YAMLs for PRs and branches as not all run always like the "release" one.
I added this now.
You can disable the per-workflow consistency check that is generated, but I would recommend to keep it there.
The YAML is basically just the output format of the Kotlin scripts and that it is there additionally should not be too much of a problem.
Until https://github.com/orgs/community/discussions/15904 is considered and realized, you just have to run the scripts manually and check in the result and thus need the consistency check.
So I'd suggest to keep the per-file consistency check there, as then the jobs defined in the respective file are also always only executed if the consistency check was successful.

leonard84 · 2023-04-12T17:01:57Z

.github/workflows/branches-and-prs.main.kts

Can we keep the .yml extension for the generated files, so that we can actually see the diff?

Yes, you can manually configure the target file name.
I changed it to ...yml, but maybe we want to return to the default after you verified the changes to not have the unnecessary configuration?

leonard84 · 2023-04-12T17:03:26Z

.github/workflows/branches-and-prs.main.kts

+ * limitations under the License.
+ */
+
+@file:DependsOn("io.github.typesafegithub:github-workflows-kt:0.40.1")


Why not add this to the kotlinScriptClasspath instead of duplicating it in every .kts?

A Kotlin .main.kts script is a standalone utility that can run on its own.
The native way to call them is to have Kotlin installed and just execute the script either explicitly with Kotlin CLI, or if you are using some sh derivative by executing the script.

The Gradle task is just a way I came up with to be able to execute the scripts as Gradle tasks.

The @DependsOn within the script is needed so that the normal way to run it works, that is for example also used by the consistency checks and also is faster than ramping up Gradle, which I just added as convenient launcher without the need to have Kotlin installed.

Also the IntelliSense in the IDE would not work if the @DependsOn were not there as it then would not know about that dependency.

Regarding the duplication, you could actually drag out common things like defining the SetupBuildEnv class or also the @DependsOn line to a common script that you then include with @file:Import. Unfortunately, this approach has still some bugs in Kotlin. For example you then sometimes loose IntelliSense completely and if there is a change in the common script, you get strange non-obvious compilation errors that you can only fix (as far as I know) by deleting the .main.kts compilation cache locally. Hence I did not yet use that approach in Spock. But you could have a look at it at https://github.com/Vampire/setup-wsl/tree/master/.github/workflows where I define some common logic in workflow-with-copyright.main.kts and live with the consequences.

I think that would be an acceptable downside to get rid of the duplication. Given that those file are not frequently modified.
I'd document the caveat and fix in a readme in the workflows folder.

Ok, I refactored some things out to a common.main.kts and added the readme, also describing the approach and different options to generate the YAML files, together with the caveats.

Thinking more about this and the issues with this approach that you documented.

A Kotlin .main.kts script is a standalone utility that can run on its own. The native way to call them is to have Kotlin installed and just execute the script either explicitly with Kotlin CLI, or if you are using some sh derivative by executing the script.

Do we really need the standalone functionality? Personally, I probably wouldn't run it that way and it seems the downsides are pretty heavy. We could replace the consistency check with a gradle task, or just rely on the GH action to validate the state.

it seems the downsides are pretty heavy.

The downsides are not related to being able to run it standalone.
The first two are bugs of the Kotlin IntelliJ plugin which are happening when editing the scripts. How you run them is irrelevant.
But in one of the according issues it was just said that they plan to improve the whole experience, so hopefully this gets fixed in forseeable future.

The third point is a bug in Kotlin itself, either the compiler or the runtime, I don't know. Indeed there would be another mitigation option, especially as they are not run often. You can disable the compilation cache by setting an evironment variable, so for running through Gradle I could just always disable the compilation cache and that would automatically mitigate the problem there. On GHA it is not a problem anyway as it runs on ephemeral environment so there is no compilation cache used anyway.

I added it to the Gradle tasks and mentioned it in the mitigation strategies should someone wish to use one of the other ways.

build.gradle

Vampire · 2023-04-17T09:12:41Z

Not seeing the advantages yet.

Well, noone really likes reading or writing YAML. :-D
And especially with such a complex DSL like GitHub workflow files.
You always have to read how you can do what where how and have to trial and error.
By using this library, you have a proper type-safe DSL with IntelliSense in the IDE that makes it nice to write and read GHA workflows.

You can also easily reuse common logic as you have a turing-complete language at hand.
So for example the logic in the custom local setup-build-env and setup-jdks workflows could just be done in a common Kotlin script, if one would be used (read above for why none is used so far).

You could then for example also centrally define the variants and Java versions, or even the complete test matrix and reuse it.

And where it also comes in handy is, for the changes in #1629 as there I can then easily change the script to calculate the size of the Matrix and set the after_n_builds accordingly without the risk of forgetting to update it.

Vampire · 2023-04-18T22:44:25Z

I now added another commit, that leverages the fact we have a programming language at hand and uses it for the variants and java versions handling, that also shows that it is really useful. :-)

Vampire · 2023-04-19T13:04:03Z

And another commit added, so that the build script and the workflow generation script use one common source for the Java versions and variants. :-)

I think I'm out of improvement ideas for now, so review-on. :-)

leonard84

By using this library, you have a proper type-safe DSL with IntelliSense in the IDE that makes it nice to write and read GHA workflows.

Unless, it is not supported, then you end up with _customArguments and unsightly mapOf, which read much worse through all the added noise IMHO.

build.gradle

leonard84 · 2023-05-05T16:42:05Z

.github/workflows/branches-and-prs.main.kts

+ id = "build-and-verify",
+ name = "Build and Verify",
+ runsOn = RunnerType.Custom(expr("matrix.os")),
+ _customArguments = mapOf(


This isn't really nice to read

Yeah, now with the common file I also added some abstration and centralization, should be much nicer now hopefully.

.github/workflows/branches-and-prs.main.kts

.github/workflows/release.main.kts

leonard84 · 2023-05-05T17:08:10Z

.github/workflows/branches-and-prs.main.kts

+ * limitations under the License.
+ */
+
+@file:DependsOn("io.github.typesafegithub:github-workflows-kt:0.40.1")


I think that would be an acceptable downside to get rid of the duplication. Given that those file are not frequently modified.
I'd document the caveat and fix in a readme in the workflows folder.

leonard84 · 2023-06-06T17:35:54Z

.github/workflows/common.main.kts

+ excludes = axes.javaVersions
+ .filter { it.toInt() >= 17 }
+ .map { javaVersion ->
+ mapOf(
+ "os" to "ubuntu-latest",
+ "variant" to "2.5",
+ "java" to javaVersion
+ )
+ },


Couldn't we have something more intuitive, now that we have a programming language?
For example having a predicate that gets the each combination and can decide to exclude it { it.java >= 17 && variant == "2.5" }?

Definitely :-)

leonard84 · 2023-06-06T17:37:45Z

.github/workflows/common.main.kts

+ mapOf(
+ "os" to "ubuntu-latest",
+ "variant" to "2.5",
+ "java" to javaVersion
+ )


Why do we not use a data class for these?

Sure, why not. :-)

Vampire force-pushed the kotlin-workflow-files branch 2 times, most recently from 4f2aa09 to c3ed268 Compare April 12, 2023 02:44

Vampire requested a review from leonard84 April 12, 2023 02:46

leonard84 requested changes Apr 12, 2023

View reviewed changes

Vampire force-pushed the kotlin-workflow-files branch 4 times, most recently from b8fd23e to 9267bae Compare April 17, 2023 08:35

Vampire requested a review from leonard84 April 17, 2023 09:12

Vampire mentioned this pull request Apr 19, 2023

Improve Codecov settings #1629

Draft

Vampire force-pushed the kotlin-workflow-files branch 2 times, most recently from e6d19ea to e65cca2 Compare April 19, 2023 13:50

Vampire changed the title ~~Write GitHub Actions workflow files in Kotlin instead of Yaml~~ Write GitHub Actions workflow files in Kotlin instead of YAML Apr 19, 2023

Vampire force-pushed the kotlin-workflow-files branch from e004e5f to e65cca2 Compare April 19, 2023 17:51

Vampire force-pushed the kotlin-workflow-files branch from e65cca2 to 8f5f426 Compare April 28, 2023 01:03

leonard84 reviewed May 5, 2023

View reviewed changes

Vampire force-pushed the kotlin-workflow-files branch from 2d52d58 to 8dadbc2 Compare June 1, 2023 16:24

Vampire requested a review from leonard84 June 1, 2023 17:04

Vampire force-pushed the kotlin-workflow-files branch 2 times, most recently from 8f578ed to 69bfb43 Compare June 6, 2023 13:04

leonard84 reviewed Jun 6, 2023

View reviewed changes

Vampire force-pushed the kotlin-workflow-files branch from 69bfb43 to 4ae5422 Compare June 6, 2023 23:00

Vampire requested a review from leonard84 June 9, 2023 19:40

Vampire force-pushed the kotlin-workflow-files branch from a2e391d to 793dffb Compare June 28, 2023 01:08

Vampire force-pushed the kotlin-workflow-files branch from 793dffb to 73058a6 Compare August 15, 2023 19:11

Vampire force-pushed the kotlin-workflow-files branch 2 times, most recently from 4c89d72 to daa6a19 Compare May 16, 2024 10:25

Vampire added 27 commits May 22, 2024 11:35

Write GitHub Actions workflow files in Kotlin instead of Yaml

7e2481d

Update to latest github-workflows-kt version and some improvements

420bf8d

Move preprocessWorkflow task creation to build-logic

cdcba21

Verify consistency of all workflow YAMLs on pull requests and branches

fc4bfe0

yaml => yml

d693dce

Use programmatic logic for variants and java versions

010e2cd

Use one central location to define the java versions and variants

bcbd7cf

Improve all script consistency check

ad52abb

Update github-workflows-kt to v0.44.0

3bd52e9

Review feedback

50846b8

Review feedback v2

8b452d4

Catch up changes from master

bc2a062

Update github-workflows-kt version to 0.50.0

35586e1

Set group for GHA tasks to 'github actions'

b415007

Catch up changes from master

8a3d343

Catch up changes from master

0585b1f

Update github-workflows-kt version to 1.3.0

c727c29

Update github-workflows-kt version to 1.4.0

e767441

Catch up changes from master

d3ab435

Update github-workflows-kt version to 1.12.0

910ad17

Catch up changes from master

d8964bf

Update Kotlin code style settings

ef8d93b

Catch up changes from master

a5641cb

Catch up changes from master

92a8701

Catch up changes from master

c3ee994

Only use zulu on arm macOS

61808df

Update github-workflows-kt version to 2.0.0

90dc92d

Vampire force-pushed the kotlin-workflow-files branch from daa6a19 to 90dc92d Compare May 22, 2024 14:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Write GitHub Actions workflow files in Kotlin instead of YAML #1630

Write GitHub Actions workflow files in Kotlin instead of YAML #1630

Vampire commented Apr 12, 2023

codecov bot commented Apr 12, 2023 •

edited

Vampire commented Apr 12, 2023

leonard84 left a comment

leonard84 Apr 12, 2023

leonard84 Apr 12, 2023 •

edited

Vampire Apr 17, 2023

leonard84 Apr 12, 2023

Vampire Apr 17, 2023

leonard84 Apr 12, 2023

Vampire Apr 17, 2023

leonard84 May 5, 2023

Vampire Jun 1, 2023

leonard84 Jun 6, 2023

Vampire Jun 9, 2023

Vampire Jun 9, 2023

Vampire commented Apr 17, 2023

Vampire commented Apr 18, 2023 •

edited

Vampire commented Apr 19, 2023

leonard84 left a comment

leonard84 May 5, 2023

Vampire Jun 1, 2023

leonard84 May 5, 2023

leonard84 Jun 6, 2023

Vampire Jun 9, 2023

leonard84 Jun 6, 2023

Vampire Jun 9, 2023

Write GitHub Actions workflow files in Kotlin instead of YAML #1630

Are you sure you want to change the base?

Write GitHub Actions workflow files in Kotlin instead of YAML #1630

Conversation

Vampire commented Apr 12, 2023

codecov bot commented Apr 12, 2023 • edited

Codecov Report

Vampire commented Apr 12, 2023

leonard84 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leonard84 Apr 12, 2023 • edited

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

Choose a reason for hiding this comment

Vampire commented Apr 17, 2023

Vampire commented Apr 18, 2023 • edited

Vampire commented Apr 19, 2023

leonard84 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

codecov bot commented Apr 12, 2023 •

edited

leonard84 Apr 12, 2023 •

edited

Vampire commented Apr 18, 2023 •

edited