Improve handling of native libraries #23
Conversation
bioball
force-pushed
the
improve-native-libs
branch
6 times, most recently
from
386c66a to
9ade31f
Compare
|
|
||
| // The thread where all tree-sitter allocations happen | ||
| val astExecutor: ExecutorService by lazy { Executors.newSingleThreadExecutor() } | ||
| val pklParser: PklParser by lazy { PklParser(this) } |
There was a problem hiding this comment.
Unrelated: make PklParser a component, and remove astExecutor because it's only used by the parser anyway.
bioball
force-pushed
the
improve-native-libs
branch
from
9ade31f to
9fe850b
Compare
This implements changes to put native libs in `~/.pkl/editor-support/`. When loading the parser, it first looks for libraries there, and if they don't exist, they get copied from bundled resources. When building libraries, the os name and architecture get included in the resource path. This enables starting the jar plainly without needing any extra argument (e.g. no need for -Djava.library.path), and eliminates the need to copy native libraries to the current working dir.
bioball
force-pushed
the
improve-native-libs
branch
from
9fe850b to
fc73ec0
Compare
| mapOf( | ||
| "version" to buildInfo.pklLspVersion, | ||
| "treeSitterVersion" to libs.versions.treeSitterRepo.get(), | ||
| "treeSitterPklVersion" to libs.versions.treeSitterPklRepo.get(), |
There was a problem hiding this comment.
Provide versions of these libraries to runtime, because this affects where these libraries get loaded from. For example, on macOS, the tree-sitter lib lives at ~/.pkl/editor-support/native-libs/tree-sitter/v0.23.0/libtree-sitter.dylib.
This ensures that the version of the native lib matches what the LSP expects.
| // chmod a+x | ||
| outFile.setExecutable(true, false) | ||
| } | ||
| } |
There was a problem hiding this comment.
Tangent: I introduced BuildInfo too, and moved both BuildInfo and ExecutableJar into buildSrc to mimick how we set things up in the pkl repo.
| val os: org.gradle.internal.os.OperatingSystem by lazy { | ||
| org.gradle.internal.os.OperatingSystem.current() | ||
| } |
There was a problem hiding this comment.
| val os: org.gradle.internal.os.OperatingSystem by lazy { | |
| org.gradle.internal.os.OperatingSystem.current() | |
| } | |
| val os: OperatingSystem by lazy { | |
| OperatingSystem.current() | |
| } |
Better to import this class.
| * https://skife.org/java/unix/2011/06/20/really_executable_jars.html | ||
| */ | ||
| abstract class ExecutableJar : DefaultTask() { |
There was a problem hiding this comment.
We don't need an executable jar. You never run the LSP by hand and the clients need to find a suitable Java version (>=22).
There was a problem hiding this comment.
Agree. But will keep that out of the scope of this PR.
| val libraryPath: Path by lazy { | ||
| when { | ||
| // optimization: if the resource file is a normal file, we can use it directly. | ||
| resourcePath.fileSystem == FileSystems.getDefault() -> resourcePath |
There was a problem hiding this comment.
I imagine this will be true for local development?
2 participants
This implements changes to put native libs in
~/.pkl/editor-support/. When loading the parser, it first looks for libraries there, and if they don't exist, they get copied from bundled resources.When building libraries, the os name and architecture get included in the resource path.
This enables starting the jar plainly without needing any extra argument (e.g. no need for CLI flag
-Djava.library.path, and no need for theLD_LIBRARY_PATHenv var), and eliminates the need to copy native libraries to the current working dir.Note: java-tree-sitter doesn't provide any way to provide the library path. To get around this, I'm doing a pretty ugly hack to rewrite the
TreeSitter.javasource file to make it load the tree-sitter library from our custom dir.