Move the srcToStore cache into fetchToStore()

[edolstra]

Motivation

This makes other users of fetchToStore() (in particular, builtins.path) benefit from the in-memory source path cache. This is useful for paths that lack a fingerprint (e.g. during non-flake evaluations using nix-instantiate).

Context

Summary by CodeRabbit

• Refactor
  • Optimized internal cache management for source-to-store path mapping to improve performance and code organization.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 902671fd-0c99-4ff6-9e68-de983361504a

📥 Commits

Reviewing files that changed from the base of the PR and between 0ef0f14 and 218796a.

📒 Files selected for processing (4)

• src/libexpr/eval.cc
• src/libexpr/include/nix/expr/eval.hh
• src/libfetchers/fetch-to-store.cc
• src/libfetchers/include/nix/fetchers/fetch-settings.hh

💤 Files with no reviewable changes (1)

• src/libexpr/include/nix/expr/eval.hh

🚧 Files skipped from review as they are similar to previous changes (3)

• src/libfetchers/include/nix/fetchers/fetch-settings.hh
• src/libfetchers/fetch-to-store.cc
• src/libexpr/eval.cc

---

📝 Walkthrough

Walkthrough

The per-session source-to-store path cache (srcToStore) is removed from EvalState and relocated into fetchers::Settings as a boost::concurrent_flat_map-backed SrcToStore struct. fetchToStore2 gains a cache lookup fast-path keyed on (SourcePath, ContentAddressMethod::raw, name) and a post-compute write-back. EvalState::copyPathToStore is simplified to call fetchToStore directly without any local caching.

Changes

srcToStore cache migration to libfetchers

Layer / File(s)	Summary
SrcToStore cache contract in libfetchers src/libfetchers/include/nix/fetchers/fetch-settings.hh, src/libfetchers/fetch-to-store.cc	fetch-settings.hh gains a forward declaration for SrcToStore, a static createSrcToStore() factory, and a const ref<SrcToStore> srcToStore member on Settings. fetch-to-store.cc defines SrcToStore using boost::concurrent_flat_map and implements createSrcToStore().
fetchToStore2 cache fast-path and write-back src/libfetchers/fetch-to-store.cc	fetchToStore2 adds an early-return branch that looks up (path, method.raw, name) in settings.srcToStore when no PathFilter is provided. After hashing or copying, results are inserted/overwritten into the cache. A stale FIXME comment is removed, and the copy log is promoted from debug to printMsg.
EvalState srcToStore removal src/libexpr/include/nix/expr/eval.hh, src/libexpr/eval.cc	EvalState's private srcToStore member is removed from the header. Its constructor initialization is dropped, and copyPathToStore is reworked to call fetchToStore directly with symlink resolution and FetchMode selection, without the old try_emplace caching pattern.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 The cache has found a better home,
No longer forced in EvalState to roam.
fetchToStore now holds the key,
A concurrent map for you and me.
Fast-paths bloom where FIXMEs grew —
Hoppity-hop, the cache moved through! ✨

🚥 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 'Move the srcToStore cache into fetchToStore()' accurately describes the main change: relocating the srcToStore cache from EvalState to the fetchToStore() function and settings, enabling broader access for callers like builtins.path.
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 move-srcToStore

---

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

[github-actions]

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/libfetchers/fetch-settings.cc (1)

1-7: ⚡ Quick win

Include the concrete cache dependencies before constructing srcToStore.

Line 7 instantiates the concurrent map type, so this translation unit should directly include the full Boost container and complete SourcePath / StorePath / Hash definitions rather than relying on transitive includes.

♻️ Proposed include cleanup

 `#include` "nix/fetchers/fetch-settings.hh"
+#include "nix/store/store-api.hh"
 `#include` "nix/util/config-global.hh"
+#include "nix/util/hash.hh"
+#include "nix/util/source-path.hh"
+
+#include <boost/unordered/concurrent_flat_map.hpp>
+#include <tuple>

🤖 Prompt for 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.

In `@src/libfetchers/fetch-settings.cc` around lines 1 - 7, The Settings
constructor on line 7 instantiates the srcToStore concurrent map type, but the
file does not directly include the necessary headers for the Boost container and
the complete type definitions needed for this instantiation. Add direct includes
for the Boost concurrent map header and the full definitions of SourcePath,
StorePath, and Hash types after the existing includes at the top of
fetch-settings.cc. This ensures all concrete cache dependencies are available
before the srcToStore member is constructed in the Settings::Settings()
constructor.

🤖 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/libfetchers/fetch-to-store.cc`:
- Around line 41-45: The fast-path cache in fetchToStore uses only the
SourcePath as the cache key on line 42 (getConcurrent call) and on line 125
(cache storage), but fetchToStore2 depends on additional parameters including
name, method, the current fingerprint, and store validity. This can return
incorrect cached results computed with different parameters. Modify the cache
key to include the full fetch identity (name, method, fingerprint) instead of
just the path. For entries without a fingerprint, ensure they are scoped
appropriately. Additionally, when returning a Copy mode hit from the cache on
line 43-44, add a temporary root and validate that the cached store path exists
and is valid in the current Store before returning it, to prevent returning
paths that are not accessible in the current context.

---

Nitpick comments:
In `@src/libfetchers/fetch-settings.cc`:
- Around line 1-7: The Settings constructor on line 7 instantiates the
srcToStore concurrent map type, but the file does not directly include the
necessary headers for the Boost container and the complete type definitions
needed for this instantiation. Add direct includes for the Boost concurrent map
header and the full definitions of SourcePath, StorePath, and Hash types after
the existing includes at the top of fetch-settings.cc. This ensures all concrete
cache dependencies are available before the srcToStore member is constructed in
the Settings::Settings() constructor.

🪄 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: a8bbf9ea-d41b-4f34-8b91-fdf50c231c0e

📥 Commits

Reviewing files that changed from the base of the PR and between 214259d and 14dbc06.

📒 Files selected for processing (7)

• src/libexpr/eval.cc
• src/libexpr/include/nix/expr/eval.hh
• src/libfetchers/fetch-settings.cc
• src/libfetchers/fetch-to-store.cc
• src/libfetchers/include/nix/fetchers/fetch-mode.hh
• src/libfetchers/include/nix/fetchers/fetch-settings.hh
• src/libfetchers/include/nix/fetchers/fetch-to-store.hh

💤 Files with no reviewable changes (1)

• src/libexpr/include/nix/expr/eval.hh