Skip to content

Register temp roots in a few places#482

Merged
edolstra merged 2 commits into
mainfrom
temp-roots
Jun 3, 2026
Merged

Register temp roots in a few places#482
edolstra merged 2 commits into
mainfrom
temp-roots

Conversation

@edolstra

@edolstra edolstra commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Motivation

Includes upstream NixOS#15720, plus a less ambitious version of NixOS#15719.

Context

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced stability of store path operations during closure fetching and path copying processes to prevent data loss scenarios.

dramforever and others added 2 commits June 3, 2026 15:15
This avoids the fetched path from getting garbage collected if it is
already valid.
Note: this only prevents the paths from being GCed during the
copy. You still need to use `nix copy --out-link ...` to prevent GC
afterwards.

Less ambitious fix than NixOS#15719.
@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 71c4bfed-2ae0-434e-834d-e5fa9ff6936e

📥 Commits

Reviewing files that changed from the base of the PR and between d5cb6d4 and 734a205.

📒 Files selected for processing (2)
  • src/libexpr/primops/fetchClosure.cc
  • src/libstore/store-api.cc

📝 Walkthrough

Walkthrough

Adds temporary GC root registrations to prevent garbage collection of store paths. Three closure-fetch helper functions in fetchClosure.cc now root their paths before validation; copyPaths in store-api.cc roots destination paths before the validity check loop.

Changes

GC Root Registration

Layer / File(s) Summary
Fetch closure GC root registrations
src/libexpr/primops/fetchClosure.cc
runFetchClosureWithRewrite registers toPathMaybe as a temporary root; runFetchClosureWithContentAddressedPath and runFetchClosureWithInputAddressedPath both register fromPath as a temporary root before validation and closure-copy operations.
Copy paths GC root registration
src/libstore/store-api.cc
copyPaths registers each destination path in the storePaths set as a temporary root via a new loop before validity checking and substitution logic.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~4 minutes

🐰 A rabbit hops with glee,
"Roots protect paths, wild and free!"
Garbage can't sweep away with care,
These stored bunny treasures fair. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.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 'Register temp roots in a few places' accurately and concisely describes the main change across both modified files—adding temporary GC root registration in fetchClosure and copyPaths functions.
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 temp-roots

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Infer (1.2.0)
src/libexpr/primops/fetchClosure.cc

src/libexpr/primops/fetchClosure.cc:1:10: fatal error: 'nix/expr/primops.hh' file not found
1 | #include "nix/expr/primops.hh"
| ^~~~~~~~~~~~~~~~~~~~~
1 error generated.
src/libexpr/primops/fetchClosure.cc:26:1-72:1: ERROR translating statement 'CompoundStmt'
Aborting translation of method 'nix::runFetchClosureWithRewrite' in file 'src/libexpr/primops/fetchClosure.cc': "Assert_failure src/clang/cAst_utils.ml:249:53"
Uncaught Internal Error: "Assert_failure src/clang/cAst_utils.ml:249:53"
Error backtrace:
Raised at ClangFrontend__CAst_utils.get_decl_from_typ_ptr in file "src/clang/cAst_utils.ml", line 249, characters 53-65
Called from ClangFrontend__CTrans.CTrans_funct.get_destructor_decl_ref in file "src/clang/cTrans.ml", line 658, characters 12-59
Called from ClangFrontend__CTrans.CTrans_funct.destructor_calls.(fun) in file "src/clang/cTrans.ml", line 2048, characters 12-69
Called from Base__List.rev_filter_map.loop in file "src/list.ml", line 944, characters 13-17

... [truncated 2200 characters] ...

in file "src/clang/cFrontend_decl.ml" (inlined), line 54, characters 4-52
Called from ClangFrontend__CFrontend_decl.CFrontend_decl_funct.function_decl in file "src/clang/cFrontend_decl.ml", line 90, characters 12-151
Called from ClangFrontend__CFrontend_decl.CFrontend_decl_funct.translate_one_declaration in file "src/clang/cFrontend_decl.ml", line 453, characters 10-56
Called from ClangFrontend__CFrontend_decl.CFrontend_decl_funct.translate_one_declaration.translate in file "src/clang/cFrontend_decl.ml" (inlined), line 448, characters 24-96
Called from Stdlib__List.iter in file "list.ml" (inlined), line 110, characters 12-15
Called from Stdlib__List.iter in file "list.ml" (inlined), line 108, characters 13-64
Called from Base__List0.iter in file "src/list0.ml" (inlined), line 25, characte

src/libstore/store-api.cc

src/libstore/store-api.cc:1:10: fatal error: 'nix/util/logging.hh' file not found
1 | #include "nix/util/logging.hh"
| ^~~~~~~~~~~~~~~~~~~~~
1 error generated.
src/libstore/store-api.cc:995:5-12: ERROR translating statement 'ReturnStmt'
Aborting translation of method 'nix::copyStorePath' in file 'src/libstore/store-api.cc': "Assert_failure src/clang/cAst_utils.ml:249:53"
Uncaught Internal Error: "Assert_failure src/clang/cAst_utils.ml:249:53"
Error backtrace:
Raised at ClangFrontend__CAst_utils.get_decl_from_typ_ptr in file "src/clang/cAst_utils.ml", line 249, characters 53-65
Called from ClangFrontend__CTrans.CTrans_funct.get_destructor_decl_ref in file "src/clang/cTrans.ml", line 658, characters 12-59
Called from ClangFrontend__CTrans.CTrans_funct.destructor_calls.(fun) in file "src/clang/cTrans.ml", line 2048, characters 12-69
Called from Base__List.rev_filter_map.loop in file "src/list.ml", line 944, characters 13-17
Called from Base__List.filter_map in file "src

... [truncated 2200 characters] ...

ine 4784, characters 10-1023
Called from ClangFrontend__CTrans.CTrans_funct.instruction in file "src/clang/cTrans.ml" (inlined), line 4765, characters 38-71
Called from ClangFrontend__CTrans.CTrans_funct.exec_with_node_creation in file "src/clang/cTrans.ml" (inlined), line 104, characters 20-38
Called from ClangFrontend__CTrans.CTrans_funct.get_clang_stmt_trans in file "src/clang/cTrans.ml" (inlined), line 5395, characters 4-69
Called from ClangFrontend__CTrans.CTrans_funct.get_custom_stmt_trans in file "src/clang/cTrans.ml", line 5401, characters 8-55
Called from ClangFrontend__CTrans.CTrans_funct.exec_trans_instrs.exec_trans_instrs_rev in file "src/clang/cTrans.ml" (inlined), line 5365, characters 28-54
Called from ClangFrontend__CTrans.CTrans_funct.exec_trans_instrs in file "src/clang/c


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

@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

@github-actions github-actions Bot temporarily deployed to pull request June 3, 2026 14:38 Inactive
@edolstra edolstra added this pull request to the merge queue Jun 3, 2026
Merged via the queue into main with commit 4723bb5 Jun 3, 2026
29 checks passed
@edolstra edolstra deleted the temp-roots branch June 3, 2026 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants