From 79aebb620410688000749775c4c52171fdb8c748 Mon Sep 17 00:00:00 2001
From: Tobias Markus <tobias@miglix.eu>
Date: Fri, 17 Feb 2023 14:14:13 +0100
Subject: [PATCH] alembic: Fix install destinations

The generated CMake targets file was referring to an incorrect
destination as the derivation manually moved the libraries during
installPhase, while CMake uses the path it thinks is going to be used
(the DESTINATION in the install rule) in the IMPORTED_LOCATION property.

By setting the install destinations via CMake flags (and patching the
DESTINATION for the binary install rules), CMake will pick up the
correct locations in the generated AlembicTargets-release.cmake file.

Along with fixing that issue, this commit also includes the following
changes:
* Remove unused unzip nativeBuildInput
* Enable unit tests
* Add missing direct dependency ilmbase:
  Previously it was only picked up indirectly, resulting in CMake
  configuration warnings
* Add ilmbase as propagatedBuildInput:
  Downstream users of Alembic (via CMake) need to add ilmbase as a
  dependency as well
  For some reason this is not discovered correctly otherwise
* Use CMake setup hooks instead of setting buildPhase/installPhase
---
 .../development/libraries/alembic/default.nix | 54 +++++++++++++------
 1 file changed, 39 insertions(+), 15 deletions(-)

diff --git a/pkgs/development/libraries/alembic/default.nix b/pkgs/development/libraries/alembic/default.nix
index c7bc4894e15b..0811a5ce09e0 100644
--- a/pkgs/development/libraries/alembic/default.nix
+++ b/pkgs/development/libraries/alembic/default.nix
@@ -1,4 +1,4 @@
-{ lib, stdenv, fetchFromGitHub, unzip, cmake, openexr, hdf5-threadsafe }:
+{ lib, stdenv, fetchFromGitHub, cmake, openexr, hdf5-threadsafe, ilmbase }:
 
 stdenv.mkDerivation rec
 {
@@ -12,26 +12,50 @@ stdenv.mkDerivation rec
     sha256 = "sha256-8dQhOQN0t2Y2kC2wOpQUqbu6Woy4DUmiLqXjf1D+mxE=";
   };
 
+  # note: out is unused (but required for outputDoc anyway)
   outputs = [ "bin" "dev" "out" "lib" ];
 
-  nativeBuildInputs = [ unzip cmake ];
-  buildInputs = [ openexr hdf5-threadsafe ];
+  # Prevent cycle between bin and dev (only occurs on Darwin for some reason)
+  propagatedBuildOutputs = [ "lib" ];
 
-  buildPhase = ''
-    cmake -DUSE_HDF5=ON -DCMAKE_INSTALL_PREFIX=$out/ -DUSE_TESTS=OFF .
+  nativeBuildInputs = [ cmake ];
 
-    mkdir $out
-    mkdir -p $bin/bin
-    mkdir -p $dev/include
-    mkdir -p $lib/lib
+  # NOTE: Alembic also support imath instead of ilmbase, but some users of Alembic (e.g. Blender)
+  # are incompatible with the imath version of Alembic
+  buildInputs = [ openexr hdf5-threadsafe ilmbase ];
+
+  # Downstream packages trying to use Alembic via CMake need ilmbase as well
+  # For some reason this won't be picked up correctly otherwise
+  propagatedBuildInputs = [ ilmbase ];
+
+  # These flags along with the postPatch step ensure that all artifacts end up
+  # in the correct output without needing to move anything
+  #
+  # - bin: Uses CMAKE_INSTALL_BINDIR (set via CMake setup hooK)
+  # - lib (contains shared libraries): Uses ALEMBIC_LIB_INSTALL_DIR
+  # - dev (headers): Uses CMAKE_INSTALL_PREFIX
+  #   (this works because every other install rule uses an absolute DESTINATION)
+  # - dev (CMake files): Uses ConfigPackageLocation
+
+  cmakeFlags = [
+    "-DUSE_HDF5=ON"
+    "-DUSE_TESTS=ON"
+    "-DALEMBIC_LIB_INSTALL_DIR=${placeholder "lib"}/lib"
+    "-DConfigPackageLocation=${placeholder "dev"}/lib/cmake/Alembic"
+    "-DCMAKE_INSTALL_PREFIX=${placeholder "dev"}"
+    "-DQUIET=ON"
+  ];
+
+  postPatch = ''
+    find bin/ -type f -name CMakeLists.txt -print -exec \
+      sed -i 's/INSTALL(TARGETS \([a-zA-Z ]*\) DESTINATION bin)/INSTALL(TARGETS \1)/' {} \;
   '';
 
-  installPhase = ''
-    make install
-
-    mv $out/bin $bin/
-    mv $out/lib $lib/
-    mv $out/include $dev/
+  doCheck = true;
+  checkPhase = ''
+    runHook preCheck
+    ctest -j 1
+    runHook postCheck
   '';
 
   meta = with lib; {