BinaryCacheStoreConfig: Change localNarCache to std::filesystem::path

Author: edolstra

src/libstore/include/nix/store/binary-cache-store.hh

@@ -38,9 +38,9 @@ struct BinaryCacheStoreConfig : virtual StoreConfig
     const Setting<std::string> secretKeyFiles{
         this, "", "secret-keys", "List of comma-separated paths to the secret keys used to sign the binary cache."};
 
-    const Setting<Path> localNarCache{
+    const Setting<std::optional<std::filesystem::path>> localNarCache{
         this,
-        "",
+        std::nullopt,
         "local-nar-cache",
         "Path to a local cache of NARs fetched from this binary cache, used by commands such as `nix store cat`."};
 

src/libstore/include/nix/store/remote-fs-accessor.hh

@@ -15,13 +15,13 @@ class RemoteFSAccessor : public SourceAccessor
 
     bool requireValidPath;
 
-    Path cacheDir;
+    std::optional<std::filesystem::path> cacheDir;
 
     std::pair<ref<SourceAccessor>, CanonPath> fetch(const CanonPath & path);
 
     friend struct BinaryCacheStore;
 
-    Path makeCacheFile(std::string_view hashPart, const std::string & ext);
+    std::filesystem::path makeCacheFile(std::string_view hashPart, const std::string & ext);
 
     ref<SourceAccessor> addToCache(std::string_view hashPart, std::string && nar);
 
@@ -33,7 +33,7 @@ public:
     std::shared_ptr<SourceAccessor> accessObject(const StorePath & path);
 
     RemoteFSAccessor(
-        ref<Store> store, bool requireValidPath = true, const /* FIXME: use std::optional */ Path & cacheDir = "");
+        ref<Store> store, bool requireValidPath = true, std::optional<std::filesystem::path> cacheDir = {});
 
     std::optional<Stat> maybeLstat(const CanonPath & path) override;
 

src/libstore/remote-fs-accessor.cc

@@ -8,24 +8,25 @@
 
 namespace nix {
 
-RemoteFSAccessor::RemoteFSAccessor(ref<Store> store, bool requireValidPath, const Path & cacheDir)
+RemoteFSAccessor::RemoteFSAccessor(
+    ref<Store> store, bool requireValidPath, std::optional<std::filesystem::path> cacheDir_)
     : store(store)
     , requireValidPath(requireValidPath)
-    , cacheDir(cacheDir)
+    , cacheDir(std::move(cacheDir_))
 {
-    if (cacheDir != "")
-        createDirs(cacheDir);
+    if (cacheDir)
+        createDirs(*cacheDir);
 }
 
-Path RemoteFSAccessor::makeCacheFile(std::string_view hashPart, const std::string & ext)
+std::filesystem::path RemoteFSAccessor::makeCacheFile(std::string_view hashPart, const std::string & ext)
 {
-    assert(cacheDir != "");
-    return fmt("%s/%s.%s", cacheDir, hashPart, ext);
+    assert(cacheDir);
+    return (*cacheDir / hashPart) + "." + ext;
 }
 
 ref<SourceAccessor> RemoteFSAccessor::addToCache(std::string_view hashPart, std::string && nar)
 {
-    if (cacheDir != "") {
+    if (cacheDir) {
         try {
             /* FIXME: do this asynchronously. */
             writeFile(makeCacheFile(hashPart, "nar"), nar);
@@ -37,7 +38,7 @@ ref<SourceAccessor> RemoteFSAccessor::addToCache(std::string_view hashPart, std:
     auto narAccessor = makeNarAccessor(std::move(nar));
     nars.emplace(hashPart, narAccessor);
 
-    if (cacheDir != "") {
+    if (cacheDir) {
         try {
             nlohmann::json j = listNarDeep(*narAccessor, CanonPath::root);
             writeFile(makeCacheFile(hashPart, "ls"), j.dump());
@@ -64,9 +65,9 @@ std::shared_ptr<SourceAccessor> RemoteFSAccessor::accessObject(const StorePath &
         return i->second;
 
     std::string listing;
-    Path cacheFile;
+    std::filesystem::path cacheFile;
 
-    if (cacheDir != "" && nix::pathExists(cacheFile = makeCacheFile(storePath.hashPart(), "nar"))) {
+    if (cacheDir && nix::pathExists(cacheFile = makeCacheFile(storePath.hashPart(), "nar"))) {
 
         try {
             listing = nix::readFile(makeCacheFile(storePath.hashPart(), "ls"));