Skip to content
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

python: add patchelf option #840

Closed

Conversation

bobvanderlinden
Copy link
Contributor

Currently Python package managers install packages with prebuilt binaries, which might refer to libraries that the program is not able to find.

LD_LIBRARY_PATH is a solution, but seeps through to sub-processes, sometimes resulting in surprising errors due to conflicting libraries/interpreters.

I have been experimenting with using autopatchelf on venv. A tool that finds ELF files, looks up their dependencies in a set of lib directories and patches the ELF files to use those lib directories specifically. It seems to work fairly well.

There are a few downsides though:

  • This mutates the files that are 'managed' by a different package manager, which could lead to confusion when using the package manager manually.
  • It adds some initialization time (~8s for my project), mostly to lookup all .so* files in the venv.
  • It doesn't solve the problem for libraries that are dynamically loaded without them being a requirements in the ELF header.
  • It isn't a solution for OSX (yet?)

I mostly wanted to give this option some consideration compared to LD_LIBRARY_PATH. Feel free to decline this. I'm open to all feedback.

'') ++ (lib.optional cfg.venv.patchelf ''
if [ -d "$VIRTUAL_ENV" ]
then
${autopatchelf}/bin/autopatchelf --libs ${config.devenv.profile}/lib --path $VIRTUAL_ENV
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know the devenv code well enough but find this approach interesting.
Here a few hints: 1. Avoid running patchelf repeatably as might change the binary on every run (sometimes with weird side effects). 2. Make sure that ${config.devenv.profile}/lib cannot be garbage collected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback!

  1. Avoid running patchelf repeatably as might change the binary on every run (sometimes with weird side effects).

Indeed. Maybe storing the profile inside VIRTUAL_ENV and comparing it is a good enough approach.

  1. Make sure that ${config.devenv.profile}/lib cannot be garbage collected.

I presumed the profile is not GCed, as it has a GC root that in .devenv. Not entirely sure whether this is a good presumption though.

@Mic92
Copy link

Mic92 commented Nov 12, 2023

macOS has install_name_tools instead of patchelf. Maybe just adding .devenv/lib with that might be good enough?

@@ -135,6 +145,12 @@ in
description = "Whether `pip install` should avoid outputting messages during devenv initialisation.";
};

venv.patchelf = lib.mkOption {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this should be a no-op on non-elf architectures to not break macOS by accident.

@Atry
Copy link
Contributor

Atry commented Nov 15, 2023

I found this PR is similar to mach-nix and dream2nix.

@domenkozar
Copy link
Member

I think the method that we use by fixing the interpreter itself now works well in 1.0 branch, we're only waiting for the merge of NixOS/nixpkgs#275701

@nevesenin
Copy link

@bobvanderlinden Something like this might be helpfull for ruby too. I added this

enterShell = ''
  ${inputs.autopatchelf.packages."${system}".default}/bin/autopatchelf --bintools ${pkgs.bintools} --patchelf ${pkgs.patchelf}/bin/patchelf --libs $DEVENV_PROFILE/lib --path $BUNDLE_PATH
'';

to a RoR projects config. I didn't find another way to make the dartsass-rails gem work.

@bobvanderlinden
Copy link
Contributor Author

The method used in NixOS/nixpkgs#275701 should also work for Ruby, but it currently has only been implemented for Python.

@nevesenin
Copy link

Oh, great. I found that PR before but lost track while reading through it.

@domenkozar
Copy link
Member

Going to close this for now, we found a way to make virtualenv work well with Nix.

To solve glibc issues we'll have to use LD_AUDIT hack or something similar.

Let me know if there's any other reason we'd have this

@domenkozar domenkozar closed this Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants