Merge remote-tracking branch 'upstream/master' into better-ca-parse-errors

8 files changed, 144 insertions, 116 deletions

diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc
index 30150eeba..9682db730 100644
--- a/src/libstore/binary-cache-store.cc
+++ b/src/libstore/binary-cache-store.cc
@@ -153,6 +153,8 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource
 
     auto [fdTemp, fnTemp] = createTempFile();
 
+    AutoDelete autoDelete(fnTemp);
+
     auto now1 = std::chrono::steady_clock::now();
 
     /* Read the NAR simultaneously into a CompressionSink+FileSink (to
@@ -167,6 +169,7 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource
     TeeSource teeSource(narSource, *compressionSink);
     narAccessor = makeNarAccessor(teeSource);
     compressionSink->finish();
+    fileSink.flush();
     }
 
     auto now2 = std::chrono::steady_clock::now();
@@ -280,7 +283,7 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource
     if (repair || !fileExists(narInfo->url)) {
         stats.narWrite++;
         upsertFile(narInfo->url,
-            std::make_shared<std::fstream>(fnTemp, std::ios_base::in),
+            std::make_shared<std::fstream>(fnTemp, std::ios_base::in | std::ios_base::binary),
             "application/x-nix-nar");
     } else
         stats.narWriteAverted++;
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index 4c4d0bdc2..6b0548538 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -1038,20 +1038,6 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
 }
 
 
-StorePath LocalStore::addToStore(const string & name, const Path & _srcPath,
-    FileIngestionMethod method, HashType hashAlgo, PathFilter & filter, RepairFlag repair)
-{
-    Path srcPath(absPath(_srcPath));
-    auto source = sinkToSource([&](Sink & sink) {
-        if (method == FileIngestionMethod::Recursive)
-            dumpPath(srcPath, sink, filter);
-        else
-            readFile(srcPath, sink);
-    });
-    return addToStoreFromDump(*source, name, method, hashAlgo, repair);
-}
-
-
 StorePath LocalStore::addToStoreFromDump(Source & source0, const string & name,
     FileIngestionMethod method, HashType hashAlgo, RepairFlag repair)
 {
diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index 5a87ec2f8..31e6587ac 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -145,16 +145,8 @@ public:
     void addToStore(const ValidPathInfo & info, Source & source,
         RepairFlag repair, CheckSigsFlag checkSigs) override;
 
-    StorePath addToStore(const string & name, const Path & srcPath,
-        FileIngestionMethod method, HashType hashAlgo,
-        PathFilter & filter, RepairFlag repair) override;
-
-    /* Like addToStore(), but the contents of the path are contained
-       in `dump', which is either a NAR serialisation (if recursive ==
-       true) or simply the contents of a regular file (if recursive ==
-       false). */
     StorePath addToStoreFromDump(Source & dump, const string & name,
-        FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) override;
+        FileIngestionMethod method, HashType hashAlgo, RepairFlag repair) override;
 
     StorePath addTextToStore(const string & name, const string & s,
         const StorePathSet & references, RepairFlag repair) override;
