Repository: TraceMachina/nativelink (language: rust)

Issue:
Broken nix flake on `aarch64-darwin`
### **Description**

I'm encountering issues when running ```nix flake show --allow-import-from-derivation``` on MacOS (aarch64-darwin). The command results in an error that halts the process. It appears that there are issues in building the nixpkgs-patched with the wrong architecture.

### **Warnings & Errors**

```bash
error:
       … while evaluating the attribute 'optionalValue.value'

         at /nix/store/l04ifn08y80fcd3iblpzfr0fj0pqc58z-source/modules.nix:856:5:

          855|
          856|     optionalValue =
             |     ^
          857|       if isDefined then { value = mergedValue; }

       … while evaluating a branch condition

         at /nix/store/l04ifn08y80fcd3iblpzfr0fj0pqc58z-source/modules.nix:857:7:

          856|     optionalValue =
          857|       if isDefined then { value = mergedValue; }
             |       ^
          858|       else {};

       (stack trace truncated; use '--show-trace' to show the full trace)

       error: a 'aarch64-linux' with features {} is required to build '/nix/store/jrhk2sdi1dij7hka434irlb1rnw4xv51-nixpkgs-patched.drv', but I am a 'aarch64-darwin' with features {apple-virt, benchmark, big-parallel, nixos-test}
```
### **Environment**

Operating System: MacOS
Architecture: aarch64-darwin
Nix Version: nix (Nix) 2.19.2
Nativelink Version: 0.5.1

### **Steps to Reproduce**
Run ```nix flake show --allow-import-from-derivation``` on MacOS with aarch64-darwin architecture.
Observe the warnings and errors during the evaluation.

### **Expected Behavior**
The ```nix flake show --allow-import-from-derivation``` command should complete successfully or provide actionable feedback without encountering fatal errors.
Migrate patches to overlays
Patching nixpkgs the way we currently do causes [IFDs](https://nix.dev/manual/nix/2.25/language/import-from-derivation), meaning that we need to realize nixpkgs before we can access store paths. When we now want to access packages from other target systems our evaluation breaks since we can't realize packages for non-native systems.

This has multiple implications:

- `nix flake show` is broken since we can't evaluate the cross-compiled variants of nativelink (#1262)
- Evaluations likely take much longer than they should since they need to evaluate nixpkgs twice. This might contribute to the slowness observed in #1485.
- If the above holds, it also likely means that we waste quite a bit of resources in most of our nix-driven CI workflows.
- It's currently impossible to implement sensible crosscompilation toolchains since we can't evaluate store paths for other systems. This prevents us from implementing cross-compilation LRE toolchains and makes it much harder to implement multi-arch images for nativelink.

The naive solution to this issue would be to run a custom fork of nixpkgs with our patches applied ahead of time. This isn't a very maintainable solution though as it would encourage drift from upstream and complicate the setups for projects importing the native-cli or LRE.

The more idomatic solution is to migrate the patches to actual overlays so that we can remove the patch logic entirely. This also makes the overlays more portable and robust to future nixpkgs version bumps.

The TODOs for this issue are the migration of the following patches:

- [x] https://github.com/TraceMachina/nativelink/blob/main/tools/nixpkgs_disable_ratehammering_pulumi_tests.diff (#1513)
- [x] https://github.com/TraceMachina/nativelink/blob/main/tools/rust-overlay_cut_libsecret.diff (#1514)
- [x] https://github.com/TraceMachina/nativelink/blob/main/tools/nixpkgs_link_libunwind_and_libcxx.diff (#1516)

Upstream is aware of these issues and improvments to the UX for naive patch application is in progress at https://github.com/NixOS/nix/pull/6530. However, at the moment we can't predict how long it'll take until we can reasonably use it.


Discussion / hints:
I can reproduce on `x86_64-linux`. This is caused by the fact that it's not possible to crosscompile for darwin, not even between `aarch64-darwin` and `x86_64-darwin`. I thought I cut these paths entirely in #1233 but apparently I overlooked something.

The solution to this is to remove the darwin flake outputs from the `packages` section on systems that don't support them. I.e.

- remove both if the host is linux
- remove `x86_64-darwin` if the host is `aarch64-darwin`
- remove `aarch64-darwin` if the host is `x86_64-darwin`

@SchahinRohani Do you want to try solving this issue?
Thanks for the feedback on the flake error. I couldn't manage the quick fix due to the high complexity involved, so I decided to redesign the basic structure and work with Nix scripts. This led me to the following proposal:

<img width="452" alt="Screenshot 2024-08-20 at 15 51 45" src="https://github.com/user-attachments/assets/8d4b8b62-bace-46f4-ae84-5a5d7a98b29c">


Setting up a multi-shell environment consisting of three shells: baseShell, nativeShell (for Rust developers), and webShell (for web projects like the docs). The baseShell would include only the essentials like linting and formatting tools, while the nativeShell would provide the full Rust development environment, including the necessary toolchains and bazel. Similarly, the webShell would include system toolchains required for things like Playwright. But the most important thing is to always modularize all scripts to an external place so that the flake.nix remains clean and only organizes the outputs.

However, given the complexity involved, I believe this is something we should definitely tackle together as a team. There's a lot going on here, and having everyone's input will be crucial to get this right.

Additionally, the CI would benefit from this setup, as it could be configured to run with only the necessary packages for each task, leading to more efficient and faster builds.

@aaronmondal  Let me know what you think!
I can see that initially it might seem a bit curious why all our CI jobs take so long to "boot up" and it's tempting to try to reduce the size of the devshell to make some jobs quicker. However, there is actually not too much to gain here.

> The baseShell would include only the essentials like linting and formatting tools

The pre-commit hooks already skip the devshell entirely since they use `nix flake check`. So for the pre-commit hooks the workflow is already optimal and does what a "baseShell" would do. Hence the ~1 min runtime for pre-commit hooks in CI on cache hits.

> the nativeShell would provide the full Rust development environment, including the necessary toolchains and bazel. Similarly, the webShell would include system toolchains required for things like Playwright.

Compared to the "nativeShell" parts, the "webShell" is almost the same in size since both run a Bazel build in the same stdenv (which makes up the majority of the devshell size). Compared to that, the additional space requirements from webdev-specific tools (playwright being the large one here) are negligible.

You also wouldn't get too much of a speedup since setting up the devshell in CI is acutally fairly fast. In this job it takes about 6 minutes: https://github.com/TraceMachina/nativelink/actions/runs/10424002033/job/28871873664#step:6:1678. Of that, 2.5 minutes are spent on building the `native-cli` due to a cache miss and 1 minute for playwright, also due to a cache miss. If we ignore those two cache misses we're at ~2.5 min startup time. Optimizing here doesn't seem too useful.

Maintenance-wise it's also a nice property to have to only check a single devshell. If we had multiple devshells we'd have to test each one. Since Nix's "core" dependencies like gcc, glibc, etc are shared, we'd end up with an overall net increase of CPU cycles instead of a reduction. To elegantly handle multiple devshells we'd likely have to use multiple envrcs. But then it becomes unclear what happens when you're in one devshell and need to invoke a tool that might behave differentlly in another devshell.

In general, when improving CI performance it should be an "efficient" improvement. That is, a speedup of 2x isn't a "real" speedup if it requires 2x the compute resources. If however we could get a 10% CPU cycle reduction in CI that would be a fantastic gain. The tricky thing here to keep in mind is that parallelizing certain jobs is only useful when we don't regress in coverage. So initially it might seem like multiple devshells speed things up, but because of the added CI jobs required to cover all usecases we'd end up with a net loss of efficiency.
Removing "good first issue" label for now as this might be a potentially highly complex thing to fix.


Fix patch:
diff --git a/flake.nix b/flake.nix
index 6294c2352..2738191ff 100644
--- a/flake.nix
+++ b/flake.nix
@@ -57,12 +57,6 @@
 
         llvmPackages = pkgs.llvmPackages_19;
 
-        customStdenv = pkgs.callPackage ./tools/llvmStdenv.nix {inherit llvmPackages;};
-
-        # TODO(aaronmondal): This doesn't work with rules_rust yet.
-        # Tracked in https://github.com/TraceMachina/nativelink/issues/477.
-        customClang = pkgs.callPackage ./tools/customClang.nix {stdenv = customStdenv;};
-
         nixSystemToRustTriple = nixSystem:
           {
             "x86_64-linux" = "x86_64-unknown-linux-musl";
@@ -251,7 +245,7 @@
 
         rbe-autogen = pkgs.callPackage ./local-remote-execution/rbe-autogen.nix {
           inherit buildImage;
-          stdenv = customStdenv;
+          inherit (pkgs.lre) stdenv;
         };
         createWorker = pkgs.callPackage ./tools/create-worker.nix {inherit buildImage self;};
         buck2-toolchain = let
@@ -292,8 +286,7 @@
           };
         };
         lre-cc = pkgs.callPackage ./local-remote-execution/lre-cc.nix {
-          inherit customClang buildImage;
-          stdenv = customStdenv;
+          inherit buildImage;
         };
         toolchain-drake = buildImage {
           name = "toolchain-drake";
@@ -348,23 +341,15 @@
 
         nativelinkCoverageForHost = nativelinkCoverageFor pkgs;
       in rec {
-        _module.args.pkgs = let
-          nixpkgs-patched = (import self.inputs.nixpkgs {inherit system;}).applyPatches {
-            name = "nixpkgs-patched";
-            src = self.inputs.nixpkgs;
-            patches = [
-              ./tools/nixpkgs_link_libunwind_and_libcxx.diff
-            ];
-          };
-        in
-          import nixpkgs-patched {
-            inherit system;
-            overlays = [
-              (import ./tools/nixpkgs-disable-ratehammering-pulumi-tests.nix)
-              (import rust-overlay)
-              (import ./tools/rust-overlay-cut-libsecret.nix)
-            ];
-          };
+        _module.args.pkgs = import self.inputs.nixpkgs {
+          inherit system;
+          overlays = [
+            self.overlays.lre
+            (import ./tools/nixpkgs-disable-ratehammering-pulumi-tests.nix)
+            (import rust-overlay)
+            (import ./tools/rust-overlay-cut-libsecret.nix)
+          ];
+        };
         apps = {
           default = {
             type = "app";
@@ -446,7 +431,7 @@
             "/run/current-system/sw/bin"
             "${binutils.bintools}/bin"
             "${uutils-coreutils-noprefix}/bin"
-            "${customClang}/bin"
+            "${pkgs.lre.clang}/bin"
             "${git}/bin"
             "${python3}/bin"
           ];
@@ -497,7 +482,7 @@
               # Additional tools from within our development environment.
               local-image-test
               generate-toolchains
-              customClang
+              pkgs.lre.clang
               native-cli
               docs
               build-chromium-tests
@@ -560,5 +545,8 @@
         local-remote-execution = ./local-remote-execution/flake-module.nix;
         nixos = ./tools/nixos/flake-module.nix;
       };
+      overlays = {
+        lre = import ./local-remote-execution/overlays/default.nix;
+      };
     };
 }
diff --git a/local-remote-execution/lre-cc.nix b/local-remote-execution/lre-cc.nix
index 3e09fe1f7..a74ed385b 100644
--- a/local-remote-execution/lre-cc.nix
+++ b/local-remote-execution/lre-cc.nix
@@ -1,7 +1,6 @@
 {
   buildImage,
-  customClang,
-  stdenv,
+  lre,
   lib,
   coreutils,
   findutils,
@@ -15,15 +14,15 @@
     # binary identical toolchains during local and remote execution.
     ("PATH="
       + (lib.strings.concatStringsSep ":" [
-        "${stdenv.cc.bintools}/bin"
-        "${customClang}/bin"
-        "${stdenv}/bin"
+        "${lre.stdenv.cc.bintools}/bin"
+        "${lre.clang}/bin"
+        "${lre.stdenv}/bin"
         "${coreutils}/bin"
         "${findutils}/bin"
         "${gnutar}/bin"
       ]))
 
-    "CC=${customClang}/bin/customClang"
+    "CC=${lre.clang}/bin/customClang"
   ];
 in
   buildImage {
diff --git a/tools/customClang.nix b/local-remote-execution/overlays/clang.nix
similarity index 100%
rename from tools/customClang.nix
rename to local-remote-execution/overlays/clang.nix
diff --git a/local-remote-execution/overlays/default.nix b/local-remote-execution/overlays/default.nix
new file mode 100644
index 000000000..3595a96fd
--- /dev/null
+++ b/local-remote-execution/overlays/default.nix
@@ -0,0 +1,12 @@
+final: _prev: {
+  lre = {
+    stdenv = final.callPackage ./stdenv.nix {
+      llvmPackages = final.llvmPackages_19;
+      targetPackages = final;
+    };
+
+    clang = final.callPackage ./clang.nix {
+      inherit (final.lre) stdenv;
+    };
+  };
+}
diff --git a/local-remote-execution/overlays/stdenv.nix b/local-remote-execution/overlays/stdenv.nix
new file mode 100644
index 000000000..918d0640c
--- /dev/null
+++ b/local-remote-execution/overlays/stdenv.nix
@@ -0,0 +1,108 @@
+{
+  lib,
+  llvmPackages,
+  overrideCC,
+  stdenv,
+  targetPackages,
+  useMoldLinker,
+  wrapCCWith,
+}: let
+  # Adapted from clangUseLLVM.
+  # See: https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/compilers/llvm/common/default.nix
+  clangVersion = "19";
+  targetLlvmLibraries = targetPackages.llvmPackages_19.libraries;
+
+  mkExtraBuildCommands0 = cc: ''
+    rsrc="$out/resource-root"
+    mkdir "$rsrc"
+    ln -s "${lib.getLib cc}/lib/clang/${clangVersion}/include" "$rsrc"
+    echo "-resource-dir=$rsrc" >> $out/nix-support/cc-cflags
+  '';
+  mkExtraBuildCommands = cc:
+    mkExtraBuildCommands0 cc
+    + ''
+      ln -s "${targetLlvmLibraries.compiler-rt.out}/lib" "$rsrc/lib"
+      ln -s "${targetLlvmLibraries.compiler-rt.out}/share" "$rsrc/share"
+    '';
+
+  toolchain' = wrapCCWith (
+    rec {
+      cc = llvmPackages.clang-unwrapped;
+      inherit (targetLlvmLibraries) libcxx;
+      inherit (llvmPackages) bintools;
+      extraPackages = [
+        targetLlvmLibraries.compiler-rt
+        targetLlvmLibraries.libunwind
+      ];
+      extraBuildCommands = mkExtraBuildCommands cc;
+    }
+    // {
+      nixSupport.cc-cflags =
+        [
+          "-rtlib=compiler-rt"
+          "-Wno-unused-command-line-argument"
+          "-B${targetLlvmLibraries.compiler-rt}/lib"
+          "-stdlib=libc++"
+        ]
+        ++ lib.optional stdenv.targetPlatform.isLinux "-fuse-ld=mold"
+        ++ lib.optional (
+          !stdenv.targetPlatform.isWasm && !stdenv.targetPlatform.isFreeBSD
+        ) "--unwindlib=libunwind"
+        ++ lib.optional (
+          !stdenv.targetPlatform.isWasm
+          && !stdenv.targetPlatform.isFreeBSD
+          && stdenv.targetPlatform.useLLVM or false
+        ) ["-lunwind" "-lc++"]
+        ++ lib.optional stdenv.targetPlatform.isWasm "-fno-exceptions";
+      nixSupport.cc-ldflags =
+        lib.optionals (
+          !stdenv.targetPlatform.isWasm && !stdenv.targetPlatform.isFreeBSD
+        ) [
+          "-L${targetLlvmLibraries.libunwind}/lib"
+          "-rpath"
+          "${targetLlvmLibraries.libunwind}/lib"
+          "-L${targetLlvmLibraries.libcxx}/lib"
+          "-rpath"
+          "${targetLlvmLibraries.libcxx}/lib"
+        ];
+    }
+  );
+
+  stdenv' =
+    overrideCC
+    llvmPackages.libcxxStdenv
+    toolchain';
+
+  toolchain =
+    if stdenv.targetPlatform.isDarwin
+    then stdenv' # Mold doesn't support darwin.
+    else useMoldLinker stdenv';
+in
+  # This toolchain uses Clang as compiler, Mold as linker, libc++ as C++
+  # standard library and compiler-rt as compiler runtime. Resulting rust
+  # binaries depend dynamically linked on the nixpkgs distribution of glibc.
+  # C++ binaries additionally depend dynamically on libc++, libunwind and
+  # libcompiler-rt. Due to a bug we also depend on libgcc_s.
+  #
+  # TODO(aaronmondal): At the moment this toolchain is only used for the Cargo
+  # build. The Bazel build uses a different mostly hermetic LLVM toolchain. We
+  # should merge the two by generating the Bazel cc_toolchain from this stdenv.
+  # This likely requires a rewrite of
+  # https://github.com/bazelbuild/bazel-toolchains as the current implementation
+  # has poor compatibility with custom container images and doesn't support
+  # generating toolchain configs from image archives.
+  #
+  # TODO(aaronmondal): Due to various issues in the nixpkgs LLVM toolchains
+  # we're not getting a pure Clang/LLVM toolchain here. My guess is that the
+  # runtimes were not built with the degenerate LLVM toolchain but with the
+  # regular GCC stdenv from nixpkgs.
+  #
+  # For instance, outputs depend on libgcc_s since libcxx seems to have been was
+  # built with a GCC toolchain. We're also not using builtin atomics, or at
+  # least we're redundantly linking libatomic.
+  #
+  # Fix this as it fixes a large number of issues, including better
+  # cross-platform compatibility, reduced closure size, and
+  # static-linking-friendly licensing. This requires building the llvm project
+  # with the correct multistage bootstrapping process.
+  toolchain
diff --git a/tools/llvmStdenv.nix b/tools/llvmStdenv.nix
deleted file mode 100644
index 963ee827f..000000000
--- a/tools/llvmStdenv.nix
+++ /dev/null
@@ -1,49 +0,0 @@
-{
-  stdenv,
-  overrideCC,
-  useMoldLinker,
-  llvmPackages,
-}: let
-  llvmPackagesNoBintools = llvmPackages.override {
-    bootBintools = null;
-    bootBintoolsNoLibc = null;
-  };
-
-  stdenv' =
-    overrideCC
-    llvmPackagesNoBintools.libcxxStdenv
-    llvmPackagesNoBintools.clangUseLLVM;
-
-  toolchain =
-    if stdenv.targetPlatform.isDarwin
-    then stdenv' # Mold doesn't support darwin.
-    else useMoldLinker stdenv';
-in
-  # This toolchain uses Clang as compiler, Mold as linker, libc++ as C++
-  # standard library and compiler-rt as compiler runtime. Resulting rust
-  # binaries depend dynamically linked on the nixpkgs distribution of glibc.
-  # C++ binaries additionally depend dynamically on libc++, libunwind and
-  # libcompiler-rt. Due to a bug we also depend on libgcc_s.
-  #
-  # TODO(aaronmondal): At the moment this toolchain is only used for the Cargo
-  # build. The Bazel build uses a different mostly hermetic LLVM toolchain. We
-  # should merge the two by generating the Bazel cc_toolchain from this stdenv.
-  # This likely requires a rewrite of
-  # https://github.com/bazelbuild/bazel-toolchains as the current implementation
-  # has poor compatibility with custom container images and doesn't support
-  # generating toolchain configs from image archives.
-  #
-  # TODO(aaronmondal): Due to various issues in the nixpkgs LLVM toolchains
-  # we're not getting a pure Clang/LLVM toolchain here. My guess is that the
-  # runtimes were not built with the degenerate LLVM toolchain but with the
-  # regular GCC stdenv from nixpkgs.
-  #
-  # For instance, outputs depend on libgcc_s since libcxx seems to have been was
-  # built with a GCC toolchain. We're also not using builtin atomics, or at
-  # least we're redundantly linking libatomic.
-  #
-  # Fix this as it fixes a large number of issues, including better
-  # cross-platform compatibility, reduced closure size, and
-  # static-linking-friendly licensing. This requires building the llvm project
-  # with the correct multistage bootstrapping process.
-  toolchain
diff --git a/tools/nixpkgs_link_libunwind_and_libcxx.diff b/tools/nixpkgs_link_libunwind_and_libcxx.diff
deleted file mode 100644
index 4ce44b416..000000000
--- a/tools/nixpkgs_link_libunwind_and_libcxx.diff
+++ /dev/null
@@ -1,32 +0,0 @@
-diff --git a/pkgs/development/compilers/llvm/common/default.nix b/pkgs/development/compilers/llvm/common/default.nix
-index 707bb91b875d..d4b4fb2b5427 100644
---- a/pkgs/development/compilers/llvm/common/default.nix
-+++ b/pkgs/development/compilers/llvm/common/default.nix
-@@ -780,7 +780,9 @@ let
-               "-rtlib=compiler-rt"
-               "-Wno-unused-command-line-argument"
-               "-B${targetLlvmLibraries.compiler-rt}/lib"
-+              "-stdlib=libc++"
-             ]
-+            ++ lib.optional stdenv.targetPlatform.isLinux "-fuse-ld=mold"
-             ++ lib.optional (
-               !stdenv.targetPlatform.isWasm && !stdenv.targetPlatform.isFreeBSD
-             ) "--unwindlib=libunwind"
-@@ -788,11 +790,16 @@ let
-               !stdenv.targetPlatform.isWasm
-               && !stdenv.targetPlatform.isFreeBSD
-               && stdenv.targetPlatform.useLLVM or false
--            ) "-lunwind"
-+            ) ["-lunwind" "-lc++"]
-             ++ lib.optional stdenv.targetPlatform.isWasm "-fno-exceptions";
-           nixSupport.cc-ldflags = lib.optionals (
-             !stdenv.targetPlatform.isWasm && !stdenv.targetPlatform.isFreeBSD
--          ) [ "-L${targetLlvmLibraries.libunwind}/lib" ];
-+          ) [
-+              "-L${targetLlvmLibraries.libunwind}/lib"
-+              "-rpath" "${targetLlvmLibraries.libunwind}/lib"
-+              "-L${targetLlvmLibraries.libcxx}/lib"
-+              "-rpath" "${targetLlvmLibraries.libcxx}/lib"
-+            ];
-         }
-       );


Test patch: