From 9fbc31a65bab50cd60a882517b3c8030485ce096 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 15 Aug 2020 16:41:28 +0000 Subject: [PATCH 1/7] Get rid of Hash::dummy from BinaryCacheStore --- src/libstore/binary-cache-store.cc | 102 +++++++++++++++++------------ src/libstore/binary-cache-store.hh | 7 ++ src/libstore/store-api.hh | 4 +- 3 files changed, 69 insertions(+), 44 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index ebc0bd6a4..6679eb16f 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -142,17 +142,10 @@ struct FileSource : FdSource } }; -void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource, - RepairFlag repair, CheckSigsFlag checkSigs) +StorePath BinaryCacheStore::addToStoreCommon( + Source & narSource, RepairFlag repair, CheckSigsFlag checkSigs, + std::function mkInfo) { - assert(info.narSize); - - if (!repair && isValidPath(info.path)) { - // FIXME: copyNAR -> null sink - narSource.drain(); - return; - } - auto [fdTemp, fnTemp] = createTempFile(); AutoDelete autoDelete(fnTemp); @@ -162,13 +155,15 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource /* Read the NAR simultaneously into a CompressionSink+FileSink (to write the compressed NAR to disk), into a HashSink (to get the NAR hash), and into a NarAccessor (to get the NAR listing). */ - HashSink fileHashSink(htSHA256); + HashSink fileHashSink { htSHA256 }; std::shared_ptr narAccessor; + HashSink narHashSink { htSHA256 }; { FdSink fileSink(fdTemp.get()); - TeeSink teeSink(fileSink, fileHashSink); - auto compressionSink = makeCompressionSink(compression, teeSink); - TeeSource teeSource(narSource, *compressionSink); + TeeSink teeSinkCompressed { fileSink, fileHashSink }; + auto compressionSink = makeCompressionSink(compression, teeSinkCompressed); + TeeSink teeSinkUncompressed { *compressionSink, narHashSink }; + TeeSource teeSource { narSource, teeSinkUncompressed }; narAccessor = makeNarAccessor(teeSource); compressionSink->finish(); fileSink.flush(); @@ -176,9 +171,10 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource auto now2 = std::chrono::steady_clock::now(); + auto info = mkInfo(narHashSink.finish()); auto narInfo = make_ref(info); - narInfo->narSize = info.narSize; - narInfo->narHash = info.narHash; + narInfo->narSize = info.narSize; // FIXME needed? + narInfo->narHash = info.narHash; // FIXME needed? narInfo->compression = compression; auto [fileHash, fileSize] = fileHashSink.finish(); narInfo->fileHash = fileHash; @@ -300,6 +296,41 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource writeNarInfo(narInfo); stats.narInfoWrite++; + + return narInfo->path; +} + +void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource, + RepairFlag repair, CheckSigsFlag checkSigs) +{ + if (!repair && isValidPath(info.path)) { + // FIXME: copyNAR -> null sink + narSource.drain(); + return; + } + + (void) addToStoreCommon(narSource, repair, checkSigs, {[&](HashResult nar) { + /* FIXME reinstate these, once we can correctly do hash modulo sink as + needed. */ + // assert(info.narHash == nar.first); + // assert(info.narSize == nar.second); + return info; + }}); +} + +StorePath BinaryCacheStore::addToStoreFromDump(Source & dump, const string & name, + FileIngestionMethod method, HashType hashAlgo, RepairFlag repair) +{ + if (method != FileIngestionMethod::Recursive || hashAlgo != htSHA256) + unsupported("addToStoreFromDump"); + return addToStoreCommon(dump, repair, CheckSigs, [&](HashResult nar) { + ValidPathInfo info { + makeFixedOutputPath(method, nar.first, name), + nar.first, + }; + info.narSize = nar.second; + return info; + }); } bool BinaryCacheStore::isValidPathUncached(const StorePath & storePath) @@ -367,11 +398,9 @@ void BinaryCacheStore::queryPathInfoUncached(const StorePath & storePath, StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath, FileIngestionMethod method, HashType hashAlgo, PathFilter & filter, RepairFlag repair) { - // FIXME: some cut&paste from LocalStore::addToStore(). - - /* Read the whole path into memory. This is not a very scalable - method for very large paths, but `copyPath' is mainly used for - small files. */ + /* FIXME: Make BinaryCacheStore::addToStoreCommon support + non-recursive+sha256 so we can just use the default + implementation of this method in terms of addToStoreFromDump. */ StringSink sink; std::optional h; if (method == FileIngestionMethod::Recursive) { @@ -383,34 +412,25 @@ StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath h = hashString(hashAlgo, s); } - ValidPathInfo info { - makeFixedOutputPath(method, *h, name), - Hash::dummy, // Will be fixed in addToStore, which recomputes nar hash - }; - auto source = StringSource { *sink.s }; - addToStore(info, source, repair, CheckSigs); - - return std::move(info.path); + return addToStoreFromDump(source, name, FileIngestionMethod::Recursive, htSHA256, repair); } StorePath BinaryCacheStore::addTextToStore(const string & name, const string & s, const StorePathSet & references, RepairFlag repair) { - ValidPathInfo info { - computeStorePathForText(name, s, references), - Hash::dummy, // Will be fixed in addToStore, which recomputes nar hash - }; - info.references = references; + auto path = computeStorePathForText(name, s, references); - if (repair || !isValidPath(info.path)) { - StringSink sink; - dumpString(s, sink); - auto source = StringSource { *sink.s }; - addToStore(info, source, repair, CheckSigs); - } + if (!repair && isValidPath(path)) + return path; - return std::move(info.path); + auto source = StringSource { s }; + return addToStoreCommon(source, repair, CheckSigs, [&](HashResult nar) { + ValidPathInfo info { path, nar.first }; + info.narSize = nar.second; + info.references = references; + return info; + }); } ref BinaryCacheStore::getFSAccessor() diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index 4b779cdd4..ce69ad3b4 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -72,6 +72,10 @@ private: void writeNarInfo(ref narInfo); + StorePath addToStoreCommon( + Source & narSource, RepairFlag repair, CheckSigsFlag checkSigs, + std::function mkInfo); + public: bool isValidPathUncached(const StorePath & path) override; @@ -85,6 +89,9 @@ public: void addToStore(const ValidPathInfo & info, Source & narSource, RepairFlag repair, CheckSigsFlag checkSigs) override; + StorePath addToStoreFromDump(Source & dump, const string & name, + FileIngestionMethod method, HashType hashAlgo, RepairFlag repair) override; + StorePath addToStore(const string & name, const Path & srcPath, FileIngestionMethod method, HashType hashAlgo, PathFilter & filter, RepairFlag repair) override; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 591140874..85a84d080 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -454,9 +454,7 @@ public: // FIXME: remove? virtual StorePath addToStoreFromDump(Source & dump, const string & name, FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) - { - throw Error("addToStoreFromDump() is not supported by this store"); - } + { unsupported("addToStoreFromDump"); } /* Like addToStore, but the contents written to the output path is a regular file containing the given string. */ From 412b3a54fb02cdf49cb084a925bd14c24e14aea8 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 23 Sep 2020 10:36:55 -0400 Subject: [PATCH 2/7] Clarify FIXME in `BinaryCacheStore::addToStoreCommon` Co-authored-by: Robert Hensing --- src/libstore/binary-cache-store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 6679eb16f..817661869 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -311,7 +311,7 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource (void) addToStoreCommon(narSource, repair, checkSigs, {[&](HashResult nar) { /* FIXME reinstate these, once we can correctly do hash modulo sink as - needed. */ + needed. We need to throw here in case we uploaded a corrupted store path. */ // assert(info.narHash == nar.first); // assert(info.narSize == nar.second); return info; From 3f226f71c185b2fbaaabb01bd0f3ba3cd4a39612 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 23 Sep 2020 14:40:41 +0000 Subject: [PATCH 3/7] Return more info from `BinaryCacheStore::addToStoreCommon` We don't need it yet, but we could/should in the future, and it's a cost-free change since we already have the reference. I like it. Co-authored-by: Robert Hensing --- src/libstore/binary-cache-store.cc | 8 ++++---- src/libstore/binary-cache-store.hh | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 817661869..f7a52a296 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -142,7 +142,7 @@ struct FileSource : FdSource } }; -StorePath BinaryCacheStore::addToStoreCommon( +ref BinaryCacheStore::addToStoreCommon( Source & narSource, RepairFlag repair, CheckSigsFlag checkSigs, std::function mkInfo) { @@ -297,7 +297,7 @@ StorePath BinaryCacheStore::addToStoreCommon( stats.narInfoWrite++; - return narInfo->path; + return narInfo; } void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource, @@ -330,7 +330,7 @@ StorePath BinaryCacheStore::addToStoreFromDump(Source & dump, const string & nam }; info.narSize = nar.second; return info; - }); + })->path; } bool BinaryCacheStore::isValidPathUncached(const StorePath & storePath) @@ -430,7 +430,7 @@ StorePath BinaryCacheStore::addTextToStore(const string & name, const string & s info.narSize = nar.second; info.references = references; return info; - }); + })->path; } ref BinaryCacheStore::getFSAccessor() diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index ce69ad3b4..5224d7ec8 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -72,7 +72,7 @@ private: void writeNarInfo(ref narInfo); - StorePath addToStoreCommon( + ref addToStoreCommon( Source & narSource, RepairFlag repair, CheckSigsFlag checkSigs, std::function mkInfo); From 5db83dd771b92bcacb0cd4dea7d4e06f767769ca Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 26 Sep 2020 03:21:36 +0000 Subject: [PATCH 4/7] BinaryCacheStore::addTextToStore include CA field --- src/libstore/binary-cache-store.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index f7a52a296..a3a73fe27 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -419,7 +419,8 @@ StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath StorePath BinaryCacheStore::addTextToStore(const string & name, const string & s, const StorePathSet & references, RepairFlag repair) { - auto path = computeStorePathForText(name, s, references); + auto textHash = hashString(htSHA256, s); + auto path = makeTextPath(name, textHash, references); if (!repair && isValidPath(path)) return path; @@ -428,6 +429,7 @@ StorePath BinaryCacheStore::addTextToStore(const string & name, const string & s return addToStoreCommon(source, repair, CheckSigs, [&](HashResult nar) { ValidPathInfo info { path, nar.first }; info.narSize = nar.second; + info.ca = TextHash { textHash }; info.references = references; return info; })->path; From 1832436526307ac92baec0146b89e9a5cf3aca35 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 26 Sep 2020 04:56:29 +0000 Subject: [PATCH 5/7] Fix up BinaryCacheStore::addToStore taking a path --- src/libstore/binary-cache-store.cc | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index a3a73fe27..b34b10fd1 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -401,19 +401,30 @@ StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath /* FIXME: Make BinaryCacheStore::addToStoreCommon support non-recursive+sha256 so we can just use the default implementation of this method in terms of addToStoreFromDump. */ - StringSink sink; - std::optional h; + + HashSink sink { hashAlgo }; if (method == FileIngestionMethod::Recursive) { dumpPath(srcPath, sink, filter); - h = hashString(hashAlgo, *sink.s); } else { - auto s = readFile(srcPath); - dumpString(s, sink); - h = hashString(hashAlgo, s); + readFile(srcPath, sink); } + auto h = sink.finish().first; - auto source = StringSource { *sink.s }; - return addToStoreFromDump(source, name, FileIngestionMethod::Recursive, htSHA256, repair); + auto source = sinkToSource([&](Sink & sink) { + dumpPath(srcPath, sink, filter); + }); + return addToStoreCommon(*source, repair, CheckSigs, [&](HashResult nar) { + ValidPathInfo info { + makeFixedOutputPath(method, h, name), + nar.first, + }; + info.narSize = nar.second; + info.ca = FixedOutputHash { + .method = method, + .hash = h, + }; + return info; + })->path; } StorePath BinaryCacheStore::addTextToStore(const string & name, const string & s, From 25fffdda865e51f68e72e0ca1775800b60391820 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 26 Sep 2020 10:17:30 -0400 Subject: [PATCH 6/7] Remove redundant nar hash and size setting Co-authored-by: Robert Hensing --- src/libstore/binary-cache-store.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index b34b10fd1..59d02cab5 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -173,8 +173,6 @@ ref BinaryCacheStore::addToStoreCommon( auto info = mkInfo(narHashSink.finish()); auto narInfo = make_ref(info); - narInfo->narSize = info.narSize; // FIXME needed? - narInfo->narHash = info.narHash; // FIXME needed? narInfo->compression = compression; auto [fileHash, fileSize] = fileHashSink.finish(); narInfo->fileHash = fileHash; From 6c31297d80b71ab9c2c1084fae22f726f7d89daa Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 28 Sep 2020 11:32:58 -0400 Subject: [PATCH 7/7] Update src/libstore/binary-cache-store.cc Co-authored-by: Eelco Dolstra --- src/libstore/binary-cache-store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 59d02cab5..f6224d6a0 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -307,7 +307,7 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource return; } - (void) addToStoreCommon(narSource, repair, checkSigs, {[&](HashResult nar) { + addToStoreCommon(narSource, repair, checkSigs, {[&](HashResult nar) { /* FIXME reinstate these, once we can correctly do hash modulo sink as needed. We need to throw here in case we uploaded a corrupted store path. */ // assert(info.narHash == nar.first);