scalafmt icon indicating copy to clipboard operation
scalafmt copied to clipboard

Support Scala Native

Open olafurpg opened this issue 7 years ago • 36 comments
trafficstars

It would be awesome to support the scalafmt cli on native to avoid JVM startup time. I managed to get it working with publishLocal a while ago and experienced ~30x speedup formatting a single file (1.5s on JVM vs 50ms on Native)

https://twitter.com/olafurpg/status/857559907876433920

The following milestones are now complete

  • https://github.com/scalameta/scalafmt/pull/1170 upgrade to Scalameta v3.7 and
  • https://github.com/scalameta/scalameta/pull/1529 cross-build Scalameta to Scala Native

We are only a few steps away from being able to properly support Scala Native. The missing pieces are

  • [x] scalacheck native, blocked by https://github.com/rickynils/scalacheck/issues/396
  • [x] paiges native, blocked by https://github.com/typelevel/paiges/pull/87
  • [x] metaconfig native
  • [ ] refactor scalafmt-cli to remove JVM stuff, or reimplement the cli from scratch (tempting, the code is not so great)
  • [ ] change the config parser to a pure Scala implementation, I'm tempted to require .scalafmt.json for Native/JS since there is no official HOCON parser written in Scala

olafurpg avatar May 10 '18 16:05 olafurpg

Any updates on this? Any remaining blockers people can help with?

virusdave avatar Aug 17 '18 14:08 virusdave

@virusdave No update. Native support has been merged into paiges but it's not yet released, scalafmt doesn't depend on paiges directly but it's brought in transitively.

Biggest blocker is that the pure Scala configuration parser is not 100% compatible with the standard HOCON parser. I'm not sure what's the best move there, I don't like the idea of having slightly incompatible parsers on Native/JVM. We could use the pure Scala parser for JVM+Native but that would mean using a non-standard configuration syntax and impose a breaking change on everybody (including non-Native users).

My favorite option at the moment is to manually port the 11,000 line Java HOCON implementation to Scala. It should be a line-by-line translation, drop-in replacement. Related discussion https://github.com/lightbend/config/issues/552

Another alternative is to support .scalafmt.json (several JSON parsers support Native already) and declare that as the official syntax moving forward.

olafurpg avatar Aug 20 '18 08:08 olafurpg

To be fair, some improvements are already in place: paiges is already available for scala-native and metaconfig seems to work for native too although it's not in Central.

I tried something different, that is AOT compilation with GraalVM's native-image, which would also avoid the VM warmup penalty and we would still enjoy Lightbend's config for HOCON.

Compilation worked fine (I did an sbt cli/assembly and then native-image -jar scalafmt.jar) but when running the ELF with something like scalafmt $PWD/SomeClass.scala execution halts due to a com.oracle.svm.core.jdk.UnsupportedFeatureError: Code that was considered unreachable by closed-world analysis was reached with this stacktrace.

I'm not sure if it's a faster approach to go towards scala-native or graalvm, but I'm raising the issue to consider it.

ssaavedra avatar Aug 30 '18 14:08 ssaavedra

Thanks for sharing @ssaavedra. How would graalvm native-image work in terms of distribution and testing? What I find compelling about Scala Native is that I can

  • run the full test suite in native via sbt test, just like on the JVM
  • publish native artifacts to maven central like normal and users link the binary for their platform with a single command coursier bootstrap com.geirsson:scalafmt-cli_native:VERSION --native. Users will not need to compile scalafmt from source.

olafurpg avatar Aug 30 '18 14:08 olafurpg

Also, if you could give steps to reproduce a working usage of scalafmt under scala-native it would be much appreciated: I have a publishLocal version of metaconfig (native) and paiges (native) and even disabled testing, but I'm still unable to make things compile. When I create a cliNative project, and build it interactively setting set scalaVersion in ThisBuild := "2.11.12" and cliNative/nativeLink, I get the following error:

[info] Done compiling.
[info] Linking (5210 ms)
[error] cannot link: @java.lang.Thread::init
[error] cannot link: @java.lang.Thread::join_unit
[error] cannot link: @java.lang.Thread::setDaemon_bool_unit
[error] cannot link: @java.lang.Thread::start_unit
[error] cannot link: @java.util.concurrent.Future
[error] cannot link: @java.util.concurrent.atomic.AtomicReference::getAndUpdate_java.util.function.UnaryOperator_java.lang.Object
[error] cannot link: @scala.concurrent.forkjoin.ForkJoinPool
[error] cannot link: @scala.concurrent.forkjoin.ForkJoinPool::execute_scala.concurrent.forkjoin.ForkJoinTask_unit
[error] cannot link: @scala.concurrent.forkjoin.ForkJoinPool::getParallelism_i32
[error] cannot link: @scala.concurrent.forkjoin.ForkJoinTask
[error] cannot link: @scala.concurrent.forkjoin.ForkJoinTask::fork_scala.concurrent.forkjoin.ForkJoinTask
[error] cannot link: @scala.concurrent.forkjoin.ForkJoinTask::join_java.lang.Object
[error] cannot link: @scala.concurrent.forkjoin.ForkJoinWorkerThread
[error] cannot link: @scala.concurrent.forkjoin.RecursiveAction
[error] cannot link: @scala.concurrent.forkjoin.RecursiveAction::init
[error] unable to link
[error] (cliNative / Compile / nativeLink) unable to link
[error] Total time: 9 s, completed Aug 30, 2018 4:23:39 PM

ssaavedra avatar Aug 30 '18 14:08 ssaavedra

The scalafmt cli currently uses parallel collections which I suspect use JVM concurrency features. To link the cli in native it probably needs a bit of refactoring.

olafurpg avatar Aug 30 '18 14:08 olafurpg

For people interested in native scalafmt, please give the branch in https://github.com/scalameta/scalafmt/pull/1266 a try and report back how it works :)

olafurpg avatar Aug 30 '18 18:08 olafurpg

Some update on this. I have ported the config library and published for JVM and have a few branches to add to a potential Scala Native 0.3.9 release. I have successfully run the bit of code used to parse scalafmt config files in Scala Native.

https://github.com/ekrich/sconfig

PR to use this in metaconfig which is used by scalafmt - I think the default still uses lightbend/config for JVM. https://github.com/olafurpg/metaconfig/commit/b2363df06365eb418e9e9531434e5956a3a95c7b

ekrich avatar Jan 07 '19 15:01 ekrich

We now have the changes in Scala Native 0.3.x to allow sconfig to cross compile to Scala Native and run the code metaconfig uses to parse the scalafmt HOCON config file.

ekrich avatar Mar 22 '19 18:03 ekrich

Paiges available now.

larsrh avatar Mar 31 '19 15:03 larsrh

Scala native 0.4.0 is just released https://scala-native.readthedocs.io/en/v0.4.0/changelog/0.4.0.html

He-Pin avatar Jan 19 '21 19:01 He-Pin

Status of sconfig - https://github.com/ekrich/sconfig/pull/139

ekrich avatar Jan 19 '21 20:01 ekrich

I have released a version of java.time, sjavatime, and a new version of sconfig that should be ready and not be blocking this anymore.

ekrich avatar Feb 25 '21 16:02 ekrich

I think biggest blocking issue is scalameta currently :/ The scala native support for removed some time ago and we need to add it back.

tgodzik avatar Feb 25 '21 16:02 tgodzik

Ok, I almost got something working : https://github.com/scalameta/scalameta/pull/2261

tgodzik avatar Mar 03 '21 10:03 tgodzik

There is a compilation error though, but we are looking into this.

tgodzik avatar Mar 03 '21 10:03 tgodzik

I wanted to point out this although I am not sure if it has been solved or not yet - https://github.com/scala-native/scala-native/issues/1575

The compilation error should be fixed in master - https://github.com/scala-native/scala-native/commit/0ded408cf0acebfac12049f875675a6efc31544f

I see you have provided a workaround in scalameta here: https://github.com/scalameta/scalameta/commit/2db5f934bf45d308a0830318658963f895b39e94#diff-865573f4d2cf3a17bdfd191d2665095bf22d743bc9433628d09b677103786d54L270-L281

ekrich avatar Mar 04 '21 23:03 ekrich

@tgodzik Let me know if you need any help.

ekrich avatar Jun 04 '21 15:06 ekrich

I am down to to errors:

[error] Found 2 missing definitions while linking
[error] Not found Top(scopt.Read$)
[error]         at /home/tgodzik/Documents/scalafmt/scalafmt-cli/shared/src/main/scala/org/scalafmt/cli/CliArgParser.scala:61
[error]         at /home/tgodzik/Documents/scalafmt/scalafmt-cli/shared/src/main/scala/org/scalafmt/cli/CliArgParser.scala:64
....
[error] Not found Top(org.scalafmt.interfaces.Scalafmt$)
[error]         at /home/tgodzik/Documents/scalafmt/scalafmt-cli/shared/src/main/scala/org/scalafmt/cli/ScalafmtDynamicRunner.scala:26

Second one is something I need to rewrite to Scala, since the default interfaces are written in Java, however I can't figure out why scopt is problematic since that is released for native. I do have to work on couple of things still, but I should get something compilable soon.

tgodzik avatar Jun 05 '21 17:06 tgodzik

Weird thing, if I do import scopt.Read by hand it works, but I get:

[info] Linking (2383 ms)
[error] missing symbols:
[error] * M12java.net.URID5toURLL12java.net.URLEO
[error]   - from M34org.ekrich.config.impl.PlatformUriD5toURLL12java.net.URLEO
[error]   - from M33org.ekrich.config.impl.Parseable$D10relativeToL12java.net.URLL16java.lang.StringL12java.net.URLEO
[error]   - from M45org.ekrich.config.impl.Parseable$ParseableURLD10relativeToL16java.lang.StringL33org.ekrich.config.ConfigParseableEO
[error]   - from M33org.ekrich.config.impl.Parseable$D6newURLL12java.net.URLL36org.ekrich.config.ConfigParseOptionsL32org.ekrich.config.impl.ParseableEO
[error]   - from M32org.ekrich.config.ConfigFactory$D8parseURLL12java.net.URLL36org.ekrich.config.ConfigParseOptionsL24org.ekrich.config.ConfigEO
[error]   - from M38org.ekrich.config.impl.SimpleIncluder$D25includeURLWithoutFallbackL38org.ekrich.config.ConfigIncludeContextL12java.net.URLL30org.ekrich.config.ConfigObjectEO

and a bunch of other things :O

tgodzik avatar Jun 06 '21 06:06 tgodzik

My current code is at https://github.com/scalameta/scalafmt/compare/master...tgodzik:add-scala-native?expand=1

There are still some other minor stuff to work out, but I am completely stumped by this one.

tgodzik avatar Jun 06 '21 06:06 tgodzik

I’ll have to look at that later today. I could have missed something there or something changed in Scala Native.

ekrich avatar Jun 06 '21 13:06 ekrich

Scala Native still has an empty URL class. Do you mind trying nativeLinkStubs := true in the build. The URI.toURL method is a stub in Scala Native.

  @scalanative.annotation.stub
  def toURL(): java.net.URL = ???

When working with Olafur previously, the code did not use the ConfigFactory.parseFile code path. Unfortunately, I was concentrating on having a smooth transition to Scala 3 and refactoring the tests for more code coverage was further down on the list for sconfig. The tested path was how the scalafmt code worked before for Scala Native and is shown here: https://github.com/ekrich/sconfig/blob/main/docs/SCALA-NATIVE.md We may be hitting an unknown code path or something else is going on.

After looking further at this it looks like PlatformUri in sconfig should not be shared with the JVM. We are just kicking the link stubs one layer deeper to more link stubs.

Wow, you had to bring the difflib across from munit too.

ekrich avatar Jun 07 '21 00:06 ekrich

