-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 CRYSTAL_LIBRARY_RPATH
for adding dynamic library lookup paths
#13499
Support CRYSTAL_LIBRARY_RPATH
for adding dynamic library lookup paths
#13499
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
I have an optional suggestion, but I'm happy with or without it.
@@ -334,7 +334,7 @@ class Crystal::Command | |||
output_format : String?, | |||
combine_rpath : Bool do | |||
def compile(output_filename = self.output_filename) | |||
compiler.emit_base_filename = original_output_filename | |||
compiler.emit_base_filename = output_filename.rchop(File.extname(output_filename)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is equivalent and more concise:
compiler.emit_base_filename = output_filename.rchop(File.extname(output_filename)) | |
compiler.emit_base_filename = Path[output_filename].stem |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is from a prior PR. That PR actually changed the behavior of -o
so that it also affects --emit
; previously those emitted files would always be in the current working directory and ignore the name given to -o
. Thus output_filename
could now contain directory separators, and calling Path#stem
would remove them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the name filename
is misleading then if it's actually a path.
But okay, don't need to fix that here.
Resolves #13490, except for the interpreter part.
The
CRYSTAL_LIBRARY_RPATH
environment variable can be used to pass a list of directories used by built programs to locate their dynamic libraries at run time.$ORIGIN
and${ORIGIN}
are expanded into the program's absolute path, and this PR adds a custom implementation for this on Windows, only available if the delay-load helper is present (i.e.-Dno_win32_delay_load
is not used). Linux's loader also expands$LIB
and$PLATFORM
, although this is not guaranteed.The compiler will automatically add its own RPATH to anything that needs to be compiled and run immediately as a child process. For Windows, this means as long as the compiler can find its own DLLs,
crystal run -Dpreview_dll
/eval
/spec
or any macrorun
insidecrystal build -Dpreview_dll
will always be able to locate the same DLLs distributed along the compiler itself, even if the compiler is not in%PATH%
. In principle, if the compiler itself is dynamically linked with aCRYSTAL_LIBRARY_RPATH
containing$ORIGIN
, then the DLLs do not even have to share the same directory as the compiler.For most Unix-like systems, this automatically passes
-Wl,-rpath,...
to the linker. It seems linkers disagree on whether that option affects RPATH or RUNPATH. macOS is excluded because it handles library lookup differently.The compiler automatically generates the
Crystal::LIBRARY_RPATH
constant; when a Crystal source is run, this constant will reflect the contents of both the environment variable and the compiler itself, as described above. The standard library also defines it explicitly in case a compiler without this PR tries to use it.