From e2641cb8907fe6344ec4d220c4764187dfb25e3f Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Thu, 14 Aug 2025 13:04:36 +0200 Subject: [PATCH] libstore/local-derivation-goal: show tree with references that lead to an output cycle Closes #551 This adds a special accessor that falls back to checking if a store-path exists within a chroot if it's not a valid path. That way, `genGraphString` can find out which files have which references before the outputs are registered. Change-Id: I03c9d508fa3c72e5c262194461a25d71f3f4de15 --- doc/manual/rl-next/output-cycle-refs.md | 25 ++++++++++ lix/libstore/build/local-derivation-goal.cc | 51 +++++++++++++++------ lix/libstore/build/local-derivation-goal.hh | 10 ++++ lix/libstore/platform/linux.cc | 45 ++++++++++++++++++ lix/libstore/platform/linux.hh | 6 +++ tests/functional/output-cycles.sh | 10 +++- 6 files changed, 132 insertions(+), 15 deletions(-) create mode 100644 doc/manual/rl-next/output-cycle-refs.md diff --git a/doc/manual/rl-next/output-cycle-refs.md b/doc/manual/rl-next/output-cycle-refs.md new file mode 100644 index 000000000..02d8640ca --- /dev/null +++ b/doc/manual/rl-next/output-cycle-refs.md @@ -0,0 +1,25 @@ +--- +synopsis: "show tree with references that lead to an output cycle" +issues: [fj#551] +category: Improvements +credits: [ma27] +--- + +When Lix determines a cyclic dependency between several outputs of a derivation, +it now displays which files in which outputs lead to an output cycle: + +``` +error: cycle detected in build of '/nix/store/gc5h2whz3rylpf34n99nswvqgkjkigmy-demo.drv' in the references of output 'bar' from output 'foo'. + + Shown below are the files inside the outputs leading to the cycle: + /nix/store/3lrgm74j85nzpnkz127rkwbx3fz5320q-demo-bar + └───lib/libfoo: …stuffbefore /nix/store/h680k7k53rjl9p15g6h7kpym33250w0y-demo-baz andafter.… + → /nix/store/h680k7k53rjl9p15g6h7kpym33250w0y-demo-baz + └───share/snenskek: …???? /nix/store/dm24c76p9y2mrvmwgpmi64rryw6x5qmm-demo-foo ....… + → /nix/store/dm24c76p9y2mrvmwgpmi64rryw6x5qmm-demo-foo + └───bin/alarm: …textexttext/nix/store/3lrgm74j85nzpnkz127rkwbx3fz5320q-demo-bar abcabcabc.… + → /nix/store/3lrgm74j85nzpnkz127rkwbx3fz5320q-demo-bar +``` + +Please note that showing the files and its contents while displaying the cycles only works +on Linux. diff --git a/lix/libstore/build/local-derivation-goal.cc b/lix/libstore/build/local-derivation-goal.cc index 7b059cf35..d65fc7af3 100644 --- a/lix/libstore/build/local-derivation-goal.cc +++ b/lix/libstore/build/local-derivation-goal.cc @@ -1829,19 +1829,44 @@ try { return dependencies; }}); - auto sortedOutputNames = std::visit(overloaded { - [&](Cycle & cycle) -> std::vector { - // TODO with more -vvvv also show the temporary paths for manual inspection. - throw BuildError( - "cycle detected in build of '%s' in the references of output '%s' from output " - "'%s'", - worker.store.printStorePath(drvPath), - cycle.path, - cycle.parent - ); + auto & localStore = getLocalStore(); + auto sortedOutputNames = TRY_AWAIT(std::visit( + overloaded{ + // NOLINTNEXTLINE(cppcoreguidelines-avoid-capturing-lambda-coroutines) + [&](Cycle & cycle) -> kj::Promise>> { + try { + auto chrootAwareAccessor = getChrootDirAwareFSAccessor(); + auto graphStr = TRY_AWAIT(genGraphString( + scratchOutputs.at(cycle.path), + scratchOutputs.at(cycle.parent), + outputGraph, + worker.store, + true, + // We need to access store-paths that aren't registered yet for + // precise=true. Hence, only do this if a chroot-aware accessor is + // implemented in for this platform. + chrootAwareAccessor.has_value(), + chrootAwareAccessor + )); + + throw BuildError( + "cycle detected in build of '%s' in the references of output '%s' from " + "output " + "'%s'.\n\nShown below are the files inside the outputs leading to the " + "cycle:\n%s", + worker.store.printStorePath(drvPath), + cycle.path, + cycle.parent, + Uncolored(graphStr) + ); + } catch (...) { + co_return result::current_exception(); + } + }, + [](auto & r) -> kj::Promise>> { co_return r; } }, - [&](auto & r) { return r; } - }, topoSortedOutputs); + topoSortedOutputs + )); std::reverse(sortedOutputNames.begin(), sortedOutputNames.end()); @@ -2111,8 +2136,6 @@ try { } } - auto & localStore = getLocalStore(); - // Check determinism and run the diff hook for input-addressed // paths if we're in check mode. // TODO: implement this for content-addressed paths too. diff --git a/lix/libstore/build/local-derivation-goal.hh b/lix/libstore/build/local-derivation-goal.hh index 3a3a2b41e..3d1c04722 100644 --- a/lix/libstore/build/local-derivation-goal.hh +++ b/lix/libstore/build/local-derivation-goal.hh @@ -3,6 +3,7 @@ #include "lix/libstore/build/derivation-goal.hh" #include "lix/libstore/local-store.hh" +#include "lix/libutil/error.hh" #include "lix/libutil/processes.hh" #include "lix/libutil/cgroup.hh" @@ -329,6 +330,15 @@ protected: */ virtual void setupSyscallFilter() {} + /** + * Create a special accessor that can access paths that were built within the sandbox's + * chroot. + */ + virtual std::optional> getChrootDirAwareFSAccessor() + { + return std::nullopt; + }; + /** * Execute the builder, replacing the current process. * Generally this means an `execve` call. diff --git a/lix/libstore/platform/linux.cc b/lix/libstore/platform/linux.cc index 4e803d943..caf5ea29b 100644 --- a/lix/libstore/platform/linux.cc +++ b/lix/libstore/platform/linux.cc @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -1176,4 +1177,48 @@ void LinuxLocalDerivationGoal::killSandbox(bool getStats) } } } + +struct ChrootDirAwareFSAccessor : public LocalStoreAccessor +{ + Path chrootDir; + + ChrootDirAwareFSAccessor(ref store, const Path & chrootDir) + : LocalStoreAccessor(store) + , chrootDir(chrootDir) + { + } + + kj::Promise> toRealPath(const Path & path, bool requireValidPath = true) override + try { + auto storePath = store->toStorePath(path).first; + if (!TRY_AWAIT(store->isValidPath(storePath))) { + auto chrootStorePath = chrootDir + "/" + path; + if (pathExists(chrootStorePath)) { + co_return chrootStorePath; + } + + if (requireValidPath) { + throw InvalidPath( + "path '%1%' does not exist in the store, neither does chrooted path '%2%'", + store->printStorePath(storePath), + chrootStorePath + ); + } + } + + co_return TRY_AWAIT(LocalStoreAccessor::toRealPath(path, false)); + } catch (...) { + co_return result::current_exception(); + } +}; + +std::optional> LinuxLocalDerivationGoal::getChrootDirAwareFSAccessor() +{ + return make_ref( + ref::unsafeFromPtr( + std::dynamic_pointer_cast(getLocalStore().shared_from_this()) + ), + chrootRootDir + ); +} } diff --git a/lix/libstore/platform/linux.hh b/lix/libstore/platform/linux.hh index 47e33f240..79c222356 100644 --- a/lix/libstore/platform/linux.hh +++ b/lix/libstore/platform/linux.hh @@ -58,6 +58,12 @@ private: RunningProgram pastaPid; + /** + * Create a special accessor that can access paths that were built within the sandbox's + * chroot. + */ + std::optional> getChrootDirAwareFSAccessor() override; + /** * Create and populate chroot */ diff --git a/tests/functional/output-cycles.sh b/tests/functional/output-cycles.sh index 6b2931ba2..1c9369dea 100644 --- a/tests/functional/output-cycles.sh +++ b/tests/functional/output-cycles.sh @@ -2,7 +2,15 @@ source common.sh clearStore -(! nix-build check-outputs.nix -A cycle) 2>&1 | grepQuiet "cycle detected in build of '.*' in the references of output 'bar' from output 'foo'" +error="$(! nix-build check-outputs.nix -A cycle 2>&1)" +grepQuiet "cycle detected in build of '.*' in the references of output 'bar' from output 'foo'" <<<"$error" + +if [[ "$(uname -s)" = Linux ]]; then + <<<"$error" grepQuiet "/store/.*-cycle-bar" + <<<"$error" grepQuiet "└───lib/libfoo: ….*cycle-baz.*" + <<<"$error" grepQuiet " →.*/store/.*-cycle-baz" + <<<"$error" grepQuiet " └───share/lalala:.*-cycle-foo.*" +fi error="$(! nix-build check-outputs.nix -A as_dependency 2>&1)"