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

Feature/add scala native #1

Closed
wants to merge 2 commits into from
Closed

Feature/add scala native #1

wants to merge 2 commits into from

Conversation

jchyb
Copy link
Owner

@jchyb jchyb commented Oct 26, 2021

No description provided.

@jchyb jchyb force-pushed the feature/add-scala-native branch 15 times, most recently from 873976d to c6668cd Compare October 29, 2021 07:49
@jchyb jchyb force-pushed the feature/add-scala-native branch 5 times, most recently from 479de67 to 3a333d5 Compare November 2, 2021 22:03
Copy link

@ekrich ekrich left a comment

Choose a reason for hiding this comment

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

I see you are using Clang 6.0 for macOS and Linux. I think I would just use the default one that gets installed for that version OS. I can't say that will change any performance outcome but I wouldn't want to use such an old version. Windows of course is a different story.

@jchyb
Copy link
Owner Author

jchyb commented Nov 3, 2021

Locally I used llvm 12 so it's probably not that. I also tried with 0.4.0 and if anything it is worse there, so at least we don't have a regression on our hands. As for CI I probably got too inspired from the scala-native repo, I'll change that before the actual PR. On windows, sometimes there's that memory error we also had on the native repo, but besides that there are also 6 other assertion errors (impressive that only six, file handling on windows can be weirdly tricky I think) - I'll try to create an issue for those. The commits are a mess now, I will rewrite the history a bit before tomorrow.

@jchyb jchyb force-pushed the feature/add-scala-native branch 5 times, most recently from 64eea37 to 19beb1d Compare November 4, 2021 14:31
.dependsOn(core, dynamic, cli)
.nativeSettings(
nativeLinkStubs := true,
scalaNativeNativeConfig
Copy link

Choose a reason for hiding this comment

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

I think you can avoid the raw settings via scalaNativeNativeConfig.withLinkStubs(true). Can it just be nativeConfig?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I can try

Copy link
Owner Author

Choose a reason for hiding this comment

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

Oh right, there are already nativeLinkStubs set, previously there were more options set the nativeConfig and I must have missed that, thank you for the catch

Copy link
Owner Author

@jchyb jchyb Nov 4, 2021

Choose a reason for hiding this comment

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

Oh wait sorry I misunderstood you completely before, of course I can

Copy link

Choose a reason for hiding this comment

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

The raw settings are suppose to be deprecated - just started looking at that in the Scala Native build as well.

@jchyb jchyb force-pushed the feature/add-scala-native branch 2 times, most recently from c2598fe to 052a296 Compare November 4, 2021 15:58
Project structure was changed in compliance with sbt-crossproject to
accomodate Scala Native support.

Co-authored-by: Tomasz Godzik <tgodzik@users.noreply.github.com>
Project structure was changed in compliance with sbt-crossproject.

googlecode's diffutils had to be removed, as Scala Native does not
support java libraries. In it's place, part of diffutils was
reimplemented in scala as part of scalafmt-cli.

For similar reasons, the java interfaces were reimplemented in scala for
Scala Native.

Since Scala Native does not yet support glob expressions, a translation
function to regex was added, heavily based on a linked stack-overflow
answer. Similarly, Scala's Process is replaced with java stdlib's
ProcessBuilder, as Process uses concurrency.

SN's regex implementation is currently based on the re2 instead of the
standard java regexes. This means there are a number of differences
between them, most notably the lack of backtracking in Scala Native.
To accomodate that, some regex function reimplementations were done to
accomodate the use of the existing scalafmt regexes in Scala Native.

Some operations concerning tests were slightly modified to work on
Native. For example, renaming a file to an existing directory path could
throw errors. This is fine, as it does not concern the core of the test,
only things like cleanup.

Assumptions about dynamic tests not working on native were added.
Scala Native does not support URLs and only has basic reflection support
comparable to the one of GraalVM, so the used and expected version of
scalafmt must match here as well.

Term Display was made single-threaded for the Scala Native
implementation, as it does not support concurrency. Since the new
implementation caused display artifacts on JVM, the old multithreaded
one was kept there.

Scala Native setup and testing was added to the CI. The windows was
temporarily disabled as it does not pass all of the tests.

scala-native sbt command was alsoadded for easier scala-native cli
building.

Co-authored-by: Tomasz Godzik <tgodzik@users.noreply.github.com>
@jchyb jchyb closed this Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants