rav1e: fix build with updated Darwin stdenv

[reckenrode]

Description of changes

The updated Darwin stdenv uses llvm-strip, but that causes issues for rav1e when using NASM and linking it with the cctools ld64. Stripping the debug information instead of everything fixes the problem.

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.

[toonn]

Is there a way to override stdenv with another cctools to test this?

[reckenrode]

rav1e was updated recently, so I’m going to test to see whether this fix is still necessary. I’m not able to reproduce using the following. The build succeeds.

Note: Using a newer LLVM is required because the older one uses the strip from cctools. #237429 must be applied to fix the build with cctools-llvm and a newer LLVM.

$ nix repl . --system x86_64-darwin
nix-repl> stdenvCctoolsLlvm = overrideCC stdenv (stdenv.cc.override {
  bintools = stdenv.cc.bintools.override {
    bintools = stdenv.cc.bintools.bintools.override {
      cctools = darwin.cctools-llvm.override { llvmPackages = llvmPackages_15; };
    };
  };
  libc = pkgs.darwin.Libsystem;
})
nix-repl> :b rav1e.override { stdenv = stdenvCctoolsLlvm; }

[toonn]

I think this override is still not working, I put a type strip in the postBuild of rav1e and strip seems to still point to cctools-port.

> nix repl .
warning: future versions of Nix will require using `--file` to load a file
Welcome to Nix 2.11.1. Type :? for help.

Loading installable ''...
Added 19369 variables.
nix-repl> stdenvCctoolsLlvm = overrideCC stdenv (stdenv.cc.override {bintools = stdenv.cc.bintools.override {bintools = stdenv.cc.bintools.bintools.override { cctools = darwin.cctools-llvm.override { llvmPackages = llvmPackages_15; }; }; };})
nix-repl> :b rav1e.override { stdenv = stdenvCctoolsLlvm; }
error: builder for '/nix/store/s0zib706v36smjh066288l1v5dp37cj1-rav1e-0.6.6.drv' failed with exit code 1;
       last 10 log lines:
       >    Compiling arg_enum_proc_macro v0.3.2
       >    Compiling simd_helpers v0.1.0
       >    Compiling scan_fmt v0.2.6
       >    Compiling y4m v0.8.0
       >    Compiling git2 v0.15.0
       >    Compiling built v0.5.2
       >    Compiling rav1e v0.6.6 (/private/tmp/nix-build-rav1e-0.6.6.drv-0/rav1e-0.6.6.tar.gz)
       >     Finished release [optimized + debuginfo] target(s) in 8m 47s
       > BLAAAAH
       > strip is /nix/store/nr4b3hbhh1izmcngzryb26rhwk37gkvz-cctools-binutils-darwin-973.0.1/bin/strip
       For full logs, run 'nix log /nix/store/s0zib706v36smjh066288l1v5dp37cj1-rav1e-0.6.6.drv'.
> ls -l /nix/store/nr4b3hbhh1izmcngzryb26rhwk37gkvz-cctools-binutils-darwin-973.0.1/bin/strip
lrwxr-xr-x  1 root  nixbld  74  1 jan  1970 /nix/store/nr4b3hbhh1izmcngzryb26rhwk37gkvz-cctools-binutils-darwin-973.0.1/bin/strip@ -> /nix/store/p42pyk9rzvdpv0gxc3rx2jx9hh4pck76-cctools-port-973.0.1/bin/strip

This is on 9bb343d628ba (HEAD, upstream/master) Merge pull request #237548 from rnhmjoj/pr-fix-rel to try to reproduce the build failure.

[reckenrode]

I have a build running against my clang 16 branch. If it fails there, I’ll try to find an override that properly includes llvm-strip, so you can test.

[reckenrode]

Actually, rav1e uses buildRustPackage. Try also overriding the stdenv in the rustPkatform it’s using.

[toonn]

Ah, that was key. Now I get a build failure on 9bb343d628ba (HEAD, upstream/master) Merge pull request #237548 from rnhmjoj/pr-fix-rel.

> nix-build --no-out-link --pure -E 'let ps = (import ./. {}); stdenvCCtools = ps.overrideCC ps.stdenv (ps.stdenv.cc.override { bintools = ps.stdenv.cc.bintools.override { bintools = ps.stdenv.cc.bintools.bintools.override { cctools = ps.darwin.cctools-llvm.override { llvmPackages = ps.llvmPackages_15; }; }; }; libc = ps.darwin.Libsystem; }); in ps.rav1e.override { stdenv = stdenvCCtools; rustPlatform = ps.makeRustPlatform (ps.rustPackages.buildRustPackages // { stdenv = stdenvCCtools; }); }'
[...]
       >   = note: ld: in /private/tmp/nix-build-rav1e-0.6.6.drv-0/rustcGil0X1/l
ibrav1e-c6b855765ab5a133.rlib(cdef_avx2.o), in section __TEXT,__text reloc 36: s
ymbol index out of range for architecture x86_64                                
       >           clang-11: error: linker command failed with exit code 1 (use 
-v to see invocation)                                                           
       >                                                                        
       >                                                                        
       > error: could not compile `rav1e` due to previous error                 
       For full logs, run 'nix log /nix/store/b4nxhwhn82gzdzrjf256gag7hg7inlsn-r
av1e-0.6.6.drv'.

And it works with this PR cherry-picked.

Does this need to target staging? I'm not quite sure because OfBorg failed so there's no label. I'm running a nixpkgs-review to check now.

[reckenrode]

Does this need to target staging?

Running nixpkg-review pr 234868 --system x86_64-darwin shows 566 packages updated.

@ofborg eval

[reckenrode]

Linux is showing 473 updates when I run nixpkgs-review there. I assume reason is the addition of the postPatch attribute even though it evaluates to an empty string.

I’d been using 500+ rebuilds as my threshold for staging versus master. If that’s too low, I can retarget.

[toonn]

Definitely should go to staging.

LGTM