diff --git a/src/libstore/path-info.hh b/src/libstore/path-info.hh
new file mode 100644
index 000000000..2a015ea3c
--- /dev/null
+++ b/src/libstore/path-info.hh
@@ -0,0 +1,111 @@
+#pragma once
+
+#include "path.hh"
+#include "hash.hh"
+#include "content-address.hh"
+
+#include <string>
+#include <optional>
+
+namespace nix {
+
+
+class Store;
+
+
+struct SubstitutablePathInfo
+{
+    std::optional<StorePath> deriver;
+    StorePathSet references;
+    uint64_t downloadSize; /* 0 = unknown or inapplicable */
+    uint64_t narSize; /* 0 = unknown */
+};
+
+typedef std::map<StorePath, SubstitutablePathInfo> SubstitutablePathInfos;
+
+
+struct ValidPathInfo
+{
+    StorePath path;
+    std::optional<StorePath> deriver;
+    // TODO document this
+    std::optional<Hash> narHash;
+    StorePathSet references;
+    time_t registrationTime = 0;
+    uint64_t narSize = 0; // 0 = unknown
+    uint64_t id; // internal use only
+
+    /* Whether the path is ultimately trusted, that is, it's a
+       derivation output that was built locally. */
+    bool ultimate = false;
+
+    StringSet sigs; // note: not necessarily verified
+
+    /* If non-empty, an assertion that the path is content-addressed,
+       i.e., that the store path is computed from a cryptographic hash
+       of the contents of the path, plus some other bits of data like
+       the "name" part of the path. Such a path doesn't need
+       signatures, since we don't have to trust anybody's claim that
+       the path is the output of a particular derivation. (In the
+       extensional store model, we have to trust that the *contents*
+       of an output path of a derivation were actually produced by
+       that derivation. In the intensional model, we have to trust
+       that a particular output path was produced by a derivation; the
+       path then implies the contents.)
+
+       Ideally, the content-addressability assertion would just be a Boolean,
+       and the store path would be computed from the name component, ‘narHash’
+       and ‘references’. However, we support many types of content addresses.
+    */
+    std::optional<ContentAddress> ca;
+
+    bool operator == (const ValidPathInfo & i) const
+    {
+        return
+            path == i.path
+            && narHash == i.narHash
+            && references == i.references;
+    }
+
+    /* Return a fingerprint of the store path to be used in binary
+       cache signatures. It contains the store path, the base-32
+       SHA-256 hash of the NAR serialisation of the path, the size of
+       the NAR, and the sorted references. The size field is strictly
+       speaking superfluous, but might prevent endless/excessive data
+       attacks. */
+    std::string fingerprint(const Store & store) const;
+
+    void sign(const Store & store, const SecretKey & secretKey);
+
+    /* Return true iff the path is verifiably content-addressed. */
+    bool isContentAddressed(const Store & store) const;
+
+    /* Functions to view references + hasSelfReference as one set, mainly for
+       compatibility's sake. */
+    StorePathSet referencesPossiblyToSelf() const;
+    void insertReferencePossiblyToSelf(StorePath && ref);
+    void setReferencesPossiblyToSelf(StorePathSet && refs);
+
+    static const size_t maxSigs = std::numeric_limits<size_t>::max();
+
+    /* Return the number of signatures on this .narinfo that were
+       produced by one of the specified keys, or maxSigs if the path
+       is content-addressed. */
+    size_t checkSignatures(const Store & store, const PublicKeys & publicKeys) const;
+
+    /* Verify a single signature. */
+    bool checkSignature(const Store & store, const PublicKeys & publicKeys, const std::string & sig) const;
+
+    Strings shortRefs() const;
+
+    ValidPathInfo(const ValidPathInfo & other) = default;
+
+    ValidPathInfo(StorePath && path) : path(std::move(path)) { };
+    ValidPathInfo(const StorePath & path) : path(path) { };
+
+    virtual ~ValidPathInfo() { }
+};
+
+typedef list<ValidPathInfo> ValidPathInfos;
+
+}
diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc
index 67935f3ba..a0a446bd3 100644
--- a/src/libstore/s3-binary-cache-store.cc
+++ b/src/libstore/s3-binary-cache-store.cc
@@ -266,6 +266,10 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore
         const std::string & mimeType,
         const std::string & contentEncoding)
     {
+        istream->seekg(0, istream->end);
+        auto size = istream->tellg();
+        istream->seekg(0, istream->beg);
+
         auto maxThreads = std::thread::hardware_concurrency();
 
         static std::shared_ptr<Aws::Utils::Threading::PooledThreadExecutor>
@@ -343,10 +347,11 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore
             std::chrono::duration_cast<std::chrono::milliseconds>(now2 - now1)
                 .count();
 
-        printInfo("uploaded 's3://%s/%s' in %d ms",
-            bucketName, path, duration);
+        printInfo("uploaded 's3://%s/%s' (%d bytes) in %d ms",
+            bucketName, path, size, duration);
 
         stats.putTimeMs += duration;
+        stats.putBytes += std::max(size, (decltype(size)) 0);
         stats.put++;
     }
 
