-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
[spatialite] fix building with x64-osx-dynamic #33841
Conversation
set(OPTIONS_RELEASE "LDFLAGS=-Wl,-rpath ${CURRENT_INSTALLED_DIR}/lib") | ||
else() | ||
set(OPTIONS_DEBUG "LDFLAGS=-Wl,-rpath ${CURRENT_INSTALLED_DIR}/debug/lib") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not work with binary caching. There is another PR to properly handle relative paths.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goal is just to trick configure into having an rpath set for conftest binaries
Cmp:
configure:4551: ./conftest
dyld[31214]: Library not loaded: '@rpath/libgeos_c.1.dylib'
Referenced from: '/Users/mkuhn/dev/vcpkg/buildtrees/libspatialite/x64-osx-dynamic-rel/conftest'
Reason: tried: '/usr/local/lib/libgeos_c.1.dylib' (no such file), '/usr/lib/libgeos_c.1.dylib' (no such file)
./../src/atialite-5-5796f3d8f9.clean/configure: line 4553: 31214 Abort trap: 6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will escape the config test context and affect the build output.
We need #32200.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#32200 looks like a step in the right direction (many libraries have hardcoded rpaths into vcpkg folders)
I am not sure it helps here, as libgeos_c.1.dylib
has an @rpath
prefix (so it looks correct already) and it's autotools not handling this correctly in the test context.
That being said, I am very open for a different approach, my autotools knowledge is basic.
@@ -188,10 +195,13 @@ else() | |||
${RTTOPO_OPTION} | |||
"--disable-examples" | |||
"--disable-minizip" | |||
"--with-geosconfig=${CURRENT_INSTALLED_DIR}/tools/geos/bin/geos-config" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The right geos should come via pkg-config, cf. pkg_config_modules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ping @m-kuhn for response, can you address the review suggestions? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will need some hints by someone else regarding the other comment, if we have a path forward there, I'm happy to look into this one here again.
Superseded |
fixes #33840
Very tentative
See also #4376 for some background on osx and rpath