WIP: Scala native #206
WIP: Scala native #206
Conversation
lihaoyi
force-pushed
the
master
branch
2 times, most recently
from
9ebd19b to
779e453
Compare
|
How the work is on this PL? Now Scala Native 0.3.7 with the new API is out so it could be very useful! |
|
Very good to know, thank you :-) |
|
Actually. I'm just about ready to push another update. I currently have it working against 0.3.7. The support for testing frameworks has been the most challenging part. I currently have utest, and scalatest mostly working. I'll push an update for comment... |
|
@ajrnz Oh, that's great. I think if testing support is the most challenging part - we could implement it as separate PR, what do you think? |
|
I think I've done all the hard work now. It works, it just needs a little tidying. Probably better to have people look at it. I just need to rebase from 0.2.0 to master and then I can update the PR. |
|
That sounds great, thank you! |
|
Don't hesitate to ask, if you need any help |
|
Ok you should be able to use this against the published version of scala-native (0.3.7). It uses the new build API. Tests should work but there may be a few rough edges. Still needs the following:
https://github.com/ajrnz/scala-native-example-app can be used to for an example of how to use the module, pending documentation. |
|
I tryed the latest commit in the PR but it fails to link: import mill._
import mill.scalalib._
import mill.scalanativelib._
object native extends ScalaNativeModule {
def scalaVersion = "2.11.12"
def scalaNativeVersion = "0.3.7"
}It gives me: |
|
It seems also that ScalaNativeModule doesn't change the mainClass according to |
|
@lolgab can you please try this repo https://github.com/ajrnz/scala-native-example-app and tell me what happens? Also try adding, |
|
Adding |
|
I got it! 😃 def ivyDeps = Agg(
ivy"com.lihaoyi::scalatags::0.6.7"
)and scalatags has a dependency on the scala native lib. If I remove it it doesn't work anymore.. |
|
Hmm strange, it should be being included. Time for some test projects... |
|
In the current state it can now build and test using published scala-native 0.3.7. It's capable of building and testing (10 test failures) upickle for example. See: https://github.com/ajrnz/upickle/tree/scala-native. You'll need to locally publish your own scalacheck 1.14.0 scala-native artifact until it's published on maven central) @lihaoyi, @rockjam It would be good to get some feedback I haven't really used it in anger yet but I have an idea for a simple project which I'll try when I get a chance |
|
@ajrnz could you update the PR description with a summary of what the major changes and moving parts are in this PR? |
|
@lihaoyi Updated. Let me know if you need anything else |
|
I'll take a look; @densh do you think you could glance over this and see if what andrew is doing makes sense to you, from a scala-native perspective? |
|
In my setup it prints by default 735 lines of informations (useless to a normal user like me) about the linking stage. With a minimal build like this: import mill._, mill.scalalib._
import mill.scalanativelib._
object build extends ScalaNativeModule {
def scalaVersion = "2.11.12"
def scalaNativeVersion = "0.3.7"
} |
|
This looks ok to me. @densh said he'll take a look |
There was a problem hiding this comment.
Thanks a lot @ajrnz , it's really nice to see SN integration in more build tools.
I left some comments below. The integration with our build api looks reasonable overall. I'd really recommend to preserve names and types of the settings to minimize surprise (e.g. in case someone has to migrate their build between the two build systems). nativeMode should really not be a boolean for example.
I don't have much comments on the mill side as I'm not 100% sure what's the idiomatic way to do things (e.g. logging) wrt to the way things are done in sbt.
| * @param args Arguments to pass to the program | ||
| * @param logger Logger to log to. | ||
| */ | ||
| class ComRunner(bin: File, |
There was a problem hiding this comment.
We're going to refactor test runner out of sbt plugin in 0.3.8 (see scala-native/scala-native#1234). It was an unfortunate oversight not to do it sooner.
There was a problem hiding this comment.
@densh Thanks for the review
There was a problem hiding this comment.
@densh what's the timeline of 0.3.8 look like? If it's soon-ish (next week or so?) maybe we should just want for it to land before merging this, rather than merging in all this copy-paste code only to remove it a week later
There was a problem hiding this comment.
(also, it's a bit hard for me to review this with all the copy-paste code; even if i try to ignore it it's pretty distracting and makes it hard to focus on the Mill-side changes)
| Logger(debugFn = msg => Unit, //err.println(msg), | ||
| infoFn = msg => out.println(msg), | ||
| warnFn = msg => out.println(msg), | ||
| errorFn = msg => err.println(msg)) |
There was a problem hiding this comment.
It would be nice to have some way to trigger debug messages being shown or not, maybe add a nativeVerbose: Boolean as a extra setting to control this. @lihaoyi is this the idiomatic way to do logging in mill?
There was a problem hiding this comment.
I agree it. It is currently way to verbose. I'm not really sure what the best way to handle this is. Happy to take suggestions @lihaoyi
There was a problem hiding this comment.
We could define any flags or configuraiton on ScalaNativeModule and plumb it down here and where-ever necessary. Then people can override it to configure
| override def scalacPluginIvyDeps = super.scalacPluginIvyDeps() ++ | ||
| Agg(ivy"org.scala-native:nscplugin_${scalaVersion()}:${scalaNativeVersion()}") | ||
|
|
||
| def releaseMode = T { false } |
There was a problem hiding this comment.
We reserve right to introduce more modes, that's why this setting is a string in the build api and sbt plugin. I'd recommend to keep it a string here as well to avoid future problems if more modes are introduced.
There was a problem hiding this comment.
OK. I'd been meaning to look at this. Will change.
There was a problem hiding this comment.
To me it is better to have a sealed trait instead of a string. So if more modes are introduced there are no problems in the future. It is how I did with scala js ModuleKinds miming the scalajs api.
| // case Level.Error => logger.error(message.message) | ||
| // case Level.Trace => message.throwable.foreach(logger.trace(_)) | ||
| // case Level.Debug => logger.debug(message.message) | ||
| // } |
There was a problem hiding this comment.
We have "no commented out code" policy in SN, is this left as an oversight?
There was a problem hiding this comment.
Just to remind me that the current situation is not ideal and it needs sorting out. Will be removed when the logging situation is resolved.
Thanks!
|
Ok the latest push should sort out the situation regarding release mode and logging configuration. I'll check out scala-native/scala-native#1234 to see that it does everything required. I'd definitely prefer not to commit copied code. |
|
Looks good to me, let's wait till 0.3.8 is out for some last minute testing before merging |
including worker/bridge
this code is not published ornot published at the correct scala version so copy it in for now
very messy at the moment also correct bridge version as much as possible with out a scala-native release
scala-native com-lihaoyi#1143 now merged
support for multiple test frameworks tidy up
better method of loading JVM test frameworks
propagate release and log levels to test projects
add ability easily compile against scala-native snapshots
The PR adds scala-native support to mill. It is integrated to mill via
mill.scalanativelib.ScalaNativeModulewhich can be extended in the same way asScalaModuleIt's built against a scala-native 0.3.8-SNAPSHOT PR (scala-native/scala-native#1234) which would need to be compiled and published locally.
In addition to the standard
scalaVersionyou must also supplyscalaNativeVersion.An optimized binary can be built using
releaseMode = ReleaseMode.ReleaseTwo test frameworks are currently supported
utestandscalatest. Support forscalacheckshould be possible when the relevant artifacts have been published.HelloNativeWorldTests.scalabuilds and tests a simple project (based onHelloJSWorldTests.scala)An example
build.scwould look like:This code code will require minor modifications to remove the -SNAPSHOT versioning once the release version of scala-native 0.3.8 is released.