fetchToStore(): Don't always respect settings.readOnlyMode

It's now up to the caller whether readOnlyMode should be applied. In some contexts (like InputScheme::fetch()), we always need to fetch.
edolstra · Feb 20, 2024 · ce374df · ce374df
1 parent b8ec2fb
commit ce374df
Show file tree

Hide file tree

Showing 8 changed files with 14 additions and 10 deletions.
diff --git a/src/libcmd/common-eval-args.cc b/src/libcmd/common-eval-args.cc
@@ -170,7 +170,7 @@ SourcePath lookupFileArg(EvalState & state, std::string_view s, const Path * bas
     if (EvalSettings::isPseudoUrl(s)) {
         auto accessor = fetchers::downloadTarball(
             EvalSettings::resolvePseudoUrl(s)).accessor;
-        auto storePath = fetchToStore(*state.store, SourcePath(accessor));
+        auto storePath = fetchToStore(*state.store, SourcePath(accessor), FetchMode::Copy);
         return state.rootPath(CanonPath(state.store->toRealPath(storePath)));
     }
 

diff --git a/src/libcmd/installable-value.cc b/src/libcmd/installable-value.cc
@@ -45,7 +45,7 @@ ref<InstallableValue> InstallableValue::require(ref<Installable> installable)
 std::optional<DerivedPathWithInfo> InstallableValue::trySinglePathToDerivedPaths(Value & v, const PosIdx pos, std::string_view errorCtx)
 {
     if (v.type() == nPath) {
-        auto storePath = fetchToStore(*state->store, v.path());
+        auto storePath = fetchToStore(*state->store, v.path(), FetchMode::Copy);
         return {{
             .path = DerivedPath::Opaque {
                 .path = std::move(storePath),

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
@@ -2342,6 +2342,7 @@ StorePath EvalState::copyPathToStore(NixStringContext & context, const SourcePat
             auto dstPath = fetchToStore(
                 *store,
                 path.resolveSymlinks(),
+                settings.readOnlyMode ? FetchMode::DryRun : FetchMode::Copy,
                 path.baseName(),
                 FileIngestionMethod::Recursive,
                 nullptr,
@@ -2795,7 +2796,7 @@ std::optional<std::string> EvalState::resolveSearchPathPath(const SearchPath::Pa
         try {
             auto accessor = fetchers::downloadTarball(
                 EvalSettings::resolvePseudoUrl(value)).accessor;
-            auto storePath = fetchToStore(*store, SourcePath(accessor));
+            auto storePath = fetchToStore(*store, SourcePath(accessor), FetchMode::Copy);
             res = { store->toRealPath(storePath) };
         } catch (Error & e) {
             logWarning({

diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
@@ -2231,6 +2231,7 @@ static void addPath(
             auto dstPath = fetchToStore(
                 *state.store,
                 path.resolveSymlinks(),
+                settings.readOnlyMode ? FetchMode::DryRun : FetchMode::Copy,
                 name,
                 method,
                 filter.get(),

diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc
@@ -474,7 +474,7 @@ static void fetch(EvalState & state, const PosIdx pos, Value * * args, Value & v
     //       https://github.com/NixOS/nix/issues/4313
     auto storePath =
         unpack
-        ? fetchToStore(*state.store, fetchers::downloadTarball(*url).accessor, name)
+        ? fetchToStore(*state.store, fetchers::downloadTarball(*url).accessor, FetchMode::Copy, name)
         : fetchers::downloadFile(state.store, *url, name, (bool) expectedHash).storePath;
 
     if (expectedHash) {

diff --git a/src/libfetchers/fetch-to-store.cc b/src/libfetchers/fetch-to-store.cc
@@ -7,6 +7,7 @@ namespace nix {
 StorePath fetchToStore(
     Store & store,
     const SourcePath & path,
+    FetchMode mode,
     std::string_view name,
     ContentAddressMethod method,
     PathFilter * filter,
@@ -33,21 +34,19 @@ StorePath fetchToStore(
     } else
         debug("source path '%s' is uncacheable", path);
 
-    auto readOnly = settings.readOnlyMode;
-
     Activity act(*logger, lvlChatty, actUnknown,
-        fmt(readOnly ? "hashing '%s'" : "copying '%s' to the store", path));
+        fmt(mode == DryRun ? "hashing '%s'" : "copying '%s' to the store", path));
 
     auto filter2 = filter ? *filter : defaultPathFilter;
 
     auto storePath =
-        readOnly
+        mode == DryRun
         ? store.computeStorePath(
             name, *path.accessor, path.path, method, HashAlgorithm::SHA256, {}, filter2).first
         : store.addToStore(
             name, *path.accessor, path.path, method, HashAlgorithm::SHA256, {}, filter2, repair);
 
-    if (cacheKey)
+    if (cacheKey && mode == Copy)
         fetchers::getCache()->add(store, *cacheKey, {}, storePath, true);
 
     return storePath;

diff --git a/src/libfetchers/fetch-to-store.hh b/src/libfetchers/fetch-to-store.hh
@@ -8,12 +8,15 @@
 
 namespace nix {
 
+enum FetchMode { DryRun, Copy };
+
 /**
  * Copy the `path` to the Nix store.
  */
 StorePath fetchToStore(
     Store & store,
     const SourcePath & path,
+    FetchMode mode,
     std::string_view name = "source",
     ContentAddressMethod method = FileIngestionMethod::Recursive,
     PathFilter * filter = nullptr,

diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc
@@ -376,7 +376,7 @@ void InputScheme::clone(const Input & input, const Path & destDir) const
 std::pair<StorePath, Input> InputScheme::fetch(ref<Store> store, const Input & input)
 {
     auto [accessor, input2] = getAccessor(store, input);
-    auto storePath = fetchToStore(*store, SourcePath(accessor), input2.getName());
+    auto storePath = fetchToStore(*store, SourcePath(accessor), FetchMode::Copy, input2.getName());
     return {storePath, input2};
 }