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

Create distributable cross-platform packages #329

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

marvinborner
Copy link
Member

Most operating systems (e.g. GNU/Linux, BSDs) treat all text after the first space as a single argument. When using env in a script it is thus not possible to specify multiple arguments.[1]

Without this change the invocation of effekt after sbt install results in the following error:

/usr/bin/env: ‘java -jar’: No such file or directory
/usr/bin/env: use -[v]S to pass options in shebang lines

MacOS/FreeBSD's env apparently behaves differently, so please verify that the resulting binary still works on them.

[1]: GNU coreutils documentation

@b-studios
Copy link
Collaborator

b-studios commented Dec 3, 2023

Hey this one line has a very long history.

We discussed it here at some point:
#16

It seems we cannot find a version that works on all platforms, so why not generate platform specific versions? Or look into https://github.com/xerial/sbt-pack ?

@marvinborner
Copy link
Member Author

marvinborner commented Dec 3, 2023

Oh, sorry for not noticing the history.

I just tried a JAR wrapper from StackOverflow which seems to work (at least for me) and bypasses the env portability issues. I'm not sure about Windows, but the previous version would've only worked with bash emulation (like cygwin) anyway, right?

I suppose an official solution like sbt-pack would be much cleaner.

Edit: The -S flag of env was released with coreutils 8.30 in 2018, so I don't really understand how it didn't work on Ubuntu 22.04.

@jiribenes
Copy link
Contributor

jiribenes commented Dec 3, 2023

Personally, I use something like:

#! /usr/bin/env bash -e

# ... add node to PATH ...

exec long/full/path/to/bin/java -jar another/long/path/to/effekt.jar "$@" 

for my version of effekt.sh both on MacOS and on Linux without any problems -- I never use the shebang on the jar file :)

(automatically generated here: https://github.com/jiribenes/effekt-nix/blob/3147a32bce206520a6d562a2469638baa9048722/effekt.nix#L24-L27; just posting for archival reasons)

However, I believe it may be wise to explore sbt pack linked above further. :)

@b-studios
Copy link
Collaborator

Maybe at some point we can also look into native image again (https://www.graalvm.org/latest/reference-manual/native-image)

So far, I never went through the troubles to set it up in CI and have it automatically build binaries for the different platforms.

@marvinborner
Copy link
Member Author

I did some experiments with sbt-scala-native (chosen over sbt-native-image since it has official support by sbt-crossproject).

Since I'm still unfamiliar with sbt/scala I have some questions:

  1. What should we set in .nativeSettings? Is leaving it empty okay for now?
  2. I get a weird error if I try effektNative/compile or something similar:
[error] -- Error: .../kiama/shared/src/main/scala/kiama/util/Source.scala:101:11
[error] 101 |case class FileSource(name: String, encoding: String = "UTF-8") extends Source {
[error]     |           ^
[error]     |   class FileSource does not contain field kiama$util$Source$$$1$$lzy2
[error] -- Error: .../kiama/shared/src/main/scala/kiama/util/Source.scala:96:11
[error] 96 |case class StringSource(content: String, name: String = "") extends Source
[error]    |           ^
[error]    |   class StringSource does not contain field kiama$util$Source$$$1$$lzy1
[error] two errors found
[error] (kiamaNative / Compile / compileIncremental) Compilation failed

Am I missing something obvious?

@marvinborner marvinborner changed the title Add argument splitting flag for GNU env Create distributable cross-platform packages Dec 4, 2023
@marvinborner marvinborner marked this pull request as draft December 4, 2023 15:22
@b-studios
Copy link
Collaborator

I am not sure whether Scala Native is in a state where we can just compile our compiler using it :(

@marvinborner
Copy link
Member Author

marvinborner commented Dec 6, 2023

Small update: GraalVM native-image sbt effektJVM/nativeImage generates a binary file that seems to work flawlessly. I did not need to use any reflection files or similar.

@b-studios
Copy link
Collaborator

b-studios commented Dec 6, 2023

Cool! Can you try to use it with the vscode plugin as a language server?

Ah, and the Repl of course

@marvinborner
Copy link
Member Author

One problem we need to solve is including the standard library in the native file. For now we need to manually specify it using --lib libraries/....

I needed to add dynamic proxies for the server to run without runtime errors.

Other than that the REPL works (as far as I can see) and effekt -s works too now. The VS code extension itself fails silently so I assume there are still some missing proxies or something. I'm not really sure where to start debugging this.

@b-studios
Copy link
Collaborator

b-studios commented Dec 6, 2023

The missing proxies are often the problem. I remember that there was a tool that would record a trace and then I needed to use as much of the language server as possible...

Edit: https://www.graalvm.org/latest/reference-manual/native-image/metadata/AutomaticMetadataCollection/

@b-studios
Copy link
Collaborator

Do the missing proxies show in the vscode log?

run: sudo apt-get install chezscheme

- name: Run tests and assemble jar file
run: sbt clean deploy
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jiribenes just told me MacOS minutes are much more expensive. Maybe we don't want to run the tests on all systems??? :)

@marvinborner
Copy link
Member Author

marvinborner commented Dec 7, 2023

I was now able to debug the server with stacktrace logs. It turns out we do still need reflection files for certain operations (especially lsp4j), so I added missing reflections to the existing config.

With the final config I was not able to produce any reflection errors, but there may still be some. I'm not sure about the general approach of releasing native images, since the reflection fixing does not really seem stable.

Also, saving files with the native build in vscode reports a NullPointerException in kiama.util.Services.formatting$$anonfun$1(Server.scala:572). I'm not sure if it's related to native-image or something else.

@b-studios
Copy link
Collaborator

I'm not sure about the general approach of releasing native images, since the reflection fixing does not really seem stable.

Yeah, that was my feeling last time as well. I would feel better if there was an automated way to query the language server and reproduce scenarios so that we can regenerate reflection traces if necessary (like when updating the LSP4J dependency).

Eventually, reimplementing LSP4J in Scala or replacing it with a Scala implementation that does not use reflection might be the best thing to do but is a LOT of work.

@marvinborner
Copy link
Member Author

Maybe writing a general test suite for the LSP server could be a good first step? In my efforts to find reflection bugs I wrote some tests using Haskell's lsp-test. I could start writing something similar (more minimal) in Scala.

@b-studios
Copy link
Collaborator

Just to make this clear: a test suite for LSP would be great. How would you run it with the graal binary? The tests are not deployed with the binary, no?

@marvinborner
Copy link
Member Author

marvinborner commented Dec 8, 2023

In the case of lsp-test, the tests simply open separate effekt -s sessions and interact with them like any other LSP client -- the tests are 100% external. I thought about implementing the test suite in a similar way (i.e. as a seperate LSP testing program). If we wanted something more performant we could perhaps use the server's debug port.

build.sbt Show resolved Hide resolved
@b-studios
Copy link
Collaborator

I was now able to build the binary. However, just invoking effekt -- that is starting the REPL -- crashes with:

java.lang.NoSuchMethodException: jline.UnixTerminal.<init>()
  _____     ______  __  __     _    _
 (_____)   |  ____|/ _|/ _|   | |  | |
   ___     | |__  | |_| |_ ___| | _| |_
  (___)    |  __| |  _|  _/ _ \ |/ / __|
  _____    | |____| | | ||  __/   <| |_
 (_____)   |______|_| |_| \___|_|\_\\__|

 Welcome to the Effekt interpreter (v0.2.1). Enter a top-level definition, or
 an expression to evaluate.

 To print the available commands, enter :help

[ERROR] Failed to construct terminal; falling back to unsupported
java.lang.InstantiationException: jline.UnixTerminal
	at java.base@11.0.17/java.lang.Class.newInstance(DynamicHub.java:571)
	at jline.TerminalFactory.getFlavor(TerminalFactory.java:205)
	at jline.TerminalFactory.create(TerminalFactory.java:96)
	at jline.TerminalFactory.get(TerminalFactory.java:180)
	at jline.TerminalFactory.get(TerminalFactory.java:186)
	at kiama.util.JLineConsole$.terminal(Console.scala:54)
	at kiama.util.JLineConsole$.withTerminal(Console.scala:62)
	at kiama.util.JLineConsole$.usingTerminal(Console.scala:76)
	at kiama.util.JLineConsole$.reader$lzyINIT1(Console.scala:88)
	at kiama.util.JLineConsole$.reader(Console.scala:83)
	at effekt.Repl.usingCommandHistory(Repl.scala:280)
	at effekt.Repl.run(Repl.scala:48)
	at effekt.Driver.run(Driver.scala:39)
	at effekt.Driver.run$(Driver.scala:25)
	at effekt.Server$.run(Server.scala:263)
	at effekt.Server$.run(Server.scala:263)
	at kiama.util.Compiler.driver(Compiler.scala:86)
	at kiama.util.Compiler.driver$(Compiler.scala:22)
	at effekt.Server$.driver(Server.scala:263)
	at kiama.util.Compiler.main(Compiler.scala:50)
	at kiama.util.Compiler.main$(Compiler.scala:22)
	at effekt.Server$.main(Server.scala:263)
	at effekt.Server.main(Server.scala)
Caused by: java.lang.NoSuchMethodException: jline.UnixTerminal.<init>()
	at java.base@11.0.17/java.lang.Class.getConstructor0(DynamicHub.java:3349)
	at java.base@11.0.17/java.lang.Class.newInstance(DynamicHub.java:556)
	... 22 more

@b-studios
Copy link
Collaborator

The native version reduces the startup significantly (for compiling and running examples/builtins.effekt):

Jar

Time (mean ± σ):      1.179 s ±  0.035 s    [User: 2.660 s, System: 0.115 s]
Range (min … max):    1.136 s …  1.267 s    10 runs

Native

Time (mean ± σ):     353.4 ms ±   1.3 ms    [User: 331.2 ms, System: 16.4 ms]
Range (min … max):   351.5 ms … 355.3 ms    10 runs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants