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

Properly set DYLD_LIBRARY_PATH for macOS wheel delocation #816

Closed
phoerious opened this issue Sep 7, 2021 · 13 comments · Fixed by #821
Closed

Properly set DYLD_LIBRARY_PATH for macOS wheel delocation #816

phoerious opened this issue Sep 7, 2021 · 13 comments · Fixed by #821

Comments

@phoerious
Copy link

phoerious commented Sep 7, 2021

I have a Github Action Workflow that builds wheels with native extensions for Linux, macOS, and Windows. External native dependencies for the macOS builds are installed via Vcpkg, so I need to set the CPATH and LIBRARY_PATH environment variables to /usr/local/share/vcpkg/installed/x64-osx/include and /usr/local/share/vcpkg/installed/x64-osx/lib in order for clang to find the headers and *.dylib binaries.

The resulting macOS binaries have shared library dependencies starting with @rpath placeholders. Since there is not RPATH hardcoded in the binaries themselves, delocate-wheel needs a properly configured DYLD_LIBRARY_PATH for resolving these. Unfortunately, I seem to be unable to set either of DYLD_LIBRARY_PATH and LD_LIBRARY_PATH on a Workflow job, so this variable is always unset, no matter what (I couldn't find any documentation, but it's probably a built-in precaution). I also tried setting it indirectly via CIBW_ENVIRONMENT, but without success.

My workaround is to add a step like that:

- name: Delocate Wheels (macOS)
  if: runner.os == 'macOS'
  run: |
    set -e
    python3 -m pip install delocate
    for w in ./wheelhouse/*-macosx*.whl; do
      DYLD_LIBRARY_PATH="$LIBRARY_PATH" python3 -m delocate.cmd.delocate_wheel -v "$w"
    done

It would be great if cibuildwheel could take care of that on its own. This issue pertains mainly to macOS, but may apply to Linux as well (not tested, because I install dependencies directly via yum there).

@Czaki
Copy link
Contributor

Czaki commented Sep 7, 2021

I seem to be unable to set either of DYLD_LIBRARY_PATH and LD_LIBRARY_PATH on a Workflow job, so this variable is always unset, no matter what (I couldn't find any documentation, but it's probably a built-in precaution).

This is macOS protection for malware injection. To disable it there is a need to run the system in a special mode which is impossible on CI. To pass these variables to setup.py file I use LIBRARY_PATH and overwrite os.environ["LD_LIBRARY_PATH"] in setup.py

My workaround is to add a step like that:

Did you try to use CIBW_REPAIR_WHEEL_COMMAND_MACOS instead of this separate step?

I use such a script to prepare libraries https://github.com/Czaki/imagecodecs_build/blob/fc23628c5eb90dfa63b854dc0d091d91151c9f2b/build_utils/fix_macos_lib.sh but fixing it on libraries update is painful.

@phoerious
Copy link
Author

phoerious commented Sep 7, 2021

This is macOS protection for malware injection.

Good to know.

Did you try to use CIBW_REPAIR_WHEEL_COMMAND_MACOS instead of this separate step?

That's not very convenient. Yes, I could do it, but it's a bit finicky with dependencies on other environment variables and it's somewhat ugly too. I would prefer a separate config setting that takes care of all that automatically.

BTW took me a while and many force-pushes to my test branch to figure out why delocate failed to find my libraries. At least a documentation entry with an acknowledgement of the problem and possible workarounds would be nice.

@Czaki
Copy link
Contributor

Czaki commented Sep 7, 2021

BTW took me a while and many force-pushes to my test branch to figure out why delocate failed to find my libraries. At least a documentation entry with an acknowledgement of the problem and possible workarounds would be nice.

I think that You may create PR. Your solution looks better than my and you still have access to logs to copy sample error messages.

@phoerious
Copy link
Author

The question would be what path we should set this to. Setting it to LIBRARY_PATH would probably make sense most of the time, but also a dedicated CIBW_LD_LIBRARY_PATH_MACOS would make sense.

@joerick
Copy link
Contributor

joerick commented Sep 9, 2021

Hi @phoerious!

I'm not sure why CIBW_ENVIRONMENT wouldn't work in this case. While there might be malware protections at the runner level, I don't know of any malware protections on macOS that would interfere with CIBW_ENVIRONMENT, given that your script above works. What kind of errors are you seeing when you try this route?

If that doesn't work, customising CIBW_REPAIR_WHEEL_COMMAND_MACOS would be my recommendation. I'm not sure what complexities you're facing, but that option is intended to deal with this sort of issue, so it would be good to learn more about this.

@phoerious
Copy link
Author

The demonstrated build step above works as a temporary solution. There are no errors, but it clutters my workflow definition.

@henryiii
Copy link
Contributor

henryiii commented Sep 9, 2021

Could you make a PR setting CIBW_ENVIRONMENT (CIBW_ENVIRONMENT_MACOS?) and then point us at the failed build logs? I am curious to know why this doesn't work, it does basically the same thing as your workaround, AFAICT.

phoerious added a commit to chatnoir-eu/chatnoir-resiliparse that referenced this issue Sep 9, 2021
phoerious added a commit to chatnoir-eu/chatnoir-resiliparse that referenced this issue Sep 9, 2021
@phoerious
Copy link
Author

phoerious commented Sep 9, 2021

I created a branch on my own repository for demonstration purposes (that's easiest, since I have everything there to build a wheel with native dependencies). I reduced the workflow file to only the steps needed to build and delocate the wheel on macOS.

I tried setting DYLD_LIBRARY_PATH both directly on the run environment and via CIBW_ENVIRONMENT. Using CIBW_BEFORE_BUILD I printed the effective environment, which you can see here: https://github.com/chatnoir-eu/chatnoir-resiliparse/runs/3556336199?check_suite_focus=true#step:5:147

You can see that LIBRARY_PATH is set, but DYLD_LIBRARY_PATH is missing. A few lines above, you can also see that cibuildwheel sees that I want to set that variable on my environment: https://github.com/chatnoir-eu/chatnoir-resiliparse/runs/3556336199?check_suite_focus=true#step:5:46, but for some reason is unable to.

And finally, here's where the build fails, because the library search path is empty:
https://github.com/chatnoir-eu/chatnoir-resiliparse/runs/3556336199?check_suite_focus=true#step:5:260

The same is reproducible with DYLD_FALLBACK_LIBRARY_PATH and LD_LIBRARY_PATH.

@Czaki
Copy link
Contributor

Czaki commented Sep 9, 2021

It looks like MacOS differently treats the environment variable and prepends the command with the variable only for this command.

Spawning children processes of processes restricted by System Integrity Protection, such as by launching a helper process in a bundle with NSTask or calling the exec(2) command, resets the Mach special ports of that child process. Any dynamic linker (dyld) environment variables, such as DYLD_LIBRARY_PATH, are purged when launching protected processes.

https://developer.apple.com/documentation/security/disabling_and_enabling_system_integrity_protection

So possible workaround on the side if cibuildwheel could check if DYLD_LIBRARY_PATH is inside CIBW_ENVIRONMENT and then use its value in DYLD_LIBRARY_PATH=... delocate

Knowledge from this thread should be transferred to documentation.

@joerick
Copy link
Contributor

joerick commented Sep 9, 2021

Ah, I see, macOS strips that env var out after each subprocess launch. So I guess that the /bin/sh that we launch to run CIBW_REPAIR_WHEEL_COMMAND gets DYLD_LIBRARY_PATH, but the shell process doesn't pass it to its children.

In that case, my recommendation would be to set

CIBW_REPAIR_WHEEL_COMMAND_MACOS: >
  DYLD_LIBRARY_PATH=$LIBRARY_PATH delocate-listdeps &&
  DYLD_LIBRARY_PATH=$LIBRARY_PATH delocate-wheel --require-archs {delocate_archs} -w {dest_dir} {wheel}

Being set by the shell process, delocate should get the environment variable. That should alleviate the need for your extra step, I think, @phoerious.

I agree, something should be added to the documentation. How does this look as a faq.md entry:


Passing DYLD_LIBRARY_PATH to delocate

macOS has built-in System Integrity protections which limits the use of DYLD_LIBRARY_PATH and LD_LIBRARY_PATH so that it does not automatically pass to children processes. This means if you set DYLD_LIBRARY_PATH before running cibuildwheel, or even set it in CIBW_ENVIRONMENT, it will be stripped out of the environment before delocate is called.

To work around this, use a different environment variable such as REPAIR_LIBRARY_PATH to store the library path, and set DYLD_LIBRARY_PATH in CIBW_REPAIR_WHEEL_COMMAND_MACOS, like this:

CIBW_REPAIR_WHEEL_COMMAND_MACOS: >
  DYLD_LIBRARY_PATH=$REPAIR_LIBRARY_PATH delocate-listdeps {wheel} &&
  DYLD_LIBRARY_PATH=$REPAIR_LIBRARY_PATH delocate-wheel --require-archs {delocate_archs} -w {dest_dir} {wheel}

@phoerious
Copy link
Author

If you write it like that, it might lead people to think that LIBRARY_PATH is just a custom name, but it's the variable the linker uses for finding the libs. It makes sense in most cases to have LIBRARY_PATH and DYLD_LIBRARY_PATH point to the same place, but there may be exceptions.

phoerious added a commit to chatnoir-eu/chatnoir-resiliparse that referenced this issue Sep 9, 2021
@phoerious
Copy link
Author

phoerious commented Sep 9, 2021

BTW, it's CIBW_REPAIR_WHEEL_COMMAND_MACOS, not CIBW_WHEEL_REPAIR_COMMAND_MACOS and there is a missing {wheel} in the first command. 🙄

@joerick
Copy link
Contributor

joerick commented Sep 9, 2021

Thanks! Yes and good point on LIBRARY_PATH, I'd forgotten that was a special variable. I've subbed it out for something else.

joerick added a commit that referenced this issue Sep 9, 2021
yngve-sk pushed a commit to equinor/Carolina that referenced this issue Nov 6, 2023
yngve-sk pushed a commit to equinor/Carolina that referenced this issue Nov 6, 2023
yngve-sk pushed a commit to equinor/Carolina that referenced this issue Nov 6, 2023
yngve-sk pushed a commit to equinor/Carolina that referenced this issue Nov 6, 2023
yngve-sk pushed a commit to equinor/Carolina that referenced this issue Nov 6, 2023
yngve-sk pushed a commit to equinor/Carolina that referenced this issue Nov 6, 2023
godlygeek added a commit to bloomberg/memray that referenced this issue May 13, 2024
We were previously setting `DYLD_LIBRARY_PATH`, but macOS's System
Integrity Protections were resulting in that environment being dropped
before `delocate` was run, causing an incorrect version of `liblz4` to
be found and vendored.

See pypa/cibuildwheel#816

Signed-off-by: Matt Wozniski <[email protected]>
godlygeek added a commit to bloomberg/memray that referenced this issue May 13, 2024
We were previously setting `DYLD_LIBRARY_PATH`, but macOS's System
Integrity Protections were resulting in that environment being dropped
before `delocate` was run, causing an incorrect version of `liblz4` to
be found and vendored.

See pypa/cibuildwheel#816

Signed-off-by: Matt Wozniski <[email protected]>
godlygeek added a commit to bloomberg/memray that referenced this issue May 13, 2024
We were previously setting `DYLD_LIBRARY_PATH`, but macOS's System
Integrity Protections were resulting in that environment being dropped
before `delocate` was run, causing an incorrect version of `liblz4` to
be found and vendored.

See pypa/cibuildwheel#816

Signed-off-by: Matt Wozniski <[email protected]>
godlygeek added a commit to bloomberg/memray that referenced this issue May 13, 2024
We were previously setting `DYLD_LIBRARY_PATH`, but macOS's System
Integrity Protections were resulting in that environment being dropped
before `delocate` was run, causing an incorrect version of `liblz4` to
be found and vendored.

See pypa/cibuildwheel#816

Also, set `PKG_CONFIG_PATH` so that `pkg-config` knows to pick up the
`liblz4` that we've built, and add a `find` to our macOS `before-all`
command so that our logs show what has been installed.

Signed-off-by: Matt Wozniski <[email protected]>
godlygeek added a commit to bloomberg/memray that referenced this issue May 13, 2024
We were previously setting `DYLD_LIBRARY_PATH`, but macOS's System
Integrity Protections were resulting in that environment variable being
dropped before `delocate` was run, causing an incorrect version of
`liblz4` to be found and vendored.

See pypa/cibuildwheel#816

Per that issue (and the docs), the workaround is overriding the
`delocate` call to pass `DYLD_LIBRARY_PATH` directly to it.

While we're at it, set `PKG_CONFIG_PATH` so that `pkg-config` knows to
pick up the `liblz4` that we've built, and add a `find` to our macOS
`before-all` command so that our logs show what has been installed.

There's one thing I still don't understand: why we need to set
`DYLD_LIBRARY_PATH` at all. I would expect `delocate` to find the right
`liblz4` via the rpath that we're setting, but it doesn't...

Signed-off-by: Matt Wozniski <[email protected]>
pablogsal pushed a commit to bloomberg/memray that referenced this issue May 13, 2024
We were previously setting `DYLD_LIBRARY_PATH`, but macOS's System
Integrity Protections were resulting in that environment variable being
dropped before `delocate` was run, causing an incorrect version of
`liblz4` to be found and vendored.

See pypa/cibuildwheel#816

Per that issue (and the docs), the workaround is overriding the
`delocate` call to pass `DYLD_LIBRARY_PATH` directly to it.

While we're at it, set `PKG_CONFIG_PATH` so that `pkg-config` knows to
pick up the `liblz4` that we've built, and add a `find` to our macOS
`before-all` command so that our logs show what has been installed.

There's one thing I still don't understand: why we need to set
`DYLD_LIBRARY_PATH` at all. I would expect `delocate` to find the right
`liblz4` via the rpath that we're setting, but it doesn't...

Signed-off-by: Matt Wozniski <[email protected]>
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 a pull request may close this issue.

4 participants