Add automatic setting of LD_LIBRARY_PATH for Posix #2718
Conversation
|
✅ PR OK, no changes in deprecations or warnings Total deprecations: 14 Total warnings: 0 Build statistics: statistics (-before, +after)
executable size=5331080 bin/dub
-rough build time=65s
+rough build time=64sFull build output |
Looks like it just needs retriggering. |
rikkimax
force-pushed
the
LD_LIBRARY_PATH
branch
from
194747a
to
7b0ce81
Compare
|
Looks like I was wrong about the parent process environment, you need |
rikkimax
force-pushed
the
LD_LIBRARY_PATH
branch
from
7b0ce81
to
b4d5111
Compare
|
Honestly, I find this approach rather strange from an outside point of view. Relying on this would mean that you can only execute the application using "dub run", without the special knowledge of setting LD_LIBRARY_PATH. To me it seems that either the application should make use of rpath for shared library that are loaded at application startup time, or modify the search path at runtime for any dynamically loaded libraries. |
|
I'm not sure that there is a better approach to this problem. Dub does not handle installation, so it shouldn't be assisting in setting Sure we can add a directive for setting Overall, we can't win. The best we can do is make |
|
Also worth considering that assuming
The successful execution of this will depend on whether you are running Windows or *nix. Given that you're running a tool that isn't being developed that is quite a problem. |
|
What I mean is that an executable that expects shared libraries that are outside of the standard search paths to be linked at startup needs to set its rpath properly in order to be useful in the general case. It would be highly unusual to require the user to set LD_LIBRARY_PATH, irrespective of whether the application has been installed as a distribution package or not. So, instead of adding the target path to LD_LIBRARY_PATH, the executable should instead have its rpath set to $ORIGIN. If DUB offers a way to build an application that way, then it should do that automatically. |
|
That also shouldn't hurt when the same executable gets distro packaged so that the libraries go into a standard library location instead of the executable directory, since rpath just adds to the normal search path. |
|
Dub currently doesn't do anything to assist you with this. It will likely be linker+target specific to perform. So there is some concern that it does mean we end up marrying dub to unknown tool behavior that we do not control. Given these facts, I'm not willing to implement it. To me at least it seems to be going outside of the bounds of dub and into installer territory which we have deemed out of scope. However, if somebody wants to do their own PR to compete, I'm happy to allow them to take responsibility for it :) |
|
Okay, but that is the basic philosophy of DUB - abstract away platform specifics as far as possible. This just seems to be another linker flag generated depending on whether there are any shared library targets that a binary depends on, but maybe I'm missing something. |
|
It is possible that setting But it may also be possible that it is linker-specific, not just compiler or target. I'm just not keen on the tradeoffs to the alternatives of this PR. Perhaps @kinke is willing to take ownership of an alternative since they are the other 'owner' of shared library support in dub. |
There was a problem hiding this comment.
IMO since this is a rather exotic use-case and may even introduce security vulnerabilities if you have random dynamic libraries lying around in your folder, e.g. you are running a dub --single test.d file in your Downloads folder. We should not do this without the user's consent. It also heavily goes against how it works in any other language, on linux systems at least. People will just get confused why running some applications will not work on its own, in the worst case they might even give up D altogether since this can easily yield to exotic unsearchable issues if you aren't aware DUB does this.
DUB projects relying on non-standard OS behavior, such as loading dynamic libraries from the project directory, should rather set
lflags "-rpath=$$ORIGIN" platform="linux"
lflags "-rpath" "@executable_path/." platform="osx"
like some DUB projects are doing already right now. (example taken from inochi-creator, other projects include rengfx, faiss-d, zased
This also makes sure that with special cases like this developers have something to start their search off in case of issues with deployment, since now they see these exotic flags in their recipe.
I am personally heavily against doing this without any user input, especially modifying/ignoring user's already set LD_LIBRARY_PATH environment variables from the recipe or environment variable. This will probably break most run scripts that already had this workaround in.
If you want to implement this, please make it a setting users need to turn on in the recipe.
All dub projects that use shared libraries as dependencies are going to need some directives that look like that to make it both run or test. This isn't a special case, it is ALL of them. It also applies to quick and dirty executables just as much as it does to well packaged up ready-to-ship production software. We need a better solution here, that IMO does not require modifying the executable.
This only affects How can it break existing scripts? It prepends, not replaces. |
|
I'll second this request. I was building this example application from dlangui and all I wanted to do is run
that's not a terrible idea, that's a great idea. anyone who wonders how this or DYLD_LIBRARY_PATH works can dig into how static and dynamic linking works. please merge this PR. |
Stating that this is a "great idea" is not an argument and does not magically make it a true statement. Also, by that same logic, you can go and just look up how dynamic linking works and set your LD_LIBRARY_PATH appropriately before calling |
It's not the same logic. Yours is an assumption. And it's an assumption on couple of things. In order to "go and just look up how dynamic linking works and set your LD_LIBRARY_PATH appropriately before calling dub run", one needs to:
What did the user want? They just wanted to try out an example. |
|
You don't need to know anything additional in the case of "dub run" over running the executable directly (apart from maybe the assumption that the parent environment gets inherited to the final process, which really is the norm). You previously dismissed the case where you run the executable directly with your "can look it up" argument, the same can be said for "dub run", simple as that. If, for whatever reason, you decide to set up your package so that the examples require a strange dynamic library setup like that, then the first question would be what you are trying to solve that way. It surely isn't what dynamic libraries are usually meant to solve, as they are still all built together. Obviously, neither dynamic loading (plugins, which need to be looked up in a particular directory anyway), nor dynamic library updates (system package manager) are the reason. Honestly, if anything, we need a proper solution here that results in a working executable/library combination. This is nothing but a hack that pushes the problem back by one step, without even hinting to a real solution. |
|
@kinke you are the other owner of shared library support in dub, any opinions? |
|
Heh, no I don't own anything in dub, except maybe for some workarounds and hacks I'm not too proud of. ;) The problem here AFAICT is that we have to deal with 2 use cases - 1) a local-only build of some executable/shared lib, to be used on the dev box exclusively, and should 'just work', and 2) the case of an application/library bundle intended for distribution. Now on Windows, there's not much room AFAIK - we generally need to copy all DLLs to the executable dir, in both cases. On Posix however, the 1st case could be handled by
But for the redistributable bundle on Posix, AFAIK one normally makes sure the .so/dylibs have an SONAME, so that the DT_NEEDED ref in the executable is a pure filename without dirs, and then copy the libs next to the executable (or some other place), and bake that directory as RUNPATH into the executable. Setting/changing RUNPATH can be done by postprocessing via I'm not sure what the best way to handle both of these use cases is, and don't really have the time/motivation to think about it carefully. |
|
Yes, my argument is that they should be handled with different solutions. Without causing a surprise when dub sets up a default that is different from what the platform defines and has security consequences. This PR only solves one of these issues, it doesn't need to solve both. |
|
Oh sorry, now I actually took a glance at the PR. ;) - Hmm yeah setting appropriate env vars for apps run through dub doesn't sound too bad to me, to cover use case 1 without breaking use case 2. Some thoughts:
|
That would be special casing, so when debugging builds you would need to question if a shared library was a dependency or not in dub. Makes things more complicated without much value I think.
There possibly are desirable traits for this. However I am unsure of the wider implications of changing how files get copied after compilation. |
|
Oh wait, we nowadays copy the dynamicLibrary artifacts to the executable's output dir automatically already, on Posix too. So yeah, nothing needed on Windows, and for non-Apple Posix here, prepending the executable dir to LD_LIBRARY_PATH should be enough. If this is restricted to dub projects having (direct or indirect) And use case 2 is covered by running patchelf to bake a suitable RUNPATH into the redistributable executable/.so/.dylib; all shared-library deps already copied to the output directory. |
See changelog for more information.
Of note (that was not mentioned), is that the environment variables were not inheriting from the parent process previously.
So while I was at it, I also added it back to the default. Most likely whoever implemented the setting of environment variables forgot that providing an AA to
std.processwill ignore the parent environment. I didn't find a ticket associated with this.EDIT: I would consider a test for this, but it's going to be extremely flaky, any change in build environment such as CI runner could change the result.