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

Set INSTALL_NAME_DIR for MacOS all the time #3948

Closed
wants to merge 2 commits into from

Conversation

pramsey
Copy link

@pramsey pramsey commented Nov 9, 2023

For XCode 15, the dyld linker behaviour changed and no longer automatically falls back to checking the contents of /usr/local/lib when trying to evaluate an @rpath dependency. As a result, runtime linking is failing on up-to-date MacOS systems. The INSTALL_NAME_DIR directive is used for APPLE builds, to ensure that the installed library advertises its install path, so libraries that link to it can find it again when dynamic linking time arrives.

https://stackoverflow.com/questions/77137284/dynamic-linker-fails-to-find-symbol-at-runtime-after-updating-to-xcode-15

If this patch is OK, it should be cherry-picked into main as well, and maybe into some earlier versions, as folks move into XCode15 this change will potentially bite them.

and no longer automatically falls back to checking
the contents of /usr/local/lib when trying to
evaluate an @rpath dependency. As a result,
runtime linking is failing on up-to-date MacOS
systems. The INSTALL_NAME_DIR directive is used
for APPLE builds, to ensure that the installed
library advertises its install path, so libraries
that link to it can find it again when dynamic
linking time arrives.

https://stackoverflow.com/questions/77137284/dynamic-linker-fails-to-find-symbol-at-runtime-after-updating-to-xcode-15
@pramsey pramsey changed the title For XCode 15, the dyld linker behaviour changed Set INSTALL_NAME_DIR for MacOS all the time Nov 9, 2023
@rouault
Copy link
Member

rouault commented Nov 10, 2023

In e1253e4 , we removed all historical rpath CMake tweaking (a topic that has a Linuxer is totally mysterious to me), as they appeared to be no longer necessary for the up-to-date Mac of the date. Seems we're back to to the olden dark days...
I've no particular objection to this PR, just that I don't fully seize the impacts. Any other devs more familiar with Mac having an opinion on that ?
It would seem to be me that CMake should do all that dark magic alone but we'll perhaps have to wait for updates on their side.

Hum, I see that in

if(APPLE)
we do define CMAKE_INSTALL_NAME_DIR. Perhaps this piece of code should be moved just before
include(lib_proj.cmake)

@rouault
Copy link
Member

rouault commented Nov 10, 2023

ok, so this breaks the test on MacOS 12: "Check that we can retrieve the resource directory in a relative way after renaming the installation prefix" :

echo "Check that we can retrieve the resource directory in a relative way after renaming the installation prefix"
. Probably because setting INSTALL_NAME_DIR prevents a "relative rpath" resolution (or whatever it is called) to work ?

@pramsey
Copy link
Author

pramsey commented Nov 10, 2023

With respect to CMAKE_INSTALL_NAME_DIR, the handling of MacOS stuff is a little snarled and conditional, with some things turned on for Framework building, but not for other things. In theory, setting CMAKE_INSTALL_NAME_DIR should just globally do the "right thing" but in practice in GEOS world, I found I had to set the property more verbosely on the target to get the behaviour I want. I agree it is all very confusing, and not at all clear what the best practice actually should be. However it does look like Apple, by accident or on purpose, has made /usr/local/lib no longer a default rpath search, so installing a bare @rpath LC_ID_DYLIB is not an option anymore.

@pramsey
Copy link
Author

pramsey commented Nov 10, 2023

Hm, so the test failure is around relocating the install manually, which fails as the binaries are built with explicit paths instead of relative paths... though I'm a little surprised it worked before, frankly, without setting a relative @rpath... I'll have to take a look at what the current main does. It seems like to achieve what I want I will need to muck with the rpath stuff again.

@pramsey
Copy link
Author

pramsey commented Nov 13, 2023

I instead did an experiment in adding the RPATH information to the PostGIS build, instead of setting the INSTALL_NAME on the library install. I'm really not sure what the "correct" approach is. Certainly 99% of the libraries on the MacOS system (including things like libraries installed by MacPorts) have their INSTALL_NAME set, and do not rely on RPATH searches.

Copy link
Contributor

The PROJ project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last two months and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add terminal output examples if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added the stale label Jan 13, 2024
Copy link
Contributor

While we hate to see this happen, this PR has been automatically closed because it has not had any activity in the last 2 months. If this pull request should be reconsidered, please follow the guidelines in the previous comment and reopen this pull request. Or, if you have any further questions, just ask! We love to help, and if there's anything the PROJ project can do to help push this PR forward please let us know how we can assist.

@github-actions github-actions bot closed this Jan 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants