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

libspatialindex: fix on Darwin #374177

Merged
merged 1 commit into from
Jan 17, 2025
Merged

libspatialindex: fix on Darwin #374177

merged 1 commit into from
Jan 17, 2025

Conversation

booxter
Copy link
Contributor

@booxter booxter commented Jan 16, 2025

Recent bump to 2.1.0 changed the soversion from 7 to 8. It broke python3-rtree as follows:

OSError: Could not load libspatialindex_c library

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Jan 16, 2025
@github-actions github-actions bot added 10.rebuild-darwin: 11-100 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Jan 16, 2025
Copy link
Contributor

@paparodeo paparodeo left a comment

Choose a reason for hiding this comment

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

can just fix the CMakeList.txt to not use @rpath

diff --git a/pkgs/by-name/li/libspatialindex/no-rpath-for-darwin.diff b/pkgs/by-name/li/libspatialindex/no-rpath-for-darwin.diff
new file mode 100644
index 000000000000..a9bb274c6271
--- /dev/null
+++ b/pkgs/by-name/li/libspatialindex/no-rpath-for-darwin.diff
@@ -0,0 +1,13 @@
+diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
+index cb62fd9..22153ff 100644
+--- a/src/CMakeLists.txt
++++ b/src/CMakeLists.txt
+@@ -248,7 +248,7 @@ if(HAVE_BCOPY)
+ endif()
+ 
+ 
+-if(APPLE)
++if(FALSE)
+   set(MACOSX_RPATH ON)
+   set_target_properties(spatialindex spatialindex_c
+                         PROPERTIES INSTALL_NAME_DIR "@rpath")
diff --git a/pkgs/by-name/li/libspatialindex/package.nix b/pkgs/by-name/li/libspatialindex/package.nix
index 1c07b6dae6ea..4e6ff6cfad4e 100644
--- a/pkgs/by-name/li/libspatialindex/package.nix
+++ b/pkgs/by-name/li/libspatialindex/package.nix
@@ -17,6 +17,10 @@ stdenv.mkDerivation (finalAttrs: {
     hash = "sha256-a2CzRLHdQMnVhHZhwYsye4X644r8gp1m6vU2CJpSRpU=";
   };
 
+  patches = [
+    ./no-rpath-for-darwin.diff
+  ];
+
   postPatch = ''
     patchShebangs test/
   '';
@@ -37,10 +41,6 @@ stdenv.mkDerivation (finalAttrs: {
 
   doCheck = true;
 
-  postFixup = lib.optionalString stdenv.hostPlatform.isDarwin ''
-    install_name_tool -change "@rpath/libspatialindex.7.dylib" "$out/lib/libspatialindex.7.dylib" $out/lib/libspatialindex_c.dylib
-  '';
-
   meta = {
     description = "Extensible spatial index library in C++";
     homepage = "https://libspatialindex.org";

@booxter
Copy link
Contributor Author

booxter commented Jan 16, 2025

@paparodeo Thank you! I've updated to your version (plus removed unnecessary buildInput for the rpath fixup hook).

Recent bump to 2.1.0 changed the soversion from 7 to 8. It broke
python3-rtree as follows:

OSError: Could not load libspatialindex_c library

Instead of bumping the version in postFixup, disable rpath in cmake
instructions.

Co-Authored-By: Reno Dakota <[email protected]>
Copy link
Contributor

@imincik imincik left a comment

Choose a reason for hiding this comment

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

@booxter @paparodeo thanks for this fix. Looks good to me.

@imincik
Copy link
Contributor

imincik commented Jan 17, 2025

Build successfully tested on Darwin:

nix-build --argstr system aarch64-darwin -A libspatialindex
nix-build --argstr system aarch64-darwin -A python3Packages.rtree

Looks good. Thanks !

@imincik imincik merged commit fadbe13 into NixOS:master Jan 17, 2025
23 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 11-100 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants