embree, openimagedenoise: Fix build failure due to TBB split

[hesiod]

Description of changes

Fixes #216390
Fixes #216580

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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/)
• 23.05 Release Notes (or backporting 22.11 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.

[Artturin]

i did try this myself and it worked but it would have to be applied always when tbb is used with cmake

[hesiod]

i did try this myself and it worked but it would have to be applied always when tbb is used with cmake

At least in case of embree, they are using their own specific CMake find module common/cmake/FindTBB.cmake, so the issue is specific to embree.

[Artturin]

sadly FindX such as FindTBB.cmake are a common thing in the cmake world

[davidak]

@Artturin Ubuntu is also splitting tbb, maybe that's still the best practice?
https://packages.ubuntu.com/bionic/libtbb-dev

Seems like a situation we want a consistent best practice in nixpkgs to be documented.

[MultisampledNight]

Hi! Not sure if this is applicable, but since openimagedenoise is affected by the exact same issue (which is also required to build Blender), could the PR maybe also include the added cmake flags on the openimagedenoise package? As in,

diff --git a/pkgs/development/libraries/openimagedenoise/default.nix b/pkgs/development/libraries/openimagedenoise/default.nix
index 8d02b976c91..39cc5ec3b52 100644
--- a/pkgs/development/libraries/openimagedenoise/default.nix
+++ b/pkgs/development/libraries/openimagedenoise/default.nix
@@ -10,6 +10,11 @@ stdenv.mkDerivation rec {
     sha256 = "sha256-i73w/Vkr5TPLB1ulPbPU4OVGwdNlky1brfarueD7akE=";
   };
 
+  cmakeFlags = [
+    "-DTBB_ROOT=${tbb}"
+    "-DTBB_INCLUDE_DIR=${tbb.dev}/include"
+  ];
+
   nativeBuildInputs = [ cmake python3 ispc ];
   buildInputs = [ tbb ];

[davidak]

@MultisampledNight i did try that and it did not work. Not sure if i'm doing something wrong.

#216580 (comment)

[davidak]

Result of nixpkgs-review pr 216844 run on x86_64-linux 1

3 packages failed to build:

• blender
• blender-hip
• python310Packages.bpycv

1 package built:

• embree

error: builder for '/nix/store/2ay0ynwd7639msidi0f8xjbx518l7k4m-openimagedenoise-1.4.3.drv' failed with exit code 1;
       last 10 log lines:
       > -- Found TBB version 2020.3 at /nix/store/6y5ms2chn50ksw6fbsig8jm62fimqspr-tbb-2020.3-dev
       > CMake Error at cmake/FindTBB.cmake:80 (message):
       >   Cannot find required components: ;tbb
       > Call Stack (most recent call first):
       >   cmake/FindTBB.cmake:487 (rk_tbb_error)
       >   CMakeLists.txt:73 (find_package)
       >
       > 
       > -- Configuring incomplete, errors occurred!
       > See also "/build/source/build/CMakeFiles/CMakeOutput.log".
       For full logs, run 'nix log /nix/store/2ay0ynwd7639msidi0f8xjbx518l7k4m-openimagedenoise-1.4.3.drv'.
error: 1 dependencies of derivation '/nix/store/6bykd889nvjj9f16b4yfxivvpv1q2cbp-blender-3.3.1.drv' failed to build
error: 1 dependencies of derivation '/nix/store/jkrm6xi93i9zfyg3n25lilfjfg42i1xj-blender-3.3.1.drv' failed to build
error: 1 dependencies of derivation '/nix/store/zfcj08d593l3xlcg1q93hgxvkwpmskdh-python3.10-bpycv-0.2.43.drv' failed to build

[MultisampledNight]

Hmm, interesting, it works (and is just building Blender fine on my local system) with this overlay, not including this PR:

      (final: prev: {
        # TODO: this overlay can be removed after https://github.com/NixOS/nixpkgs/pull/216844 is merged
        embree = prev.embree.overrideAttrs(old: rec {
          cmakeFlags = old.cmakeFlags ++ ["-DTBB_ROOT=${final.tbb}" "-DTBB_INCLUDE_DIR=${final.tbb.dev}/include"];
        });
        openimagedenoise = prev.openimagedenoise.overrideAttrs(old: rec {
          cmakeFlags = ["-DTBB_ROOT=${final.tbb}" "-DTBB_INCLUDE_DIR=${final.tbb.dev}/include"];
        });
      })

EDIT: anyway I'll stop now since this isn't really relevant for the PR

[davidak]

Changes look good

fixes the issue

[hesiod]

@MultisampledNight I would also look into the openimagedenoise issue, but it only makes sense if @Artturin is OK with that instead of the revert PR. I want to avoid both PRs being merged at the same time resulting in a mess, that's why it's still marked as a draft

[hesiod]

@MultisampledNight @davidak It turns out that openimagedenoise can be fixed by the exact same flags. I added a commit to fix openimagedenoise as well. That should resolve all breakage due to #212787

@davidak the reason your change didn't work was that you used TBB_ROOT_DIR (vs TBB_ROOT)

[davidak]

I have a PR that fixes both versions of openimagedenoise. If you want it in one PR, you can cherry-pick my commit from #216853.

It would make sense to have both in one PR because both break the same packages.

[hesiod]

Right, I missed the other openimagedenoise... sigh.. my nixpkgs-review run was already halfway through compiling Blender, but I guess I'll have to start again
Edit: Integrated your change into my second commit (I wanted to use my commit message, for consistency, hope you don't mind 😄 )

[davidak]

Result of nixpkgs-review pr 216844 run on x86_64-linux 1

1 package failed to build:

• openimagedenoise_1_2_x

5 packages built:

• blender
• blender-hip
• embree
• openimagedenoise
• python310Packages.bpycv

openimagedenoise_1_2_x issue unrelated. was already broken before (#216870)

[davidak]

Change looks good

Fixes the issue

[hesiod]

Result of nixpkgs-review pr 216844 run on x86_64-linux 1

5 packages built:

• blender
• blender-hip
• embree
• openimagedenoise
• python310Packages.bpycv

Note: This was before the force-push to include the fix for openimagedenoise_1_2_x