diff --git a/src/libstore/s3-binary-cache-store.hh b/src/libstore/s3-binary-cache-store.hh
index b2b75d498..4d43fe4d2 100644
--- a/src/libstore/s3-binary-cache-store.hh
+++ b/src/libstore/s3-binary-cache-store.hh
@@ -19,6 +19,7 @@ public:
     struct Stats
     {
         std::atomic<uint64_t> put{0};
+        std::atomic<uint64_t> putBytes{0};
         std::atomic<uint64_t> putTimeMs{0};
         std::atomic<uint64_t> get{0};
         std::atomic<uint64_t> getBytes{0};
diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
index 76324cf08..ea666f098 100644
--- a/src/libstore/store-api.cc
+++ b/src/libstore/store-api.cc
@@ -239,6 +239,20 @@ StorePath Store::computeStorePathForText(const string & name, const string & s,
 }
 
 
+StorePath Store::addToStore(const string & name, const Path & _srcPath,
+    FileIngestionMethod method, HashType hashAlgo, PathFilter & filter, RepairFlag repair)
+{
+    Path srcPath(absPath(_srcPath));
+    auto source = sinkToSource([&](Sink & sink) {
+        if (method == FileIngestionMethod::Recursive)
+            dumpPath(srcPath, sink, filter);
+        else
+            readFile(srcPath, sink);
+    });
+    return addToStoreFromDump(*source, name, method, hashAlgo, repair);
+}
+
+
 /*
 The aim of this function is to compute in one pass the correct ValidPathInfo for
 the files that we are trying to add to the store. To accomplish that in one
diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh
index 49da19496..e94d975c5 100644
--- a/src/libstore/store-api.hh
+++ b/src/libstore/store-api.hh
@@ -10,6 +10,7 @@
 #include "globals.hh"
 #include "config.hh"
 #include "derivations.hh"
+#include "path-info.hh"
 
 #include <atomic>
 #include <limits>
@@ -101,95 +102,6 @@ struct GCResults
 };
 
 
-struct SubstitutablePathInfo
-{
-    std::optional<StorePath> deriver;
-    StorePathSet references;
-    uint64_t downloadSize; /* 0 = unknown or inapplicable */
-    uint64_t narSize; /* 0 = unknown */
-};
-
-typedef std::map<StorePath, SubstitutablePathInfo> SubstitutablePathInfos;
-
-struct ValidPathInfo
-{
-    StorePath path;
-    std::optional<StorePath> deriver;
-    // TODO document this
-    std::optional<Hash> narHash;
-    StorePathSet references;
-    time_t registrationTime = 0;
-    uint64_t narSize = 0; // 0 = unknown
-    uint64_t id; // internal use only
-
-    /* Whether the path is ultimately trusted, that is, it's a
-       derivation output that was built locally. */
-    bool ultimate = false;
-
-    StringSet sigs; // note: not necessarily verified
-
-    /* If non-empty, an assertion that the path is content-addressed,
-       i.e., that the store path is computed from a cryptographic hash
-       of the contents of the path, plus some other bits of data like
-       the "name" part of the path. Such a path doesn't need
-       signatures, since we don't have to trust anybody's claim that
-       the path is the output of a particular derivation. (In the
-       extensional store model, we have to trust that the *contents*
-       of an output path of a derivation were actually produced by
-       that derivation. In the intensional model, we have to trust
-       that a particular output path was produced by a derivation; the
-       path then implies the contents.)
-
-       Ideally, the content-addressability assertion would just be a Boolean,
-       and the store path would be computed from the name component, ‘narHash’
-       and ‘references’. However, we support many types of content addresses.
-    */
-    std::optional<ContentAddress> ca;
-
-    bool operator == (const ValidPathInfo & i) const
-    {
-        return
-            path == i.path
-            && narHash == i.narHash
-            && references == i.references;
-    }
-
-    /* Return a fingerprint of the store path to be used in binary
-       cache signatures. It contains the store path, the base-32
-       SHA-256 hash of the NAR serialisation of the path, the size of
-       the NAR, and the sorted references. The size field is strictly
-       speaking superfluous, but might prevent endless/excessive data
-       attacks. */
-    std::string fingerprint(const Store & store) const;
-
-    void sign(const Store & store, const SecretKey & secretKey);
-
-    /* Return true iff the path is verifiably content-addressed. */
-    bool isContentAddressed(const Store & store) const;
-
-    static const size_t maxSigs = std::numeric_limits<size_t>::max();
-
-    /* Return the number of signatures on this .narinfo that were
-       produced by one of the specified keys, or maxSigs if the path
-       is content-addressed. */
-    size_t checkSignatures(const Store & store, const PublicKeys & publicKeys) const;
-
-    /* Verify a single signature. */
-    bool checkSignature(const Store & store, const PublicKeys & publicKeys, const std::string & sig) const;
-
-    Strings shortRefs() const;
-
-    ValidPathInfo(const ValidPathInfo & other) = default;
-
-    ValidPathInfo(StorePath && path) : path(std::move(path)) { };
-    ValidPathInfo(const StorePath & path) : path(path) { };
-
-    virtual ~ValidPathInfo() { }
-};
-
-typedef list<ValidPathInfo> ValidPathInfos;
-
-
 enum BuildMode { bmNormal, bmRepair, bmCheck };
 
 
@@ -456,7 +368,7 @@ public:
        libutil/archive.hh). */
     virtual StorePath addToStore(const string & name, const Path & srcPath,
         FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256,
-        PathFilter & filter = defaultPathFilter, RepairFlag repair = NoRepair) = 0;
+        PathFilter & filter = defaultPathFilter, RepairFlag repair = NoRepair);
 
     /* Copy the contents of a path to the store and register the
        validity the resulting path, using a constant amount of
@@ -465,6 +377,10 @@ public:
         FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256,
         std::optional<Hash> expectedCAHash = {});
 
+    /* Like addToStore(), but the contents of the path are contained
+       in `dump', which is either a NAR serialisation (if recursive ==
+       true) or simply the contents of a regular file (if recursive ==
+       false). */
     // FIXME: remove?
     virtual StorePath addToStoreFromDump(Source & dump, const string & name,
         FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair)