Adding missing rpath to Python on Apple Silicon

[colincornaby]

On Apple Silicon, Homebrew has moved the location its installed its libraries. That means if you are installing a version of Cairo from Homebrew, Python/python3-cairosvg cannot find Cairo, and vcpkg will fail to build python3-cairosvg.

Homebrew's version of Python passes an additional rpath to the configuration phase of the Python build to fix this: https://github.com/carlocab/homebrew-core/blob/d83d8d4665518494ef7fc3e5a64ba937e69cb06c/Formula/python%403.9.rb#L150

This patch to our Python port also introduces the same patch. I'm still pretty new with vcpkg so I possibly haven't defined this correctly from a maintainable vcpkg perspective. But this fix does block Apple Silicon builds out of the box. I've had to do some manual patching each vcpkg update to work around it.

[dpogue]

Two things I'm wondering here:

1. This should have no ill-effects if the Homebrew directory does not exist?

2. This does not somehow introduce a potential security hole whereby the game Python interpreter can load modules from the homebrew path? (I assume not, but good to make sure)

I seem to recall that on Windows, we have to build Python as a dynamic library so that we get a working interpreter at build-time that can load the cairo module. I know you're trying to make sure Python is built as a static library on macOS, but do we run into the same issue there when trying to use it at build time? (grumble grumble back to https://github.com/H-uru/Plasma/discussions/1423 and the inability of CMake to find a host Python interpreter and a target Python library)

[colincornaby]

It should not cause any failure if the Homebrew path does not exist.

To answer 2 and your follow on question…. It gets weird.

The rpath argument is embedded into the binary and parsed by the dynamic linker. But we build a static version of Python, so this rpath does absolutely nothing for the static library built for Plasma. In since Python is a static library I’m not even sure dynamic loader commands could be embedded at all. What it does effect is the Python executable that is built by vcpkg - which is what the Python Cairo package uses to test it’s integration.

[dpogue]

Interesting... okay. So to confirm, on macOS we're able to build both a working Python interpreter executable and a Python static library from vcpkg at the same time?

[colincornaby]

That seems to be what is occurring. In the vcpkg output there is also an interpreter that the Cairo module is executing tests against. On Apple Silicon the Cairo module fails during it’s test phase.

[dgelessus]

Hmm, I don't understand why anything should be pulled in from Homebrew when using vcpkg to build the dependencies... Shouldn't Cairo also come from vcpkg in that case?

[Hoikas]

PSF seems to think this is incorrect https://bugs.python.org/issue46381

[colincornaby]

Reading the full thread, it seems ok because we use LDFLAGS_NODIST which is the intended mechanism.

https://github.com/python/cpython/issues/90539#issuecomment-1125478404

[Hoikas]

Ok, in that case, it would probably be better to, instead of patching the Python build, pass through the configure option CONFIGURE_LDFLAGS_NODIST from the triplet. That way, we could (potentially) de-vendor the port someday. So, in the triplet, you'll want to do something like:

if(PORT STREQUAL python3)
    set(VCPKG_MAKE_CONFIGURE_OPTIONS "CONFIGURE_LDFLAGS_NODIST=-rpath /opt/homebrew/lib")
endif()

and toss the patch.

[colincornaby]

Oooooo thanks. That’s the sort of insight I was looking for. I’ll edit the commit history to reflect only this suggestion.

[colincornaby]

That chunk is applying during the Python build - but the output isn't correct. I'm tinkering with it to see if I can figure out the issue.

[colincornaby]

Ok - there is an issue on the vcpkg side. I can submit a PR - but something is going to get broken possibly.

VCPKG_MAKE_CONFIGURE_OPTIONS is indeed a documented option: https://learn.microsoft.com/en-us/vcpkg/users/triplets#vcpkg_make_configure_options

This change seems to have intentionally or unintentionally renamed that option to VCPKG_CONFIGURE_MAKE_OPTIONS - without updating the docs: https://github.com/microsoft/vcpkg/commit/5283cdb37039860a89d00bae0e162a1fe360a80c#diff-b92b07d5c0c681687d71463c6a368fae98e9ffedb241e2aadbde3aac688899a9L519

(Line 519, scripts/cmake/vcpkg_configure_make.cmake)

So that's why it wasn't working. I can use VCPKG_CONFIGURE_MAKE_OPTIONS for now. But something is not right.

[colincornaby]

I've gone ahead and implemented the change through a history rewrite - accounting for the issue noted in the previous comment.

[dgelessus]

Why do we need to add Homebrew paths to the linker flags at all though? If a library is missing, the proper fix would be to add it as a dependency in our vcpkg.json, not pull it in from a different package manager - right?

[dpogue]

If a library is missing, the proper fix would be to add it as a dependency in our vcpkg.json, not pull it in from a different package manager - right?

For the case of cairo, since it's an optional dependency used only on the host system to generate resource.dat at build time, we've been pulling it from package managers on macOS and Linux. On Windows we do pull it in with vcpkg though, so that's an option if it makes our lives easier: https://github.com/H-uru/Plasma/blob/d8630785def8d0bb717e372a58354233952933b0/vcpkg.json#L55-L59

[colincornaby]

I'm not completely sure that would work on macOS - but I could try. When vcpkg builds the library it won't add it to a search path, taking us right back where we started. The Python interpreter won't be able to use Cairo because it won't be in a known path.

[dgelessus]

For the case of cairo, [...], we've been pulling it from package managers on macOS and Linux.

On Linux it's not a problem in practice, because often you already have Cairo installed system-wide as a dependency of the usual GUI environments. On macOS you usually need to install it manually, and when you do, it will be in a location that isn't searched by default by many things, which is suboptimal.

since it's an optional dependency used only on the host system to generate resource.dat at build time

Ah okay, that makes some sense. But is there any disadvantage to using vcpkg for such dependencies as well (especially if we already do that on Windows)? There's even already some code in VcpkgToolchain.cmake that skips installing Cairo if you turn off PLASMA_BUILD_RESOURCE_DAT, so you can always do that if the extra dependency is an issue.

When vcpkg builds the library it won't add it to a search path, taking us right back where we started. The Python interpreter won't be able to use Cairo because it won't be in a known path.

That would make things more difficult... but in that case, I'd argue that the right fix/workaround is to put vcpkg's directories onto the Python search path, rather than depending on Homebrew.

(Side note: I personally use MacPorts and don't even have Homebrew installed, so I get a bit peeved when things have special defaults that only work for Homebrew 🙂 But regardless of that, I think we shouldn't depend on more than one package manager, unless there's a very good reason for it.)

[Hoikas]

I'm not completely sure that would work on macOS - but I could try. When vcpkg builds the library it won't add it to a search path, taking us right back where we started. The Python interpreter won't be able to use Cairo because it won't be in a known path.

If we went this route, we would need to copy the cairo DLL into the Python site-packages in the install step in our python3-cairosvg port.

[colincornaby]

I think I probably could just point the rpath at the vcpkg dir instead of the Homebrew dir.

Overall what would be required is:

• Building Cairo as a dylib instead of as a static library (easy)
• Building Cairo for the host architecture instead of the destination architecture (easy)
• Getting the path to Cairo lib folder within the triplet (not sure how to do this)

[Hoikas]

Building Cairo for the host architecture instead of the destination architecture (easy)

python3-cairosvg should probably be marked "host": true

[colincornaby]

Alright - let's try this approach. :)

This pulls Cairo from vcpkg. It also sets up the rpath to look for Cairo within the vcpkg installed directory. Cairo is no longer required from Homebrew.

I didn't remove the Homebrew install from Github actions yet. If this builds successfully in Github Actions I'll go ahead and do so.

[colincornaby]

I've updated the commit to remove the Windows gate for the Cairo dependency.