Fix p7zip_jll environment setup
#39155
Conversation
On most platforms, setting `LD_LIBRARY_PATH` and friends was not necessary for invoking `7z` because we fell back to the natural library search paths for things like `libstdc++`, but in truly minimal environments (like Alpine linux) this does not work; so let's set these paths properly. We do this work manually in v1.6, in v1.7 we hope to have a JLLWrappers-based solution.
|
IIRC there is a way to set the environment for spawning a Cmd without modifying the global ENV. |
|
There is, I added it. :) this provides both ways of doing it, for backwards compatibility. |
| return env | ||
| end | ||
|
|
||
| function p7zip(f::Function; adjust_PATH::Bool = true, adjust_LIBPATH::Bool = true) |
There was a problem hiding this comment.
Should this method just be deprecated?
There was a problem hiding this comment.
Or maybe we can pass an appropriate Cmd object, rather than the string?
There was a problem hiding this comment.
I guess that doesn't buy us anything over the other method.
There was a problem hiding this comment.
We could deprecate it, but we're in a bit of an ecosystem issue in that if a package, which uses JLLs, wants to support 1.3-1.6, they will then get deprecation warnings on 1.6 for using the older, compatible interface. I think I would rather let things be for a bit, and then start introducing deprecation warnings once we have the data showing that we have a majority of users on 1.6+, and that package devs can actually switch over without annoying too many users.
| const LIBPATH_env = "LD_LIBRARY_PATH" | ||
| const LIBPATH_default = "" | ||
| const pathsep = ':' | ||
| end |
There was a problem hiding this comment.
Should this definition be in Base somewhere?
There was a problem hiding this comment.
Yeah, probably. In v1.7 this is all going to be generated by a stdlib-vendored copy of JLLWrappers, this is all just copied from there for the interim.
|
I guess this should be backported for v1.6? |
5 participants
On most platforms, setting
LD_LIBRARY_PATHand friends was notnecessary for invoking
7zbecause we fell back to the natural librarysearch paths for things like
libstdc++, but in truly minimalenvironments (like Alpine linux) this does not work; so let's set these
paths properly. We do this work manually in v1.6, in v1.7 we hope to
have a JLLWrappers-based solution.