Merge remote-tracking branch 'upstream/master' into validPathInfo-ca-proper-datatype

11 files changed, 162 insertions, 31 deletions

diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc
index 7d681218e..75d70c1b4 100644
--- a/src/libfetchers/git.cc
+++ b/src/libfetchers/git.cc
@@ -282,7 +282,10 @@ struct GitInput : Input
                 // FIXME: git stderr messes up our progress indicator, so
                 // we're using --quiet for now. Should process its stderr.
                 try {
-                    runProgram("git", true, { "-C", repoDir, "fetch", "--quiet", "--force", "--", actualUrl, fmt("%s:%s", *input->ref, *input->ref) });
+                    auto fetchRef = input->ref->compare(0, 5, "refs/") == 0
+                        ? *input->ref
+                        : "refs/heads/" + *input->ref;
+                    runProgram("git", true, { "-C", repoDir, "fetch", "--quiet", "--force", "--", actualUrl, fmt("%s:%s", fetchRef, fetchRef) });
                 } catch (Error & e) {
                     if (!pathExists(localRefFile)) throw;
                     warn("could not update local clone of Git repository '%s'; continuing with the most recent version", actualUrl);
@@ -418,7 +421,7 @@ struct GitInputScheme : InputScheme
 
         auto input = std::make_unique<GitInput>(parseURL(getStrAttr(attrs, "url")));
         if (auto ref = maybeGetStrAttr(attrs, "ref")) {
-            if (!std::regex_match(*ref, refRegex))
+            if (std::regex_search(*ref, badGitRefRegex))
                 throw BadURL("invalid Git branch/tag name '%s'", *ref);
             input->ref = *ref;
         }
diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc
index 8efb8d68c..e8e5231d2 100644
--- a/src/libfetchers/tarball.cc
+++ b/src/libfetchers/tarball.cc
@@ -74,7 +74,8 @@ DownloadFileResult downloadFile(
             .method = FileIngestionMethod::Flat,
             .hash = hash,
         };
-        store->addToStore(info, sink.s, NoRepair, NoCheckSigs);
+        auto source = StringSource { *sink.s };
+        store->addToStore(info, source, NoRepair, NoCheckSigs);
         storePath = std::move(info.path);
     }
 
diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc
index 259ffaf59..583f9d8ac 100644
--- a/src/libstore/binary-cache-store.cc
+++ b/src/libstore/binary-cache-store.cc
@@ -113,9 +113,12 @@ void BinaryCacheStore::writeNarInfo(ref<NarInfo> narInfo)
         diskCache->upsertNarInfo(getUri(), hashPart, std::shared_ptr<NarInfo>(narInfo));
 }
 
