if we don't remove these 3 lines it fixes llvmPackages.clang rebuilding + various other packages like casadi
but will cause ~1k additional rebuilds elsewhere eg all php packages, which seems to be down to llvmPackages.stdenv.cc becoming different

I don't understand how replacing just the clang attr with an attrset merge // outside the overrideScope call can cause llvm scope's stdenv attr to change so I'm probably badly misunderstanding something going on here.

with clang inherited from prevStage here llvmPackages.stdenv.cc, php etc are different. llvmPackages. prefix only, stdenv.cc is always not a rebuild:

nix-diff (nix derivation show github:nixos/nixpkgs/staging-next#legacyPackages.aarch64-darwin.llvmPackages.stdenv.cc | jq -r 'keys[0]') (nix derivation show .#legacyPackages.aarch64-darwin.llvmPackages.stdenv.cc | jq -r 'keys[0]')

with clang not inherited from prevStage here llvmPackages.clang, casadi etc are different:

nix-diff (nix derivation show github:nixos/nixpkgs/staging-next#legacyPackages.aarch64-darwin.llvmPackages.clang | jq -r 'keys[0]') (nix derivation show .#legacyPackages.aarch64-darwin.llvmPackages.clang | jq -r 'keys[0]')

See https://github.com/NixOS/nixpkgs/pull/445668/files#r2450184270, but that appears to eliminate rebuilds on Darwin.

With that applied we're back to php* rebuilds. :c

That’s really strange because nix-diff showed no differences for me. 😞

I think I have a real solution. This results in no differences for php and for casadi.

diff --git a/pkgs/stdenv/darwin/default.nix b/pkgs/stdenv/darwin/default.nix
index dc8ac58328..f7c5262acc 100644
--- a/pkgs/stdenv/darwin/default.nix
+++ b/pkgs/stdenv/darwin/default.nix
@@ -1245,9 +1245,6 @@
                   // {
                     inherit (super."llvmPackages_${llvmVersion}") llvm-manpages;
                   }
-                  // lib.optionalAttrs (super.stdenv.targetPlatform == localSystem) {
-                    inherit (prevStage.llvmPackages) systemLibcxxClang;
-                  }
                 ))
                 // {
                   inherit (super."llvmPackages_${llvmVersion}") override;
@@ -1284,7 +1281,24 @@
     assert prevStage.libiconv == prevStage.darwin.libiconv;
 
     {
-      inherit (prevStage) config overlays stdenv;
+      inherit (prevStage) config overlays;
+      # This should be done in the `overrideScope` above, but it causes rebuilds.
+      # TODO: Move it there once https://github.com/NixOS/nixpkgs/pull/445668 is merged.
+      stdenv = prevStage.stdenv // {
+        overrides =
+          self: super:
+          (prevStage.stdenv.overrides self super)
+          // lib.optionalAttrs (super.stdenv.targetPlatform == localSystem) (
+            let
+              llvmVersion = lib.versions.major prevStage.llvmPackages.release_version;
+            in
+            {
+              "llvmPackages_${llvmVersion}" = prevStage."llvmPackages_${llvmVersion}" // {
+                inherit (prevStage) clang;
+              };
+            }
+          );
+      };
     }
   )
 ]

Running nixpkgs-review with this patch results in only one rebuild: flang_21.

Yay! Down to zero rebuilds now.

stdenv mess to get it there was not fun, thanks for helping out.

This wasn’t here previously; is it related to rebuild avoidance?

Newly introduced, to me it made sense to include with introducing overrideScope usage here as it doesn't cause a rebuild. Without overrideScope there wasn't a way to enforce this.

I think I lost track of what the plan is with these. This will mean that llvmPackages.override { } will produce super.llvmPackages, without the overrides from this stage. That may be required for rebuild avoidance, of course, but do we know what we’re going to do for eating the rebuilds? It seems quite important for the final llvmPackages.override after the bootstrap to behave correctly, and to correctly override the overridden scope.

I guess this is an existing problem – clang != (llvmPackages.override { }).stdenv on Darwin right now, which is quite distressing. But we really ought to fix that, so I’m just checking if we know how we’re going to. @ShamrockLee’s proposal for a general solution to composable overrides seems sound, and I’ve spent an inordinate amount of time thinking about how this stuff should look lately myself. But we should have a short‐term solution for llvmPackages at least, when we’re switching over how that scope is assembled.

I think that llvmPackages = callPackage (lib.mirrorFunctionArgs llvmPackages.override (args: (llvmPackages.override args).overrideScope …)); might work, at the expense of being quite horrendous – although AIUI @Artturin says you should always use callPackage in overlays anyway for splicing reasons, so maybe it would actually be good?? I suppose this is analogous to the way overrideResult works, which would more directly be llvmPackages = lib.makeOverridable (lib.mirrorFunctionArgs llvmPackages.override (args: (llvmPackages.override args).overrideScope …)). I frankly have no clue which one would be better and hope one of the people I just pinged has a better idea :)

We could abstract this into an overrideLlvmPackages function in this file. This is certainly a problem we need to solve more generally, for overriding LLVM to work well, let alone many other cases where our overrides system falls down. Motivation to keep working on my related grant plans…

I feel okay about this plan for now.

although AIUI @Artturin says you should always use callPackage in overlays anyway for splicing reasons, so maybe it would actually be good??

Is there discussion elsewhere about this to follow?

I forget where I read @Artturin saying this, I’m afraid. But it’s because of final.callPackage having access to the spliced packages but final not.