Scala Native still has an empty URL class. Do you mind trying nativeLinkStubs := true in the build. The URI.toURL method is a stub in Scala Native.

That helped. I am down to two issuses:

[error] * M26java.text.SimpleDateFormatRL16java.lang.StringE
...
[error] * T26java.text.SimpleDateFormat

Is SimpleDateFormat implemented in sjavatime or do we need to add it there?

[error] * M38java.util.concurrent.ConcurrentHashMapRE
...
[error] * M40java.util.concurrent.LinkedBlockingDequeRE
...

The concurrent collections is something that I can probably replace.

[error] * M18scopt.OptionParserRL16java.lang.StringE
...
[error] * T15java.io.Console

No idea about console and scoopt yet.

Wow, you had to bring the difflib across from munit too.

I might try to release it as a separate library.

tgodzik avatar Jun 07 '21 08:06 tgodzik

Ok, got it all compiling at least for now, but getting an exception:

scala.NotImplementedError: an implementation is missing
	at java.lang.Throwable.fillInStackTrace(Unknown Source)
	at scala.Predef$.???(Unknown Source)
	at java.net.URI.toURL(Unknown Source)
	at org.ekrich.config.impl.PlatformUri.toURL(Unknown Source)
	at org.ekrich.config.impl.SimpleConfigOrigin$.newFile(Unknown Source)
	at org.ekrich.config.impl.Parseable$ParseableFile.createOrigin(Unknown Source)
	at org.ekrich.config.impl.Parseable.postConstruct(Unknown Source)
	at org.ekrich.config.impl.Parseable$.newFile(Unknown Source)
	at org.ekrich.config.ConfigFactory$.parseFile(Unknown Source)
	at org.ekrich.config.ConfigFactory$.parseFile(Unknown Source)
	at metaconfig.sconfig.SConfig2Class$.$anonfun$gimmeConfFromFile$1(Unknown Source)
	at metaconfig.sconfig.SConfig2Class$$$Lambda$2.apply(Unknown Source)
	at metaconfig.sconfig.SConfig2Class$.gimmeSafeConf(Unknown Source)
	at metaconfig.sconfig.SConfig2Class$.gimmeConfFromFile(Unknown Source)
	at metaconfig.sconfig.package$.metaconfig$sconfig$package$$$anonfun$sConfigMetaconfigParser$1(Unknown Source)
	at metaconfig.sconfig.package$$anonfun$1.fromInput(Unknown Source)
	at metaconfig.Input$InputImplicits.parse(Unknown Source)
	at metaconfig.Input$InputImplicits.$anonfun$parse$1(Unknown Source)
	at metaconfig.Input$InputImplicits$$Lambda$1.apply(Unknown Source)
	at scala.Option.fold(Unknown Source)
	at metaconfig.Input$InputImplicits.parse(Unknown Source)
	at org.scalafmt.config.Config$.$anonfun$hoconFileToConf$2(Unknown Source)
	at org.scalafmt.config.Config$$$Lambda$2.apply(Unknown Source)
	at metaconfig.Configured.andThen(Unknown Source)
	at org.scalafmt.config.Config$.hoconFileToConf(Unknown Source)
	at org.scalafmt.cli.CliOptions.$anonfun$hoconOpt$3(Unknown Source)
	at org.scalafmt.cli.CliOptions$$Lambda$8.apply(Unknown Source)
	at scala.Option.map(Unknown Source)
	at org.scalafmt.cli.CliOptions.hoconOpt$lzycompute(Unknown Source)
	at org.scalafmt.cli.CliOptions.getHoconValueOpt(Unknown Source)
	at org.scalafmt.cli.CliOptions.getVersionIfDifferent(Unknown Source)
	at org.scalafmt.cli.Cli.findRunner(Unknown Source)
	at org.scalafmt.cli.Cli.run(Unknown Source)
	at org.scalafmt.cli.Cli.mainWithOptions(Unknown Source)
	at org.scalafmt.cli.Cli$.main(Unknown Source)
	at <none>.main(Unknown Source)
	at <none>.__libc_start_main(Unknown Source)
	at <none>._start(Unknown Source)

