From 51b2764e9fcd4ef768de7590ce796d619ef82cd2 Mon Sep 17 00:00:00 2001 From: Connor Baker Date: Wed, 22 Jan 2025 11:05:13 -0800 Subject: [PATCH] no-broken-symlinks: provide only dontCheckForBrokenSymlinks and test against absolute symlinks --- doc/stdenv/stdenv.chapter.md | 6 +- .../setup-hooks/no-broken-symlinks.sh | 28 +--- pkgs/test/stdenv/no-broken-symlinks.nix | 142 +++++++++++++----- 3 files changed, 115 insertions(+), 61 deletions(-) diff --git a/doc/stdenv/stdenv.chapter.md b/doc/stdenv/stdenv.chapter.md index dedeb8d45c92d..868a7543d9ad3 100644 --- a/doc/stdenv/stdenv.chapter.md +++ b/doc/stdenv/stdenv.chapter.md @@ -1375,9 +1375,11 @@ This hook only runs when compiling for Linux. This setup hook checks for, reports, and (by default) fails builds when "broken" symlinks are found. A symlink is considered "broken" if it's dangling (the target doesn't exist) or reflexive (it refers to itself). -By setting `allowDanglingSymlinks` or `allowReflexiveSymlinks` the hook can be configured to allow symlinks with non-existent targets or symlinks which are reflexive, respectively. +This hook can be disabled by setting `dontCheckForBrokenSymlinks`. -This hook can be disabled entirely by setting `dontCheckForBrokenSymlinks`. +::: {.note} +The check for reflexivity is direct and does not account for transitivity, so this hook will not prevent cycles in symlinks. +::: ### `set-source-date-epoch-to-latest.sh` {#set-source-date-epoch-to-latest.sh} diff --git a/pkgs/build-support/setup-hooks/no-broken-symlinks.sh b/pkgs/build-support/setup-hooks/no-broken-symlinks.sh index 6711e0deace4b..d622322a2747f 100644 --- a/pkgs/build-support/setup-hooks/no-broken-symlinks.sh +++ b/pkgs/build-support/setup-hooks/no-broken-symlinks.sh @@ -19,11 +19,10 @@ noBrokenSymlinks() { local path local pathParent local symlinkTarget - local errorMessage local -i numDanglingSymlinks=0 local -i numReflexiveSymlinks=0 - # TODO(@connorbaker): This hook doesn't check for cycles in symlinks. + # NOTE(@connorbaker): This hook doesn't check for cycles in symlinks. if [[ ! -e $output ]]; then nixWarnLog "skipping non-existent output $output" @@ -52,28 +51,13 @@ noBrokenSymlinks() { fi if [[ $path == "$symlinkTarget" ]]; then - # symlinkTarget is reflexive - errorMessage="the symlink $path is reflexive $symlinkTarget" - if [[ -z ${allowReflexiveSymlinks-} ]]; then - nixErrorLog "$errorMessage" - numReflexiveSymlinks+=1 - else - nixInfoLog "$errorMessage" - fi - + nixErrorLog "the symlink $path is reflexive $symlinkTarget" + numReflexiveSymlinks+=1 elif [[ ! -e $symlinkTarget ]]; then - # symlinkTarget does not exist - errorMessage="the symlink $path points to a missing target $symlinkTarget" - if [[ -z ${allowDanglingSymlinks-} ]]; then - nixErrorLog "$errorMessage" - numDanglingSymlinks+=1 - else - nixInfoLog "$errorMessage" - fi - + nixErrorLog "the symlink $path points to a missing target $symlinkTarget" + numDanglingSymlinks+=1 else - # symlinkTarget exists and is irreflexive - nixInfoLog "the symlink $path is irreflexive and points to a target which exists" + nixDebugLog "the symlink $path is irreflexive and points to a target which exists" fi done < <(find "$output" -type l -print0) diff --git a/pkgs/test/stdenv/no-broken-symlinks.nix b/pkgs/test/stdenv/no-broken-symlinks.nix index 85963542a04a3..0eb0ef0f982e6 100644 --- a/pkgs/test/stdenv/no-broken-symlinks.nix +++ b/pkgs/test/stdenv/no-broken-symlinks.nix @@ -9,17 +9,17 @@ let inherit (pkgs) runCommand; inherit (pkgs.testers) testBuildFailure; - mkDanglingSymlink = '' - ln -sr "$out/dangling" "$out/dangling-symlink" + mkDanglingSymlink = absolute: '' + ln -s${if absolute then "r" else ""} "$out/dangling" "$out/dangling-symlink" ''; - mkReflexiveSymlink = '' - ln -sr "$out/reflexive-symlink" "$out/reflexive-symlink" + mkReflexiveSymlink = absolute: '' + ln -s${if absolute then "r" else ""} "$out/reflexive-symlink" "$out/reflexive-symlink" ''; - mkValidSymlink = '' + mkValidSymlink = absolute: '' touch "$out/valid" - ln -sr "$out/valid" "$out/valid-symlink" + ln -s${if absolute then "r" else ""} "$out/valid" "$out/valid-symlink" ''; testBuilder = @@ -47,13 +47,12 @@ let ); in { - # Dangling symlinks (allowDanglingSymlinks) - fail-dangling-symlink = - runCommand "fail-dangling-symlink" + fail-dangling-symlink-relative = + runCommand "fail-dangling-symlink-relative" { failed = testBuildFailure (testBuilder { - name = "fail-dangling-symlink-inner"; - commands = [ mkDanglingSymlink ]; + name = "fail-dangling-symlink-relative-inner"; + commands = [ (mkDanglingSymlink false) ]; }); } '' @@ -62,19 +61,38 @@ in touch $out ''; - pass-dangling-symlink-allowed = testBuilder { - name = "pass-symlink-dangling-allowed"; - commands = [ mkDanglingSymlink ]; - derivationArgs.allowDanglingSymlinks = true; + pass-dangling-symlink-relative-allowed = testBuilder { + name = "pass-dangling-symlink-relative-allowed"; + commands = [ (mkDanglingSymlink false) ]; + derivationArgs.dontCheckForBrokenSymlinks = true; + }; + + fail-dangling-symlink-absolute = + runCommand "fail-dangling-symlink-absolute" + { + failed = testBuildFailure (testBuilder { + name = "fail-dangling-symlink-absolute-inner"; + commands = [ (mkDanglingSymlink true) ]; + }); + } + '' + (( 1 == "$(cat "$failed/testBuildFailure.exit")" )) + grep -F 'found 1 dangling symlinks and 0 reflexive symlinks' "$failed/testBuildFailure.log" + touch $out + ''; + + pass-dangling-symlink-absolute-allowed = testBuilder { + name = "pass-dangling-symlink-absolute-allowed"; + commands = [ (mkDanglingSymlink true) ]; + derivationArgs.dontCheckForBrokenSymlinks = true; }; - # Reflexive symlinks (allowReflexiveSymlinks) - fail-reflexive-symlink = - runCommand "fail-reflexive-symlink" + fail-reflexive-symlink-relative = + runCommand "fail-reflexive-symlink-relative" { failed = testBuildFailure (testBuilder { - name = "fail-reflexive-symlink-inner"; - commands = [ mkReflexiveSymlink ]; + name = "fail-reflexive-symlink-relative-inner"; + commands = [ (mkReflexiveSymlink false) ]; }); } '' @@ -83,21 +101,40 @@ in touch $out ''; - pass-reflexive-symlink-allowed = testBuilder { - name = "pass-reflexive-symlink-allowed"; - commands = [ mkReflexiveSymlink ]; - derivationArgs.allowReflexiveSymlinks = true; + pass-reflexive-symlink-relative-allowed = testBuilder { + name = "pass-reflexive-symlink-relative-allowed"; + commands = [ (mkReflexiveSymlink false) ]; + derivationArgs.dontCheckForBrokenSymlinks = true; }; - # Global (dontCheckForBrokenSymlinks) - fail-broken-symlinks = - runCommand "fail-broken-symlinks" + fail-reflexive-symlink-absolute = + runCommand "fail-reflexive-symlink-absolute" { failed = testBuildFailure (testBuilder { - name = "fail-broken-symlinks-inner"; + name = "fail-reflexive-symlink-absolute-inner"; + commands = [ (mkReflexiveSymlink true) ]; + }); + } + '' + (( 1 == "$(cat "$failed/testBuildFailure.exit")" )) + grep -F 'found 0 dangling symlinks and 1 reflexive symlinks' "$failed/testBuildFailure.log" + touch $out + ''; + + pass-reflexive-symlink-absolute-allowed = testBuilder { + name = "pass-reflexive-symlink-absolute-allowed"; + commands = [ (mkReflexiveSymlink true) ]; + derivationArgs.dontCheckForBrokenSymlinks = true; + }; + + fail-broken-symlinks-relative = + runCommand "fail-broken-symlinks-relative" + { + failed = testBuildFailure (testBuilder { + name = "fail-broken-symlinks-relative-inner"; commands = [ - mkDanglingSymlink - mkReflexiveSymlink + (mkDanglingSymlink false) + (mkReflexiveSymlink false) ]; }); } @@ -107,17 +144,48 @@ in touch $out ''; - pass-broken-symlinks-allowed = testBuilder { - name = "fail-broken-symlinks-allowed"; + pass-broken-symlinks-relative-allowed = testBuilder { + name = "pass-broken-symlinks-relative-allowed"; commands = [ - mkDanglingSymlink - mkReflexiveSymlink + (mkDanglingSymlink false) + (mkReflexiveSymlink false) ]; derivationArgs.dontCheckForBrokenSymlinks = true; }; - pass-valid-symlink = testBuilder { - name = "pass-valid-symlink"; - commands = [ mkValidSymlink ]; + fail-broken-symlinks-absolute = + runCommand "fail-broken-symlinks-absolute" + { + failed = testBuildFailure (testBuilder { + name = "fail-broken-symlinks-absolute-inner"; + commands = [ + (mkDanglingSymlink true) + (mkReflexiveSymlink true) + ]; + }); + } + '' + (( 1 == "$(cat "$failed/testBuildFailure.exit")" )) + grep -F 'found 1 dangling symlinks and 1 reflexive symlinks' "$failed/testBuildFailure.log" + touch $out + ''; + + pass-broken-symlinks-absolute-allowed = testBuilder { + name = "pass-broken-symlinks-absolute-allowed"; + commands = [ + (mkDanglingSymlink true) + (mkReflexiveSymlink true) + ]; + derivationArgs.dontCheckForBrokenSymlinks = true; + }; + + pass-valid-symlink-relative = testBuilder { + name = "pass-valid-symlink-relative"; + commands = [ (mkValidSymlink false) ]; + }; + + pass-valid-symlink-absolute = testBuilder { + name = "pass-valid-symlink-absolute"; + commands = [ (mkValidSymlink true) ]; }; }