Skip to content

[lib-path] Add a few common env variables #792

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

Merged
merged 1 commit into from
Mar 21, 2023
Merged

Conversation

mikeland73
Copy link
Contributor

@mikeland73 mikeland73 commented Mar 21, 2023

Summary

Adds LD_LIBRARY_PATH and LIBRARY_PATH env vars to shellenv (pointing to profile lib directory).

This is a bit of a blunt approach, so not sure if it's best. This would fix #760 and also lets us simplify the rust plugin

Two questions/considerations:

  • I considered adding to the flake.nix file instead, but wasn't example sure how to point to the right directory (without hard coding). It might look something like LD_LIBRARY_PATH = pkgs.lib.makeLibraryPath...
  • I also read the following:

Not setting LD_LIBRARY_PATH is usually the right choice because usually the native binaries find their libraries using RUNPATH which is specific to the binary. Setting LD_LIBRARY_PATH has an obvious problem of potentially messing up other programs.

@gcurtis any idea how accurate this is?

How was it tested?

devbox shell and confirmed env vars are set.

Also tested with this example: #710 (comment)

@mikeland73 mikeland73 requested review from gcurtis and Lagoja March 21, 2023 00:35
@gcurtis
Copy link
Collaborator

gcurtis commented Mar 21, 2023

tl;dr I think this is a good idea and we should do it.

The Python and Rust issues are two different things. If I remember, the rustc issues were because the linker couldn't find libraries at build time. Setting LIBRARY_PATH would fix that, provided that the user installed the libraries their program needs.

The advice about not setting LD_LIBRARY_PATH is generally correct, but I don't think it has too much of a downside here. As far as I know, most Nix binaries are built (or patched) to use absolute paths pointing to the shared libraries they need. You can run otool -L <binary> on macOS to see what those paths are.

If those weren't full paths, then the linker would look in LD_LIBRARY_PATH followed by a few other places. That's why I think setting it is okay and won't break other Devbox packages. Otherwise, LD_LIBRARY_PATH can cause problems if two binaries depend on the same library, but they were each tested against different versions of that library. Unless that library is perfectly backwards-compatible, one of the programs may encounter bugs at runtime. Nix side-steps this problem by explicitly setting all runtime dependencies, which is part of what makes it so stable.

So why do Python modules with native dependencies break? Because Python programs are interpreted, they don't really use shared libraries in the same way. The Python interpreter itself links against libraries at runtime, but programs written in Python need to manually find and load (with a syscall) the native libraries they need. On most distros this just works because you can assume that the library is in /lib, /usr/lib, or some other well-known directory. Nix doesn't have those, so things blow up.

Fortunately, Python provides functions for finding dynamic libraries and those functions respect LD_LIBRARY_PATH. That's why setting it fixes the import error.

@mikeland73
Copy link
Contributor Author

@gcurtis awesome, thanks for the explanation! Gives me more confidence about this. I may follow up this PR to make 2 changes:

  • Remove env var from rust plugin (since no longer needed)
  • Move env variable to flake

@mikeland73 mikeland73 merged commit 5ee847b into main Mar 21, 2023
@mikeland73 mikeland73 deleted the landau/ld-flag branch March 21, 2023 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Set LD_LIBRARY_PATH to point to the .devbox/.../lib folder
2 participants