Make LocalStore::optimisePath_() non-recursive

[edolstra]

Motivation

I encountered this failure during a test:

$ nix-instantiate --store /tmp/nix-gc-test '<nixpkgs>' -A hello --add-root x --auto-optimise-store
error: stack overflow (possible infinite recursion)

This turned out to be due to the recursion in optimisePath_() when called from a coroutine (which has a small stack):

  #0  0x00007ffff7e3e45d in nix::drainFD (fd=0, sink=..., opts=...) at ../src/libutil/file-descriptor.cc:131
  #1  0x00007ffff7ee6369 in nix::PosixSourceAccessor::readFile (this=0x55555617e2d0, path=..., sink=..., sizeCallback=...) at ../src/libutil/posix-source-accessor.cc:66
  #2  0x00007ffff7d38ad5 in nix::SourceAccessor::dumpPath(nix::CanonPath const&, nix::Sink&, nix::fun<bool (std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>&)::$_0::operator()(nix::CanonPath const&) const (this=0x7fffa7d64978, path=...) at ../src/libutil/archive.cc:48
  #3  0x00007ffff7d36efc in nix::SourceAccessor::dumpPath(nix::CanonPath const&, nix::Sink&, nix::fun<bool (std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>&)::$_1::operator()<$_1>(this $_1 const&, nix::CanonPath const&, unsigned long) (dump=..., path=..., depth=0) at ../src/libutil/archive.cc:72
  #4  0x00007ffff7d36bec in nix::SourceAccessor::dumpPath (this=0x55555617e310, path=..., sink=..., filter=...) at ../src/libutil/archive.cc:58
  #5  0x00007ffff7f1e3fa in nix::SourcePath::dumpPath (this=0x7fffa7d65f90, sink=..., filter=...) at ../src/libutil/source-path.cc:49
  #6  0x00007ffff7e3cb85 in nix::dumpPath (path=..., sink=..., method=nix::FileSerialisationMethod::NixArchive, filter=...) at ../src/libutil/file-content-address.cc:73
  #7  0x00007ffff7e3ccbd in nix::hashPath (path=..., method=nix::FileSerialisationMethod::NixArchive, ha=nix::HashAlgorithm::SHA256, filter=...) at ../src/libutil/file-content-address.cc:93
  #8  0x00007ffff763f02f in nix::LocalStore::optimisePath_ (this=0x555555eaeb00, act=0x0, stats=..., path=Python Exception <class 'gdb.error'>: No type named std::filesystem::__cxx11::path::_Cmpt.

      filesystem::path "/tmp/nix-gc-test/nix/store/9figz32vbiblaw5x3li1hc8q9qwlwf27-source/pkgs/desktops/pantheon/libraries/granite/7/default.nix", inodeHash=..., repair=nix::NoRepair)
      at ../src/libstore/optimise-store.cc:162
  #9  0x00007ffff763e70b in nix::LocalStore::optimisePath_ (this=0x555555eaeb00, act=0x0, stats=..., path=Python Exception <class 'gdb.error'>: No type named std::filesystem::__cxx11::path::_Cmpt.

      filesystem::path "/tmp/nix-gc-test/nix/store/9figz32vbiblaw5x3li1hc8q9qwlwf27-source/pkgs/desktops/pantheon/libraries/granite/7", inodeHash=..., repair=nix::NoRepair)
      at ../src/libstore/optimise-store.cc:125
  ...
  #25 0x00007ffff7f058ad in nix::sourceToSink(nix::fun<void (nix::Source&)>)::SourceToSink::operator()(std::basic_string_view<char, std::char_traits<char> >)::{lambda(boost::coroutines2::detail::pull_coroutine<bool>&)#1}::operator()(boost::coroutines2::detail::pull_coroutine<bool>&) const (this=0x7fffa7d73c98, yield=...) at ../src/libutil/serialise.cc:361
  ...
  #32 0x00007ffff3af918f in make_fcontext () from /nix/store/5xkyx9gi1s8mqp1ki1hsx8hwpr4dncxk-boost-1.87.0/lib/libboost_context.so.1.87.0

So let's make optimisePath_() non-recursive to ensure constant stack space.

Context

Summary by CodeRabbit

• Refactor
  • Internal optimization of store optimization logic with no changes to user-facing functionality or behavior.

[coderabbitai]

📝 Walkthrough

Walkthrough

LocalStore::optimisePath_ in src/libstore/optimise-store.cc is refactored from recursive traversal to iterative traversal using an explicit std::vector<std::filesystem::path> stack. Directory entries are pushed onto the stack instead of triggering recursive calls. All hardlink-deduplication logic—eligibility checks, hashing, repair, permission toggling, atomic rename, and stats reporting—is preserved and adapted to the loop structure.

Changes

Iterative rewrite of LocalStore::optimisePath_

Layer / File(s)	Summary
Stack setup, main loop, and directory enumeration src/libstore/optimise-store.cc	Introduces a std::vector path stack seeded with the input path; the while loop pops entries, checks for interrupts, lstats each path, applies the macOS .app/Contents exclusion via continue, and pushes directory children onto the stack instead of recursing.
File eligibility checks, hashing, and corrupted-link repair src/libstore/optimise-store.cc	Non-regular and non-allowed-symlink entries, suspicious writable files, and already-linked inodes are all skipped via continue. For eligible files, hash computation, linkPath derivation, and corrupted/size-mismatched link removal (with re-hash on repair) are embedded in the loop. Hard-link creation into linksDir handles file_exists and no_space_on_device without throwing.
Inode comparison, atomic replacement, and stats src/libstore/optimise-store.cc	Inside the loop, inode identity is checked to skip redundant work; the containing directory's permissions are toggled via MakeReadOnly; a temporary hard link is created and atomically renamed over the original path, with too_many_links handled at both steps. On success, stats.filesLinked, stats.bytesFreed, and act->result(resFileLinked, ...) are updated.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 No more deep dives down the rabbit hole,
A stack now guides me, keeping control.
Each path I push, each path I pop,
Hard links made without a stack overflow drop.
The forest traversed, one hop at a time—
Iterative bliss, in rhythmic rhyme! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and clearly describes the main change: converting LocalStore::optimisePath_() from a recursive to a non-recursive (iterative) implementation, which is the primary objective of the pull request.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch non-recursive-optimise-store

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/libstore/optimise-store.cc`:
- Around line 113-114: The code uses std::vector<std::filesystem::path> for the
stack variable declaration without an explicit `#include` <vector> directive,
relying on transitive includes. Add an explicit `#include` <vector> header at the
top of the file in the includes section to improve code clarity and
maintainability, ensuring the std::vector template is directly available when
the stack variable is declared.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e1a739cc-1f17-4285-a0ea-1ff14ce72cf3

📥 Commits

Reviewing files that changed from the base of the PR and between 21ed367 and d9b1aa9.

📒 Files selected for processing (1)

• src/libstore/optimise-store.cc

[github-actions]

🚀 Deployed on https://6a31574ad8352300ba504a4b--determinate-nix.netlify.app