Skip to content

Add missing temp roots#373

Merged
grahamc merged 2 commits intomainfrom
add-missing-temp-roots
Feb 27, 2026
Merged

Add missing temp roots#373
grahamc merged 2 commits intomainfrom
add-missing-temp-roots

Conversation

@edolstra
Copy link
Collaborator

@edolstra edolstra commented Feb 27, 2026 •
edited by coderabbitai bot
Loading

Motivation

Backport of NixOS#15237 + a similar fix to AsyncPathWriter.

Context

Summary by CodeRabbit

  • Bug Fixes
    • Improved cache protection mechanisms to prevent accidental garbage collection of cached inputs across store operations. Temporary root management has been added during path storage, async write operations, and derivation processing to ensure cached data persists reliably and reduces the risk of unintended data loss.

@coderabbitai
Copy link

coderabbitai bot commented Feb 27, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 908be63 and 618c4df.

📒 Files selected for processing (3)
  • src/libfetchers/fetch-to-store.cc
  • src/libstore/async-path-writer.cc
  • src/libstore/derivations.cc
📝 Walkthrough

Walkthrough

The changes add temporary root registration across three store modules to prevent accidental garbage collection. Each modification calls addTempRoot() before performing store operations on paths, ensuring cached inputs and derivations remain valid during processing.

Changes

Cohort / File(s) Summary
Temporary Root Registration
src/libfetchers/fetch-to-store.cc, src/libstore/async-path-writer.cc, src/libstore/derivations.cc		 Adds addTempRoot() calls before store path operations to prevent accidental garbage collection of cached inputs and derivations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

Hops through the store with roots held tight,
Temp anchors placed to save the night,
No garbage shall steal treasures here,
Three guards stand watch, both far and near! 🐰✨

🚥 Pre-merge checks | ✅ 2 | ❌ 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 (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add missing temp roots' directly summarizes the main objective of the pull request, which is to add missing temporary root registrations across multiple components.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch add-missing-temp-roots

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

@github-actions
Copy link

github-actions bot commented Feb 27, 2026

🚀 Deployed on https://69a1d0fe94b83265cb465bf0--determinate-nix.netlify.app

@github-actions github-actions bot temporarily deployed to pull request February 27, 2026 17:14 Inactive
@grahamc grahamc added this pull request to the merge queue Feb 27, 2026
Merged via the queue into main with commit f7e9e71 Feb 27, 2026
28 checks passed
@grahamc grahamc deleted the add-missing-temp-roots branch February 27, 2026 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@grahamc grahamc grahamc approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@edolstra @grahamc @xokdvium