From cd6dbf951a0ea903d0e9f2671fb127ae419e5ed2 Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Mon, 8 Jun 2020 11:34:37 +0200 Subject: [PATCH 1/5] Add compression unit tests --- src/libutil/tests/compression.cc | 78 ++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 src/libutil/tests/compression.cc diff --git a/src/libutil/tests/compression.cc b/src/libutil/tests/compression.cc new file mode 100644 index 000000000..5b7a2c5b9 --- /dev/null +++ b/src/libutil/tests/compression.cc @@ -0,0 +1,78 @@ +#include "compression.hh" +#include + +namespace nix { + + /* ---------------------------------------------------------------------------- + * compress / decompress + * --------------------------------------------------------------------------*/ + + TEST(compress, compressWithUnknownMethod) { + ASSERT_THROW(compress("invalid-method", "something-to-compress"), UnknownCompressionMethod); + } + + TEST(compress, noneMethodDoesNothingToTheInput) { + ref 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 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 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 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); + } + +} From 237d88c97e1f08f8c1513261ac5fea847d5917ff Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 19 Jun 2020 14:47:10 +0000 Subject: [PATCH 2/5] FileSystemHash -> DerivationOutputHash --- src/libexpr/primops.cc | 6 +++--- src/libstore/derivations.cc | 10 +++++----- src/libstore/derivations.hh | 12 ++++++------ src/nix/develop.cc | 2 +- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 8288dd6a1..175fccf39 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -776,7 +776,7 @@ 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 { ingestionMethod, std::move(h) }, }); } @@ -792,7 +792,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * drv.outputs.insert_or_assign(i, DerivationOutput { .path = StorePath::dummy, - .hash = std::optional {}, + .hash = std::optional {}, }); } @@ -804,7 +804,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(), + .hash = std::optional(), }); } } diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 826b336e0..51a01feac 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -8,7 +8,7 @@ namespace nix { -std::string FileSystemHash::printMethodAlgo() const { +std::string DerivationOutputHash::printMethodAlgo() const { return makeFileIngestionPrefix(method) + printHashType(hash.type); } @@ -113,7 +113,7 @@ static DerivationOutput parseDerivationOutput(const Store & store, istringstream expect(str, ","); const auto hash = parseString(str); expect(str, ")"); - std::optional fsh; + std::optional fsh; if (hashAlgo != "") { auto method = FileIngestionMethod::Flat; if (string(hashAlgo, 0, 2) == "r:") { @@ -123,7 +123,7 @@ static DerivationOutput parseDerivationOutput(const Store & store, istringstream const HashType hashType = parseHashType(hashAlgo); if (hashType == htUnknown) throw Error("unknown hash hashAlgorithm '%s'", hashAlgo); - fsh = FileSystemHash { + fsh = DerivationOutputHash { std::move(method), Hash(hash, hashType), }; @@ -413,7 +413,7 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store) auto hashAlgo = readString(in); const auto hash = readString(in); - std::optional fsh; + std::optional fsh; if (hashAlgo != "") { auto method = FileIngestionMethod::Flat; if (string(hashAlgo, 0, 2) == "r:") { @@ -423,7 +423,7 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store) const HashType hashType = parseHashType(hashAlgo); if (hashType == htUnknown) throw Error("unknown hash hashAlgorithm '%s'", hashAlgo); - fsh = FileSystemHash { + fsh = DerivationOutputHash { std::move(method), Hash(hash, hashType), }; diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 3b4eb7ec4..f3eff2748 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -13,23 +13,23 @@ namespace nix { /* Abstract syntax of derivations. */ /// Pair of a hash, and how the file system was ingested -struct FileSystemHash { +struct DerivationOutputHash { FileIngestionMethod method; Hash hash; - FileSystemHash(FileIngestionMethod method, Hash hash) + DerivationOutputHash(FileIngestionMethod method, Hash hash) : method(std::move(method)) , hash(std::move(hash)) { } - FileSystemHash(const FileSystemHash &) = default; - FileSystemHash(FileSystemHash &&) = default; - FileSystemHash & operator = (const FileSystemHash &) = default; + DerivationOutputHash(const DerivationOutputHash &) = default; + DerivationOutputHash(DerivationOutputHash &&) = default; + DerivationOutputHash & operator = (const DerivationOutputHash &) = default; std::string printMethodAlgo() const; }; struct DerivationOutput { StorePath path; - std::optional hash; /* hash used for expected hash computation */ + std::optional hash; /* hash used for expected hash computation */ void parseHashInfo(FileIngestionMethod & recursive, Hash & hash) const; }; diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 8abb710c2..0845c65fa 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -137,7 +137,7 @@ StorePath getDerivationEnvironment(ref store, const StorePath & drvPath) auto shellOutPath = store->makeOutputPath("out", h, drvName); drv.outputs.insert_or_assign("out", DerivationOutput { .path = shellOutPath, - .hash = FileSystemHash { + .hash = DerivationOutputHash { FileIngestionMethod::Flat, Hash { } }, }); From 145d88cb2a160871968285fb1898732090f4e14c Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 19 Jun 2020 14:58:30 +0000 Subject: [PATCH 3/5] Use designated initializers for `DerivationOutputHash` --- src/libexpr/primops.cc | 5 ++++- src/libstore/derivations.cc | 8 ++++---- src/libstore/derivations.hh | 3 --- src/nix/develop.cc | 3 ++- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 175fccf39..bf3c17997 100644 --- 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 = DerivationOutputHash { ingestionMethod, std::move(h) }, + .hash = DerivationOutputHash { + .method = ingestionMethod, + .hash = std::move(h), + }, }); } diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 51a01feac..528b7ccea 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -124,8 +124,8 @@ static DerivationOutput parseDerivationOutput(const Store & store, istringstream if (hashType == htUnknown) throw Error("unknown hash hashAlgorithm '%s'", hashAlgo); fsh = DerivationOutputHash { - std::move(method), - Hash(hash, hashType), + .method = std::move(method), + .hash = Hash(hash, hashType), }; } @@ -424,8 +424,8 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store) if (hashType == htUnknown) throw Error("unknown hash hashAlgorithm '%s'", hashAlgo); fsh = DerivationOutputHash { - std::move(method), - Hash(hash, hashType), + .method = std::move(method), + .hash = Hash(hash, hashType), }; } diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index f3eff2748..292861065 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -20,9 +20,6 @@ struct DerivationOutputHash { : method(std::move(method)) , hash(std::move(hash)) { } - DerivationOutputHash(const DerivationOutputHash &) = default; - DerivationOutputHash(DerivationOutputHash &&) = default; - DerivationOutputHash & operator = (const DerivationOutputHash &) = default; std::string printMethodAlgo() const; }; diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 0845c65fa..8b85caf82 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -138,7 +138,8 @@ StorePath getDerivationEnvironment(ref store, const StorePath & drvPath) drv.outputs.insert_or_assign("out", DerivationOutput { .path = shellOutPath, .hash = DerivationOutputHash { - FileIngestionMethod::Flat, Hash { } + .method = FileIngestionMethod::Flat, + .hash = Hash { }, }, }); drv.env["out"] = store->printStorePath(shellOutPath); From b90cac3bad41715c2fefc9d725630d0abb9af725 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 19 Jun 2020 15:00:38 +0000 Subject: [PATCH 4/5] Remove uneeded `= default` for Hash --- src/libutil/hash.hh | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 92e10ee6e..180fb7633 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -44,12 +44,6 @@ struct Hash string. */ Hash(std::string_view s, HashType type = htUnknown); - Hash(const Hash &) = default; - - Hash(Hash &&) = default; - - Hash & operator = (const Hash &) = default; - void init(); /* Check whether a hash is set. */ From fb39a5e00c03c8fb3b893263e1d2b151c3ba11aa Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 19 Jun 2020 15:11:11 +0000 Subject: [PATCH 5/5] Remove unneeded constructor for `DerivationOutputHash` --- src/libstore/derivations.hh | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 292861065..7b677ca49 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -16,10 +16,6 @@ namespace nix { struct DerivationOutputHash { FileIngestionMethod method; Hash hash; - DerivationOutputHash(FileIngestionMethod method, Hash hash) - : method(std::move(method)) - , hash(std::move(hash)) - { } std::string printMethodAlgo() const; };