@ekrich I think that's something we should fix in sconfig?

tgodzik avatar Jun 07 '21 12:06 tgodzik

@tgodzik if you could only call ConfigFactory.parseString, I think it would work. parseFile can read URLs from the file and URL is a mine field in Scala Native and Scala.js - no support basically. Please do something like this if possible.

import java.nio.file.{Files, Paths}
val bytes = Files.readAllBytes(Paths.get("src/main/resources/myapp.conf"))
val configStr = new String(bytes)
val conf = ConfigFactory.parseString(configStr)

We don't have java.text support so maybe java.util.Formatter?

I put together sjavatime as a stop gap until https://github.com/cquiroz/scala-java-time could be updated but nobody has managed to get this library updated yet.

Hope this helps.

ekrich avatar Jun 07 '21 15:06 ekrich

I will do with toString for now, it's not that bad. Turns out though there is a bunch of other issues:

java.lang.UnsupportedOperationException
	at java.lang.Throwable.fillInStackTrace(Unknown Source)
	at java.nio.file.PathMatcherImpl$.apply(Unknown Source)
	at scala.scalanative.nio.fs.UnixFileSystem.getPathMatcher(Unknown Source)
	at org.scalafmt.config.ProjectFiles$FileMatcher$.$anonfun$nio$1(Unknown Source)
	at org.scalafmt.config.ProjectFiles$FileMatcher$$$Lambda$2.apply(Unknown Source)
	at scala.collection.immutable.List.map(Unknown Source)
	at scala.collection.immutable.List.map(Unknown Source)
	at org.scalafmt.config.ProjectFiles$FileMatcher$.create(Unknown Source)
	at org.scalafmt.config.ProjectFiles$FileMatcher$.nio(Unknown Source)
	at org.scalafmt.config.ProjectFiles$FileMatcher$.apply(Unknown Source)
	at org.scalafmt.cli.ScalafmtCoreRunner$.$anonfun$run$2(Unknown Source)
	at org.scalafmt.cli.ScalafmtCoreRunner$$$Lambda$2.apply(Unknown Source)
	at metaconfig.Configured$ConfiguredImplicit.fold(Unknown Source)
	at org.scalafmt.cli.ScalafmtCoreRunner$.run(Unknown Source)
	at org.scalafmt.cli.Cli.runWithRunner(Unknown Source)
	at org.scalafmt.cli.Cli.run(Unknown Source)
	at org.scalafmt.cli.Cli.mainWithOptions(Unknown Source)
	at org.scalafmt.cli.Cli$.main(Unknown Source)
	at <none>.main(Unknown Source)
	at <none>.__libc_start_main(Unknown Source)
	at <none>._start(Unknown Source)

Scalafmt uses getPathMatcher which seems to be not implemented and that is used in a bunch of places. I also found some failing regexes, which I asked Wojciech to investigate.

I also found another issue with running git:

        sys.process
          .Process(cmd, workingDirectory.jfile)
          .!!(swallowStderr)
          .trim

Seems that it's not possible currently to run a process?

There is one last issue that deals with Threads, but I guess I can work around it for Scala Native.

tgodzik avatar Jun 07 '21 16:06 tgodzik

We do support java.lang.Process and java.lang.ProcessBuilder. I looked and it seems that scala.sys.process.ProcessImpl requires threads.

ekrich avatar Jun 07 '21 16:06 ekrich

Ach ok, that fixed it. The remaining issues are:

  • scala.scalanative.nio.fs.UnixFileSystem.getPathMatcher maybe there is a way to work around it
  • failing regexes - I don't think there is a way around those currently
  • non-thread implementation of UpdateDisplayThread

tgodzik avatar Jun 07 '21 17:06 tgodzik

What is the issue with scala.scalanative.nio.fs.UnixFileSystem.getPathMatcher?

cc/ Our regex expert @LeeTibbert

ekrich avatar Jun 07 '21 17:06 ekrich