treewide: autoPatchelfHook / fixDarwinDylibNames only on Linux / Darwin

[OPNA2608]

Description of changes

#149731 rewrote autoPatchelfHook in a way where it won't silently skip over non-ELF binaries anymore. I assume that wasn't an intended change, but it's prolly better to fix some packages so they don't even pull in this hook when it's not needed.

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/)
• 22.05 Release Notes (or backporting 21.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
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[OPNA2608]

Note that dmd will still fail with this, but at a different point in the build. That failure wasn't introduced by the autoPatchelfHook rewrite and affects master too, it will be fixed by #163989.

[symphorien]

maybe you can add meta.platforms = linux to autoPatchelfHook so that forgetting to make it conditional results in an explicit error?

[OPNA2608]

None of the other setup hook derivations (i.e. fixDarwinDylibNames, but it won't fail on Linux anyway) do it and makeSetupHook doesn't seem to support meta afaict. What would be the best way of doing that?

  autoPatchelfHook = (makeSetupHook {
    name = "auto-patchelf-hook";
    deps = [ bintools ];
    substitutions = {
      pythonInterpreter = "${python3.withPackages (ps: [ ps.pyelftools ])}/bin/python";
      autoPatchelfScript = ../build-support/setup-hooks/auto-patchelf.py;
    };
  } ../build-support/setup-hooks/auto-patchelf.sh).overrideAttrs (oa: {
    meta.platforms = lib.platforms.linux;
  });

Works but maybe there's a better way?

[symphorien]

None of the other setup hook derivations (i.e. fixDarwinDylibNames, but it won't fail on Linux anyway) do it

If you see a value in modifying derivations to not use autoPatchelfHook on darwin, then it must fail loudly when not fixed otherwise it will bitrot.

Works but maybe there's a better way?

The way you found look good enough to me, but modifying makeSetupHook to optionally take meta as argument would be better.

[OPNA2608]

If you see a value in modifying derivations to not use autoPatchelfHook on darwin, then it must fail loudly when not fixed

Well, it already fails quite loudly. That's what I'm trying to fix here:

I guess nobody bothered with testing that PR on Darwin, so it wasn't noticed.

[symphorien]

Ah ok, weird that nobody noticed it then.
Setting meta.platforms would still improve the error message, but I leave it up to you.

[siraben]

Result of nixpkgs-review pr 163924 run on aarch64-darwin 1

22 packages marked as broken and skipped:

• bazel-watcher
• bazel_0_29
• bazel_1
• libtensorflow
• python39Packages.augmax
• python39Packages.baselines
• python39Packages.dm-haiku
• python39Packages.edward
• python39Packages.einops
• python39Packages.elegy
• python39Packages.flax
• python39Packages.optax
• python39Packages.optuna
• python39Packages.rl-coach
• python39Packages.scikit-tda
• python39Packages.tensorflow
• python39Packages.tensorflow-datasets
• python39Packages.tensorflow-probability
• python39Packages.tensorflowWithoutCuda
• python39Packages.tflearn
• python39Packages.treex
• python39Packages.umap-learn

2 packages failed to build:

• bazel
• gtkd

10 packages built:

• Literate
• bazel_4
• dtools
• dub
• elasticsearch
• ldc
• mspds-bin
• powershell
• rund
• ssm-session-manager-plugin

[OPNA2608]

I was hoping to keep the changes small to get them into staging-next before the merge, but I suppose we can aim abit bigger now that that's over.

@symphorien We can do that, I guess there usually isn't much use to trying to patch ELF files for a Darwin target. For consistency's sake I'll do the same to fixDarwinDylibNames as well then.

[siraben]

Coq also builds. Not sure what the previous errors were caused by.

[kubukoz]

Just wondering, what's the next step to get this into master?

[OPNA2608]

The next step is for this to get picked up by a staging-next iteration. It missed the current one, so it'll happen in the next one. After that it'll get merged into master, and from there into the various channels: https://nixpk.gs/pr-tracker.html?pr=163924

[kubukoz]

lovely, thanks. I'm following master in my flakes so that point will be good enough for me :)