Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generalize handling of source folders in cross-platform/version scenarios #2531

Merged
merged 10 commits into from
May 20, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion docs/modules/ROOT/pages/Web_Build_Examples.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,8 @@ include::example/web/5-webapp-scalajs-shared.adoc[]

== Publishing Cross-Platform Scala Modules

include::example/web/6-cross-platform-publishing.adoc[]
include::example/web/6-cross-version-platform-publishing.adoc[]

== Publishing Cross-Platform Scala Modules Alternative

include::example/web/7-cross-platform-version-publishing.adoc[]
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,5 @@ Publishing Artifact(com.lihaoyi,foo-bar_sjs1_3,0.0.1) to ivy repo...
Publishing Artifact(com.lihaoyi,foo-bar_3,0.0.1) to ivy repo...
Publishing Artifact(com.lihaoyi,foo-qux_sjs1_3,0.0.1) to ivy repo...
Publishing Artifact(com.lihaoyi,foo-qux_3,0.0.1) to ivy repo...

*/
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package bar
object BarVersionSpecific {
def text(): String = "Specific code for Scala 2.x"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package bar
object BarVersionSpecific {
def text(): String = "Specific code for Scala 3.x"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package bar
import scalatags.Text.all._
object Bar {
val value = p("world", " ", BarVersionSpecific.text())
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package bar
import utest._
object BarTests extends TestSuite {
def tests = Tests {
test("test") {
val result = Bar.value.toString
val matcher = "<p>world Specific code for Scala [23].x</p>".r
assert(matcher.matches(result))
result
}
}
}
100 changes: 100 additions & 0 deletions example/web/7-cross-platform-version-publishing/build.sc
Copy link
Member

Choose a reason for hiding this comment

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

file needs scalafmt

Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import mill._, scalalib._, scalajslib._, publish._

trait Shared extends CrossScalaModule with PlatformScalaModule with PublishModule {
def publishVersion = "0.0.1"

def pomSettings = PomSettings(
description = "Hello",
organization = "com.lihaoyi",
url = "https://github.com/lihaoyi/example",
licenses = Seq(License.MIT),
versionControl = VersionControl.github("lihaoyi", "example"),
developers = Seq(Developer("lihaoyi", "Li Haoyi", "https://github.com/lihaoyi"))
)

def ivyDeps = Agg(ivy"com.lihaoyi::scalatags::0.12.0")

object test extends Tests {
def ivyDeps = Agg(ivy"com.lihaoyi::utest::0.7.11")
def testFramework = "utest.runner.Framework"
}
}

trait SharedJS extends Shared with ScalaJSModule {
def scalaJSVersion = "1.13.0"
}

val scalaVersions = Seq("2.13.8", "3.2.2")

object bar extends Module {
object jvm extends Cross[JvmModule](scalaVersions)
trait JvmModule extends Shared

object js extends Cross[JsModule](scalaVersions)
trait JsModule extends SharedJS
}

object qux extends Module{
object jvm extends Cross[JvmModule](scalaVersions)
trait JvmModule extends Shared{
def moduleDeps = Seq(bar.jvm())
def ivyDeps = super.ivyDeps() ++ Agg(ivy"com.lihaoyi::upickle::3.0.0")
}

object js extends Cross[JsModule](scalaVersions)
trait JsModule extends SharedJS {
def moduleDeps = Seq(bar.js())
}
}

// This example demonstrates an alternative way of defining your cross-platform
// cross-version modules: rather than wrapping them all in a `foo`
// cross-module to provide the different versions, we instead give each module
// `bar.jvm`, `bar.js`, `qux.jvm`, `qux.js` its own `Cross` module. This
// approach can be useful if the different cross modules need to support
// different sets of Scala versions, as it allows you to specify the
// `scalaVersions` passed to each individual cross module separately.

/** Usage

> ./mill show qux.js[3.2.2].sources
[
".../qux/src",
".../qux/src-js",
".../qux/src-3.2.2",
".../qux/src-3.2.2-js",
".../qux/src-3.2",
".../qux/src-3.2-js",
".../qux/src-3",
".../qux/src-3-js"
]

> ./mill show qux.js[3.2.2].test.sources
[
".../qux/test/src",
".../qux/test/src-js",
".../qux/test/src-3.2.2",
".../qux/test/src-3.2.2-js",
".../qux/test/src-3.2",
".../qux/test/src-3.2-js",
".../qux/test/src-3",
".../qux/test/src-3-js"
]

> ./mill qux.jvm[2.13.8].run
Bar.value: <p>world Specific code for Scala 2.x</p>
Parsing JSON with ujson.read
Qux.main: Set(<p>i</p>, <p>cow</p>, <p>me</p>)

> ./mill __.publishLocal
...
Publishing Artifact(com.lihaoyi,bar_sjs1_2.13,0.0.1) to ivy repo...
Publishing Artifact(com.lihaoyi,bar_2.13,0.0.1) to ivy repo...
Publishing Artifact(com.lihaoyi,qux_sjs1_2.13,0.0.1) to ivy repo...
Publishing Artifact(com.lihaoyi,qux_2.13,0.0.1) to ivy repo...
Publishing Artifact(com.lihaoyi,bar_sjs1_3,0.0.1) to ivy repo...
Publishing Artifact(com.lihaoyi,bar_3,0.0.1) to ivy repo...
Publishing Artifact(com.lihaoyi,qux_sjs1_3,0.0.1) to ivy repo...
Publishing Artifact(com.lihaoyi,qux_3,0.0.1) to ivy repo...

*/
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package bar
object BarVersionSpecific {
def text(): String = "Specific code for Scala 2.x"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package bar
object BarVersionSpecific {
def text(): String = "Specific code for Scala 3.x"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package bar
import scalatags.Text.all._
object Bar {
val value = p("world", " ", BarVersionSpecific.text())
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package bar
import utest._
object BarTests extends TestSuite {
def tests = Tests {
test("test") {
val result = Bar.value.toString
val matcher = "<p>world Specific code for Scala [23].x</p>".r
assert(matcher.matches(result))
result
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package qux
import scala.scalajs.js
object QuxPlatformSpecific {
def parseJsonGetKeys(s: String): Set[String] = {
println("Parsing JSON with js.JSON.parse")
js.JSON.parse(s).asInstanceOf[js.Dictionary[_]].keys.toSet
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package qux
object QuxPlatformSpecific {
def parseJsonGetKeys(s: String): Set[String] = {
println("Parsing JSON with ujson.read")
ujson.read(s).obj.keys.toSet
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package qux
import scalatags.Text.all._
object Qux {
def main(args: Array[String]): Unit = {
println("Bar.value: " + bar.Bar.value)
val string = """{"i": "am", "cow": "hear", "me": "moo"}"""
println("Qux.main: " + QuxPlatformSpecific.parseJsonGetKeys(string).map(p(_)))
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package qux
import utest._
object QuxTests extends TestSuite {
def tests = Tests {
test("parseJsonGetKeys") {
val string = """{"i": "am", "cow": "hear", "me": "moo}"""
val keys = QuxPlatformSpecific.parseJsonGetKeys(string)
assert(keys == Set("i", "cow", "me"))
keys
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package qux
import scala.scalajs.js
object QuxPlatformSpecific {
def parseJsonGetKeys(s: String): Set[String] = {
println("Parsing JSON with js.JSON.parse")
js.JSON.parse(s).asInstanceOf[js.Dictionary[_]].keys.toSet
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package qux
object QuxPlatformSpecific {
def parseJsonGetKeys(s: String): Set[String] = {
println("Parsing JSON with ujson.read")
ujson.read(s).obj.keys.toSet
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package qux
import scalatags.Text.all._
object Qux {
def main(args: Array[String]): Unit = {
println("Bar.value: " + bar.Bar.value)
val string = """{"i": "am", "cow": "hear", "me": "moo"}"""
println("Qux.main: " + QuxPlatformSpecific.parseJsonGetKeys(string).map(p(_)))
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package qux
import utest._
object QuxTests extends TestSuite {
def tests = Tests {
test("parseJsonGetKeys") {
val string = """{"i": "am", "cow": "hear", "me": "moo}"""
val keys = QuxPlatformSpecific.parseJsonGetKeys(string)
assert(keys == Set("i", "cow", "me"))
keys
}
}
}
12 changes: 9 additions & 3 deletions scalajslib/src/mill/scalajslib/ScalaJSModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,16 @@ trait ScalaJSModule extends scalalib.ScalaModule { outer =>
}

override def mandatoryScalacOptions = T {
super.mandatoryScalacOptions() ++ {
if (ZincWorkerUtil.isScala3(scalaVersion())) Seq("-scalajs")
// Don't add flag twice, e.g. if a test suite inherits it both directly
// ScalaJSModule as well as from the enclosing non-test ScalaJSModule
val scalajsFlag =
if (
ZincWorkerUtil.isScala3(scalaVersion()) &&
!super.mandatoryScalacOptions().contains("-scalajs")
) Seq("-scalajs")
else Seq.empty
}

super.mandatoryScalacOptions() ++ scalajsFlag
}

override def scalacPluginIvyDeps = T {
Expand Down
8 changes: 0 additions & 8 deletions scalalib/src/mill/scalalib/CrossScalaModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,4 @@ trait CrossScalaModule extends ScalaModule with CrossModuleBase {
super.sources() ++
scalaVersionDirectoryNames.map(s => PathRef(millSourcePath / s"src-$s"))
}

trait CrossScalaModuleTests extends ScalaModuleTests {
override def sources = T.sources {
super.sources() ++
scalaVersionDirectoryNames.map(s => PathRef(millSourcePath / s"src-$s"))
}
}
trait Tests extends CrossScalaModuleTests
}
6 changes: 6 additions & 0 deletions scalalib/src/mill/scalalib/JavaModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,13 @@ trait JavaModule
override def zincWorker: ZincWorkerModule = outer.zincWorker
override def skipIdea: Boolean = outer.skipIdea
override def runUseArgsFile: Target[Boolean] = T { outer.runUseArgsFile() }
override def sources = T.sources {
for (src <- outer.sources()) yield {
PathRef(this.millSourcePath / src.path.relativeTo(outer.millSourcePath))
Copy link
Member

Choose a reason for hiding this comment

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

What if the user added additional source folders, maybe pointing outside of outer.millSourcePath?

Copy link
Member Author

@lihaoyi lihaoyi May 20, 2023

Choose a reason for hiding this comment

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

Seems reasonable to mirror them inside test I think? That's what I'm finding as I port builds over, e.g. maybe someone added src-2.13+/ or src-js-jvm/, and it would make perfect sense to mirror them inside the test submodule. Especially v.s. the current status quo, where the source folder layout of test folders is pretty manual and thus ad-hoc, "same as parent module" seems like a big simplification both in UX as well as internal machinery

We could filter these folders somehow if we really want, but IMO it's not necessary. In most common cases the only downside would be the test module gets a few unused-but-reasonable source folders, and worst comes to worst if someone wants to change this mirroring they can always override it, which is what the CrossSbtModule#Tests already do

Copy link
Member Author

Choose a reason for hiding this comment

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

One more consideration is that the current approach of shadowing the Tests trait does not compose. While it works for one use case in CrossScalaModule, you cannot stack a second use case in PlatformScalaModule and have them work together to define the final sources list

We also already delegate a lot of targets from test module to host module, so delegating another one would fit right in

}
}
}

trait Tests extends JavaModuleTests

def defaultCommandName(): String = "run"
Expand Down
16 changes: 9 additions & 7 deletions scalalib/src/mill/scalalib/PlatformScalaModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@ trait PlatformScalaModule extends ScalaModule {
override def millSourcePath = super.millSourcePath / os.up

override def sources = T.sources {
val platform = millModuleSegments.parts.last
super.sources().flatMap(source =>
Seq(
source,
PathRef(source.path / os.up / s"${source.path.last}-${platform}")
)
)
val platform = millModuleSegments
.value
.collect { case l: mill.define.Segment.Label => l.value }
.last

super.sources().flatMap { source =>
val platformPath = PathRef(source.path / _root_.os.up / s"${source.path.last}-${platform}")
Seq(source, platformPath)
}
}

override def artifactNameParts = super.artifactNameParts().dropRight(1)
Expand Down