Add support for CRYSTAL_PATH resolution relative to compiler #10877
Conversation
| # to produce a portable binary that resolves the standard library path | ||
| # relative to the compiler location, independent of the absolute path. | ||
| if executable_path = Process.executable_path | ||
| path %= {"COMPILER_DIR": File.dirname(executable_path)} |
There was a problem hiding this comment.
This makes any path that happens to contain % blow up. Also could have security implications because the format string is taken from user input.
I think at the very least this needs a different implementation for the substitution. (e.g. literally replacing %{COMPILER_DIR} would probably already be better). But I'd hope that there would be some placeholder that's more or less guaranteed to not be encountered.
There is precedent in LD_LIBRARY_PATH to specially understand the exact string $ORIGIN at the beginning of the path, and I don't see any reason to prefer a completely new convention over that one.
https://www.google.com/search?q=rpath+origin
And I would like to stress that I think it's quite important to recognize the string only at the beginning of a path.
There was a problem hiding this comment.
Using an already established mechanism is a great suggestions. I suppose @executable_path (from macos' linker) could also be an option for this. But I'm fine with $ORIGIN.
You're absolutely right about the implications of sprintf expansion. I'll change that.
|
I suppose we should add the same relative path resolution mechanism for |
|
Closing. I created a fresh PR at #11030 which should be easier to review. The final diff isn't that much different but it's bit cleaner now. |
Resolves #7391
This patch adds support for runtime expansion of the
CRYSTAL_PATHenvironment variable. It allows resolving the path to the standard library relative to the compiler location at runtime. This avoids the need to bake absolute paths into the compiler and using a wrapper script for distribution (https://github.com/crystal-lang/distribution-scripts/blob/8bc01e26291dc518390129e15df8f757d687871c/linux/files/crystal-wrapper). In fact, this essentially replicates the behaviour of the wrapper script, just embedded in the compiler directly.It integrates perfectly with all existing usage. The new behaviour only becomes active when
CRYSTAL_PATHincludes the pattern%{COMPILER_DIR}. Existing use cases such as the wrapper script or baking ing absolute paths continue to work and will continue to do so.The real value comes when this is baked in via
CRYSTAL_CONFIG_PATHbecause that makes the compiler binary work standalone, as long as it finds the standard library in the designated path relative to its location.So the default path
%{COMPILER_DIR}/../share/crystal/srcis configured in theMakefile. This location fits the typical install paths/usr/bin/crystaland/usr/share/crystal/src. This will become even more useful withmake install(#10702) and adaptation in distribution-scripts.