First draft of the mill plugin for smithy4s #450

daddykotex · 2022-09-12T14:45:44Z

This brings mill support on top of `multimodule

Co-authored-by: David Francoeur <dfrancoeur04@gmail.com>

build.sbt

daddykotex · 2022-09-12T18:25:19Z

modules/mill-codegen-plugin/src/smithy4s/codegen/mill/Smithy4sModule.scala

+
+  /** Input directory for .smithy files */
+  protected def smithy4sInputDir: Source = T.source {
+    PathRef(millSourcePath / "smithy")


changed from os.pwd

modules/mill-codegen-plugin/test/src/smithy4s/codegen/mill/Smithy4sModuleSpec.scala

daddykotex · 2022-09-12T18:34:54Z

modules/mill-codegen-plugin/test/src/smithy4s/codegen/mill/Smithy4sModuleSpec.scala

+
+  test("multi-module codegen works") {
+    val fooEv = testKit.staticTestEvaluator(foo)(FullName("multi-module-foo"))
+    val barEv = testKit.staticTestEvaluator(foo)(FullName("multi-module-bar"))


also the implicit FullName here is rather pointless

I'd rather have the evaluator for foo to write in foo

in this example if I don't provide it, it writes to barEv and if you have multiple tests that use an evaluator with the same name, they write to the same out directory.

I think this is more a criticism of https://github.com/com-lihaoyi/mill/pull/1893/files rather than this PR though

I think this is more a criticism of https://github.com/com-lihaoyi/mill/pull/1893/files rather than this PR though

UX can be improved as we learn more by using it. MiMa has not been set on this module, specifically to be able to iterate.

.github/workflows/ci.yml

Baccata · 2022-09-13T13:16:13Z

modules/mill-codegen-plugin/src/smithy4s/codegen/mill/Smithy4sModule.scala

+  def smithy4sCodegenDependencies: T[List[String]] = List.empty[String]
+
+  def smithy4sLocalJars: T[List[os.Path]] = T {
+    T.traverse(moduleDeps)(_.jar)


.mill-version

daddykotex · 2022-09-14T14:43:24Z

that's great:

it's common sense but I never thought github would do that automatically

daddykotex · 2022-09-14T15:04:17Z

the new tests add quite a bit of overhead

daddykotex · 2022-09-14T16:07:13Z

artifact name of other mill plugin:

    <name>de.tobiasroeser.mill.integrationtest_mill0.10_2.13</name>
    <groupId>de.tototec</groupId>
    <artifactId>de.tobiasroeser.mill.integrationtest_mill0.10_2.13</artifactId>

kubukoz · 2022-09-14T16:15:08Z

the new tests add quite a bit of overhead

@daddykotex I've been thinking the IDE experience hasn't been great recently due to how large the matrix is 😬 I think Oli mentioned one time that a Mill build would work better in Metals.

As for new tests slowing this down, maybe we can run scripted tests in parallel with some others (on a separate job)?

Baccata · 2022-09-14T19:12:11Z

It's not about the build being large, it's that IJ sucks at cross-built modules.

As for new tests slowing this down, maybe we can run scripted tests in parallel with some others

Meh, the scripted tests need some compilation resulting from this job, so it'd be suboptimal to parallelise only to compile the same thing twice.

Baccata · 2022-09-14T19:14:34Z

@daddykotex looks like this is what we want, let's add it to this PR

daddykotex · 2022-09-14T19:21:48Z

yeah, I've been working on that for the last two hours, I can't figure out the right config
I had:

lazy val millPublishSettings = Def.settings(
  // EG:
  // sbt => com/disneystreaming/smithy4s/smithy4s-sbt-codegen_2.12_1.0/0.15.3/
  // mill => com/disneystreaming/smithy4s/smithy4s-mill-codegen-plugin_mill0.10_2.13/0.15.3
  Compile / sbt.Keys.artifact := {
    val sv = scalaBinaryVersion.value
    val artifact = sbt.Keys.artifact.value
    artifact.withName(
      s"${artifact.name}_mill${millPlatform(Dependencies.Mill.millVersion)}_${sv}"
    )
  }
)

But this produce:

[info] [millCodegenPlugin]      published smithy4s-mill-codegen-plugin_2.13 to local/com.disneystreaming.smithy4s/smithy4s-mill-codegen-plugin_2.13/dev-SNAPSHOT/poms/smithy4s-mill-codegen-plugin_2.13.pom
[info] [millCodegenPlugin]      published smithy4s-mill-codegen-plugin_mill0.10_2.13_2.13 to local/com.disneystreaming.smithy4s/smithy4s-mill-codegen-plugin_2.13/dev-SNAPSHOT/jars/smithy4s-mill-codegen-plugin_mill0.10_2.13_2.13.jar
[info] [millCodegenPlugin]      published smithy4s-mill-codegen-plugin_mill0.10_2.13_2.13 to local/com.disneystreaming.smithy4s/smithy4s-mill-codegen-plugin_2.13/dev-SNAPSHOT/srcs/smithy4s-mill-codegen-plugin_mill0.10_2.13_2.13-sources.jar
[info] [millCodegenPlugin]      published smithy4s-mill-codegen-plugin_mill0.10_2.13_2.13 to local/com.disneystreaming.smithy4s/smithy4s-mill-codegen-plugin_2.13/dev-SNAPSHOT/docs/smithy4s-mill-codegen-plugin_mill0.10_2.13_2.13-javadoc.jar

which is slightly off, we want:
com.disneystreaming.smithy4s/smithy4s-mill-codegen-plugin_mill0.10_2.13/dev-SNAPSHOT/

daddykotex · 2022-09-14T19:28:40Z

reached out to discord

daddykotex · 2022-09-14T19:42:49Z

Added a bit of logic to override the artifact to change the name following the logic of this mill-integrationtest: // mill => de/tototec/de.tobiasroeser.mill.integrationtest_mill0.10_2.13/0.5.1-2-5042de/

from this url, we can see that the artifact contain the mill and the scala version like: ed a bit of logic to override the artifactto change the name following the logic of thismill-integrationtest: [// mill => $GROUP/$ARTIFACT_mill${MILL_VERSION}_${SCALA_VERSION}/$VERSION/

I've tested locally, given a dummy project like:

tree .
.
├── build.sc
└── demo
    └── src
        └── Test.scala

2 directories, 2 files

I can update my build.sc from:

import mill._, mill.scalalib._
object demo extends ScalaModule {
  def scalaVersion = "2.13.8"
}

to:

import $ivy.`com.disneystreaming.smithy4s::smithy4s-mill-codegen-plugin::dev-SNAPSHOT`
import smithy4s.codegen.mill._

import mill._, mill.scalalib._
object demo extends ScalaModule with Smithy4sModule {
  def scalaVersion = "2.13.8"
  override def ivyDeps = Agg(
    ivy"com.disneystreaming.smithy4s::smithy4s-core:dev-SNAPSHOT"
  )
}

Then add a smithy/somefile.smithy under demo/ and get codegen to work!

daddykotex · 2022-09-14T19:44:58Z

you can also use: ivy"com.disneystreaming.smithy4s::smithy4s-core:${smithy4sVersion()}"

kubukoz · 2022-09-14T19:48:58Z

It's not about the build being large, it's that IJ sucks at cross-built modules.

I meant metals though!

daddykotex · 2022-09-14T19:54:04Z

From the 2.13 job:

[info] [millCodegenPlugin] :: delivering :: com.disneystreaming.smithy4s#smithy4s-mill-codegen-plugin_mill0.10_2.13;0.16-176-35b69e1 :: 0.16-176-35b69e1 :: integration :: Wed Sep 14 19:50:01 UTC 2022
[info] [millCodegenPlugin] 	delivering ivy file to /home/runner/work/smithy4s/smithy4s/modules/mill-codegen-plugin/target/jvm-2.13/ivy-0.16-176-35b69e1.xml
[info] [millCodegenPlugin] 	published smithy4s-mill-codegen-plugin_mill0.10_2.13 to /home/runner/.ivy2/local/com.disneystreaming.smithy4s/smithy4s-mill-codegen-plugin_mill0.10_2.13/0.16-176-35b69e1/poms/smithy4s-mill-codegen-plugin_mill0.10_2.13.pom
[info] [millCodegenPlugin] 	published smithy4s-mill-codegen-plugin_mill0.10_2.13 to /home/runner/.ivy2/local/com.disneystreaming.smithy4s/smithy4s-mill-codegen-plugin_mill0.10_2.13/0.16-176-35b69e1/jars/smithy4s-mill-codegen-plugin_mill0.10_2.13.jar
[info] [millCodegenPlugin] 	published smithy4s-mill-codegen-plugin_mill0.10_2.13 to /home/runner/.ivy2/local/com.disneystreaming.smithy4s/smithy4s-mill-codegen-plugin_mill0.10_2.13/0.16-176-35b69e1/srcs/smithy4s-mill-codegen-plugin_mill0.10_2.13-sources.jar
[info] [millCodegenPlugin] 	published smithy4s-mill-codegen-plugin_mill0.10_2.13 to /home/runner/.ivy2/local/com.disneystreaming.smithy4s/smithy4s-mill-codegen-plugin_mill0.10_2.13/0.16-176-35b69e1/docs/smithy4s-mill-codegen-plugin_mill0.10_2.13-javadoc.jar

👍

Baccata · 2022-09-15T19:59:43Z

@daddykotex could you add some docs in a subsequent PR ?

daddykotex · 2022-09-15T20:32:04Z

oh yes, I forgot about that

daddykotex · 2022-09-16T14:59:23Z

docs PR: #458

yisraelU and others added 4 commits September 12, 2022 10:40

First draft of the mill plugin for smithy4s

61f4be8

Co-authored-by: David Francoeur <dfrancoeur04@gmail.com>

Use millSourcePath to build the input directory

d8c7cb2

Add internal modules jars to localJars arg

1d8b18f

Mill integration tests

fa2bcbf

daddykotex force-pushed the dfrancoeur/mill branch from 44c4223 to fa2bcbf Compare September 12, 2022 14:46

daddykotex added 4 commits September 12, 2022 10:57

Set global mill version, install mill in CI

826776a

Publish before test

a50ef9b

Use mill-main-testkit

e50a94c

Drop integration tests

6df6821

daddykotex marked this pull request as ready for review September 12, 2022 18:23

daddykotex commented Sep 12, 2022

View reviewed changes

build.sbt Show resolved Hide resolved

daddykotex commented Sep 12, 2022

View reviewed changes

modules/mill-codegen-plugin/test/src/smithy4s/codegen/mill/Smithy4sModuleSpec.scala Outdated Show resolved Hide resolved

Grab version from buildinfo

492b2a8

daddykotex commented Sep 12, 2022

View reviewed changes

modules/mill-codegen-plugin/test/src/smithy4s/codegen/mill/Smithy4sModuleSpec.scala Show resolved Hide resolved

daddykotex commented Sep 12, 2022

View reviewed changes

Make sure other artifacts are published locally

d40cbb6

Baccata reviewed Sep 13, 2022

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

Baccata reviewed Sep 13, 2022

View reviewed changes

daddykotex added 2 commits September 13, 2022 09:25

Mill is not required since we use the testkit

5b6fa72

Use files in resources for mill tests

8077c7e

Base automatically changed from multimodule-codegen to 0.16.0 September 13, 2022 18:20

Baccata reviewed Sep 14, 2022

View reviewed changes

.mill-version Outdated Show resolved Hide resolved

daddykotex added 3 commits September 14, 2022 10:32

Remove leftover mill version

38830e3

Left over settings using openApi name

3715a70

Merge remote-tracking branch 'origin/0.16.0' into dfrancoeur/mill

0959ba3

Tweak artifact name to include mill version

ab4cd3c

Baccata approved these changes Sep 15, 2022

View reviewed changes

Baccata merged commit 5a17d8a into 0.16.0 Sep 15, 2022

Baccata deleted the dfrancoeur/mill branch September 15, 2022 19:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First draft of the mill plugin for smithy4s #450

First draft of the mill plugin for smithy4s #450

daddykotex commented Sep 12, 2022 •

edited

Loading

daddykotex Sep 12, 2022

daddykotex Sep 12, 2022

Baccata Sep 13, 2022 •

edited

Loading

Baccata Sep 13, 2022

daddykotex commented Sep 14, 2022

daddykotex commented Sep 14, 2022

daddykotex commented Sep 14, 2022

kubukoz commented Sep 14, 2022

Baccata commented Sep 14, 2022

Baccata commented Sep 14, 2022

daddykotex commented Sep 14, 2022

daddykotex commented Sep 14, 2022

daddykotex commented Sep 14, 2022

daddykotex commented Sep 14, 2022

kubukoz commented Sep 14, 2022

daddykotex commented Sep 14, 2022

Baccata commented Sep 15, 2022

daddykotex commented Sep 15, 2022

daddykotex commented Sep 16, 2022

First draft of the mill plugin for smithy4s #450

First draft of the mill plugin for smithy4s #450

Conversation

daddykotex commented Sep 12, 2022 • edited Loading

daddykotex Sep 12, 2022

Choose a reason for hiding this comment

daddykotex Sep 12, 2022

Choose a reason for hiding this comment

Baccata Sep 13, 2022 • edited Loading

Choose a reason for hiding this comment

Baccata Sep 13, 2022

Choose a reason for hiding this comment

daddykotex commented Sep 14, 2022

daddykotex commented Sep 14, 2022

daddykotex commented Sep 14, 2022

kubukoz commented Sep 14, 2022

Baccata commented Sep 14, 2022

Baccata commented Sep 14, 2022

daddykotex commented Sep 14, 2022

daddykotex commented Sep 14, 2022

daddykotex commented Sep 14, 2022

daddykotex commented Sep 14, 2022

kubukoz commented Sep 14, 2022

daddykotex commented Sep 14, 2022

Baccata commented Sep 15, 2022

daddykotex commented Sep 15, 2022

daddykotex commented Sep 16, 2022

daddykotex commented Sep 12, 2022 •

edited

Loading

Baccata Sep 13, 2022 •

edited

Loading