-void BinaryCacheStore::addToStore(const ValidPathInfo & info, const ref<std::string> & nar,
+void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource,
     RepairFlag repair, CheckSigsFlag checkSigs, std::shared_ptr<FSAccessor> accessor)
 {
+    // FIXME: See if we can use the original source to reduce memory usage.
+    auto nar = make_ref<std::string>(narSource.drain());
+
     if (!repair && isValidPath(info.path)) return;
 
     /* Verify that all references are valid. This may do some .narinfo
@@ -347,7 +350,8 @@ StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath
 
     ValidPathInfo info(makeFixedOutputPath(method, h, name));
 
-    addToStore(info, sink.s, repair, CheckSigs, nullptr);
+    auto source = StringSource { *sink.s };
+    addToStore(info, source, repair, CheckSigs, nullptr);
 
     return std::move(info.path);
 }
@@ -361,7 +365,8 @@ StorePath BinaryCacheStore::addTextToStore(const string & name, const string & s
     if (repair || !isValidPath(info.path)) {
         StringSink sink;
         dumpString(s, sink);
-        addToStore(info, sink.s, repair, CheckSigs, nullptr);
+        auto source = StringSource { *sink.s };
+        addToStore(info, source, repair, CheckSigs, nullptr);
     }
 
     return std::move(info.path);
diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh
index 4ff5609ed..52ef8aa7a 100644
--- a/src/libstore/binary-cache-store.hh
+++ b/src/libstore/binary-cache-store.hh
@@ -74,7 +74,7 @@ public:
     std::optional<StorePath> queryPathFromHashPart(const std::string & hashPart) override
     { unsupported("queryPathFromHashPart"); }
 
-    void addToStore(const ValidPathInfo & info, const ref<std::string> & nar,
+    void addToStore(const ValidPathInfo & info, Source & narSource,
         RepairFlag repair, CheckSigsFlag checkSigs,
         std::shared_ptr<FSAccessor> accessor) override;
 
diff --git a/src/libstore/export-import.cc b/src/libstore/export-import.cc
index 8a5e9d08e..c9731a6bd 100644
--- a/src/libstore/export-import.cc
+++ b/src/libstore/export-import.cc
@@ -1,3 +1,4 @@
+#include "serialise.hh"
 #include "store-api.hh"
 #include "archive.hh"
 #include "worker-protocol.hh"
@@ -100,7 +101,9 @@ StorePaths Store::importPaths(Source & source, std::shared_ptr<FSAccessor> acces
         if (readInt(source) == 1)
             readString(source);
 
-        addToStore(info, tee.source.data, NoRepair, checkSigs, accessor);
+        // Can't use underlying source, which would have been exhausted
+        auto source = StringSource { *tee.source.data };
+        addToStore(info, source, NoRepair, checkSigs, accessor);
 
         res.push_back(info.path.clone());
     }
diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
index 0715f64f3..ea30ed105 100644
--- a/src/libstore/store-api.cc
+++ b/src/libstore/store-api.cc
@@ -813,21 +813,6 @@ Strings ValidPathInfo::shortRefs() const
 }
 
 
-void Store::addToStore(const ValidPathInfo & info, Source & narSource,
-    RepairFlag repair, CheckSigsFlag checkSigs,
-    std::shared_ptr<FSAccessor> accessor)
-{
-    addToStore(info, make_ref<std::string>(narSource.drain()), repair, checkSigs, accessor);
-}
-
-void Store::addToStore(const ValidPathInfo & info, const ref<std::string> & nar,
-    RepairFlag repair, CheckSigsFlag checkSigs,
-    std::shared_ptr<FSAccessor> accessor)
-{
-    StringSource source(*nar);
-    addToStore(info, source, repair, checkSigs, accessor);
-}
-
 }
 
 
diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh
index faf549fe1..7836c04dc 100644
--- a/src/libstore/store-api.hh
+++ b/src/libstore/store-api.hh
@@ -440,12 +440,7 @@ public:
     /* Import a path into the store. */
     virtual void addToStore(const ValidPathInfo & info, Source & narSource,
         RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs,
-        std::shared_ptr<FSAccessor> accessor = 0);
-
-    // FIXME: remove
-    virtual void addToStore(const ValidPathInfo & info, const ref<std::string> & nar,
-        RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs,
-        std::shared_ptr<FSAccessor> accessor = 0);
+        std::shared_ptr<FSAccessor> accessor = 0) = 0;
 
     /* Copy the contents of a path to the store and register the
        validity the resulting path.  The resulting path is returned.
diff --git a/src/libutil/tests/lru-cache.cc b/src/libutil/tests/lru-cache.cc
new file mode 100644
index 000000000..091d3d5ed
--- /dev/null
+++ b/src/libutil/tests/lru-cache.cc
@@ -0,0 +1,130 @@
+#include "lru-cache.hh"
+#include <gtest/gtest.h>
+
+namespace nix {
+
+    /* ----------------------------------------------------------------------------
+     * size
+     * --------------------------------------------------------------------------*/
+
+    TEST(LRUCache, sizeOfEmptyCacheIsZero) {
+        LRUCache<std::string, std::string> c(10);
+        ASSERT_EQ(c.size(), 0);
+    }
+
+    TEST(LRUCache, sizeOfSingleElementCacheIsOne) {
+        LRUCache<std::string, std::string> c(10);
+        c.upsert("foo", "bar");
+        ASSERT_EQ(c.size(), 1);
+    }
+
+    /* ----------------------------------------------------------------------------
+     * upsert / get
+     * --------------------------------------------------------------------------*/
+
+    TEST(LRUCache, getFromEmptyCache) {
+        LRUCache<std::string, std::string> c(10);
+        auto val = c.get("x");
+        ASSERT_EQ(val.has_value(), false);
+    }
+
+    TEST(LRUCache, getExistingValue) {
+        LRUCache<std::string, std::string> c(10);
+        c.upsert("foo", "bar");
+        auto val = c.get("foo");
+        ASSERT_EQ(val, "bar");
+    }
+
+    TEST(LRUCache, getNonExistingValueFromNonEmptyCache) {
+        LRUCache<std::string, std::string> c(10);
+        c.upsert("foo", "bar");
+        auto val = c.get("another");
+        ASSERT_EQ(val.has_value(), false);
+    }
+
+    TEST(LRUCache, upsertOnZeroCapacityCache) {
+        LRUCache<std::string, std::string> c(0);
+        c.upsert("foo", "bar");
+        auto val = c.get("foo");
+        ASSERT_EQ(val.has_value(), false);
+    }
+
+    TEST(LRUCache, updateExistingValue) {
+        LRUCache<std::string, std::string> c(1);
+        c.upsert("foo", "bar");
+
+        auto val = c.get("foo");
+        ASSERT_EQ(val.value_or("error"), "bar");
+        ASSERT_EQ(c.size(), 1);
+
+        c.upsert("foo", "changed");
+        val = c.get("foo");
+        ASSERT_EQ(val.value_or("error"), "changed");
+        ASSERT_EQ(c.size(), 1);
+    }
+
+    TEST(LRUCache, overwriteOldestWhenCapacityIsReached) {
+        LRUCache<std::string, std::string> c(3);
+        c.upsert("one", "eins");
+        c.upsert("two", "zwei");
+        c.upsert("three", "drei");
+
+        ASSERT_EQ(c.size(), 3);
+        ASSERT_EQ(c.get("one").value_or("error"), "eins");
+
+        // exceed capacity
+        c.upsert("another", "whatever");
+
+        ASSERT_EQ(c.size(), 3);
+        // Retrieving "one" makes it the most recent element thus
+        // two will be the oldest one and thus replaced.
+        ASSERT_EQ(c.get("two").has_value(), false);
+        ASSERT_EQ(c.get("another").value(), "whatever");
+    }
+
+    /* ----------------------------------------------------------------------------
+     * clear
+     * --------------------------------------------------------------------------*/
+
+    TEST(LRUCache, clearEmptyCache) {
+        LRUCache<std::string, std::string> c(10);
+        c.clear();
+        ASSERT_EQ(c.size(), 0);
+    }
+
+    TEST(LRUCache, clearNonEmptyCache) {
+        LRUCache<std::string, std::string> c(10);
+        c.upsert("one", "eins");
+        c.upsert("two", "zwei");
+        c.upsert("three", "drei");
+        ASSERT_EQ(c.size(), 3);
+        c.clear();
+        ASSERT_EQ(c.size(), 0);
+    }
+
+    /* ----------------------------------------------------------------------------
+     * erase
+     * --------------------------------------------------------------------------*/
+
+    TEST(LRUCache, eraseFromEmptyCache) {
+        LRUCache<std::string, std::string> c(10);
+        ASSERT_EQ(c.erase("foo"), false);
+        ASSERT_EQ(c.size(), 0);
+    }
+
+    TEST(LRUCache, eraseMissingFromNonEmptyCache) {
+        LRUCache<std::string, std::string> c(10);
+        c.upsert("one", "eins");
+        ASSERT_EQ(c.erase("foo"), false);
+        ASSERT_EQ(c.size(), 1);
+        ASSERT_EQ(c.get("one").value_or("error"), "eins");
+    }
+
+    TEST(LRUCache, eraseFromNonEmptyCache) {
+        LRUCache<std::string, std::string> c(10);
+        c.upsert("one", "eins");
+        ASSERT_EQ(c.erase("one"), true);
+        ASSERT_EQ(c.size(), 0);
+        ASSERT_EQ(c.get("one").value_or("empty"), "empty");
+    }
+}
diff --git a/src/libutil/url.cc b/src/libutil/url.cc
index 5d5328e5d..88c09eef9 100644
--- a/src/libutil/url.cc
+++ b/src/libutil/url.cc
@@ -4,6 +4,7 @@
 namespace nix {
 
 std::regex refRegex(refRegexS, std::regex::ECMAScript);
+std::regex badGitRefRegex(badGitRefRegexS, std::regex::ECMAScript);
 std::regex revRegex(revRegexS, std::regex::ECMAScript);
 std::regex flakeIdRegex(flakeIdRegexS, std::regex::ECMAScript);
 
diff --git a/src/libutil/url.hh b/src/libutil/url.hh
index 1503023a2..4a0d4071b 100644
--- a/src/libutil/url.hh
+++ b/src/libutil/url.hh
@@ -49,6 +49,12 @@ const static std::string pathRegex = "(?:" + segmentRegex + "(?:/" + segmentRege
 const static std::string refRegexS = "[a-zA-Z0-9][a-zA-Z0-9_.-]*"; // FIXME: check
 extern std::regex refRegex;
 
+// Instead of defining what a good Git Ref is, we define what a bad Git Ref is
+// This is because of the definition of a ref in refs.c in https://github.com/git/git
+// See tests/fetchGitRefs.sh for the full definition
+const static std::string badGitRefRegexS = "//|^[./]|/\\.|\\.\\.|[[:cntrl:][:space:]:?^~\[]|\\\\|\\*|\\.lock$|\\.lock/|@\\{|[/.]$|^@$|^$";
+extern std::regex badGitRefRegex;
+
 // A Git revision (a SHA-1 commit hash).
 const static std::string revRegexS = "[0-9a-fA-F]{40}";
 extern std::regex revRegex;
diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc
index dc34a83ca..b92597cc2 100644
--- a/src/nix/add-to-store.cc
+++ b/src/nix/add-to-store.cc
@@ -53,8 +53,10 @@ struct CmdAddToStore : MixDryRun, StoreCommand
             .hash = info.narHash
         };
 
-        if (!dryRun)
-            store->addToStore(info, sink.s);
+        if (!dryRun) {
+            auto source = StringSource { *sink.s };
+            store->addToStore(info, source);
+        }
 
         logger->stdout("%s", store->printStorePath(info.path));
     }