Merge remote-tracking branch 'me/no-stringly-typed-derivation-output'…

… into validPathInfo-ca-proper-datatype
NixOS · Jun 19, 2020 · babb27c · babb27c
2 parents 3fc58a9 + fb39a5e
commit babb27c
Show file tree

Hide file tree

Showing 14 changed files with 108 additions and 42 deletions.
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
@@ -776,7 +776,10 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * *
         if (!jsonObject) drv.env["out"] = state.store->printStorePath(outPath);
         drv.outputs.insert_or_assign("out", DerivationOutput {
             .path = std::move(outPath),
-            .hash = FileSystemHash { ingestionMethod, std::move(h) },
+            .hash = DerivationOutputHash {
+                .method = ingestionMethod,
+                .hash = std::move(h),
+            },
         });
     }
 
@@ -792,7 +795,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * *
             drv.outputs.insert_or_assign(i,
                 DerivationOutput {
                     .path = StorePath::dummy,
-                    .hash = std::optional<FileSystemHash> {},
+                    .hash = std::optional<DerivationOutputHash> {},
                 });
         }
 
@@ -804,7 +807,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * *
             drv.outputs.insert_or_assign(i,
                 DerivationOutput {
                     .path = std::move(outPath),
-                    .hash = std::optional<FileSystemHash>(),
+                    .hash = std::optional<DerivationOutputHash>(),
                 });
         }
     }

diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc
@@ -70,7 +70,7 @@ DownloadFileResult downloadFile(
         ValidPathInfo info(store->makeFixedOutputPath(FileIngestionMethod::Flat, hash, name));
         info.narHash = hashString(htSHA256, *sink.s);
         info.narSize = sink.s->size();
-        info.ca = FileSystemHash {
+        info.ca = FixedOutputHash {
             FileIngestionMethod::Flat,
             hash,
         };

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
@@ -3764,7 +3764,7 @@ void DerivationGoal::registerOutputs()
             else
                 assert(worker.store.parseStorePath(path) == dest);
 
-            ca = FileSystemHash { i.second.hash->method, h2 };
+            ca = FixedOutputHash { i.second.hash->method, h2 };
         }
 
         /* Get rid of all weird permissions.  This also checks that

diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc
@@ -2,7 +2,7 @@
 
 namespace nix {
 
-std::string FileSystemHash::printMethodAlgo() const {
+std::string FixedOutputHash::printMethodAlgo() const {
     return makeFileIngestionPrefix(method) + printHashType(*hash.type);
 }
 
@@ -33,7 +33,7 @@ std::string renderContentAddress(ContentAddress ca) {
         [](TextHash th) {
             return "text:" + th.hash.to_string(Base32, true);
         },
-        [](FileSystemHash fsh) {
+        [](FixedOutputHash fsh) {
             return makeFixedOutputCA(fsh.method, fsh.hash);
         }
     }, ca);
@@ -55,10 +55,10 @@ ContentAddress parseContentAddress(std::string_view rawCa) {
             auto methodAndHash = rawCa.substr(prefixSeparator+1, string::npos);
             if (methodAndHash.substr(0,2) == "r:") {
                 std::string_view hashRaw = methodAndHash.substr(2,string::npos);
-                return FileSystemHash { FileIngestionMethod::Recursive, Hash(string(hashRaw)) };
+                return FixedOutputHash { FileIngestionMethod::Recursive, Hash(string(hashRaw)) };
             } else {
                 std::string_view hashRaw = methodAndHash;
-                return FileSystemHash { FileIngestionMethod::Flat, Hash(string(hashRaw)) };
+                return FixedOutputHash { FileIngestionMethod::Flat, Hash(string(hashRaw)) };
             }
         } else {
             throw Error("parseContentAddress: format not recognized; has to be text or fixed");

diff --git a/src/libstore/content-address.hh b/src/libstore/content-address.hh
@@ -12,22 +12,12 @@ enum struct FileIngestionMethod : uint8_t {
 
 struct TextHash {
     Hash hash;
-    TextHash(const TextHash &) = default;
-    TextHash(TextHash &&) = default;
-    TextHash & operator = (const TextHash &) = default;
 };
 
 /// Pair of a hash, and how the file system was ingested
-struct FileSystemHash {
+struct FixedOutputHash {
     FileIngestionMethod method;
     Hash hash;
-    FileSystemHash(FileIngestionMethod method, Hash hash)
-        : method(std::move(method))
-        , hash(std::move(hash))
-    { }
-    FileSystemHash(const FileSystemHash &) = default;
-    FileSystemHash(FileSystemHash &&) = default;
-    FileSystemHash & operator = (const FileSystemHash &) = default;
     std::string printMethodAlgo() const;
 };
 
@@ -44,7 +34,7 @@ struct FileSystemHash {
 */
 typedef std::variant<
     TextHash, // for paths computed by makeTextPath() / addTextToStore
-    FileSystemHash // for path computed by makeFixedOutputPath
+    FixedOutputHash // for path computed by makeFixedOutputPath
 > ContentAddress;
 
 /* Compute the prefix to the hash algorithm which indicates how the files were

diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc
@@ -108,17 +108,17 @@ static DerivationOutput parseDerivationOutput(const Store & store, istringstream
     expect(str, ","); const auto hash = parseString(str);
     expect(str, ")");
 
-    std::optional<FileSystemHash> fsh;
+    std::optional<DerivationOutputHash> fsh;
     if (hashAlgo != "") {
         auto method = FileIngestionMethod::Flat;
         if (string(hashAlgo, 0, 2) == "r:") {
             method = FileIngestionMethod::Recursive;
             hashAlgo = string(hashAlgo, 2);
         }
         const HashType hashType = parseHashType(hashAlgo);
-        fsh = FileSystemHash {
-            std::move(method),
-            Hash(hash, hashType),
+        fsh = DerivationOutputHash {
+            .method = std::move(method),
+            .hash = Hash(hash, hashType),
         };
     }
 
@@ -406,17 +406,17 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store)
     auto hashAlgo = readString(in);
     const auto hash = readString(in);
 
-    std::optional<FileSystemHash> fsh;
+    std::optional<DerivationOutputHash> fsh;
     if (hashAlgo != "") {
         auto method = FileIngestionMethod::Flat;
         if (string(hashAlgo, 0, 2) == "r:") {
             method = FileIngestionMethod::Recursive;
             hashAlgo = string(hashAlgo, 2);
         }
         const HashType hashType = parseHashType(hashAlgo);
-        fsh = FileSystemHash {
-            std::move(method),
-            Hash(hash, hashType),
+        fsh = DerivationOutputHash {
+            .method = std::move(method),
+            .hash = Hash(hash, hashType),
         };
     }
 

diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh
@@ -16,7 +16,7 @@ namespace nix {
 struct DerivationOutput
 {
     StorePath path;
-    std::optional<FileSystemHash> hash; /* hash used for expected hash computation */
+    std::optional<FixedOutputHash> hash; /* hash used for expected hash computation */
 };
 
 typedef std::map<string, DerivationOutput> DerivationOutputs;

diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
@@ -1079,7 +1079,7 @@ StorePath LocalStore::addToStoreFromDump(const string & dump, const string & nam
             ValidPathInfo info(dstPath);
             info.narHash = hash.first;
             info.narSize = hash.second;
-            info.ca = FileSystemHash { method, h };
+            info.ca = FixedOutputHash { method, h };
             registerValidPath(info);
         }
 

diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
@@ -769,7 +769,7 @@ bool ValidPathInfo::isContentAddressed(const Store & store) const
         [&](TextHash th) {
             return store.makeTextPath(path.name(), th.hash, references);
         },
-        [&](FileSystemHash fsh) {
+        [&](FixedOutputHash fsh) {
             auto refs = references;
             bool hasSelfReference = false;
             if (refs.count(path)) {

diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh
@@ -48,12 +48,6 @@ struct Hash
     // hash type must be part of string
     Hash(std::string_view s);
 
-    Hash(const Hash &) = default;
-
-    Hash(Hash &&) = default;
-
-    Hash & operator = (const Hash &) = default;
-
     void init();
 
     /* Check whether a hash is set. */

diff --git a/src/libutil/tests/compression.cc b/src/libutil/tests/compression.cc
@@ -0,0 +1,78 @@
+#include "compression.hh"
+#include <gtest/gtest.h>
+
+namespace nix {
+
+    /* ----------------------------------------------------------------------------
+     * compress / decompress
+     * --------------------------------------------------------------------------*/
+
+    TEST(compress, compressWithUnknownMethod) {
+        ASSERT_THROW(compress("invalid-method", "something-to-compress"), UnknownCompressionMethod);
+    }
+
+    TEST(compress, noneMethodDoesNothingToTheInput) {
+        ref<std::string> o = compress("none", "this-is-a-test");
+
+        ASSERT_EQ(*o, "this-is-a-test");
+    }
+
+    TEST(decompress, decompressXzCompressed) {
+        auto method = "xz";
+        auto str = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf";
+        ref<std::string> o = decompress(method, *compress(method, str));
+
+        ASSERT_EQ(*o, str);
+    }
+
+    TEST(decompress, decompressBzip2Compressed) {
+        auto method = "bzip2";
+        auto str = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf";
+        ref<std::string> o = decompress(method, *compress(method, str));
+
+        ASSERT_EQ(*o, str);
+    }
+
+    TEST(decompress, decompressBrCompressed) {
+        auto method = "br";
+        auto str = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf";
+        ref<std::string> o = decompress(method, *compress(method, str));
+
+        ASSERT_EQ(*o, str);
+    }
+
+    TEST(decompress, decompressInvalidInputThrowsCompressionError) {
+        auto method = "bzip2";
+        auto str = "this is a string that does not qualify as valid bzip2 data";
+
+        ASSERT_THROW(decompress(method, str), CompressionError);
+    }
+
+    /* ----------------------------------------------------------------------------
+     * compression sinks
+     * --------------------------------------------------------------------------*/
+
+    TEST(makeCompressionSink, noneSinkDoesNothingToInput) {
+        StringSink strSink;
+        auto inputString = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf";
+        auto sink = makeCompressionSink("none", strSink);
+        (*sink)(inputString);
+        sink->finish();
+
+        ASSERT_STREQ((*strSink.s).c_str(), inputString);
+    }
+
+    TEST(makeCompressionSink, compressAndDecompress) {
+        StringSink strSink;
+        auto inputString = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf";
+        auto decompressionSink = makeDecompressionSink("bzip2", strSink);
+        auto sink = makeCompressionSink("bzip2", *decompressionSink);
+
+        (*sink)(inputString);
+        sink->finish();
+        decompressionSink->finish();
+
+        ASSERT_STREQ((*strSink.s).c_str(), inputString);
+    }
+
+}
diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc
@@ -48,7 +48,7 @@ struct CmdAddToStore : MixDryRun, StoreCommand
         ValidPathInfo info(store->makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, *namePart));
         info.narHash = narHash;
         info.narSize = sink.s->size();
-        info.ca = std::optional { FileSystemHash {
+        info.ca = std::optional { FixedOutputHash {
             FileIngestionMethod::Recursive,
             info.narHash,
         } };

diff --git a/src/nix/develop.cc b/src/nix/develop.cc
@@ -137,8 +137,9 @@ StorePath getDerivationEnvironment(ref<Store> store, const StorePath & drvPath)
     auto shellOutPath = store->makeOutputPath("out", h, drvName);
     drv.outputs.insert_or_assign("out", DerivationOutput {
         .path = shellOutPath,
-        .hash = FileSystemHash {
-            FileIngestionMethod::Flat, Hash { }
+        .hash = DerivationOutputHash {
+            .method = FileIngestionMethod::Flat,
+            .hash = Hash { },
         },
     });
     drv.env["out"] = store->printStorePath(shellOutPath);

diff --git a/src/nix/make-content-addressable.cc b/src/nix/make-content-addressable.cc
@@ -82,7 +82,7 @@ struct CmdMakeContentAddressable : StorePathsCommand, MixJSON
             if (hasSelfReference) info.references.insert(info.path);
             info.narHash = narHash;
             info.narSize = sink.s->size();
-            info.ca = FileSystemHash {
+            info.ca = FixedOutputHash {
                 FileIngestionMethod::Recursive,
                 info.narHash,
             };