From bcde5456cc3295061a0726881c3e441444dd6680 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 21 Mar 2020 11:07:16 -0400 Subject: Flip dependency so store-api.hh includes derivations.hh I think it makes more sense to define the data model (derivations), before the operations (store api). --- src/libexpr/get-drvs.cc | 2 +- src/libexpr/primops/context.cc | 2 +- src/libstore/derivations.hh | 3 ++- src/libstore/parsed-derivations.hh | 2 +- src/libstore/store-api.hh | 4 +--- 5 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc index ca9c547fa..16f7b5637 100644 --- a/src/libexpr/get-drvs.cc +++ b/src/libexpr/get-drvs.cc @@ -1,7 +1,7 @@ #include "get-drvs.hh" #include "util.hh" #include "eval-inline.hh" -#include "derivations.hh" +#include "store-api.hh" #include #include diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index 94fa0158c..66d8bab1f 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -1,6 +1,6 @@ #include "primops.hh" #include "eval-inline.hh" -#include "derivations.hh" +#include "store-api.hh" namespace nix { diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 7222d25e5..a3651de23 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -1,8 +1,8 @@ #pragma once +#include "path.hh" #include "types.hh" #include "hash.hh" -#include "store-api.hh" #include @@ -76,6 +76,7 @@ struct Derivation : BasicDerivation class Store; +enum RepairFlag : bool { NoRepair = false, Repair = true }; /* Write a derivation to the Nix store, and return its path. */ StorePath writeDerivation(ref store, diff --git a/src/libstore/parsed-derivations.hh b/src/libstore/parsed-derivations.hh index f4df5dd54..d0cc70f9c 100644 --- a/src/libstore/parsed-derivations.hh +++ b/src/libstore/parsed-derivations.hh @@ -1,4 +1,4 @@ -#include "derivations.hh" +#include "store-api.hh" #include diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 0fa59be6a..396709633 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -8,6 +8,7 @@ #include "sync.hh" #include "globals.hh" #include "config.hh" +#include "derivations.hh" #include #include @@ -30,15 +31,12 @@ MakeError(SubstituteGone, Error); MakeError(SubstituterDisabled, Error); -struct BasicDerivation; -struct Derivation; class FSAccessor; class NarInfoDiskCache; class Store; class JSONPlaceholder; -enum RepairFlag : bool { NoRepair = false, Repair = true }; enum CheckSigsFlag : bool { NoCheckSigs = false, CheckSigs = true }; enum SubstituteFlag : bool { NoSubstitute = false, Substitute = true }; enum AllowInvalidFlag : bool { DisallowInvalid = false, AllowInvalid = true }; -- cgit v1.2.3 From 0e9438b6d381a87946ddda8d4bdd06707f9b0a48 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Mon, 1 Jun 2020 17:32:27 -0400 Subject: Create new file-hash files --- src/libstore/derivations.cc | 5 ----- src/libstore/derivations.hh | 15 +-------------- src/libstore/file-hash.cc | 27 +++++++++++++++++++++++++++ src/libstore/file-hash.hh | 34 ++++++++++++++++++++++++++++++++++ src/libstore/path.hh | 5 ----- src/libstore/store-api.cc | 19 ------------------- src/libstore/store-api.hh | 10 +--------- src/nix/hash.cc | 1 + 8 files changed, 64 insertions(+), 52 deletions(-) create mode 100644 src/libstore/file-hash.cc create mode 100644 src/libstore/file-hash.hh diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index ad4d02c2c..f1569bf22 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -8,11 +8,6 @@ namespace nix { -std::string FileSystemHash::printMethodAlgo() const { - return makeFileIngestionPrefix(method) + printHashType(hash.type); -} - - BasicDerivation::BasicDerivation(const BasicDerivation & other) : platform(other.platform) , builder(other.builder) diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 5fc3b37da..e314beac1 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -3,6 +3,7 @@ #include "path.hh" #include "types.hh" #include "hash.hh" +#include "file-hash.hh" #include @@ -12,20 +13,6 @@ namespace nix { /* Abstract syntax of derivations. */ -/// Pair of a hash, and how the file system was ingested -struct FileSystemHash { - 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; -}; - struct DerivationOutput { StorePath path; diff --git a/src/libstore/file-hash.cc b/src/libstore/file-hash.cc new file mode 100644 index 000000000..549540db2 --- /dev/null +++ b/src/libstore/file-hash.cc @@ -0,0 +1,27 @@ +#include "file-hash.hh" + +namespace nix { + +std::string FileSystemHash::printMethodAlgo() const { + return makeFileIngestionPrefix(method) + printHashType(hash.type); +} + +std::string makeFileIngestionPrefix(const FileIngestionMethod m) { + switch (m) { + case FileIngestionMethod::Flat: + return ""; + case FileIngestionMethod::Recursive: + return "r:"; + default: + throw Error("impossible, caught both cases"); + } +} + +std::string makeFixedOutputCA(FileIngestionMethod method, const Hash & hash) +{ + return "fixed:" + + makeFileIngestionPrefix(method) + + hash.to_string(); +} + +} diff --git a/src/libstore/file-hash.hh b/src/libstore/file-hash.hh new file mode 100644 index 000000000..94122db07 --- /dev/null +++ b/src/libstore/file-hash.hh @@ -0,0 +1,34 @@ +#pragma once + +#include "hash.hh" + +namespace nix { + +enum struct FileIngestionMethod : uint8_t { + Flat = false, + Recursive = true +}; + +/// Pair of a hash, and how the file system was ingested +struct FileSystemHash { + 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; +}; + +/* Compute the prefix to the hash algorithm which indicates how the files were + ingested. */ +std::string makeFileIngestionPrefix(const FileIngestionMethod m); + +/* Compute the content-addressability assertion (ValidPathInfo::ca) + for paths created by makeFixedOutputPath() / addToStore(). */ +std::string makeFixedOutputCA(FileIngestionMethod method, const Hash & hash); + +} diff --git a/src/libstore/path.hh b/src/libstore/path.hh index f3ed6ca39..186976855 100644 --- a/src/libstore/path.hh +++ b/src/libstore/path.hh @@ -87,11 +87,6 @@ const size_t storePathHashLen = 32; // i.e. 160 bits /* Extension of derivations in the Nix store. */ const std::string drvExtension = ".drv"; -enum struct FileIngestionMethod : uint8_t { - Flat = false, - Recursive = true -}; - struct StorePathWithOutputs { StorePath path; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index b7ba9e4f2..3c72dc56f 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -814,25 +814,6 @@ Strings ValidPathInfo::shortRefs() const } -std::string makeFileIngestionPrefix(const FileIngestionMethod m) { - switch (m) { - case FileIngestionMethod::Flat: - return ""; - case FileIngestionMethod::Recursive: - return "r:"; - default: - throw Error("impossible, caught both cases"); - } -} - -std::string makeFixedOutputCA(FileIngestionMethod method, const Hash & hash) -{ - return "fixed:" - + makeFileIngestionPrefix(method) - + hash.to_string(); -} - - void Store::addToStore(const ValidPathInfo & info, Source & narSource, RepairFlag repair, CheckSigsFlag checkSigs, std::shared_ptr accessor) diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 598798570..d89e10c94 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -2,6 +2,7 @@ #include "path.hh" #include "hash.hh" +#include "file-hash.hh" #include "serialise.hh" #include "crypto.hh" #include "lru-cache.hh" @@ -846,15 +847,6 @@ std::optional decodeValidPathInfo( std::istream & str, bool hashGiven = false); -/* Compute the prefix to the hash algorithm which indicates how the files were - ingested. */ -std::string makeFileIngestionPrefix(const FileIngestionMethod m); - -/* Compute the content-addressability assertion (ValidPathInfo::ca) - for paths created by makeFixedOutputPath() / addToStore(). */ -std::string makeFixedOutputCA(FileIngestionMethod method, const Hash & hash); - - /* Split URI into protocol+hierarchy part and its parameter set. */ std::pair splitUriAndParams(const std::string & uri); diff --git a/src/nix/hash.cc b/src/nix/hash.cc index 3362ffd0d..ec23b77bc 100644 --- a/src/nix/hash.cc +++ b/src/nix/hash.cc @@ -1,5 +1,6 @@ #include "command.hh" #include "hash.hh" +#include "file-hash.hh" #include "legacy.hh" #include "shared.hh" #include "references.hh" -- cgit v1.2.3 From da39092a39bbcca80144041b5efc8428cc3b2e4a Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Mon, 1 Jun 2020 18:53:31 -0400 Subject: WIP --- src/libstore/file-hash.hh | 17 +++++++++++++++++ src/libstore/path.hh | 10 ++++++++++ src/libstore/store-api.cc | 2 +- src/libstore/store-api.hh | 32 ++++++++++++++++++-------------- src/nix-store/nix-store.cc | 2 +- 5 files changed, 47 insertions(+), 16 deletions(-) diff --git a/src/libstore/file-hash.hh b/src/libstore/file-hash.hh index 94122db07..9d2b78688 100644 --- a/src/libstore/file-hash.hh +++ b/src/libstore/file-hash.hh @@ -1,5 +1,6 @@ #pragma once +#include #include "hash.hh" namespace nix { @@ -23,6 +24,22 @@ struct FileSystemHash { std::string printMethodAlgo() const; }; +/* + We've accumulated several types of content-addressed paths over the years; + fixed-output derivations support multiple hash algorithms and serialisation + methods (flat file vs NAR). Thus, ‘ca’ has one of the following forms: + + * ‘text:sha256:’: For paths + computed by makeTextPath() / addTextToStore(). + + * ‘fixed:::’: For paths computed by + makeFixedOutputPath() / addToStore(). +*/ +typedef std::variant< + Hash, // for paths computed by makeTextPath() / addTextToStore + FileSystemHash // for path computed by makeFixedOutputPath +> ContentAddress; + /* Compute the prefix to the hash algorithm which indicates how the files were ingested. */ std::string makeFileIngestionPrefix(const FileIngestionMethod m); diff --git a/src/libstore/path.hh b/src/libstore/path.hh index 186976855..5268b3bbf 100644 --- a/src/libstore/path.hh +++ b/src/libstore/path.hh @@ -1,6 +1,7 @@ #pragma once #include "rust-ffi.hh" +#include "file-hash.hh" namespace nix { @@ -87,6 +88,15 @@ const size_t storePathHashLen = 32; // i.e. 160 bits /* Extension of derivations in the Nix store. */ const std::string drvExtension = ".drv"; +std::string to_string(FileIngestionMethod m) { + switch(m) { + case FileIngestionMethod::Flat: + return "false"; + case FileIngestionMethod::Recursive: + return "true"; + } +} + struct StorePathWithOutputs { StorePath path; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 3c72dc56f..0a36dcb2b 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -467,7 +467,7 @@ void Store::pathInfoToJSON(JSONPlaceholder & jsonOut, const StorePathSet & store jsonRefs.elem(printStorePath(ref)); } - if (info->ca != "") + if (info->ca) jsonPath.attr("ca", info->ca); std::pair closureSizes; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index d89e10c94..1f1c1e0eb 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -19,6 +19,7 @@ #include #include #include +#include namespace nix { @@ -110,6 +111,19 @@ struct SubstitutablePathInfo typedef std::map SubstitutablePathInfos; +template struct overloaded : Ts... { using Ts::operator()...; }; +template overloaded(Ts...) -> overloaded; + +std::string renderContentAddress(ContentAddress ca) { + return std::visit(overloaded { + [](Hash hash) { + return "text:" + hash.to_string(); + }, + [](FileSystemHash fsh) { + return makeFixedOutputCA(fsh.method, fsh.hash); + } + }, ca); +} struct ValidPathInfo { @@ -139,21 +153,11 @@ struct ValidPathInfo 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, - 1) we've accumulated several types of content-addressed paths - over the years; and 2) fixed-output derivations support - multiple hash algorithms and serialisation methods (flat file - vs NAR). Thus, ‘ca’ has one of the following forms: - - * ‘text:sha256:’: For paths - computed by makeTextPath() / addTextToStore(). - - * ‘fixed:::’: For paths computed by - makeFixedOutputPath() / addToStore(). + 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::string ca; + std::optional ca; bool operator == (const ValidPathInfo & i) const { diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index ace593cde..6f3a59002 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -856,7 +856,7 @@ static void opServe(Strings opFlags, Strings opArgs) out << info->narSize // downloadSize << info->narSize; if (GET_PROTOCOL_MINOR(clientVersion) >= 4) - out << (info->narHash ? info->narHash.to_string() : "") << info->ca << info->sigs; + out << (info->narHash ? info->narHash.to_string() : "") << renderContentAddress(info->ca) << info->sigs; } catch (InvalidPath &) { } } -- cgit v1.2.3 From 754c910953901ced4fbd27a8b86d36d57cb72996 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 1 Jun 2020 19:26:40 -0400 Subject: WIP more progress --- src/libstore/file-hash.cc | 18 ++++++++++++++++++ src/libstore/file-hash.hh | 10 +++++++++- src/libstore/path.hh | 9 --------- src/libstore/store-api.hh | 14 -------------- src/libutil/tests/libutil-tests | Bin 14929616 -> 14929616 bytes src/nix/path-info.cc | 2 +- 6 files changed, 28 insertions(+), 25 deletions(-) diff --git a/src/libstore/file-hash.cc b/src/libstore/file-hash.cc index 549540db2..ebd732759 100644 --- a/src/libstore/file-hash.cc +++ b/src/libstore/file-hash.cc @@ -24,4 +24,22 @@ std::string makeFixedOutputCA(FileIngestionMethod method, const Hash & hash) + hash.to_string(); } +template struct overloaded : Ts... { using Ts::operator()...; }; +template overloaded(Ts...) -> overloaded; + +std::string renderContentAddress(ContentAddress ca) { + return std::visit(overloaded { + [](TextHash th) { + return "text:" + th.hash.to_string(); + }, + [](FileSystemHash fsh) { + return makeFixedOutputCA(fsh.method, fsh.hash); + } + }, ca); +} + +std::string renderContentAddress(std::optionalContent
ca) { + return ca ? renderContentAddress(*ca) else ""; +} + } diff --git a/src/libstore/file-hash.hh b/src/libstore/file-hash.hh index 9d2b78688..e33878bf9 100644 --- a/src/libstore/file-hash.hh +++ b/src/libstore/file-hash.hh @@ -10,6 +10,10 @@ enum struct FileIngestionMethod : uint8_t { Recursive = true }; +struct TextHash { + Hash hash; +}; + /// Pair of a hash, and how the file system was ingested struct FileSystemHash { FileIngestionMethod method; @@ -36,7 +40,7 @@ struct FileSystemHash { makeFixedOutputPath() / addToStore(). */ typedef std::variant< - Hash, // for paths computed by makeTextPath() / addTextToStore + TextHash, // for paths computed by makeTextPath() / addTextToStore FileSystemHash // for path computed by makeFixedOutputPath > ContentAddress; @@ -48,4 +52,8 @@ std::string makeFileIngestionPrefix(const FileIngestionMethod m); for paths created by makeFixedOutputPath() / addToStore(). */ std::string makeFixedOutputCA(FileIngestionMethod method, const Hash & hash); +std::string renderContentAddress(ContentAddress ca); + +std::string renderContentAddress(std::optional ca); + } diff --git a/src/libstore/path.hh b/src/libstore/path.hh index 5268b3bbf..dfc0a9531 100644 --- a/src/libstore/path.hh +++ b/src/libstore/path.hh @@ -88,15 +88,6 @@ const size_t storePathHashLen = 32; // i.e. 160 bits /* Extension of derivations in the Nix store. */ const std::string drvExtension = ".drv"; -std::string to_string(FileIngestionMethod m) { - switch(m) { - case FileIngestionMethod::Flat: - return "false"; - case FileIngestionMethod::Recursive: - return "true"; - } -} - struct StorePathWithOutputs { StorePath path; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 1f1c1e0eb..faf549fe1 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -111,20 +111,6 @@ struct SubstitutablePathInfo typedef std::map SubstitutablePathInfos; -template struct overloaded : Ts... { using Ts::operator()...; }; -template overloaded(Ts...) -> overloaded; - -std::string renderContentAddress(ContentAddress ca) { - return std::visit(overloaded { - [](Hash hash) { - return "text:" + hash.to_string(); - }, - [](FileSystemHash fsh) { - return makeFixedOutputCA(fsh.method, fsh.hash); - } - }, ca); -} - struct ValidPathInfo { StorePath path; diff --git a/src/libutil/tests/libutil-tests b/src/libutil/tests/libutil-tests index 919c401a1..a4f9bbbc1 100755 Binary files a/src/libutil/tests/libutil-tests and b/src/libutil/tests/libutil-tests differ diff --git a/src/nix/path-info.cc b/src/nix/path-info.cc index 91d62bcec..ffe07a3ee 100644 --- a/src/nix/path-info.cc +++ b/src/nix/path-info.cc @@ -115,7 +115,7 @@ struct CmdPathInfo : StorePathsCommand, MixJSON std::cout << '\t'; Strings ss; if (info->ultimate) ss.push_back("ultimate"); - if (info->ca != "") ss.push_back("ca:" + info->ca); + if (info->ca != "") ss.push_back("ca:" + renderContentAddress(*info->ca)); for (auto & sig : info->sigs) ss.push_back(sig); std::cout << concatStringsSep(" ", ss); } -- cgit v1.2.3 From efcd30da891eccdb3518e9c07afc6f907e9ac5e6 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 2 Jun 2020 00:37:43 +0000 Subject: WIP --- src/libfetchers/tarball.cc | 5 ++++- src/libstore/build.cc | 12 +++++++++--- src/libstore/daemon.cc | 9 +++++++-- src/libstore/file-hash.cc | 12 ++++++++++-- src/libstore/file-hash.hh | 7 +++++++ src/libstore/legacy-ssh-store.cc | 8 ++++++-- src/libstore/local-store.cc | 23 ++++++++++++----------- src/libstore/nar-info-disk-cache.cc | 4 ++-- src/libstore/nar-info.cc | 9 +++++---- src/libstore/remote-store.cc | 6 ++++-- src/libstore/store-api.cc | 15 +++++++-------- src/libutil/tests/libutil-tests | Bin 14929616 -> 0 bytes src/nix-store/nix-store.cc | 6 +++++- src/nix/add-to-store.cc | 5 ++++- src/nix/make-content-addressable.cc | 5 ++++- src/nix/path-info.cc | 2 +- src/nix/verify.cc | 2 +- 17 files changed, 88 insertions(+), 42 deletions(-) delete mode 100755 src/libutil/tests/libutil-tests diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index 3e9004223..8efb8d68c 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -70,7 +70,10 @@ DownloadFileResult downloadFile( ValidPathInfo info(store->makeFixedOutputPath(FileIngestionMethod::Flat, hash, name)); info.narHash = hashString(HashType::SHA256, *sink.s); info.narSize = sink.s->size(); - info.ca = makeFixedOutputCA(FileIngestionMethod::Flat, hash); + info.ca = FileSystemHash { + .method = FileIngestionMethod::Flat, + .hash = hash, + }; store->addToStore(info, sink.s, NoRepair, NoCheckSigs); storePath = std::move(info.path); } diff --git a/src/libstore/build.cc b/src/libstore/build.cc index e35044532..7f79d6a2a 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3708,7 +3708,7 @@ void DerivationGoal::registerOutputs() /* Check that fixed-output derivations produced the right outputs (i.e., the content hash should match the specified hash). */ - std::string ca; + std::optional ca; if (fixedOutput) { @@ -3757,7 +3757,10 @@ void DerivationGoal::registerOutputs() else assert(worker.store.parseStorePath(path) == dest); - ca = makeFixedOutputCA(i.second.hash->method, h2); + ca = FileSystemHash { + .method = i.second.hash->method, + .hash = h2, + }; } /* Get rid of all weird permissions. This also checks that @@ -3830,7 +3833,10 @@ void DerivationGoal::registerOutputs() info.ca = ca; worker.store.signPathInfo(info); - if (!info.references.empty()) info.ca.clear(); + if (!info.references.empty()) { + // FIXME don't we have an experimental feature for fixed output with references? + info.ca = {}; + } infos.emplace(i.first, std::move(info)); } diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 1f950de63..45bfa6753 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -650,7 +650,7 @@ static void performOp(TunnelLogger * logger, ref store, if (GET_PROTOCOL_MINOR(clientVersion) >= 16) { to << info->ultimate << info->sigs - << info->ca; + << renderContentAddress(info->ca); } } else { assert(GET_PROTOCOL_MINOR(clientVersion) >= 17); @@ -708,7 +708,12 @@ static void performOp(TunnelLogger * logger, ref store, info.references = readStorePaths(*store, from); from >> info.registrationTime >> info.narSize >> info.ultimate; info.sigs = readStrings(from); - from >> info.ca >> repair >> dontCheckSigs; + { + string caOptRaw; + from >> caOptRaw; + info.ca = parseCaOpt(caOptRaw); + } + from >> repair >> dontCheckSigs; if (!trusted && dontCheckSigs) dontCheckSigs = false; if (!trusted) diff --git a/src/libstore/file-hash.cc b/src/libstore/file-hash.cc index ebd732759..494079c18 100644 --- a/src/libstore/file-hash.cc +++ b/src/libstore/file-hash.cc @@ -38,8 +38,16 @@ std::string renderContentAddress(ContentAddress ca) { }, ca); } -std::string renderContentAddress(std::optionalContent
ca) { - return ca ? renderContentAddress(*ca) else ""; +ContentAddress parseCa(std::string_view rawCa) { + throw Error("TODO"); +}; + +std::optional parseCaOpt(std::string_view rawCaOpt) { + return rawCaOpt == "" ? std::optional {} : parseCa(rawCaOpt); +}; + +std::string renderContentAddress(std::optional ca) { + return ca ? renderContentAddress(*ca) : ""; } } diff --git a/src/libstore/file-hash.hh b/src/libstore/file-hash.hh index e33878bf9..9d12f5fe7 100644 --- a/src/libstore/file-hash.hh +++ b/src/libstore/file-hash.hh @@ -12,6 +12,9 @@ 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 @@ -56,4 +59,8 @@ std::string renderContentAddress(ContentAddress ca); std::string renderContentAddress(std::optional ca); +ContentAddress parseCa(std::string_view rawCa); + +std::optional parseCaOpt(std::string_view rawCaOpt); + } diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 61e7603b7..8471ed073 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -114,7 +114,11 @@ struct LegacySSHStore : public Store if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 4) { auto s = readString(conn->from); info->narHash = s.empty() ? Hash() : Hash(s); - conn->from >> info->ca; + { + std::string rawCaOpt; + conn->from >> rawCaOpt; + info->ca = parseCaOpt(rawCaOpt); + } info->sigs = readStrings(conn->from); } @@ -146,7 +150,7 @@ struct LegacySSHStore : public Store << info.narSize << info.ultimate << info.sigs - << info.ca; + << renderContentAddress(info.ca); try { copyNAR(source, conn->to); } catch (...) { diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 9b60bf887..fe346f96a 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -576,7 +576,7 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat uint64_t LocalStore::addValidPath(State & state, const ValidPathInfo & info, bool checkOutputs) { - if (info.ca != "" && !info.isContentAddressed(*this)) + if (info.ca && !info.isContentAddressed(*this)) throw Error("cannot add path '%s' to the Nix store because it claims to be content-addressed but isn't", printStorePath(info.path)); @@ -588,7 +588,7 @@ uint64_t LocalStore::addValidPath(State & state, (info.narSize, info.narSize != 0) (info.ultimate ? 1 : 0, info.ultimate) (concatStringsSep(" ", info.sigs), !info.sigs.empty()) - (info.ca, !info.ca.empty()) + (renderContentAddress(info.ca), (bool) info.ca) .exec(); uint64_t id = sqlite3_last_insert_rowid(state.db); @@ -662,7 +662,7 @@ void LocalStore::queryPathInfoUncached(const StorePath & path, if (s) info->sigs = tokenizeString(s, " "); s = (const char *) sqlite3_column_text(state->stmtQueryPathInfo, 7); - if (s) info->ca = s; + if (s) info->ca = parseCaOpt(s); /* Get the references. */ auto useQueryReferences(state->stmtQueryReferences.use()(info->id)); @@ -685,7 +685,7 @@ void LocalStore::updatePathInfo(State & state, const ValidPathInfo & info) (info.narHash.to_string(Base::Base16)) (info.ultimate ? 1 : 0, info.ultimate) (concatStringsSep(" ", info.sigs), !info.sigs.empty()) - (info.ca, !info.ca.empty()) + (renderContentAddress(info.ca), (bool) info.ca) (printStorePath(info.path)) .exec(); } @@ -999,15 +999,13 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, deletePath(realPath); - if (info.ca != "" && - !((hasPrefix(info.ca, "text:") && !info.references.count(info.path)) - || info.references.empty())) + if (info.ca && !info.references.empty() && !std::holds_alternative(*info.ca)) settings.requireExperimentalFeature("ca-references"); /* While restoring the path from the NAR, compute the hash of the NAR. */ std::unique_ptr hashSink; - if (info.ca == "" || !info.references.count(info.path)) + if (info.ca || !info.references.count(info.path)) hashSink = std::make_unique(HashType::SHA256); else hashSink = std::make_unique(HashType::SHA256, storePathToHash(printStorePath(info.path))); @@ -1093,7 +1091,10 @@ StorePath LocalStore::addToStoreFromDump(const string & dump, const string & nam ValidPathInfo info(dstPath.clone()); info.narHash = hash.first; info.narSize = hash.second; - info.ca = makeFixedOutputCA(method, h); + info.ca = FileSystemHash { + .method = method, + .hash = h, + }; registerValidPath(info); } @@ -1157,7 +1158,7 @@ StorePath LocalStore::addTextToStore(const string & name, const string & s, info.narHash = narHash; info.narSize = sink.s->size(); info.references = cloneStorePathSet(references); - info.ca = "text:" + hash.to_string(); + info.ca = TextHash { .hash = hash }; registerValidPath(info); } @@ -1265,7 +1266,7 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair) printMsg(Verbosity::Talkative, "checking contents of '%s'", printStorePath(i)); std::unique_ptr hashSink; - if (info->ca == "" || !info->references.count(info->path)) + if (info->ca || !info->references.count(info->path)) hashSink = std::make_unique(info->narHash.type); else hashSink = std::make_unique(info->narHash.type, storePathToHash(printStorePath(info->path))); diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index 442541330..b4346d4d1 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -203,7 +203,7 @@ public: narInfo->deriver = StorePath::fromBaseName(queryNAR.getStr(9)); for (auto & sig : tokenizeString(queryNAR.getStr(10), " ")) narInfo->sigs.insert(sig); - narInfo->ca = queryNAR.getStr(11); + narInfo->ca = parseCaOpt(queryNAR.getStr(11)); return {oValid, narInfo}; }); @@ -237,7 +237,7 @@ public: (concatStringsSep(" ", info->shortRefs())) (info->deriver ? std::string(info->deriver->to_string()) : "", (bool) info->deriver) (concatStringsSep(" ", info->sigs)) - (info->ca) + (renderContentAddress(info->ca)) (time(0)).exec(); } else { diff --git a/src/libstore/nar-info.cc b/src/libstore/nar-info.cc index 8592ee968..4f7c732ce 100644 --- a/src/libstore/nar-info.cc +++ b/src/libstore/nar-info.cc @@ -67,8 +67,9 @@ NarInfo::NarInfo(const Store & store, const std::string & s, const std::string & else if (name == "Sig") sigs.insert(value); else if (name == "CA") { - if (!ca.empty()) corrupt(); - ca = value; + if (ca) corrupt(); + // FIXME: allow blank ca or require skipping field? + ca = parseCaOpt(value); } pos = eol + 1; @@ -104,8 +105,8 @@ std::string NarInfo::to_string(const Store & store) const for (auto sig : sigs) res += "Sig: " + sig + "\n"; - if (!ca.empty()) - res += "CA: " + ca + "\n"; + if (ca) + res += "CA: " + renderContentAddress(*ca) + "\n"; return res; } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 2037a55f4..2744245f3 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -381,7 +381,9 @@ void RemoteStore::queryPathInfoUncached(const StorePath & path, if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 16) { conn->from >> info->ultimate; info->sigs = readStrings(conn->from); - conn->from >> info->ca; + string caOptRaw; + conn->from >> caOptRaw; + info->ca = parseCaOpt(caOptRaw); } } callback(std::move(info)); @@ -474,7 +476,7 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, << info.narHash.to_string(Base::Base16, false); writeStorePaths(*this, conn->to, info.references); conn->to << info.registrationTime << info.narSize - << info.ultimate << info.sigs << info.ca + << info.ultimate << info.sigs << renderContentAddress(info.ca) << repair << !checkSigs; bool tunnel = GET_PROTOCOL_MINOR(conn->daemonVersion) >= 21; if (!tunnel) copyNAR(source, conn->to); diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 0a36dcb2b..0715f64f3 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -468,7 +468,7 @@ void Store::pathInfoToJSON(JSONPlaceholder & jsonOut, const StorePathSet & store } if (info->ca) - jsonPath.attr("ca", info->ca); + jsonPath.attr("ca", renderContentAddress(info->ca)); std::pair closureSizes; @@ -760,24 +760,23 @@ bool ValidPathInfo::isContentAddressed(const Store & store) const printError("warning: path '%s' claims to be content-addressed but isn't", store.printStorePath(path)); }; - if (hasPrefix(ca, "text:")) { - Hash hash(std::string(ca, 5)); - if (store.makeTextPath(path.name(), hash, references) == path) + if (! ca) {} + + else if (auto p = std::get_if(&*ca)) { + if (store.makeTextPath(path.name(), p->hash, references) == path) return true; else warn(); } - else if (hasPrefix(ca, "fixed:")) { - FileIngestionMethod recursive { ca.compare(6, 2, "r:") == 0 }; - Hash hash(std::string(ca, recursive == FileIngestionMethod::Recursive ? 8 : 6)); + else if (auto p = std::get_if(&*ca)) { auto refs = cloneStorePathSet(references); bool hasSelfReference = false; if (refs.count(path)) { hasSelfReference = true; refs.erase(path); } - if (store.makeFixedOutputPath(recursive, hash, path.name(), refs, hasSelfReference) == path) + if (store.makeFixedOutputPath(p->method, p->hash, path.name(), refs, hasSelfReference) == path) return true; else warn(); diff --git a/src/libutil/tests/libutil-tests b/src/libutil/tests/libutil-tests deleted file mode 100755 index a4f9bbbc1..000000000 Binary files a/src/libutil/tests/libutil-tests and /dev/null differ diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 6f3a59002..64ee9fd45 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -944,7 +944,11 @@ static void opServe(Strings opFlags, Strings opArgs) info.references = readStorePaths(*store, in); in >> info.registrationTime >> info.narSize >> info.ultimate; info.sigs = readStrings(in); - in >> info.ca; + { + std::string rawCA; + in >> rawCA; + info.ca = parseCaOpt(rawCA); + } if (info.narSize == 0) throw Error("narInfo is too old and missing the narSize field"); diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index 39d49721a..dc34a83ca 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -48,7 +48,10 @@ struct CmdAddToStore : MixDryRun, StoreCommand ValidPathInfo info(store->makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, *namePart)); info.narHash = narHash; info.narSize = sink.s->size(); - info.ca = makeFixedOutputCA(FileIngestionMethod::Recursive, info.narHash); + *info.ca = FileSystemHash { + .method = FileIngestionMethod::Recursive, + .hash = info.narHash + }; if (!dryRun) store->addToStore(info, sink.s); diff --git a/src/nix/make-content-addressable.cc b/src/nix/make-content-addressable.cc index bd948a983..5003ff459 100644 --- a/src/nix/make-content-addressable.cc +++ b/src/nix/make-content-addressable.cc @@ -82,7 +82,10 @@ struct CmdMakeContentAddressable : StorePathsCommand, MixJSON if (hasSelfReference) info.references.insert(info.path.clone()); info.narHash = narHash; info.narSize = sink.s->size(); - info.ca = makeFixedOutputCA(FileIngestionMethod::Recursive, info.narHash); + info.ca = FileSystemHash { + .method = FileIngestionMethod::Recursive, + .hash = info.narHash, + }; if (!json) printError("rewrote '%s' to '%s'", pathS, store->printStorePath(info.path)); diff --git a/src/nix/path-info.cc b/src/nix/path-info.cc index ffe07a3ee..cda371129 100644 --- a/src/nix/path-info.cc +++ b/src/nix/path-info.cc @@ -115,7 +115,7 @@ struct CmdPathInfo : StorePathsCommand, MixJSON std::cout << '\t'; Strings ss; if (info->ultimate) ss.push_back("ultimate"); - if (info->ca != "") ss.push_back("ca:" + renderContentAddress(*info->ca)); + if (info->ca) ss.push_back("ca:" + renderContentAddress(*info->ca)); for (auto & sig : info->sigs) ss.push_back(sig); std::cout << concatStringsSep(" ", ss); } diff --git a/src/nix/verify.cc b/src/nix/verify.cc index 0c3478ff5..d8636435c 100644 --- a/src/nix/verify.cc +++ b/src/nix/verify.cc @@ -87,7 +87,7 @@ struct CmdVerify : StorePathsCommand if (!noContents) { std::unique_ptr hashSink; - if (info->ca == "") + if (info->ca) hashSink = std::make_unique(info->narHash.type); else hashSink = std::make_unique(info->narHash.type, storePathToHash(store->printStorePath(info->path))); -- cgit v1.2.3 From a5d820a0a396db01e6297dfcadaa3218a1e918c7 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 2 Jun 2020 11:00:10 -0400 Subject: Change parseCa(Opt) to parseContentAddress(Opt) --- src/libstore/daemon.cc | 2 +- src/libstore/file-hash.cc | 6 +++--- src/libstore/file-hash.hh | 4 ++-- src/libstore/legacy-ssh-store.cc | 2 +- src/libstore/local-store.cc | 2 +- src/libstore/nar-info-disk-cache.cc | 2 +- src/libstore/nar-info.cc | 2 +- src/libstore/remote-store.cc | 2 +- src/nix-store/nix-store.cc | 2 +- 9 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 45bfa6753..0734f990a 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -711,7 +711,7 @@ static void performOp(TunnelLogger * logger, ref store, { string caOptRaw; from >> caOptRaw; - info.ca = parseCaOpt(caOptRaw); + info.ca = parseContentAddressOpt(caOptRaw); } from >> repair >> dontCheckSigs; if (!trusted && dontCheckSigs) diff --git a/src/libstore/file-hash.cc b/src/libstore/file-hash.cc index 494079c18..26153a424 100644 --- a/src/libstore/file-hash.cc +++ b/src/libstore/file-hash.cc @@ -38,12 +38,12 @@ std::string renderContentAddress(ContentAddress ca) { }, ca); } -ContentAddress parseCa(std::string_view rawCa) { +ContentAddress parseContentAddress(std::string_view rawCa) { throw Error("TODO"); }; -std::optional parseCaOpt(std::string_view rawCaOpt) { - return rawCaOpt == "" ? std::optional {} : parseCa(rawCaOpt); +std::optional parseContentAddressOpt(std::string_view rawCaOpt) { + return rawCaOpt == "" ? std::optional {} : parseContentAddress(rawCaOpt); }; std::string renderContentAddress(std::optional ca) { diff --git a/src/libstore/file-hash.hh b/src/libstore/file-hash.hh index 9d12f5fe7..64d514751 100644 --- a/src/libstore/file-hash.hh +++ b/src/libstore/file-hash.hh @@ -59,8 +59,8 @@ std::string renderContentAddress(ContentAddress ca); std::string renderContentAddress(std::optional ca); -ContentAddress parseCa(std::string_view rawCa); +ContentAddress parseContentAddress(std::string_view rawCa); -std::optional parseCaOpt(std::string_view rawCaOpt); +std::optional parseContentAddressOpt(std::string_view rawCaOpt); } diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 8471ed073..9b6e6e6d7 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -117,7 +117,7 @@ struct LegacySSHStore : public Store { std::string rawCaOpt; conn->from >> rawCaOpt; - info->ca = parseCaOpt(rawCaOpt); + info->ca = parseContentAddressOpt(rawCaOpt); } info->sigs = readStrings(conn->from); } diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index fe346f96a..2067343c7 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -662,7 +662,7 @@ void LocalStore::queryPathInfoUncached(const StorePath & path, if (s) info->sigs = tokenizeString(s, " "); s = (const char *) sqlite3_column_text(state->stmtQueryPathInfo, 7); - if (s) info->ca = parseCaOpt(s); + if (s) info->ca = parseContentAddressOpt(s); /* Get the references. */ auto useQueryReferences(state->stmtQueryReferences.use()(info->id)); diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index b4346d4d1..def514840 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -203,7 +203,7 @@ public: narInfo->deriver = StorePath::fromBaseName(queryNAR.getStr(9)); for (auto & sig : tokenizeString(queryNAR.getStr(10), " ")) narInfo->sigs.insert(sig); - narInfo->ca = parseCaOpt(queryNAR.getStr(11)); + narInfo->ca = parseContentAddressOpt(queryNAR.getStr(11)); return {oValid, narInfo}; }); diff --git a/src/libstore/nar-info.cc b/src/libstore/nar-info.cc index 4f7c732ce..fe37d67ec 100644 --- a/src/libstore/nar-info.cc +++ b/src/libstore/nar-info.cc @@ -69,7 +69,7 @@ NarInfo::NarInfo(const Store & store, const std::string & s, const std::string & else if (name == "CA") { if (ca) corrupt(); // FIXME: allow blank ca or require skipping field? - ca = parseCaOpt(value); + ca = parseContentAddressOpt(value); } pos = eol + 1; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 2744245f3..6dab9dc16 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -383,7 +383,7 @@ void RemoteStore::queryPathInfoUncached(const StorePath & path, info->sigs = readStrings(conn->from); string caOptRaw; conn->from >> caOptRaw; - info->ca = parseCaOpt(caOptRaw); + info->ca = parseContentAddressOpt(caOptRaw); } } callback(std::move(info)); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 64ee9fd45..5d8d04252 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -947,7 +947,7 @@ static void opServe(Strings opFlags, Strings opArgs) { std::string rawCA; in >> rawCA; - info.ca = parseCaOpt(rawCA); + info.ca = parseContentAddressOpt(rawCA); } if (info.narSize == 0) -- cgit v1.2.3 From 25e61812f341a4467ab4fdffb12f63b48a3d0272 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 2 Jun 2020 12:47:18 -0400 Subject: Apply suggestions from code review Co-authored-by: Matthew Bauer --- src/libstore/legacy-ssh-store.cc | 6 +----- src/libstore/local-store.cc | 4 +++- src/libstore/remote-store.cc | 4 +--- src/nix-store/nix-store.cc | 6 +----- src/nix/verify.cc | 2 +- 5 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 9b6e6e6d7..b43e34484 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -114,11 +114,7 @@ struct LegacySSHStore : public Store if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 4) { auto s = readString(conn->from); info->narHash = s.empty() ? Hash() : Hash(s); - { - std::string rawCaOpt; - conn->from >> rawCaOpt; - info->ca = parseContentAddressOpt(rawCaOpt); - } + info->ca = parseContentAddressOpt(readString(conn->from)); info->sigs = readStrings(conn->from); } diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 2067343c7..93697ae47 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -999,7 +999,9 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, deletePath(realPath); - if (info.ca && !info.references.empty() && !std::holds_alternative(*info.ca)) + // text hashing has long been allowed to have non-self-references because it is used for drv files. + bool refersToSelf = info.references.count(info.path) > 0; + if (info.ca && !info.references.empty() && !(std::holds_alternative(*info.ca) && !refersToSelf)) settings.requireExperimentalFeature("ca-references"); /* While restoring the path from the NAR, compute the hash diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 6dab9dc16..0faa4d824 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -381,9 +381,7 @@ void RemoteStore::queryPathInfoUncached(const StorePath & path, if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 16) { conn->from >> info->ultimate; info->sigs = readStrings(conn->from); - string caOptRaw; - conn->from >> caOptRaw; - info->ca = parseContentAddressOpt(caOptRaw); + info->ca = parseContentAddressOpt(readString(conn->from)); } } callback(std::move(info)); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 5d8d04252..c74847394 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -944,11 +944,7 @@ static void opServe(Strings opFlags, Strings opArgs) info.references = readStorePaths(*store, in); in >> info.registrationTime >> info.narSize >> info.ultimate; info.sigs = readStrings(in); - { - std::string rawCA; - in >> rawCA; - info.ca = parseContentAddressOpt(rawCA); - } + info.ca = parseContentAddressOpt(readString(in)); if (info.narSize == 0) throw Error("narInfo is too old and missing the narSize field"); diff --git a/src/nix/verify.cc b/src/nix/verify.cc index d8636435c..d9b200591 100644 --- a/src/nix/verify.cc +++ b/src/nix/verify.cc @@ -87,7 +87,7 @@ struct CmdVerify : StorePathsCommand if (!noContents) { std::unique_ptr hashSink; - if (info->ca) + if (!info->ca) hashSink = std::make_unique(info->narHash.type); else hashSink = std::make_unique(info->narHash.type, storePathToHash(store->printStorePath(info->path))); -- cgit v1.2.3 From a33270ce1d815069d3fec43225a10e33c8e94287 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 2 Jun 2020 17:04:21 +0000 Subject: Clean up `ValidPathInfo::isContentAddressed` with `std::visit` --- src/libstore/file-hash.cc | 1 + src/libstore/store-api.cc | 45 ++++++++++++++++++++++----------------------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/libstore/file-hash.cc b/src/libstore/file-hash.cc index 26153a424..4a3280653 100644 --- a/src/libstore/file-hash.cc +++ b/src/libstore/file-hash.cc @@ -24,6 +24,7 @@ std::string makeFixedOutputCA(FileIngestionMethod method, const Hash & hash) + hash.to_string(); } +// FIXME Put this somewhere? template struct overloaded : Ts... { using Ts::operator()...; }; template overloaded(Ts...) -> overloaded; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index ea30ed105..4bd3121ed 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -753,36 +753,35 @@ void ValidPathInfo::sign(const Store & store, const SecretKey & secretKey) sigs.insert(secretKey.signDetached(fingerprint(store))); } +// FIXME Put this somewhere? +template struct overloaded : Ts... { using Ts::operator()...; }; +template overloaded(Ts...) -> overloaded; bool ValidPathInfo::isContentAddressed(const Store & store) const { - auto warn = [&]() { - printError("warning: path '%s' claims to be content-addressed but isn't", store.printStorePath(path)); - }; + if (! ca) return false; - if (! ca) {} + auto caPath = std::visit(overloaded { + [&](TextHash th) { + return store.makeTextPath(path.name(), th.hash, references); + }, + [&](FileSystemHash fsh) { + auto refs = cloneStorePathSet(references); + bool hasSelfReference = false; + if (refs.count(path)) { + hasSelfReference = true; + refs.erase(path); + } + return store.makeFixedOutputPath(fsh.method, fsh.hash, path.name(), refs, hasSelfReference); + } + }, *ca); - else if (auto p = std::get_if(&*ca)) { - if (store.makeTextPath(path.name(), p->hash, references) == path) - return true; - else - warn(); - } + bool res = caPath == path; - else if (auto p = std::get_if(&*ca)) { - auto refs = cloneStorePathSet(references); - bool hasSelfReference = false; - if (refs.count(path)) { - hasSelfReference = true; - refs.erase(path); - } - if (store.makeFixedOutputPath(p->method, p->hash, path.name(), refs, hasSelfReference) == path) - return true; - else - warn(); - } + if (!res) + printError("warning: path '%s' claims to be content-addressed but isn't", store.printStorePath(path)); - return false; + return res; } -- cgit v1.2.3 From 390bf64858e03b948a9b65fe138b34dd2776f8ef Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 2 Jun 2020 14:15:38 -0400 Subject: WIP --- src/libstore/derivations.hh | 1 - src/libstore/file-hash.cc | 55 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index e314beac1..dba9318d9 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -24,7 +24,6 @@ struct DerivationOutput DerivationOutput(const DerivationOutput &) = default; DerivationOutput(DerivationOutput &&) = default; DerivationOutput & operator = (const DerivationOutput &) = default; - void parseHashInfo(FileIngestionMethod & recursive, Hash & hash) const; }; typedef std::map DerivationOutputs; diff --git a/src/libstore/file-hash.cc b/src/libstore/file-hash.cc index 4a3280653..9cae8f80e 100644 --- a/src/libstore/file-hash.cc +++ b/src/libstore/file-hash.cc @@ -40,7 +40,60 @@ std::string renderContentAddress(ContentAddress ca) { } ContentAddress parseContentAddress(std::string_view rawCa) { - throw Error("TODO"); + auto prefixSeparator = rawCa.find(':'); + if (prefixSeparator != string::npos) { + auto prefix = string(rawCa, 0, prefixSeparator); + if (prefix == "text") { + auto hashTypeAndHash = rawCa.substr(prefixSeparator+1, string::npos); + auto hashSeparator = hashTypeAndHash.find(':'); + if (hashSeparator != string::npos) { + std::string_view hashTypeRaw = hashTypeAndHash.substr(0, hashSeparator); + std::string_view hashRaw = hashTypeAndHash.substr(hashSeparator+1, string::npos); + auto hashType = parseHashType(string(hashTypeRaw)); + return TextHash { Hash(string(hashRaw), hashType) }; + } else { + throw "parseContentAddress: hash type not found"; + } + } else if (prefix == "fixed") { + 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, } + } + + + + // break; + // } else { + // throw "parseContentAddress: invalid prefix"; + } + + } else { + throw "Not a content address because it lacks an appropriate prefix"; + } + + + + // if (getString(rawCa, 5) == "text:") { + // auto hashTypeAndHash = string::substr(5, string::npos); + // auto sep = hashTypeAndHash.find(':'); + // if (sep != string::npos) { + // string hashTypeRaw = string(hashTypeAndHash, 0, sep); + // auto hashType = parseHashType(hashTypeRaw); + // } + // break; + + // // } else if (getString (rawCa, 6) = "fixed:") { + // } else if (true) { + // break; + // } + + // auto sep = rawCa.find(':'); + // if (sep == string::npos) + // if(string(rawCa, 5) == "text:") { + // break; + // } else if {} + // throw Error("TODO"); }; std::optional parseContentAddressOpt(std::string_view rawCaOpt) { -- cgit v1.2.3 From 343c20a404055d266227b7562f775f7fad37103f Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 2 Jun 2020 15:18:05 -0400 Subject: WIP Completed implementation --- src/libstore/file-hash.cc | 48 ++++++++--------------------------------------- 1 file changed, 8 insertions(+), 40 deletions(-) diff --git a/src/libstore/file-hash.cc b/src/libstore/file-hash.cc index 9cae8f80e..caf9a83e9 100644 --- a/src/libstore/file-hash.cc +++ b/src/libstore/file-hash.cc @@ -45,55 +45,23 @@ ContentAddress parseContentAddress(std::string_view rawCa) { auto prefix = string(rawCa, 0, prefixSeparator); if (prefix == "text") { auto hashTypeAndHash = rawCa.substr(prefixSeparator+1, string::npos); - auto hashSeparator = hashTypeAndHash.find(':'); - if (hashSeparator != string::npos) { - std::string_view hashTypeRaw = hashTypeAndHash.substr(0, hashSeparator); - std::string_view hashRaw = hashTypeAndHash.substr(hashSeparator+1, string::npos); - auto hashType = parseHashType(string(hashTypeRaw)); - return TextHash { Hash(string(hashRaw), hashType) }; - } else { - throw "parseContentAddress: hash type not found"; - } + return TextHash { Hash(string(hashTypeAndHash)) }; } else if (prefix == "fixed") { + // This has to be an inverse to makeFixedOutputCA 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, } + return FileSystemHash { FileIngestionMethod::Recursive, Hash(string(hashRaw)) }; + } else { + std::string_view hashRaw = methodAndHash; + return FileSystemHash { FileIngestionMethod::Flat, Hash(string(hashRaw)) }; } - - - - // break; - // } else { - // throw "parseContentAddress: invalid prefix"; + } else { + throw "parseContentAddress: format not recognized; has to be text or fixed"; } - } else { throw "Not a content address because it lacks an appropriate prefix"; } - - - - // if (getString(rawCa, 5) == "text:") { - // auto hashTypeAndHash = string::substr(5, string::npos); - // auto sep = hashTypeAndHash.find(':'); - // if (sep != string::npos) { - // string hashTypeRaw = string(hashTypeAndHash, 0, sep); - // auto hashType = parseHashType(hashTypeRaw); - // } - // break; - - // // } else if (getString (rawCa, 6) = "fixed:") { - // } else if (true) { - // break; - // } - - // auto sep = rawCa.find(':'); - // if (sep == string::npos) - // if(string(rawCa, 5) == "text:") { - // break; - // } else if {} - // throw Error("TODO"); }; std::optional parseContentAddressOpt(std::string_view rawCaOpt) { -- cgit v1.2.3 From fd2eb41e6433e72516ae149949b8b0050305293d Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 2 Jun 2020 15:44:58 -0400 Subject: Move file-hash to content-address --- src/libstore/content-address.cc | 75 +++++++++++++++++++++++++++++++++++++++++ src/libstore/content-address.hh | 66 ++++++++++++++++++++++++++++++++++++ src/libstore/derivations.hh | 2 +- src/libstore/file-hash.cc | 75 ----------------------------------------- src/libstore/file-hash.hh | 66 ------------------------------------ src/libstore/path.hh | 2 +- src/libstore/store-api.hh | 2 +- src/nix/hash.cc | 2 +- 8 files changed, 145 insertions(+), 145 deletions(-) create mode 100644 src/libstore/content-address.cc create mode 100644 src/libstore/content-address.hh delete mode 100644 src/libstore/file-hash.cc delete mode 100644 src/libstore/file-hash.hh diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc new file mode 100644 index 000000000..421127f5a --- /dev/null +++ b/src/libstore/content-address.cc @@ -0,0 +1,75 @@ +#include "content-address.hh" + +namespace nix { + +std::string FileSystemHash::printMethodAlgo() const { + return makeFileIngestionPrefix(method) + printHashType(hash.type); +} + +std::string makeFileIngestionPrefix(const FileIngestionMethod m) { + switch (m) { + case FileIngestionMethod::Flat: + return ""; + case FileIngestionMethod::Recursive: + return "r:"; + default: + throw Error("impossible, caught both cases"); + } +} + +std::string makeFixedOutputCA(FileIngestionMethod method, const Hash & hash) +{ + return "fixed:" + + makeFileIngestionPrefix(method) + + hash.to_string(); +} + +// FIXME Put this somewhere? +template struct overloaded : Ts... { using Ts::operator()...; }; +template overloaded(Ts...) -> overloaded; + +std::string renderContentAddress(ContentAddress ca) { + return std::visit(overloaded { + [](TextHash th) { + return "text:" + th.hash.to_string(); + }, + [](FileSystemHash fsh) { + return makeFixedOutputCA(fsh.method, fsh.hash); + } + }, ca); +} + +ContentAddress parseContentAddress(std::string_view rawCa) { + auto prefixSeparator = rawCa.find(':'); + if (prefixSeparator != string::npos) { + auto prefix = string(rawCa, 0, prefixSeparator); + if (prefix == "text") { + auto hashTypeAndHash = rawCa.substr(prefixSeparator+1, string::npos); + return TextHash { Hash(string(hashTypeAndHash)) }; + } else if (prefix == "fixed") { + // This has to be an inverse to makeFixedOutputCA + 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)) }; + } else { + std::string_view hashRaw = methodAndHash; + return FileSystemHash { FileIngestionMethod::Flat, Hash(string(hashRaw)) }; + } + } else { + throw "parseContentAddress: format not recognized; has to be text or fixed"; + } + } else { + throw "Not a content address because it lacks an appropriate prefix"; + } +}; + +std::optional parseContentAddressOpt(std::string_view rawCaOpt) { + return rawCaOpt == "" ? std::optional {} : parseContentAddress(rawCaOpt); +}; + +std::string renderContentAddress(std::optional ca) { + return ca ? renderContentAddress(*ca) : ""; +} + +} diff --git a/src/libstore/content-address.hh b/src/libstore/content-address.hh new file mode 100644 index 000000000..64d514751 --- /dev/null +++ b/src/libstore/content-address.hh @@ -0,0 +1,66 @@ +#pragma once + +#include +#include "hash.hh" + +namespace nix { + +enum struct FileIngestionMethod : uint8_t { + Flat = false, + Recursive = true +}; + +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 { + 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; +}; + +/* + We've accumulated several types of content-addressed paths over the years; + fixed-output derivations support multiple hash algorithms and serialisation + methods (flat file vs NAR). Thus, ‘ca’ has one of the following forms: + + * ‘text:sha256:’: For paths + computed by makeTextPath() / addTextToStore(). + + * ‘fixed:::’: For paths computed by + makeFixedOutputPath() / addToStore(). +*/ +typedef std::variant< + TextHash, // for paths computed by makeTextPath() / addTextToStore + FileSystemHash // for path computed by makeFixedOutputPath +> ContentAddress; + +/* Compute the prefix to the hash algorithm which indicates how the files were + ingested. */ +std::string makeFileIngestionPrefix(const FileIngestionMethod m); + +/* Compute the content-addressability assertion (ValidPathInfo::ca) + for paths created by makeFixedOutputPath() / addToStore(). */ +std::string makeFixedOutputCA(FileIngestionMethod method, const Hash & hash); + +std::string renderContentAddress(ContentAddress ca); + +std::string renderContentAddress(std::optional ca); + +ContentAddress parseContentAddress(std::string_view rawCa); + +std::optional parseContentAddressOpt(std::string_view rawCaOpt); + +} diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index dba9318d9..838b63358 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -3,7 +3,7 @@ #include "path.hh" #include "types.hh" #include "hash.hh" -#include "file-hash.hh" +#include "content-address.hh" #include diff --git a/src/libstore/file-hash.cc b/src/libstore/file-hash.cc deleted file mode 100644 index caf9a83e9..000000000 --- a/src/libstore/file-hash.cc +++ /dev/null @@ -1,75 +0,0 @@ -#include "file-hash.hh" - -namespace nix { - -std::string FileSystemHash::printMethodAlgo() const { - return makeFileIngestionPrefix(method) + printHashType(hash.type); -} - -std::string makeFileIngestionPrefix(const FileIngestionMethod m) { - switch (m) { - case FileIngestionMethod::Flat: - return ""; - case FileIngestionMethod::Recursive: - return "r:"; - default: - throw Error("impossible, caught both cases"); - } -} - -std::string makeFixedOutputCA(FileIngestionMethod method, const Hash & hash) -{ - return "fixed:" - + makeFileIngestionPrefix(method) - + hash.to_string(); -} - -// FIXME Put this somewhere? -template struct overloaded : Ts... { using Ts::operator()...; }; -template overloaded(Ts...) -> overloaded; - -std::string renderContentAddress(ContentAddress ca) { - return std::visit(overloaded { - [](TextHash th) { - return "text:" + th.hash.to_string(); - }, - [](FileSystemHash fsh) { - return makeFixedOutputCA(fsh.method, fsh.hash); - } - }, ca); -} - -ContentAddress parseContentAddress(std::string_view rawCa) { - auto prefixSeparator = rawCa.find(':'); - if (prefixSeparator != string::npos) { - auto prefix = string(rawCa, 0, prefixSeparator); - if (prefix == "text") { - auto hashTypeAndHash = rawCa.substr(prefixSeparator+1, string::npos); - return TextHash { Hash(string(hashTypeAndHash)) }; - } else if (prefix == "fixed") { - // This has to be an inverse to makeFixedOutputCA - 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)) }; - } else { - std::string_view hashRaw = methodAndHash; - return FileSystemHash { FileIngestionMethod::Flat, Hash(string(hashRaw)) }; - } - } else { - throw "parseContentAddress: format not recognized; has to be text or fixed"; - } - } else { - throw "Not a content address because it lacks an appropriate prefix"; - } -}; - -std::optional parseContentAddressOpt(std::string_view rawCaOpt) { - return rawCaOpt == "" ? std::optional {} : parseContentAddress(rawCaOpt); -}; - -std::string renderContentAddress(std::optional ca) { - return ca ? renderContentAddress(*ca) : ""; -} - -} diff --git a/src/libstore/file-hash.hh b/src/libstore/file-hash.hh deleted file mode 100644 index 64d514751..000000000 --- a/src/libstore/file-hash.hh +++ /dev/null @@ -1,66 +0,0 @@ -#pragma once - -#include -#include "hash.hh" - -namespace nix { - -enum struct FileIngestionMethod : uint8_t { - Flat = false, - Recursive = true -}; - -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 { - 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; -}; - -/* - We've accumulated several types of content-addressed paths over the years; - fixed-output derivations support multiple hash algorithms and serialisation - methods (flat file vs NAR). Thus, ‘ca’ has one of the following forms: - - * ‘text:sha256:’: For paths - computed by makeTextPath() / addTextToStore(). - - * ‘fixed:::’: For paths computed by - makeFixedOutputPath() / addToStore(). -*/ -typedef std::variant< - TextHash, // for paths computed by makeTextPath() / addTextToStore - FileSystemHash // for path computed by makeFixedOutputPath -> ContentAddress; - -/* Compute the prefix to the hash algorithm which indicates how the files were - ingested. */ -std::string makeFileIngestionPrefix(const FileIngestionMethod m); - -/* Compute the content-addressability assertion (ValidPathInfo::ca) - for paths created by makeFixedOutputPath() / addToStore(). */ -std::string makeFixedOutputCA(FileIngestionMethod method, const Hash & hash); - -std::string renderContentAddress(ContentAddress ca); - -std::string renderContentAddress(std::optional ca); - -ContentAddress parseContentAddress(std::string_view rawCa); - -std::optional parseContentAddressOpt(std::string_view rawCaOpt); - -} diff --git a/src/libstore/path.hh b/src/libstore/path.hh index dfc0a9531..fdbc906ef 100644 --- a/src/libstore/path.hh +++ b/src/libstore/path.hh @@ -1,7 +1,7 @@ #pragma once #include "rust-ffi.hh" -#include "file-hash.hh" +#include "content-address.hh" namespace nix { diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 7836c04dc..441e3efc5 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -2,7 +2,7 @@ #include "path.hh" #include "hash.hh" -#include "file-hash.hh" +#include "content-address.hh" #include "serialise.hh" #include "crypto.hh" #include "lru-cache.hh" diff --git a/src/nix/hash.cc b/src/nix/hash.cc index ec23b77bc..26bbcef1c 100644 --- a/src/nix/hash.cc +++ b/src/nix/hash.cc @@ -1,6 +1,6 @@ #include "command.hh" #include "hash.hh" -#include "file-hash.hh" +#include "content-address.hh" #include "legacy.hh" #include "shared.hh" #include "references.hh" -- cgit v1.2.3 From a5cdf1867efdf50f8cef324bbc36d1b840e13f8c Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 2 Jun 2020 16:13:08 -0400 Subject: Add assertions for SHA256 in fixed case --- src/libstore/content-address.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 421127f5a..4c3af18fd 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -51,10 +51,14 @@ 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)) }; + Hash hash = Hash(string(hashRaw)); + assert(hash.type == HashType::SHA256); + return FileSystemHash { FileIngestionMethod::Recursive, hash }; } else { std::string_view hashRaw = methodAndHash; - return FileSystemHash { FileIngestionMethod::Flat, Hash(string(hashRaw)) }; + Hash hash = Hash(string(hashRaw)); + assert(hash.type == HashType::SHA256); + return FileSystemHash { FileIngestionMethod::Flat, hash }; } } else { throw "parseContentAddress: format not recognized; has to be text or fixed"; -- cgit v1.2.3 From 78f137e931eff3c5133fe0a58f5d469f50959556 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 2 Jun 2020 16:20:22 -0400 Subject: Validate text version instead, throw Errors --- src/libstore/content-address.cc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 4c3af18fd..8a8112fd0 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -45,26 +45,26 @@ ContentAddress parseContentAddress(std::string_view rawCa) { auto prefix = string(rawCa, 0, prefixSeparator); if (prefix == "text") { auto hashTypeAndHash = rawCa.substr(prefixSeparator+1, string::npos); - return TextHash { Hash(string(hashTypeAndHash)) }; + Hash hash = Hash(string(hashTypeAndHash)); + if (hash.type != HashType::SHA256) { + throw Error("parseContentAddress: the text hash should have type SHA256"); + } + return TextHash { hash }; } else if (prefix == "fixed") { // This has to be an inverse to makeFixedOutputCA auto methodAndHash = rawCa.substr(prefixSeparator+1, string::npos); if (methodAndHash.substr(0,2) == "r:") { std::string_view hashRaw = methodAndHash.substr(2,string::npos); - Hash hash = Hash(string(hashRaw)); - assert(hash.type == HashType::SHA256); - return FileSystemHash { FileIngestionMethod::Recursive, hash }; + return FileSystemHash { FileIngestionMethod::Recursive, Hash(string(hashRaw)) }; } else { std::string_view hashRaw = methodAndHash; - Hash hash = Hash(string(hashRaw)); - assert(hash.type == HashType::SHA256); - return FileSystemHash { FileIngestionMethod::Flat, hash }; + return FileSystemHash { FileIngestionMethod::Flat, Hash(string(hashRaw)) }; } } else { - throw "parseContentAddress: format not recognized; has to be text or fixed"; + throw Error("parseContentAddress: format not recognized; has to be text or fixed"); } } else { - throw "Not a content address because it lacks an appropriate prefix"; + throw Error("Not a content address because it lacks an appropriate prefix"); } }; -- cgit v1.2.3 From 75d2581390e70c9f5820311ff25ae13521ad07e2 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 2 Jun 2020 16:21:18 -0400 Subject: Typo --- src/libstore/content-address.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 8a8112fd0..718c7ffc3 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -51,7 +51,7 @@ ContentAddress parseContentAddress(std::string_view rawCa) { } return TextHash { hash }; } else if (prefix == "fixed") { - // This has to be an inverse to makeFixedOutputCA + // This has to be an inverse of makeFixedOutputCA auto methodAndHash = rawCa.substr(prefixSeparator+1, string::npos); if (methodAndHash.substr(0,2) == "r:") { std::string_view hashRaw = methodAndHash.substr(2,string::npos); -- cgit v1.2.3 From 39ba87be9b426d75de50d35c9199661067218adc Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 2 Jun 2020 21:36:53 +0000 Subject: Missing `#include ` in `lru-cache.hh` This was a latent bug that just appeared because of the tests that were added. Remember to wait for CI! :) --- src/libutil/lru-cache.hh | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libutil/lru-cache.hh b/src/libutil/lru-cache.hh index 8b83f842c..6ef4a3e06 100644 --- a/src/libutil/lru-cache.hh +++ b/src/libutil/lru-cache.hh @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include -- cgit v1.2.3 From 53bc8ff152091173e11f2bead245541e3037d769 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 3 Jun 2020 20:35:14 -0400 Subject: No C++ designated initializers yet with Clang 7 --- src/libfetchers/tarball.cc | 4 ++-- src/libstore/build.cc | 5 +---- src/libstore/local-store.cc | 5 +---- src/nix/add-to-store.cc | 4 ++-- src/nix/make-content-addressable.cc | 4 ++-- 5 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index e8e5231d2..f8813ea1a 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -71,8 +71,8 @@ DownloadFileResult downloadFile( info.narHash = hashString(HashType::SHA256, *sink.s); info.narSize = sink.s->size(); info.ca = FileSystemHash { - .method = FileIngestionMethod::Flat, - .hash = hash, + FileIngestionMethod::Flat, + hash, }; auto source = StringSource { *sink.s }; store->addToStore(info, source, NoRepair, NoCheckSigs); diff --git a/src/libstore/build.cc b/src/libstore/build.cc index ea7319150..3eb103e94 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3757,10 +3757,7 @@ void DerivationGoal::registerOutputs() else assert(worker.store.parseStorePath(path) == dest); - ca = FileSystemHash { - .method = i.second.hash->method, - .hash = h2, - }; + ca = FileSystemHash { i.second.hash->method, h2 }; } /* Get rid of all weird permissions. This also checks that diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 5f0053753..af6c6f390 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1093,10 +1093,7 @@ StorePath LocalStore::addToStoreFromDump(const string & dump, const string & nam ValidPathInfo info(dstPath.clone()); info.narHash = hash.first; info.narSize = hash.second; - info.ca = FileSystemHash { - .method = method, - .hash = h, - }; + info.ca = FileSystemHash { method, h }; registerValidPath(info); } diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index b92597cc2..3162e0c62 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -49,8 +49,8 @@ struct CmdAddToStore : MixDryRun, StoreCommand info.narHash = narHash; info.narSize = sink.s->size(); *info.ca = FileSystemHash { - .method = FileIngestionMethod::Recursive, - .hash = info.narHash + FileIngestionMethod::Recursive, + info.narHash, }; if (!dryRun) { diff --git a/src/nix/make-content-addressable.cc b/src/nix/make-content-addressable.cc index 5003ff459..d19e681bd 100644 --- a/src/nix/make-content-addressable.cc +++ b/src/nix/make-content-addressable.cc @@ -83,8 +83,8 @@ struct CmdMakeContentAddressable : StorePathsCommand, MixJSON info.narHash = narHash; info.narSize = sink.s->size(); info.ca = FileSystemHash { - .method = FileIngestionMethod::Recursive, - .hash = info.narHash, + FileIngestionMethod::Recursive, + info.narHash, }; if (!json) -- cgit v1.2.3 From 574d5460f02c46e9e5bcca93a14ccf782cb2c8f8 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 4 Jun 2020 20:33:28 +0000 Subject: Make sure info.ca tag bit is set in `nix add-to-store` --- src/nix/add-to-store.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index 3162e0c62..e6f6ef27b 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -48,10 +48,10 @@ struct CmdAddToStore : MixDryRun, StoreCommand ValidPathInfo info(store->makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, *namePart)); info.narHash = narHash; info.narSize = sink.s->size(); - *info.ca = FileSystemHash { + info.ca = std::optional { FileSystemHash { FileIngestionMethod::Recursive, info.narHash, - }; + } }; if (!dryRun) { auto source = StringSource { *sink.s }; -- cgit v1.2.3 From ed86acf02aa5810044b995dc57744d7b51867bb0 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 4 Jun 2020 20:42:02 +0000 Subject: Use some `std::optional::has_value` for clarity --- src/libstore/local-store.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index af6c6f390..804d4c514 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -576,7 +576,7 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat uint64_t LocalStore::addValidPath(State & state, const ValidPathInfo & info, bool checkOutputs) { - if (info.ca && !info.isContentAddressed(*this)) + if (info.ca.has_value() && !info.isContentAddressed(*this)) throw Error("cannot add path '%s' to the Nix store because it claims to be content-addressed but isn't", printStorePath(info.path)); @@ -1001,13 +1001,13 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, // text hashing has long been allowed to have non-self-references because it is used for drv files. bool refersToSelf = info.references.count(info.path) > 0; - if (info.ca && !info.references.empty() && !(std::holds_alternative(*info.ca) && !refersToSelf)) + if (info.ca.has_value() && !info.references.empty() && !(std::holds_alternative(*info.ca) && !refersToSelf)) settings.requireExperimentalFeature("ca-references"); /* While restoring the path from the NAR, compute the hash of the NAR. */ std::unique_ptr hashSink; - if (info.ca || !info.references.count(info.path)) + if (info.ca.has_value() || !info.references.count(info.path)) hashSink = std::make_unique(HashType::SHA256); else hashSink = std::make_unique(HashType::SHA256, storePathToHash(printStorePath(info.path))); -- cgit v1.2.3 From 2041499b5e62ab45b23c3f2ceee337cf8d2193bf Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 4 Jun 2020 20:42:25 +0000 Subject: Flip boolean Thanks Matt! --- src/libstore/local-store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 804d4c514..ba5436ca4 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1007,7 +1007,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, /* While restoring the path from the NAR, compute the hash of the NAR. */ std::unique_ptr hashSink; - if (info.ca.has_value() || !info.references.count(info.path)) + if (!info.ca.has_value() || !info.references.count(info.path)) hashSink = std::make_unique(HashType::SHA256); else hashSink = std::make_unique(HashType::SHA256, storePathToHash(printStorePath(info.path))); -- cgit v1.2.3 From 94ddea9e2fffe2c0818653c509f881ae00f14d93 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 4 Jun 2020 20:54:55 +0000 Subject: Use `readString` rather than `>> temporary` Fixed the rest of these before, but this one slipped through. --- src/libstore/daemon.cc | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 0734f990a..63b8d4bfe 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -708,11 +708,7 @@ static void performOp(TunnelLogger * logger, ref store, info.references = readStorePaths(*store, from); from >> info.registrationTime >> info.narSize >> info.ultimate; info.sigs = readStrings(from); - { - string caOptRaw; - from >> caOptRaw; - info.ca = parseContentAddressOpt(caOptRaw); - } + info.ca = parseContentAddressOpt(readString(from)); from >> repair >> dontCheckSigs; if (!trusted && dontCheckSigs) dontCheckSigs = false; -- cgit v1.2.3 From d614166cb6864f2448b9e03f8dccccf301dc541e Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Thu, 4 Jun 2020 17:21:21 -0400 Subject: Fix condition error and make test suite pass --- src/libstore/local-store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index ba5436ca4..37c9a2bc3 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1265,7 +1265,7 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair) printMsg(Verbosity::Talkative, "checking contents of '%s'", printStorePath(i)); std::unique_ptr hashSink; - if (info->ca || !info->references.count(info->path)) + if (!info->ca || !info->references.count(info->path)) hashSink = std::make_unique(*info->narHash.type); else hashSink = std::make_unique(*info->narHash.type, storePathToHash(printStorePath(info->path))); -- cgit v1.2.3 From 3fc58a96387f55226ad78a0d8e765a0f041e8da0 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 19 Jun 2020 00:24:47 +0000 Subject: Remove some `Base::` that crept in --- src/libstore/content-address.cc | 2 +- src/libstore/derivations.cc | 4 ++-- src/libutil/hash.cc | 2 +- src/nix/show-derivation.cc | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 5bf51cb97..7de5c0874 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -31,7 +31,7 @@ template overloaded(Ts...) -> overloaded; std::string renderContentAddress(ContentAddress ca) { return std::visit(overloaded { [](TextHash th) { - return "text:" + th.hash.to_string(Base::Base32, true); + return "text:" + th.hash.to_string(Base32, true); }, [](FileSystemHash fsh) { return makeFixedOutputCA(fsh.method, fsh.hash); diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 1ba957c4a..6383e31f4 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -267,7 +267,7 @@ string Derivation::unparse(const Store & store, bool maskOutputs, s += ','; printUnquotedString(s, maskOutputs ? "" : store.printStorePath(i.second.path)); s += ','; printUnquotedString(s, i.second.hash ? i.second.hash->printMethodAlgo() : ""); s += ','; printUnquotedString(s, - i.second.hash ? i.second.hash->hash.to_string(Base::Base16, false) : ""); + i.second.hash ? i.second.hash->hash.to_string(Base16, false) : ""); s += ')'; } @@ -467,7 +467,7 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr out << i.first << store.printStorePath(i.second.path) << i.second.hash->printMethodAlgo() - << i.second.hash->hash.to_string(Base::Base16, false); + << i.second.hash->hash.to_string(Base16, false); writeStorePaths(store, out, drv.inputSrcs); out << drv.platform << drv.builder << drv.args; out << drv.env.size(); diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 7a8d091df..b187249e9 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -223,7 +223,7 @@ Hash newHashAllowEmpty(std::string hashStr, std::optional ht) if (!ht) throw BadHash("empty hash requires explicit hash type"); Hash h(*ht); - warn("found empty hash, assuming '%s'", h.to_string(Base::SRI, true)); + warn("found empty hash, assuming '%s'", h.to_string(SRI, true)); return h; } else return Hash(hashStr, ht); diff --git a/src/nix/show-derivation.cc b/src/nix/show-derivation.cc index 9132c75b1..5d77cfdca 100644 --- a/src/nix/show-derivation.cc +++ b/src/nix/show-derivation.cc @@ -72,7 +72,7 @@ struct CmdShowDerivation : InstallablesCommand outputObj.attr("path", store->printStorePath(output.second.path)); if (output.second.hash) { outputObj.attr("hashAlgo", output.second.hash->printMethodAlgo()); - outputObj.attr("hash", output.second.hash->hash.to_string(Base::Base16, false)); + outputObj.attr("hash", output.second.hash->hash.to_string(Base16, false)); } } } -- cgit v1.2.3 From 911fc88bcb467ff779c5d0ab49e0a5525f284e19 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 19 Jun 2020 17:42:56 +0000 Subject: More designated initializers --- src/libfetchers/tarball.cc | 4 ++-- src/libstore/build.cc | 5 ++++- src/libstore/content-address.cc | 10 ++++++++-- src/libstore/local-store.cc | 2 +- src/nix/add-to-store.cc | 4 ++-- src/nix/make-content-addressable.cc | 4 ++-- 6 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index da8d20d98..f5356f0af 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -71,8 +71,8 @@ DownloadFileResult downloadFile( info.narHash = hashString(htSHA256, *sink.s); info.narSize = sink.s->size(); info.ca = FixedOutputHash { - FileIngestionMethod::Flat, - hash, + .method = FileIngestionMethod::Flat, + .hash = hash, }; auto source = StringSource { *sink.s }; store->addToStore(info, source, NoRepair, NoCheckSigs); diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 1126c186f..0c25897f8 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3764,7 +3764,10 @@ void DerivationGoal::registerOutputs() else assert(worker.store.parseStorePath(path) == dest); - ca = FixedOutputHash { i.second.hash->method, h2 }; + ca = FixedOutputHash { + .method = i.second.hash->method, + .hash = 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 index 98b3677bb..3d753836f 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -55,10 +55,16 @@ 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 FixedOutputHash { FileIngestionMethod::Recursive, Hash(string(hashRaw)) }; + return FixedOutputHash { + .method = FileIngestionMethod::Recursive, + .hash = Hash(string(hashRaw)), + }; } else { std::string_view hashRaw = methodAndHash; - return FixedOutputHash { FileIngestionMethod::Flat, Hash(string(hashRaw)) }; + return FixedOutputHash { + .method = FileIngestionMethod::Flat, + .hash = Hash(string(hashRaw)), + }; } } else { throw Error("parseContentAddress: format not recognized; has to be text or fixed"); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index f9b2db2e3..0dfbed9fc 100644 --- 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 = FixedOutputHash { method, h }; + info.ca = FixedOutputHash { .method = method, .hash = h }; registerValidPath(info); } diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index 6d90e93e8..f9d6de16e 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -49,8 +49,8 @@ struct CmdAddToStore : MixDryRun, StoreCommand info.narHash = narHash; info.narSize = sink.s->size(); info.ca = std::optional { FixedOutputHash { - FileIngestionMethod::Recursive, - info.narHash, + .method = FileIngestionMethod::Recursive, + .hash = info.narHash, } }; if (!dryRun) { diff --git a/src/nix/make-content-addressable.cc b/src/nix/make-content-addressable.cc index c506eabd8..fb36fc410 100644 --- a/src/nix/make-content-addressable.cc +++ b/src/nix/make-content-addressable.cc @@ -83,8 +83,8 @@ struct CmdMakeContentAddressable : StorePathsCommand, MixJSON info.narHash = narHash; info.narSize = sink.s->size(); info.ca = FixedOutputHash { - FileIngestionMethod::Recursive, - info.narHash, + .method = FileIngestionMethod::Recursive, + .hash = info.narHash, }; if (!json) -- cgit v1.2.3