-
-
Notifications
You must be signed in to change notification settings - Fork 372
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
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
9ff11aa
.
lihaoyi 3c20ced
wip
lihaoyi 50a41d7
.
lihaoyi e46a2bd
.
lihaoyi 6f7147b
.
lihaoyi 8505cde
.
lihaoyi fec317a
.
lihaoyi f4bc130
fix test suites in examples
lihaoyi 3c6fef4
.
lihaoyi 276bbd3
.
lihaoyi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 changes: 4 additions & 0 deletions
4
example/web/7-cross-platform-version-publishing/bar/src-2/BarVersionSpecific.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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" | ||
} |
4 changes: 4 additions & 0 deletions
4
example/web/7-cross-platform-version-publishing/bar/src-3/BarVersionSpecific.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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" | ||
} |
5 changes: 5 additions & 0 deletions
5
example/web/7-cross-platform-version-publishing/bar/src/Bar.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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()) | ||
} |
12 changes: 12 additions & 0 deletions
12
example/web/7-cross-platform-version-publishing/bar/test/src/BarTests.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 | ||
} | ||
} | ||
} |
112 changes: 112 additions & 0 deletions
112
example/web/7-cross-platform-version-publishing/build.sc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
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") | ||
|
||
trait SharedTestModule 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 test extends Tests with SharedTestModule | ||
} | ||
|
||
object js extends Cross[JsModule](scalaVersions) | ||
trait JsModule extends SharedJS{ | ||
object test extends Tests with SharedTestModule | ||
} | ||
} | ||
|
||
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 test extends Tests with SharedTestModule | ||
} | ||
|
||
object js extends Cross[JsModule](scalaVersions) | ||
trait JsModule extends SharedJS { | ||
def moduleDeps = Seq(bar.js()) | ||
|
||
object test extends Tests with SharedTestModule | ||
} | ||
} | ||
|
||
// 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 __.js[3.2.2].test | ||
+ bar.BarTests.test ... <p>world Specific code for Scala 3.x</p> | ||
+ qux.QuxTests.parseJsonGetKeys ... Set(i, cow, me) | ||
|
||
> ./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... | ||
|
||
*/ |
8 changes: 8 additions & 0 deletions
8
example/web/7-cross-platform-version-publishing/qux/src-js/QuxPlatformSpecific.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 | ||
} | ||
} |
7 changes: 7 additions & 0 deletions
7
example/web/7-cross-platform-version-publishing/qux/src-jvm/QuxPlatformSpecific.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 | ||
} | ||
} |
9 changes: 9 additions & 0 deletions
9
example/web/7-cross-platform-version-publishing/qux/src/Qux.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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(_))) | ||
} | ||
} |
12 changes: 12 additions & 0 deletions
12
example/web/7-cross-platform-version-publishing/qux/test/src/QuxTests.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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
?There was a problem hiding this comment.
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 addedsrc-2.13+/
orsrc-js-jvm/
, and it would make perfect sense to mirror them inside thetest
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 machineryWe 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 theCrossSbtModule#Tests
already doThere was a problem hiding this comment.
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 inCrossScalaModule
, you cannot stack a second use case inPlatformScalaModule
and have them work together to define the final sources listWe also already delegate a lot of targets from test module to host module, so delegating another one would fit right in