-
Notifications
You must be signed in to change notification settings - Fork 276
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
Support Scala Native #1172
Comments
Any updates on this? Any remaining blockers people can help with? |
@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 lightbend/config#552 Another alternative is to support .scalafmt.json (several JSON parsers support Native already) and declare that as the official syntax moving forward. |
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 Compilation worked fine (I did an 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. |
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
|
Also, if you could give steps to reproduce a working usage of scalafmt under scala-native it would be much appreciated: I have a
|
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. |
For people interested in native scalafmt, please give the branch in #1266 a try and report back how it works :) |
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 https://github.com/ekrich/sconfig PR to use this in metaconfig which is used by |
We now have the changes in Scala Native 0.3.x to allow sconfig to cross compile to Scala Native and run the code |
Paiges available now. |
Scala native 0.4.0 is just released https://scala-native.readthedocs.io/en/v0.4.0/changelog/0.4.0.html |
Status of |
I think biggest blocking issue is scalameta currently :/ The scala native support for removed some time ago and we need to add it back. |
Ok, I almost got something working : scalameta/scalameta#2261 |
There is a compilation error though, but we are looking into this. |
I wanted to point out this although I am not sure if it has been solved or not yet - scala-native/scala-native#1575 The compilation error should be fixed in master - scala-native/scala-native@0ded408 I see you have provided a workaround in |
@tgodzik Let me know if you need any help. |
I am down to to errors:
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. |
Weird thing, if I do
and a bunch of other things :O |
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. |
I’ll have to look at that later today. I could have missed something there or something changed in Scala Native. |
Scala Native still has an empty @scalanative.annotation.stub
def toURL(): java.net.URL = ??? When working with Olafur previously, the code did not use the After looking further at this it looks like Wow, you had to bring the difflib across from |
That helped. I am down to two issuses:
Is SimpleDateFormat implemented in sjavatime or do we need to add it there?
The concurrent collections is something that I can probably replace.
No idea about console and scoopt yet.
I might try to release it as a separate library. |
Ok, got it all compiling at least for now, but getting an exception:
@ekrich I think that's something we should fix in sconfig? |
@tgodzik if you could only call 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 I put together Hope this helps. |
I will do with
Scalafmt uses I also found another issue with running git:
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. |
We do support |
Ach ok, that fixed it. The remaining issues are:
|
What is the issue with cc/ Our regex expert @LeeTibbert |
It's not currently implemented for Scala Native, we could try turning the glob patterns into regexes aka https://stackoverflow.com/questions/1247772/is-there-an-equivalent-of-java-util-regex-for-glob-type-patterns Anyway, I am trying out that solution (seems to do the trick for most cases), but another thing popped up after implementing that:
I pushed the current state to the branch if anyone wants to take a look. Edit: That seems to be an issue with the regexes also (run a file separately):
|
Ok, we worked around the Regexes thanks to @sjrd - we might need to fix some issues with the workaround still - but everything finally compiles and runs! I've put a draft PR here: https://github.com/tgodzik/scalafmt/pull/13/files I still need to:
This might still be a bit of work. Next step would be the publishing story, I think we could try the same as in Bloop, publish artifact to github and link to them from coursier, which would allow us to install binaries using |
Some updates. I took over from @tgodzik to finish the PR and I mostly did, it's pretty much done (jchyb#1). Testing on a my small unrelated repo yielded very promising results. However. I decided to do some real benchmarks on the scala-native repo itself, and... there are some problems. Some of the bigger files take extremely long to process (from about 1s sys time on JVM to 10s on Native). I have reasons to believe it's related to scala-native/scala-native#1713, but I need to investigate further. I don't think it makes much sense to merge before fixing the underlying issue. EDIT: I just realized I should check how it works with another GC to confirm the issue. |
@jchyb I know @densh took a look at the performance issues recently and posted on the Scala Native discord. I am wondering too if it might be a good idea to introduce changes made in the Scala Native PR branch here that can be merged. It seems reasonable to have the code support Scala Native even if the usability is sub par. I feel that runtime or other issues are somewhat tangential to this issue. It might make it easier for others to get involved as well. Currently, 3 of the 5 check boxes are completed. |
@ekrich I have been meaning to get back to this. For now I would like to properly implement glob support on the SN side. Should not be too difficult and the regex wrapping system used before was not great and on further consideration I would not want to introduce too many SN-specific considerations for the scalafmt team. Additionally @kitbellew review (for which I am very grateful) revealed some issues with f.e. the lack of test cases there. In terms of performance, I will also try testing scalafmt with the newest SN version and report my findings today in the issue. The recent changes to Strings seem substantial. As for the check boxes they look slightly outdated, I was under the impression that sconfig was already used on the metaconfig side (but I don't remember exactly at this point) |
@jchyb Sounds like a plan - sconfig was merged for the Scala Native cross project as far as I know. |
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
We are only a few steps away from being able to properly support Scala Native. The missing pieces are
The text was updated successfully, but these errors were encountered: