From c66b12ff90768587735a54e94e0d448558cf5046 Mon Sep 17 00:00:00 2001
From: DavHau <hsngrmpf+github@gmail.com>
Date: Thu, 29 May 2025 13:39:50 +0700
Subject: [PATCH] make-symlinks-relative.sh: optimize - use multiple cores

This optimizes the performance of the hook on machines with multiple cores.

The previous implementation was slow, as it was launching two extra processes per symlink (readlink and ln) while not making use of multiprocessing.

The following improvements were made:
- don't even execute the hook if dontRewriteSymlinks is set
- replace multiple find calls with single find call over all outputs
- use xargs -P to process symlinks in parallel
- add test to check that the hook operates as expected
---
 .../setup-hooks/make-symlinks-relative.sh     | 52 ++++++-----
 pkgs/test/stdenv/default.nix                  |  7 +-
 pkgs/test/stdenv/hooks.nix                    | 90 ++++++++++++-------
 3 files changed, 90 insertions(+), 59 deletions(-)

diff --git a/pkgs/build-support/setup-hooks/make-symlinks-relative.sh b/pkgs/build-support/setup-hooks/make-symlinks-relative.sh
index b07b0c5ae804c..478543afed115 100644
--- a/pkgs/build-support/setup-hooks/make-symlinks-relative.sh
+++ b/pkgs/build-support/setup-hooks/make-symlinks-relative.sh
@@ -1,37 +1,35 @@
 # symlinks are often created in postFixup
 # don't use fixupOutputHooks, it is before postFixup
-postFixupHooks+=(_makeSymlinksRelativeInAllOutputs)
+if [[ -z "${dontRewriteSymlinks-}" ]]; then
+  postFixupHooks+=(_makeSymlinksRelative)
+fi
+
 
 # For every symlink in $output that refers to another file in $output
-# ensure that the symlink is relative. This removes references to the output
-# has from the resulting store paths and thus the NAR files.
+# ensure that the symlink is relative.
+# This increases the chance that NAR files can be deduplicated.
 _makeSymlinksRelative() {
-    local symlinkTarget
-
-    if [ "${dontRewriteSymlinks-}" ] || [ ! -e "$prefix" ]; then
-       return
-    fi
-
-    while IFS= read -r -d $'\0' f; do
-        symlinkTarget=$(readlink "$f")
-        if [[ "$symlinkTarget"/ != "$prefix"/* ]]; then
-            # skip this symlink as it doesn't point to $prefix
-            continue
-        fi
+  local prefixes
+  prefixes=()
+  for output in $(getAllOutputNames); do
+    [ ! -e "${!output}" ] && continue
+    prefixes+=( "${!output}" )
+  done
+  find "${prefixes[@]}" -type l -printf '%H\0%p\0' \
+    | xargs -0 -n2 -r -P "$NIX_BUILD_CORES" sh -c '
+      output="$1"
+      link="$2"
 
-        if [ ! -e "$symlinkTarget" ]; then
-            echo "the symlink $f is broken, it points to $symlinkTarget (which is missing)"
-        fi
+      linkTarget=$(readlink "$link")
 
-        echo "rewriting symlink $f to be relative to $prefix"
-        ln -snrf "$symlinkTarget" "$f"
+      # only touch links that point inside the same output tree
+      [[ $linkTarget == "$output"/* ]] || exit 0
 
-    done < <(find $prefix -type l -print0)
-}
+      if [ ! -e "$linkTarget" ]; then
+        echo "the symlink $link is broken, it points to $linkTarget (which is missing)"
+      fi
 
-_makeSymlinksRelativeInAllOutputs() {
-  local output
-  for output in $(getAllOutputNames); do
-    prefix="${!output}" _makeSymlinksRelative
-  done
+      echo "making symlink relative: $link"
+      ln -snrf "$linkTarget" "$link"
+    ' _
 }
diff --git a/pkgs/test/stdenv/default.nix b/pkgs/test/stdenv/default.nix
index cf09258d6fbc0..ecc7bbaadac05 100644
--- a/pkgs/test/stdenv/default.nix
+++ b/pkgs/test/stdenv/default.nix
@@ -9,6 +9,9 @@
 }:
 
 let
+  # tests can be based on builtins.derivation and bootstrapTools directly to minimize rebuilds
+  # see test 'make-symlinks-relative' in ./hooks.nix as an example.
+  bootstrapTools = stdenv.bootstrapTools;
   # early enough not to rebuild gcc but late enough to have patchelf
   earlyPkgs = stdenv.__bootPackages.stdenv.__bootPackages;
   earlierPkgs =
@@ -223,7 +226,7 @@ in
     import ./hooks.nix {
       stdenv = bootStdenv;
       pkgs = earlyPkgs;
-      inherit lib;
+      inherit bootstrapTools lib;
     }
   );
 
@@ -433,7 +436,7 @@ in
       import ./hooks.nix {
         stdenv = bootStdenvStructuredAttrsByDefault;
         pkgs = earlyPkgs;
-        inherit lib;
+        inherit bootstrapTools lib;
       }
     );
 
diff --git a/pkgs/test/stdenv/hooks.nix b/pkgs/test/stdenv/hooks.nix
index aa138040d50fe..70daaf5d403e5 100644
--- a/pkgs/test/stdenv/hooks.nix
+++ b/pkgs/test/stdenv/hooks.nix
@@ -1,4 +1,5 @@
 {
+  bootstrapTools,
   stdenv,
   pkgs,
   lib,
@@ -28,36 +29,65 @@
         [[ -e $out/share/man/small-man.1.gz ]]
       '';
     };
-  make-symlinks-relative = stdenv.mkDerivation {
-    name = "test-make-symlinks-relative";
-    outputs = [
-      "out"
-      "man"
-    ];
-    buildCommand = ''
-      mkdir -p $out/{bar,baz}
-      mkdir -p $man/share/{x,y}
-      source1="$out/bar/foo"
-      destination1="$out/baz/foo"
-      source2="$man/share/x/file1"
-      destination2="$man/share/y/file2"
-      echo foo > $source1
-      echo foo > $source2
-      ln -s $source1 $destination1
-      ln -s $source2 $destination2
-      echo "symlink before patching: $(readlink $destination1)"
-      echo "symlink before patching: $(readlink $destination2)"
-
-      _makeSymlinksRelativeInAllOutputs
-
-      echo "symlink after patching: $(readlink $destination1)"
-      ([[ -e $destination1 ]] && echo "symlink isn't broken") || (echo "symlink is broken" && exit 1)
-      ([[ $(readlink $destination1) == "../bar/foo" ]] && echo "absolute symlink was made relative") || (echo "symlink was not made relative" && exit 1)
-      echo "symlink after patching: $(readlink $destination2)"
-      ([[ -e $destination2 ]] && echo "symlink isn't broken") || (echo "symlink is broken" && exit 1)
-      ([[ $(readlink $destination2) == "../x/file1" ]] && echo "absolute symlink was made relative") || (echo "symlink was not made relative" && exit 1)
-    '';
-  };
+  # test based on bootstrapTools to minimize rebuilds
+  make-symlinks-relative =
+    (derivation {
+      name = "test-make-symlinks-relative";
+      system = stdenv.system;
+      builder = "${bootstrapTools}/bin/bash";
+      initialPath = "${bootstrapTools}";
+      outputs = [
+        "out"
+        "out2"
+      ];
+      args = [
+        "-c"
+        ''
+          set -euo pipefail
+          . ${../../stdenv/generic/setup.sh}
+          . ${../../build-support/setup-hooks/make-symlinks-relative.sh}
+
+          mkdir -p $out $out2
+
+          # create symlink targets
+          touch $out/target $out2/target
+
+          # link within out
+          ln -s $out/target $out/linkToOut
+
+          # link across different outputs
+          ln -s $out2/target $out/linkToOut2
+
+          # broken link
+          ln -s $out/does-not-exist $out/brokenLink
+
+          # call hook
+          _makeSymlinksRelative
+
+          # verify link within out became relative
+          echo "readlink linkToOut: $(readlink $out/linkToOut)"
+          if test "$(readlink $out/linkToOut)" != 'target'; then
+            echo "Expected relative link, got: $(readlink $out/linkToOut)"
+            exit 1
+          fi
+
+          # verify link across outputs is still absolute
+          if test "$(readlink $out/linkToOut2)" != "$out2/target"; then
+            echo "Expected absolute link, got: $(readlink $out/linkToOut2)"
+            exit 1
+          fi
+
+          # verify broken link was made relative
+          if test "$(readlink $out/brokenLink)" != 'does-not-exist'; then
+            echo "Expected relative broken link, got: $(readlink $out/brokenLink)"
+            exit 1
+          fi
+        ''
+      ];
+    })
+    // {
+      meta = { };
+    };
   move-docs = stdenv.mkDerivation {
     name = "test-move-docs";
     buildCommand = ''