From 55d4bd6e0ea23dcf55dc33a964514230ee785bbe Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 22 Jun 2020 18:08:27 +0000 Subject: [PATCH 01/30] Improve content address parsing - Ensure hash is in form - and not SRI. - Better errors if something goes wrong - string_view for no coppying --- src/libstore/content-address.cc | 77 ++++++++++++++++++++------------- src/libutil/hash.cc | 4 +- src/libutil/hash.hh | 4 +- 3 files changed, 50 insertions(+), 35 deletions(-) diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 3d753836f..216d7eb03 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -1,3 +1,4 @@ +#include "args.hh" #include "content-address.hh" namespace nix { @@ -40,38 +41,52 @@ std::string renderContentAddress(ContentAddress 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); - Hash hash = Hash(string(hashTypeAndHash)); - if (*hash.type != htSHA256) { - throw Error("parseContentAddress: the text hash should have type SHA256"); - } - return TextHash { hash }; - } else if (prefix == "fixed") { - // 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); - return FixedOutputHash { - .method = FileIngestionMethod::Recursive, - .hash = Hash(string(hashRaw)), - }; - } else { - std::string_view hashRaw = methodAndHash; - return FixedOutputHash { - .method = FileIngestionMethod::Flat, - .hash = Hash(string(hashRaw)), - }; - } - } else { - throw Error("parseContentAddress: format not recognized; has to be text or fixed"); + auto rest = rawCa; + + // Ensure prefix + const auto prefixSeparator = rawCa.find(':'); + if (prefixSeparator == string::npos) + throw UsageError("not a content address because it is not in the form \":\": %s", rawCa); + auto prefix = rest.substr(0, prefixSeparator); + rest = rest.substr(prefixSeparator + 1); + + auto parseHashType_ = [&](){ + // Parse hash type + auto algoSeparator = rest.find(':'); + HashType hashType; + if (algoSeparator == string::npos) + throw UsageError("content address hash must be in form \":\", but found: %s", rest); + hashType = parseHashType(rest.substr(0, algoSeparator)); + + rest = rest.substr(algoSeparator + 1); + + return std::move(hashType); + }; + + // Switch on prefix + if (prefix == "text") { + // No parsing of the method, "text" only support flat. + HashType hashType = parseHashType_(); + if (hashType != htSHA256) + throw Error("text content address hash should use %s, but instead uses %s", + printHashType(htSHA256), printHashType(hashType)); + return TextHash { + .hash = Hash { rest, std::move(hashType) }, + }; + } else if (prefix == "fixed") { + // Parse method + auto method = FileIngestionMethod::Flat; + if (rest.substr(0, 2) == "r:") { + method = FileIngestionMethod::Recursive; + rest = rest.substr(2); } - } else { - throw Error("Not a content address because it lacks an appropriate prefix"); - } + HashType hashType = parseHashType_(); + return FixedOutputHash { + .method = method, + .hash = Hash { rest, std::move(hashType) }, + }; + } else + throw UsageError("content address prefix \"%s\" is unrecognized. Recogonized prefixes are \"text\" or \"fixed\"", prefix); }; std::optional parseContentAddressOpt(std::string_view rawCaOpt) { diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index c8fcdfed0..012fa727d 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -342,7 +342,7 @@ Hash compressHash(const Hash & hash, unsigned int newSize) } -std::optional parseHashTypeOpt(const string & s) +std::optional parseHashTypeOpt(std::string_view s) { if (s == "md5") return htMD5; else if (s == "sha1") return htSHA1; @@ -351,7 +351,7 @@ std::optional parseHashTypeOpt(const string & s) else return std::optional {}; } -HashType parseHashType(const string & s) +HashType parseHashType(std::string_view s) { auto opt_h = parseHashTypeOpt(s); if (opt_h) diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 0d9916508..e4abe72ce 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -121,9 +121,9 @@ HashResult hashPath(HashType ht, const Path & path, Hash compressHash(const Hash & hash, unsigned int newSize); /* Parse a string representing a hash type. */ -HashType parseHashType(const string & s); +HashType parseHashType(std::string_view s); /* Will return nothing on parse error */ -std::optional parseHashTypeOpt(const string & s); +std::optional parseHashTypeOpt(std::string_view s); /* And the reverse. */ string printHashType(HashType ht); From 7ba0fae0ddc815d7d7b2c3a9cd3d60879baeab54 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 30 Jun 2020 11:57:09 -0400 Subject: [PATCH 02/30] Create the spitPrefix function in parser.hh --- src/libutil/parser.hh | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 src/libutil/parser.hh diff --git a/src/libutil/parser.hh b/src/libutil/parser.hh new file mode 100644 index 000000000..64689e283 --- /dev/null +++ b/src/libutil/parser.hh @@ -0,0 +1,25 @@ +#pragma once + +#include + +namespace nix { + +// If `separator` is found, we return the portion of the string before the +// separator, and modify the string argument to contain only the part after the +// separator. Otherwise, wer return `std::nullopt`, and we leave the argument +// string alone. +std::optional splitPrefix(std::string_view & string, char separator); + +std::optional splitPrefix(std::string_view & string, char separator) { + auto sepInstance = string.find(separator); + + if (sepInstance != std::string_view::npos) { + auto prefix = string.substr(0, sepInstance); + string.remove_prefix(sepInstance+1); + return prefix; + } + + return std::nullopt; +} + +} From 77b51f4598763f0b3a435ed0e4ce98178d9e99da Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 30 Jun 2020 11:57:46 -0400 Subject: [PATCH 03/30] Factor the prefix splitting in content-address --- src/libstore/content-address.cc | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 4599cd924..8152e5215 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -1,5 +1,6 @@ #include "args.hh" #include "content-address.hh" +#include "parser.hh" namespace nix { @@ -43,23 +44,19 @@ std::string renderContentAddress(ContentAddress ca) { ContentAddress parseContentAddress(std::string_view rawCa) { auto rest = rawCa; - // Ensure prefix - const auto prefixSeparator = rawCa.find(':'); - if (prefixSeparator == string::npos) - throw UsageError("not a content address because it is not in the form \":\": %s", rawCa); - auto prefix = rest.substr(0, prefixSeparator); - rest = rest.substr(prefixSeparator + 1); + std::string_view prefix; + { + auto optPrefix = splitPrefix(rest, ':'); + if (!optPrefix) + throw UsageError("not a content address because it is not in the form \":\": %s", rawCa); + prefix = *optPrefix; + } auto parseHashType_ = [&](){ - // Parse hash type - auto algoSeparator = rest.find(':'); - HashType hashType; - if (algoSeparator == string::npos) - throw UsageError("content address hash must be in form \":\", but found: %s", rest); - hashType = parseHashType(rest.substr(0, algoSeparator)); - - rest = rest.substr(algoSeparator + 1); - + auto hashTypeRaw = splitPrefix(rest, ':'); + if (!hashTypeRaw) + throw UsageError("content address hash must be in form \":\", but found: %s", rawCa); + HashType hashType = parseHashType(*hashTypeRaw); return std::move(hashType); }; From a1f66d1d9e70d4204a3686002b02277465a6b7ab Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 30 Jun 2020 12:49:00 -0400 Subject: [PATCH 04/30] Factor the prefix splitting in hash --- src/libutil/hash.cc | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index e060700d9..26ab5a110 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -7,6 +7,7 @@ #include "args.hh" #include "hash.hh" #include "archive.hh" +#include "parser.hh" #include "util.hh" #include "istringstream_nocopy.hh" @@ -144,17 +145,15 @@ Hash::Hash(std::string_view original, std::optional optType) // Parse the has type before the separater, if there was one. std::optional optParsedType; { - auto sep = rest.find(':'); - if (sep == std::string_view::npos) { - sep = rest.find('-'); - if (sep != std::string_view::npos) + auto hashRaw = splitPrefix(rest, ':'); + + if (!hashRaw) { + hashRaw = splitPrefix(rest, '-'); + if (hashRaw) isSRI = true; } - if (sep != std::string_view::npos) { - auto hashRaw = rest.substr(0, sep); - optParsedType = parseHashType(hashRaw); - rest = rest.substr(sep + 1); - } + if (hashRaw) + optParsedType = parseHashType(*hashRaw); } // Either the string or user must provide the type, if they both do they From b798efb829415eb47a532e9479523afdff74eca7 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 30 Jun 2020 14:10:30 -0400 Subject: [PATCH 05/30] WIP initial design --- src/libutil/hash.cc | 17 ++++++++++++----- src/libutil/hash.hh | 7 +++++-- src/libutil/parser.hh | 5 ++--- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 26ab5a110..36de293bb 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -132,10 +132,13 @@ std::string Hash::to_string(Base base, bool includeType) const return s; } -Hash::Hash(std::string_view s, HashType type) : Hash(s, std::optional { type }) { } -Hash::Hash(std::string_view s) : Hash(s, std::optional{}) { } +Hash fromSRI(std::string_view original) { -Hash::Hash(std::string_view original, std::optional optType) +} + +Hash::Hash(std::string_view s) : Hash(s, std::nullopt) { } + +static HashType newFunction(std::string_view & rest, std::optional optType) { auto rest = original; @@ -161,13 +164,17 @@ Hash::Hash(std::string_view original, std::optional optType) if (!optParsedType && !optType) { throw BadHash("hash '%s' does not include a type, nor is the type otherwise known from context.", rest); } else { - this->type = optParsedType ? *optParsedType : *optType; if (optParsedType && optType && *optParsedType != *optType) throw BadHash("hash '%s' should have type '%s'", original, printHashType(*optType)); + return optParsedType ? *optParsedType : *optType; } +} - init(); +// mutates the string_view +Hash::Hash(std::string_view original, std::optional optType) + : Hash(original, newFunction(original, optType)) +Hash::Hash(std::string_view original, HashType type) : Hash(type) { if (!isSRI && rest.size() == base16Len()) { auto parseHexDigit = [&](char c) { diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index a55295912..42bd585a2 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -40,13 +40,16 @@ struct Hash is not present, then the hash type must be specified in the string. */ Hash(std::string_view s, std::optional type); - // type must be provided - Hash(std::string_view s, HashType type); // hash type must be part of string Hash(std::string_view s); +private: + // type must be provided, s must not include prefix + Hash(std::string_view s, HashType type); + void init(); +public: /* Check whether a hash is set. */ operator bool () const { return (bool) type; } diff --git a/src/libutil/parser.hh b/src/libutil/parser.hh index 64689e283..d3bfafe75 100644 --- a/src/libutil/parser.hh +++ b/src/libutil/parser.hh @@ -1,6 +1,7 @@ #pragma once #include +#include namespace nix { @@ -8,9 +9,7 @@ namespace nix { // separator, and modify the string argument to contain only the part after the // separator. Otherwise, wer return `std::nullopt`, and we leave the argument // string alone. -std::optional splitPrefix(std::string_view & string, char separator); - -std::optional splitPrefix(std::string_view & string, char separator) { +static inline std::optional splitPrefix(std::string_view & string, char separator) { auto sepInstance = string.find(separator); if (sepInstance != std::string_view::npos) { From c2e7f7a7129fc5366a4cad337fcd6ae319a58ce5 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Wed, 1 Jul 2020 17:32:06 -0400 Subject: [PATCH 06/30] Fixed build, we still have test errors --- src/libutil/hash.cc | 46 +++++++++++++++++++++++++++++---------------- src/libutil/hash.hh | 4 +++- src/libutil/util.cc | 2 +- 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 36de293bb..e8d290d13 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -132,17 +132,24 @@ std::string Hash::to_string(Base base, bool includeType) const return s; } -Hash fromSRI(std::string_view original) { +Hash Hash::fromSRI(std::string_view original) { + auto rest = original; + // Parse the has type before the separater, if there was one. + auto hashRaw = splitPrefix(rest, '-'); + if (!hashRaw) + throw BadHash("hash '%s' is not SRI", original); + HashType parsedType = parseHashType(*hashRaw); + + return Hash(rest, std::make_pair(parsedType, true)); } -Hash::Hash(std::string_view s) : Hash(s, std::nullopt) { } +Hash::Hash(std::string_view s) : Hash(s, std::nullopt) {} -static HashType newFunction(std::string_view & rest, std::optional optType) +static std::pair newFunction(std::string_view & original, std::optional optType) { auto rest = original; - size_t pos = 0; bool isSRI = false; // Parse the has type before the separater, if there was one. @@ -166,16 +173,23 @@ static HashType newFunction(std::string_view & rest, std::optional opt } else { if (optParsedType && optType && *optParsedType != *optType) throw BadHash("hash '%s' should have type '%s'", original, printHashType(*optType)); - return optParsedType ? *optParsedType : *optType; + return { + optParsedType ? *optParsedType : *optType, + isSRI, + }; } } // mutates the string_view Hash::Hash(std::string_view original, std::optional optType) - : Hash(original, newFunction(original, optType)) + : Hash(original, newFunction(original, optType)) {} -Hash::Hash(std::string_view original, HashType type) : Hash(type) { - if (!isSRI && rest.size() == base16Len()) { +Hash::Hash(std::string_view original, std::pair typeAndSRI) + : Hash(typeAndSRI.first) +{ + auto [type, isSRI] = std::move(typeAndSRI); + + if (!isSRI && original.size() == base16Len()) { auto parseHexDigit = [&](char c) { if (c >= '0' && c <= '9') return c - '0'; @@ -186,15 +200,15 @@ Hash::Hash(std::string_view original, HashType type) : Hash(type) { for (unsigned int i = 0; i < hashSize; i++) { hash[i] = - parseHexDigit(rest[pos + i * 2]) << 4 - | parseHexDigit(rest[pos + i * 2 + 1]); + parseHexDigit(original[i * 2]) << 4 + | parseHexDigit(original[i * 2 + 1]); } } - else if (!isSRI && rest.size() == base32Len()) { + else if (!isSRI && original.size() == base32Len()) { - for (unsigned int n = 0; n < rest.size(); ++n) { - char c = rest[rest.size() - n - 1]; + for (unsigned int n = 0; n < original.size(); ++n) { + char c = original[original.size() - n - 1]; unsigned char digit; for (digit = 0; digit < base32Chars.size(); ++digit) /* !!! slow */ if (base32Chars[digit] == c) break; @@ -214,8 +228,8 @@ Hash::Hash(std::string_view original, HashType type) : Hash(type) { } } - else if (isSRI || rest.size() == base64Len()) { - auto d = base64Decode(rest); + else if (isSRI || original.size() == base64Len()) { + auto d = base64Decode(original); if (d.size() != hashSize) throw BadHash("invalid %s hash '%s'", isSRI ? "SRI" : "base-64", original); assert(hashSize); @@ -223,7 +237,7 @@ Hash::Hash(std::string_view original, HashType type) : Hash(type) { } else - throw BadHash("hash '%s' has wrong length for hash type '%s'", rest, printHashType(this->type)); + throw BadHash("hash '%s' has wrong length for hash type '%s'", original, printHashType(this->type)); } Hash newHashAllowEmpty(std::string hashStr, std::optional ht) diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 42bd585a2..570e50dd4 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -43,9 +43,11 @@ struct Hash // hash type must be part of string Hash(std::string_view s); + Hash fromSRI(std::string_view original); + private: // type must be provided, s must not include prefix - Hash(std::string_view s, HashType type); + Hash(std::string_view s, std::pair typeAndSRI); void init(); diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 1268b146a..ed43c403f 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1433,7 +1433,7 @@ string base64Decode(std::string_view s) char digit = decode[(unsigned char) c]; if (digit == -1) - throw Error("invalid character in Base64 string"); + throw Error("invalid character in Base64 string: '%c'", c); bits += 6; d = d << 6 | digit; From 274a8136fbf3d0fffb564f33464da26aab924b60 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Wed, 1 Jul 2020 17:47:15 -0400 Subject: [PATCH 07/30] Correct FIXMEs in libfetchers --- src/libfetchers/fetchers.cc | 3 +-- src/libfetchers/tarball.cc | 6 ++---- src/libutil/hash.hh | 2 +- src/libutil/util.cc | 1 + 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 9174c3de4..91d0d6a1d 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -35,8 +35,7 @@ std::unique_ptr inputFromAttrs(const Attrs & attrs) auto res = inputScheme->inputFromAttrs(attrs2); if (res) { if (auto narHash = maybeGetStrAttr(attrs, "narHash")) - // FIXME: require SRI hash. - res->narHash = newHashAllowEmpty(*narHash, {}); + res->narHash = Hash::fromSRI(*narHash); return res; } } diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index f5356f0af..732fac8c3 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -242,15 +242,13 @@ struct TarballInputScheme : InputScheme auto hash = input->url.query.find("hash"); if (hash != input->url.query.end()) { - // FIXME: require SRI hash. - input->hash = Hash(hash->second); + input->hash = Hash::fromSRI(hash->second); input->url.query.erase(hash); } auto narHash = input->url.query.find("narHash"); if (narHash != input->url.query.end()) { - // FIXME: require SRI hash. - input->narHash = Hash(narHash->second); + input->narHash = Hash::fromSRI(narHash->second); input->url.query.erase(narHash); } diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 570e50dd4..887952ce5 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -43,7 +43,7 @@ struct Hash // hash type must be part of string Hash(std::string_view s); - Hash fromSRI(std::string_view original); + static Hash fromSRI(std::string_view original); private: // type must be provided, s must not include prefix diff --git a/src/libutil/util.cc b/src/libutil/util.cc index ed43c403f..ea28ee70d 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1,5 +1,6 @@ #include "lazy.hh" #include "util.hh" +#include "hash.hh" #include "affinity.hh" #include "sync.hh" #include "finally.hh" From 6faeec3b2a5446e3fa511ce8cb4b1a12621c6186 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Wed, 1 Jul 2020 17:50:34 -0400 Subject: [PATCH 08/30] Keep the previous name, for diffing --- src/libutil/hash.cc | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index e8d290d13..448eb25f9 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -184,36 +184,36 @@ static std::pair newFunction(std::string_view & original, std::o Hash::Hash(std::string_view original, std::optional optType) : Hash(original, newFunction(original, optType)) {} -Hash::Hash(std::string_view original, std::pair typeAndSRI) +Hash::Hash(std::string_view rest, std::pair typeAndSRI) : Hash(typeAndSRI.first) { auto [type, isSRI] = std::move(typeAndSRI); - if (!isSRI && original.size() == base16Len()) { + if (!isSRI && rest.size() == base16Len()) { auto parseHexDigit = [&](char c) { if (c >= '0' && c <= '9') return c - '0'; if (c >= 'A' && c <= 'F') return c - 'A' + 10; if (c >= 'a' && c <= 'f') return c - 'a' + 10; - throw BadHash("invalid base-16 hash '%s'", original); + throw BadHash("invalid base-16 hash '%s'", rest); }; for (unsigned int i = 0; i < hashSize; i++) { hash[i] = - parseHexDigit(original[i * 2]) << 4 - | parseHexDigit(original[i * 2 + 1]); + parseHexDigit(rest[i * 2]) << 4 + | parseHexDigit(rest[i * 2 + 1]); } } - else if (!isSRI && original.size() == base32Len()) { + else if (!isSRI && rest.size() == base32Len()) { - for (unsigned int n = 0; n < original.size(); ++n) { - char c = original[original.size() - n - 1]; + for (unsigned int n = 0; n < rest.size(); ++n) { + char c = rest[rest.size() - n - 1]; unsigned char digit; for (digit = 0; digit < base32Chars.size(); ++digit) /* !!! slow */ if (base32Chars[digit] == c) break; if (digit >= 32) - throw BadHash("invalid base-32 hash '%s'", original); + throw BadHash("invalid base-32 hash '%s'", rest); unsigned int b = n * 5; unsigned int i = b / 8; unsigned int j = b % 8; @@ -223,21 +223,21 @@ Hash::Hash(std::string_view original, std::pair typeAndSRI) hash[i + 1] |= digit >> (8 - j); } else { if (digit >> (8 - j)) - throw BadHash("invalid base-32 hash '%s'", original); + throw BadHash("invalid base-32 hash '%s'", rest); } } } - else if (isSRI || original.size() == base64Len()) { - auto d = base64Decode(original); + else if (isSRI || rest.size() == base64Len()) { + auto d = base64Decode(rest); if (d.size() != hashSize) - throw BadHash("invalid %s hash '%s'", isSRI ? "SRI" : "base-64", original); + throw BadHash("invalid %s hash '%s'", isSRI ? "SRI" : "base-64", rest); assert(hashSize); memcpy(hash, d.data(), hashSize); } else - throw BadHash("hash '%s' has wrong length for hash type '%s'", original, printHashType(this->type)); + throw BadHash("hash '%s' has wrong length for hash type '%s'", rest, printHashType(this->type)); } Hash newHashAllowEmpty(std::string hashStr, std::optional ht) From d63a5ded76079008b09256aa36ef0c222919e8fa Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Wed, 1 Jul 2020 17:53:24 -0400 Subject: [PATCH 09/30] Remove unused import --- src/libutil/util.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index ea28ee70d..ed43c403f 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1,6 +1,5 @@ #include "lazy.hh" #include "util.hh" -#include "hash.hh" #include "affinity.hh" #include "sync.hh" #include "finally.hh" From c8c4bcf90e065b47c3ee2984b1f8ff696da889af Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Wed, 1 Jul 2020 18:03:22 -0400 Subject: [PATCH 10/30] Inline Hash::init() --- src/libutil/hash.cc | 2 +- src/libutil/hash.hh | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 448eb25f9..2087e3464 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -27,7 +27,7 @@ static size_t regularHashSize(HashType type) { abort(); } -void Hash::init() +Hash::Hash(HashType type) : type(type) { hashSize = regularHashSize(type); assert(hashSize <= maxHashSize); diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 887952ce5..766009438 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -32,7 +32,7 @@ struct Hash HashType type; /* Create a zero-filled hash object. */ - Hash(HashType type) : type(type) { init(); }; + Hash(HashType type); /* Initialize the hash from a string representation, in the format "[:]" or "-" (a @@ -49,8 +49,6 @@ private: // type must be provided, s must not include prefix Hash(std::string_view s, std::pair typeAndSRI); - void init(); - public: /* Check whether a hash is set. */ operator bool () const { return (bool) type; } From 263ccdd48923b730fd7e6f687583160d7b24039b Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Wed, 1 Jul 2020 18:34:18 -0400 Subject: [PATCH 11/30] Rename two hash constructors to proper functions --- src/libexpr/primops/fetchGit.cc | 2 +- src/libexpr/primops/fetchMercurial.cc | 2 +- src/libfetchers/git.cc | 8 ++++---- src/libfetchers/github.cc | 8 ++++---- src/libfetchers/mercurial.cc | 6 +++--- src/libfetchers/path.cc | 4 ++-- src/libstore/content-address.cc | 4 ++-- src/libstore/daemon.cc | 2 +- src/libstore/derivations.cc | 4 ++-- src/libstore/legacy-ssh-store.cc | 2 +- src/libstore/local-store.cc | 2 +- src/libstore/nar-info-disk-cache.cc | 4 ++-- src/libstore/nar-info.cc | 2 +- src/libstore/remote-store.cc | 2 +- src/libstore/store-api.cc | 2 +- src/libutil/hash.cc | 14 ++++++++++---- src/libutil/hash.hh | 6 ++++-- src/nix-prefetch-url/nix-prefetch-url.cc | 2 +- src/nix-store/nix-store.cc | 4 ++-- src/nix/hash.cc | 2 +- 20 files changed, 45 insertions(+), 37 deletions(-) diff --git a/src/libexpr/primops/fetchGit.cc b/src/libexpr/primops/fetchGit.cc index dd7229a3d..0421318d1 100644 --- a/src/libexpr/primops/fetchGit.cc +++ b/src/libexpr/primops/fetchGit.cc @@ -29,7 +29,7 @@ static void prim_fetchGit(EvalState & state, const Pos & pos, Value * * args, Va else if (n == "ref") ref = state.forceStringNoCtx(*attr.value, *attr.pos); else if (n == "rev") - rev = Hash(state.forceStringNoCtx(*attr.value, *attr.pos), htSHA1); + rev = Hash::parseAny(state.forceStringNoCtx(*attr.value, *attr.pos), htSHA1); else if (n == "name") name = state.forceStringNoCtx(*attr.value, *attr.pos); else if (n == "submodules") diff --git a/src/libexpr/primops/fetchMercurial.cc b/src/libexpr/primops/fetchMercurial.cc index 9bace8f89..236219a6f 100644 --- a/src/libexpr/primops/fetchMercurial.cc +++ b/src/libexpr/primops/fetchMercurial.cc @@ -31,7 +31,7 @@ static void prim_fetchMercurial(EvalState & state, const Pos & pos, Value * * ar // be both a revision or a branch/tag name. auto value = state.forceStringNoCtx(*attr.value, *attr.pos); if (std::regex_match(value, revRegex)) - rev = Hash(value, htSHA1); + rev = Hash::parseAny(value, htSHA1); else ref = value; } diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 75ce5ee8b..909bac78d 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -225,14 +225,14 @@ struct GitInput : Input if (isLocal) { if (!input->rev) - input->rev = Hash(chomp(runProgram("git", true, { "-C", actualUrl, "rev-parse", *input->ref })), htSHA1); + input->rev = Hash::parseAny(chomp(runProgram("git", true, { "-C", actualUrl, "rev-parse", *input->ref })), htSHA1); repoDir = actualUrl; } else { if (auto res = getCache()->lookup(store, mutableAttrs)) { - auto rev2 = Hash(getStrAttr(res->first, "rev"), htSHA1); + auto rev2 = Hash::parseAny(getStrAttr(res->first, "rev"), htSHA1); if (!rev || rev == rev2) { input->rev = rev2; return makeResult(res->first, std::move(res->second)); @@ -301,7 +301,7 @@ struct GitInput : Input } if (!input->rev) - input->rev = Hash(chomp(readFile(localRefFile)), htSHA1); + input->rev = Hash::parseAny(chomp(readFile(localRefFile)), htSHA1); } bool isShallow = chomp(runProgram("git", true, { "-C", repoDir, "rev-parse", "--is-shallow-repository" })) == "true"; @@ -426,7 +426,7 @@ struct GitInputScheme : InputScheme input->ref = *ref; } if (auto rev = maybeGetStrAttr(attrs, "rev")) - input->rev = Hash(*rev, htSHA1); + input->rev = Hash::parseAny(*rev, htSHA1); input->shallow = maybeGetBoolAttr(attrs, "shallow").value_or(false); diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index 0bee1d6b3..449481092 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -76,7 +76,7 @@ struct GitHubInput : Input readFile( store->toRealPath( downloadFile(store, url, "source", false).storePath))); - rev = Hash(std::string { json["sha"] }, htSHA1); + rev = Hash::parseAny(std::string { json["sha"] }, htSHA1); debug("HEAD revision for '%s' is %s", url, rev->gitRev()); } @@ -140,7 +140,7 @@ struct GitHubInputScheme : InputScheme if (path.size() == 2) { } else if (path.size() == 3) { if (std::regex_match(path[2], revRegex)) - input->rev = Hash(path[2], htSHA1); + input->rev = Hash::parseAny(path[2], htSHA1); else if (std::regex_match(path[2], refRegex)) input->ref = path[2]; else @@ -152,7 +152,7 @@ struct GitHubInputScheme : InputScheme if (name == "rev") { if (input->rev) throw BadURL("GitHub URL '%s' contains multiple commit hashes", url.url); - input->rev = Hash(value, htSHA1); + input->rev = Hash::parseAny(value, htSHA1); } else if (name == "ref") { if (!std::regex_match(value, refRegex)) @@ -185,7 +185,7 @@ struct GitHubInputScheme : InputScheme input->repo = getStrAttr(attrs, "repo"); input->ref = maybeGetStrAttr(attrs, "ref"); if (auto rev = maybeGetStrAttr(attrs, "rev")) - input->rev = Hash(*rev, htSHA1); + input->rev = Hash::parseAny(*rev, htSHA1); return input; } }; diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index 2e0d4bf4d..29f2a9d5b 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -167,7 +167,7 @@ struct MercurialInput : Input }); if (auto res = getCache()->lookup(store, mutableAttrs)) { - auto rev2 = Hash(getStrAttr(res->first, "rev"), htSHA1); + auto rev2 = Hash::parseAny(getStrAttr(res->first, "rev"), htSHA1); if (!rev || rev == rev2) { input->rev = rev2; return makeResult(res->first, std::move(res->second)); @@ -210,7 +210,7 @@ struct MercurialInput : Input runProgram("hg", true, { "log", "-R", cacheDir, "-r", revOrRef, "--template", "{node} {rev} {branch}" })); assert(tokens.size() == 3); - input->rev = Hash(tokens[0], htSHA1); + input->rev = Hash::parseAny(tokens[0], htSHA1); auto revCount = std::stoull(tokens[1]); input->ref = tokens[2]; @@ -293,7 +293,7 @@ struct MercurialInputScheme : InputScheme input->ref = *ref; } if (auto rev = maybeGetStrAttr(attrs, "rev")) - input->rev = Hash(*rev, htSHA1); + input->rev = Hash::parseAny(*rev, htSHA1); return input; } }; diff --git a/src/libfetchers/path.cc b/src/libfetchers/path.cc index ba2cc192e..1caab4165 100644 --- a/src/libfetchers/path.cc +++ b/src/libfetchers/path.cc @@ -101,7 +101,7 @@ struct PathInputScheme : InputScheme for (auto & [name, value] : url.query) if (name == "rev") - input->rev = Hash(value, htSHA1); + input->rev = Hash::parseAny(value, htSHA1); else if (name == "revCount") { uint64_t revCount; if (!string2Int(value, revCount)) @@ -129,7 +129,7 @@ struct PathInputScheme : InputScheme for (auto & [name, value] : attrs) if (name == "rev") - input->rev = Hash(getStrAttr(attrs, "rev"), htSHA1); + input->rev = Hash::parseAny(getStrAttr(attrs, "rev"), htSHA1); else if (name == "revCount") input->revCount = getIntAttr(attrs, "revCount"); else if (name == "lastModified") diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 8152e5215..2d96fb0c0 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -68,7 +68,7 @@ ContentAddress parseContentAddress(std::string_view rawCa) { throw Error("text content address hash should use %s, but instead uses %s", printHashType(htSHA256), printHashType(hashType)); return TextHash { - .hash = Hash { rest, std::move(hashType) }, + .hash = Hash::parseAny(rest, std::move(hashType)), }; } else if (prefix == "fixed") { // Parse method @@ -80,7 +80,7 @@ ContentAddress parseContentAddress(std::string_view rawCa) { HashType hashType = parseHashType_(); return FixedOutputHash { .method = method, - .hash = Hash { rest, std::move(hashType) }, + .hash = Hash::parseAny(rest, std::move(hashType)), }; } else throw UsageError("content address prefix \"%s\" is unrecognized. Recogonized prefixes are \"text\" or \"fixed\"", prefix); diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 0b48e04c0..cc4827e64 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -706,7 +706,7 @@ static void performOp(TunnelLogger * logger, ref store, auto deriver = readString(from); if (deriver != "") info.deriver = store->parseStorePath(deriver); - info.narHash = Hash(readString(from), htSHA256); + info.narHash = Hash::parseAny(readString(from), htSHA256); info.references = readStorePaths(*store, from); from >> info.registrationTime >> info.narSize >> info.ultimate; info.sigs = readStrings(from); diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 42551ef6b..30ce32354 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -118,7 +118,7 @@ static DerivationOutput parseDerivationOutput(const Store & store, istringstream const HashType hashType = parseHashType(hashAlgo); fsh = FixedOutputHash { .method = std::move(method), - .hash = Hash(hash, hashType), + .hash = Hash::parseAny(hash, hashType), }; } @@ -416,7 +416,7 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store) auto hashType = parseHashType(hashAlgo); fsh = FixedOutputHash { .method = std::move(method), - .hash = Hash(hash, hashType), + .hash = Hash::parseAny(hash, hashType), }; } diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 35caf23e7..f9d95fd2b 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -113,7 +113,7 @@ struct LegacySSHStore : public Store if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 4) { auto s = readString(conn->from); - info->narHash = s.empty() ? std::optional{} : Hash{s}; + info->narHash = s.empty() ? std::optional{} : Hash::parseAnyPrefixed(s); 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 9259d8f61..b75e2bdfe 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -647,7 +647,7 @@ void LocalStore::queryPathInfoUncached(const StorePath & path, info->id = useQueryPathInfo.getInt(0); try { - info->narHash = Hash(useQueryPathInfo.getStr(1)); + info->narHash = Hash::parseAnyPrefixed(useQueryPathInfo.getStr(1)); } catch (BadHash & e) { throw Error("in valid-path entry for '%s': %s", printStorePath(path), e.what()); } diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index 9ddb9957f..92da14e23 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -193,9 +193,9 @@ public: narInfo->url = queryNAR.getStr(2); narInfo->compression = queryNAR.getStr(3); if (!queryNAR.isNull(4)) - narInfo->fileHash = Hash(queryNAR.getStr(4)); + narInfo->fileHash = Hash::parseAnyPrefixed(queryNAR.getStr(4)); narInfo->fileSize = queryNAR.getInt(5); - narInfo->narHash = Hash(queryNAR.getStr(6)); + narInfo->narHash = Hash::parseAnyPrefixed(queryNAR.getStr(6)); narInfo->narSize = queryNAR.getInt(7); for (auto & r : tokenizeString(queryNAR.getStr(8), " ")) narInfo->references.insert(StorePath(r)); diff --git a/src/libstore/nar-info.cc b/src/libstore/nar-info.cc index c403d4bec..2a52e4098 100644 --- a/src/libstore/nar-info.cc +++ b/src/libstore/nar-info.cc @@ -12,7 +12,7 @@ NarInfo::NarInfo(const Store & store, const std::string & s, const std::string & auto parseHashField = [&](const string & s) { try { - return Hash(s); + return Hash::parseAnyPrefixed(s); } catch (BadHash &) { throw corrupt(); } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 305a47340..890d96388 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -375,7 +375,7 @@ void RemoteStore::queryPathInfoUncached(const StorePath & path, info = std::make_shared(StorePath(path)); auto deriver = readString(conn->from); if (deriver != "") info->deriver = parseStorePath(deriver); - info->narHash = Hash(readString(conn->from), htSHA256); + info->narHash = Hash::parseAny(readString(conn->from), htSHA256); info->references = readStorePaths(*this, conn->from); conn->from >> info->registrationTime >> info->narSize; if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 16) { diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index e4083bbe1..080ce9823 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -703,7 +703,7 @@ std::optional decodeValidPathInfo(const Store & store, std::istre if (hashGiven) { string s; getline(str, s); - info.narHash = Hash(s, htSHA256); + info.narHash = Hash::parseAny(s, htSHA256); getline(str, s); if (!string2Int(s, info.narSize)) throw Error("number expected"); } diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 2087e3464..fcc0b9eb7 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -144,7 +144,10 @@ Hash Hash::fromSRI(std::string_view original) { return Hash(rest, std::make_pair(parsedType, true)); } -Hash::Hash(std::string_view s) : Hash(s, std::nullopt) {} +Hash Hash::parseAnyPrefixed(std::string_view s) +{ + return parseAny(s, std::nullopt); +} static std::pair newFunction(std::string_view & original, std::optional optType) { @@ -181,8 +184,11 @@ static std::pair newFunction(std::string_view & original, std::o } // mutates the string_view -Hash::Hash(std::string_view original, std::optional optType) - : Hash(original, newFunction(original, optType)) {} +Hash Hash::parseAny(std::string_view original, std::optional optType) +{ + auto typeAndSRI = newFunction(original, optType); + return Hash(original, typeAndSRI); +} Hash::Hash(std::string_view rest, std::pair typeAndSRI) : Hash(typeAndSRI.first) @@ -249,7 +255,7 @@ Hash newHashAllowEmpty(std::string hashStr, std::optional ht) warn("found empty hash, assuming '%s'", h.to_string(SRI, true)); return h; } else - return Hash(hashStr, ht); + return Hash::parseAny(hashStr, ht); } diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 766009438..3e413a52c 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -39,9 +39,11 @@ struct Hash Subresource Integrity hash expression). If the 'type' argument is not present, then the hash type must be specified in the string. */ - Hash(std::string_view s, std::optional type); + static Hash parseAny(std::string_view s, std::optional type); // hash type must be part of string - Hash(std::string_view s); + static Hash parseAnyPrefixed(std::string_view s); + // prefix parsed separately; non SRI hash + static Hash parseAnyUnprefixed(std::string_view s, HashType type); static Hash fromSRI(std::string_view original); diff --git a/src/nix-prefetch-url/nix-prefetch-url.cc b/src/nix-prefetch-url/nix-prefetch-url.cc index 22410c44c..f752e0448 100644 --- a/src/nix-prefetch-url/nix-prefetch-url.cc +++ b/src/nix-prefetch-url/nix-prefetch-url.cc @@ -156,7 +156,7 @@ static int _main(int argc, char * * argv) Hash hash(ht), expectedHash(ht); std::optional storePath; if (args.size() == 2) { - expectedHash = Hash(args[1], ht); + expectedHash = Hash::parseAny(args[1], ht); const auto recursive = unpack ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; storePath = store->makeFixedOutputPath(recursive, expectedHash, name); if (store->isValidPath(*storePath)) diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index ff04cbefc..cc3b07c31 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -208,7 +208,7 @@ static void opPrintFixedPath(Strings opFlags, Strings opArgs) string hash = *i++; string name = *i++; - cout << fmt("%s\n", store->printStorePath(store->makeFixedOutputPath(recursive, Hash(hash, hashAlgo), name))); + cout << fmt("%s\n", store->printStorePath(store->makeFixedOutputPath(recursive, Hash::parseAny(hash, hashAlgo), name))); } @@ -950,7 +950,7 @@ static void opServe(Strings opFlags, Strings opArgs) auto deriver = readString(in); if (deriver != "") info.deriver = store->parseStorePath(deriver); - info.narHash = Hash(readString(in), htSHA256); + info.narHash = Hash::parseAny(readString(in), htSHA256); info.references = readStorePaths(*store, in); in >> info.registrationTime >> info.narSize >> info.ultimate; info.sigs = readStrings(in); diff --git a/src/nix/hash.cc b/src/nix/hash.cc index b97c6d21f..fb0843ce2 100644 --- a/src/nix/hash.cc +++ b/src/nix/hash.cc @@ -103,7 +103,7 @@ struct CmdToBase : Command void run() override { for (auto s : args) - logger->stdout(Hash(s, ht).to_string(base, base == SRI)); + logger->stdout(Hash::parseAny(s, ht).to_string(base, base == SRI)); } }; From 343d1569b193e4456e2e5365921371bfd6e37205 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Thu, 2 Jul 2020 10:48:47 -0400 Subject: [PATCH 12/30] Fix test suite --- src/libutil/hash.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index fcc0b9eb7..084d24170 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -149,9 +149,9 @@ Hash Hash::parseAnyPrefixed(std::string_view s) return parseAny(s, std::nullopt); } -static std::pair newFunction(std::string_view & original, std::optional optType) +static std::pair newFunction(std::string_view & rest, std::optional optType) { - auto rest = original; + auto original = rest; bool isSRI = false; From 27c8029573ab49ad11c069ef547e3c087c73939f Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Thu, 2 Jul 2020 10:58:29 -0400 Subject: [PATCH 13/30] Inline newFunction --- src/libutil/hash.cc | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 084d24170..2908a3445 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -149,11 +149,12 @@ Hash Hash::parseAnyPrefixed(std::string_view s) return parseAny(s, std::nullopt); } -static std::pair newFunction(std::string_view & rest, std::optional optType) +Hash Hash::parseAny(std::string_view original, std::optional optType) { - auto original = rest; + auto rest = original; bool isSRI = false; + HashType hashType; // Parse the has type before the separater, if there was one. std::optional optParsedType; @@ -171,23 +172,13 @@ static std::pair newFunction(std::string_view & rest, std::optio // Either the string or user must provide the type, if they both do they // must agree. - if (!optParsedType && !optType) { + if (!optParsedType && !optType) throw BadHash("hash '%s' does not include a type, nor is the type otherwise known from context.", rest); - } else { - if (optParsedType && optType && *optParsedType != *optType) - throw BadHash("hash '%s' should have type '%s'", original, printHashType(*optType)); - return { - optParsedType ? *optParsedType : *optType, - isSRI, - }; - } -} + else if (optParsedType && optType && *optParsedType != *optType) + throw BadHash("hash '%s' should have type '%s'", original, printHashType(*optType)); -// mutates the string_view -Hash Hash::parseAny(std::string_view original, std::optional optType) -{ - auto typeAndSRI = newFunction(original, optType); - return Hash(original, typeAndSRI); + hashType = optParsedType ? *optParsedType : *optType; + return Hash(rest, std::make_pair(hashType, isSRI)); } Hash::Hash(std::string_view rest, std::pair typeAndSRI) From f61bc45d192809a040a359b22f3dbd0722613af6 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Thu, 2 Jul 2020 11:09:04 -0400 Subject: [PATCH 14/30] Get rid of the std::pair --- src/libutil/hash.cc | 10 ++++------ src/libutil/hash.hh | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 2908a3445..2f006ce1e 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -141,7 +141,7 @@ Hash Hash::fromSRI(std::string_view original) { throw BadHash("hash '%s' is not SRI", original); HashType parsedType = parseHashType(*hashRaw); - return Hash(rest, std::make_pair(parsedType, true)); + return Hash(rest, parsedType, true); } Hash Hash::parseAnyPrefixed(std::string_view s) @@ -178,14 +178,12 @@ Hash Hash::parseAny(std::string_view original, std::optional optType) throw BadHash("hash '%s' should have type '%s'", original, printHashType(*optType)); hashType = optParsedType ? *optParsedType : *optType; - return Hash(rest, std::make_pair(hashType, isSRI)); + return Hash(rest, hashType, isSRI); } -Hash::Hash(std::string_view rest, std::pair typeAndSRI) - : Hash(typeAndSRI.first) +Hash::Hash(std::string_view rest, HashType type, bool isSRI) + : Hash(type) { - auto [type, isSRI] = std::move(typeAndSRI); - if (!isSRI && rest.size() == base16Len()) { auto parseHexDigit = [&](char c) { diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 3e413a52c..4e3591a04 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -49,7 +49,7 @@ struct Hash private: // type must be provided, s must not include prefix - Hash(std::string_view s, std::pair typeAndSRI); + Hash(std::string_view s, HashType type, bool isSRI); public: /* Check whether a hash is set. */ From 9462d8a50b5443bc2dac616f94ded9ad37020094 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Thu, 2 Jul 2020 11:11:18 -0400 Subject: [PATCH 15/30] Rename fromSRI to parseSRI for constistency --- src/libfetchers/fetchers.cc | 2 +- src/libfetchers/tarball.cc | 4 ++-- src/libutil/hash.cc | 2 +- src/libutil/hash.hh | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 91d0d6a1d..b1782feab 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -35,7 +35,7 @@ std::unique_ptr inputFromAttrs(const Attrs & attrs) auto res = inputScheme->inputFromAttrs(attrs2); if (res) { if (auto narHash = maybeGetStrAttr(attrs, "narHash")) - res->narHash = Hash::fromSRI(*narHash); + res->narHash = Hash::parseSRI(*narHash); return res; } } diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index 732fac8c3..6ca51269b 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -242,13 +242,13 @@ struct TarballInputScheme : InputScheme auto hash = input->url.query.find("hash"); if (hash != input->url.query.end()) { - input->hash = Hash::fromSRI(hash->second); + input->hash = Hash::parseSRI(hash->second); input->url.query.erase(hash); } auto narHash = input->url.query.find("narHash"); if (narHash != input->url.query.end()) { - input->narHash = Hash::fromSRI(narHash->second); + input->narHash = Hash::parseSRI(narHash->second); input->url.query.erase(narHash); } diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 2f006ce1e..a077d40a0 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -132,7 +132,7 @@ std::string Hash::to_string(Base base, bool includeType) const return s; } -Hash Hash::fromSRI(std::string_view original) { +Hash Hash::parseSRI(std::string_view original) { auto rest = original; // Parse the has type before the separater, if there was one. diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 4e3591a04..d321cc8e1 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -45,7 +45,7 @@ struct Hash // prefix parsed separately; non SRI hash static Hash parseAnyUnprefixed(std::string_view s, HashType type); - static Hash fromSRI(std::string_view original); + static Hash parseSRI(std::string_view original); private: // type must be provided, s must not include prefix From 36cbc74689321399aeae26ff506809b8d9b24674 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Thu, 2 Jul 2020 11:21:00 -0400 Subject: [PATCH 16/30] Inline and simplify in parseAnyPrefixed --- src/libutil/hash.cc | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index a077d40a0..75f8f319c 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -144,9 +144,32 @@ Hash Hash::parseSRI(std::string_view original) { return Hash(rest, parsedType, true); } -Hash Hash::parseAnyPrefixed(std::string_view s) +Hash Hash::parseAnyPrefixed(std::string_view original) { - return parseAny(s, std::nullopt); + auto rest = original; + + bool isSRI = false; + + // Parse the has type before the separater, if there was one. + std::optional optParsedType; + { + auto hashRaw = splitPrefix(rest, ':'); + + if (!hashRaw) { + hashRaw = splitPrefix(rest, '-'); + if (hashRaw) + isSRI = true; + } + if (hashRaw) + optParsedType = parseHashType(*hashRaw); + } + + // Either the string or user must provide the type, if they both do they + // must agree. + if (!optParsedType) + throw BadHash("hash '%s' does not include a type.", rest); + + return Hash(rest, *optParsedType, isSRI); } Hash Hash::parseAny(std::string_view original, std::optional optType) From ea48e3a5b5f1051c251184792417326c513bf00f Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Thu, 2 Jul 2020 11:29:33 -0400 Subject: [PATCH 17/30] Abstract common parsing functionality --- src/libutil/hash.cc | 37 +++---------------------------------- src/libutil/parser.hh | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 34 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 75f8f319c..7d6b8d96e 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -147,22 +147,7 @@ Hash Hash::parseSRI(std::string_view original) { Hash Hash::parseAnyPrefixed(std::string_view original) { auto rest = original; - - bool isSRI = false; - - // Parse the has type before the separater, if there was one. - std::optional optParsedType; - { - auto hashRaw = splitPrefix(rest, ':'); - - if (!hashRaw) { - hashRaw = splitPrefix(rest, '-'); - if (hashRaw) - isSRI = true; - } - if (hashRaw) - optParsedType = parseHashType(*hashRaw); - } + auto [optParsedType, isSRI] = getParsedTypeAndSRI(rest); // Either the string or user must provide the type, if they both do they // must agree. @@ -175,23 +160,7 @@ Hash Hash::parseAnyPrefixed(std::string_view original) Hash Hash::parseAny(std::string_view original, std::optional optType) { auto rest = original; - - bool isSRI = false; - HashType hashType; - - // Parse the has type before the separater, if there was one. - std::optional optParsedType; - { - auto hashRaw = splitPrefix(rest, ':'); - - if (!hashRaw) { - hashRaw = splitPrefix(rest, '-'); - if (hashRaw) - isSRI = true; - } - if (hashRaw) - optParsedType = parseHashType(*hashRaw); - } + auto [optParsedType, isSRI] = getParsedTypeAndSRI(rest); // Either the string or user must provide the type, if they both do they // must agree. @@ -200,7 +169,7 @@ Hash Hash::parseAny(std::string_view original, std::optional optType) else if (optParsedType && optType && *optParsedType != *optType) throw BadHash("hash '%s' should have type '%s'", original, printHashType(*optType)); - hashType = optParsedType ? *optParsedType : *optType; + HashType hashType = optParsedType ? *optParsedType : *optType; return Hash(rest, hashType, isSRI); } diff --git a/src/libutil/parser.hh b/src/libutil/parser.hh index d3bfafe75..d20e4dfc6 100644 --- a/src/libutil/parser.hh +++ b/src/libutil/parser.hh @@ -21,4 +21,26 @@ static inline std::optional splitPrefix(std::string_view & str return std::nullopt; } +// Mutates the string to eliminate the prefixes when found +std::pair, bool> getParsedTypeAndSRI(std::string_view & rest) { + bool isSRI = false; + + // Parse the has type before the separater, if there was one. + std::optional optParsedType; + { + auto hashRaw = splitPrefix(rest, ':'); + + if (!hashRaw) { + hashRaw = splitPrefix(rest, '-'); + if (hashRaw) + isSRI = true; + } + if (hashRaw) + optParsedType = parseHashType(*hashRaw); + } + + return std::make_pair(optParsedType, isSRI); +} + + } From b6b10b1d4cb1cd487bbb5d2cc063ca743ae79004 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Thu, 2 Jul 2020 11:34:40 -0400 Subject: [PATCH 18/30] Write the implementation for parseNonSRIUnprefixed --- src/libutil/hash.cc | 5 +++++ src/libutil/hash.hh | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 7d6b8d96e..1150e74ed 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -173,6 +173,11 @@ Hash Hash::parseAny(std::string_view original, std::optional optType) return Hash(rest, hashType, isSRI); } +Hash Hash::parseNonSRIUnprefixed(std::string_view s, HashType type) +{ + return Hash(s, type, false); +} + Hash::Hash(std::string_view rest, HashType type, bool isSRI) : Hash(type) { diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index d321cc8e1..af11a028d 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -43,7 +43,7 @@ struct Hash // hash type must be part of string static Hash parseAnyPrefixed(std::string_view s); // prefix parsed separately; non SRI hash - static Hash parseAnyUnprefixed(std::string_view s, HashType type); + static Hash parseNonSRIUnprefixed(std::string_view s, HashType type); static Hash parseSRI(std::string_view original); From 1fc835aa223520f37e4945fa8626a096f170b188 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Thu, 2 Jul 2020 11:57:21 -0400 Subject: [PATCH 19/30] Tighten parsing for drv files and pathinfo --- src/libstore/content-address.cc | 4 ++-- src/libstore/derivations.cc | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 2d96fb0c0..02ab6710f 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -68,7 +68,7 @@ ContentAddress parseContentAddress(std::string_view rawCa) { throw Error("text content address hash should use %s, but instead uses %s", printHashType(htSHA256), printHashType(hashType)); return TextHash { - .hash = Hash::parseAny(rest, std::move(hashType)), + .hash = Hash::parseNonSRIUnprefixed(rest, std::move(hashType)), }; } else if (prefix == "fixed") { // Parse method @@ -80,7 +80,7 @@ ContentAddress parseContentAddress(std::string_view rawCa) { HashType hashType = parseHashType_(); return FixedOutputHash { .method = method, - .hash = Hash::parseAny(rest, std::move(hashType)), + .hash = Hash::parseNonSRIUnprefixed(rest, std::move(hashType)), }; } else throw UsageError("content address prefix \"%s\" is unrecognized. Recogonized prefixes are \"text\" or \"fixed\"", prefix); diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 30ce32354..149994e3e 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -118,7 +118,7 @@ static DerivationOutput parseDerivationOutput(const Store & store, istringstream const HashType hashType = parseHashType(hashAlgo); fsh = FixedOutputHash { .method = std::move(method), - .hash = Hash::parseAny(hash, hashType), + .hash = Hash::parseNonSRIUnprefixed(hash, hashType), }; } @@ -416,7 +416,7 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store) auto hashType = parseHashType(hashAlgo); fsh = FixedOutputHash { .method = std::move(method), - .hash = Hash::parseAny(hash, hashType), + .hash = Hash::parseNonSRIUnprefixed(hash, hashType), }; } From a7cd7425d9341cf8a2c3af80b84cc55e874515c6 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 2 Jul 2020 23:10:11 +0000 Subject: [PATCH 20/30] Move `getParsedTypeAndSRI` to a more suitable location Also mark it static --- src/libutil/hash.cc | 21 +++++++++++++++++++++ src/libutil/parser.hh | 21 --------------------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 1150e74ed..76fa67086 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -144,6 +144,27 @@ Hash Hash::parseSRI(std::string_view original) { return Hash(rest, parsedType, true); } +// Mutates the string to eliminate the prefixes when found +static std::pair, bool> getParsedTypeAndSRI(std::string_view & rest) { + bool isSRI = false; + + // Parse the has type before the separater, if there was one. + std::optional optParsedType; + { + auto hashRaw = splitPrefix(rest, ':'); + + if (!hashRaw) { + hashRaw = splitPrefix(rest, '-'); + if (hashRaw) + isSRI = true; + } + if (hashRaw) + optParsedType = parseHashType(*hashRaw); + } + + return {optParsedType, isSRI}; +} + Hash Hash::parseAnyPrefixed(std::string_view original) { auto rest = original; diff --git a/src/libutil/parser.hh b/src/libutil/parser.hh index d20e4dfc6..a6a83ce89 100644 --- a/src/libutil/parser.hh +++ b/src/libutil/parser.hh @@ -21,26 +21,5 @@ static inline std::optional splitPrefix(std::string_view & str return std::nullopt; } -// Mutates the string to eliminate the prefixes when found -std::pair, bool> getParsedTypeAndSRI(std::string_view & rest) { - bool isSRI = false; - - // Parse the has type before the separater, if there was one. - std::optional optParsedType; - { - auto hashRaw = splitPrefix(rest, ':'); - - if (!hashRaw) { - hashRaw = splitPrefix(rest, '-'); - if (hashRaw) - isSRI = true; - } - if (hashRaw) - optParsedType = parseHashType(*hashRaw); - } - - return std::make_pair(optParsedType, isSRI); -} - } From 13796be78dfa9d3a189ea6b482659c56b1301634 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 2 Jul 2020 23:16:57 +0000 Subject: [PATCH 21/30] Have `splitPrefix` and `splitPrefixTo` parser helpers --- src/libstore/content-address.cc | 8 +++----- src/libutil/hash.cc | 6 +++--- src/libutil/parser.hh | 10 +++++++++- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 02ab6710f..470cc62c9 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -46,14 +46,14 @@ ContentAddress parseContentAddress(std::string_view rawCa) { std::string_view prefix; { - auto optPrefix = splitPrefix(rest, ':'); + auto optPrefix = splitPrefixTo(rest, ':'); if (!optPrefix) throw UsageError("not a content address because it is not in the form \":\": %s", rawCa); prefix = *optPrefix; } auto parseHashType_ = [&](){ - auto hashTypeRaw = splitPrefix(rest, ':'); + auto hashTypeRaw = splitPrefixTo(rest, ':'); if (!hashTypeRaw) throw UsageError("content address hash must be in form \":\", but found: %s", rawCa); HashType hashType = parseHashType(*hashTypeRaw); @@ -73,10 +73,8 @@ ContentAddress parseContentAddress(std::string_view rawCa) { } else if (prefix == "fixed") { // Parse method auto method = FileIngestionMethod::Flat; - if (rest.substr(0, 2) == "r:") { + if (splitPrefix(rest, "r:")) method = FileIngestionMethod::Recursive; - rest = rest.substr(2); - } HashType hashType = parseHashType_(); return FixedOutputHash { .method = method, diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 76fa67086..35054462c 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -136,7 +136,7 @@ Hash Hash::parseSRI(std::string_view original) { auto rest = original; // Parse the has type before the separater, if there was one. - auto hashRaw = splitPrefix(rest, '-'); + auto hashRaw = splitPrefixTo(rest, '-'); if (!hashRaw) throw BadHash("hash '%s' is not SRI", original); HashType parsedType = parseHashType(*hashRaw); @@ -151,10 +151,10 @@ static std::pair, bool> getParsedTypeAndSRI(std::string_ // Parse the has type before the separater, if there was one. std::optional optParsedType; { - auto hashRaw = splitPrefix(rest, ':'); + auto hashRaw = splitPrefixTo(rest, ':'); if (!hashRaw) { - hashRaw = splitPrefix(rest, '-'); + hashRaw = splitPrefixTo(rest, '-'); if (hashRaw) isSRI = true; } diff --git a/src/libutil/parser.hh b/src/libutil/parser.hh index a6a83ce89..d19d7d8ed 100644 --- a/src/libutil/parser.hh +++ b/src/libutil/parser.hh @@ -3,13 +3,15 @@ #include #include +#include "util.hh" + namespace nix { // If `separator` is found, we return the portion of the string before the // separator, and modify the string argument to contain only the part after the // separator. Otherwise, wer return `std::nullopt`, and we leave the argument // string alone. -static inline std::optional splitPrefix(std::string_view & string, char separator) { +static inline std::optional splitPrefixTo(std::string_view & string, char separator) { auto sepInstance = string.find(separator); if (sepInstance != std::string_view::npos) { @@ -21,5 +23,11 @@ static inline std::optional splitPrefix(std::string_view & str return std::nullopt; } +static inline bool splitPrefix(std::string_view & string, std::string_view prefix) { + bool res = hasPrefix(string, prefix); + if (res) + string.remove_prefix(prefix.length()); + return res; +} } From d291be444bf6e50ba13235be975db4443b162903 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 3 Jul 2020 14:49:22 +0000 Subject: [PATCH 22/30] Fix Perl --- perl/lib/Nix/Store.xs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs index 2a2a0d429..595153e36 100644 --- a/perl/lib/Nix/Store.xs +++ b/perl/lib/Nix/Store.xs @@ -285,7 +285,7 @@ SV * addToStore(char * srcPath, int recursive, char * algo) SV * makeFixedOutputPath(int recursive, char * algo, char * hash, char * name) PPCODE: try { - Hash h(hash, parseHashType(algo)); + auto h = Hash::parseAny(hash, parseHashType(algo)); auto method = recursive ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; auto path = store()->makeFixedOutputPath(method, h, name); XPUSHs(sv_2mortal(newSVpv(store()->printStorePath(path).c_str(), 0))); From d4250fef239b28b4a098e70e1deac889616dbb9a Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 3 Jul 2020 15:17:20 +0000 Subject: [PATCH 23/30] Fix Perl, again... --- perl/lib/Nix/Store.xs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs index 595153e36..97cc4d94e 100644 --- a/perl/lib/Nix/Store.xs +++ b/perl/lib/Nix/Store.xs @@ -224,7 +224,7 @@ SV * hashString(char * algo, int base32, char * s) SV * convertHash(char * algo, char * s, int toBase32) PPCODE: try { - Hash h(s, parseHashType(algo)); + auto h = Hash::parseAny(s, parseHashType(algo)); string s = h.to_string(toBase32 ? Base32 : Base16, false); XPUSHs(sv_2mortal(newSVpv(s.c_str(), 0))); } catch (Error & e) { From bf61871271971aa45237fb9ba7fa4c63ae083ff2 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 20 Jul 2020 17:42:34 +0000 Subject: [PATCH 24/30] parser.hh -> split.hh --- src/libstore/content-address.cc | 2 +- src/libutil/hash.cc | 2 +- src/libutil/{parser.hh => split.hh} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename src/libutil/{parser.hh => split.hh} (100%) diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index a562f2d23..749551d1a 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -1,6 +1,6 @@ #include "args.hh" #include "content-address.hh" -#include "parser.hh" +#include "split.hh" namespace nix { diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index c7ccb809d..65ba1dc81 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -7,7 +7,7 @@ #include "args.hh" #include "hash.hh" #include "archive.hh" -#include "parser.hh" +#include "split.hh" #include "util.hh" #include diff --git a/src/libutil/parser.hh b/src/libutil/split.hh similarity index 100% rename from src/libutil/parser.hh rename to src/libutil/split.hh From c58c6165c554d671f87b463c9ab1d47a5d75bbbb Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 20 Jul 2020 17:43:19 +0000 Subject: [PATCH 25/30] Remove period at the end of the exception message --- src/libutil/hash.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 65ba1dc81..dfb3668f1 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -177,7 +177,7 @@ Hash Hash::parseAnyPrefixed(std::string_view original) // Either the string or user must provide the type, if they both do they // must agree. if (!optParsedType) - throw BadHash("hash '%s' does not include a type.", rest); + throw BadHash("hash '%s' does not include a type", rest); return Hash(rest, *optParsedType, isSRI); } From 8065c6d1606402e936b048aa75ad98ffdd7c8d60 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 27 Jul 2020 20:45:34 +0000 Subject: [PATCH 26/30] Abstract out topo sorting logic --- src/libstore/misc.cc | 51 +++++++++++++--------------------------- src/libutil/topo-sort.hh | 40 +++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 35 deletions(-) create mode 100644 src/libutil/topo-sort.hh diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index c4d22a634..34a14d30d 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -4,6 +4,7 @@ #include "local-store.hh" #include "store-api.hh" #include "thread-pool.hh" +#include "topo-sort.hh" namespace nix { @@ -246,41 +247,21 @@ void Store::queryMissing(const std::vector & targets, StorePaths Store::topoSortPaths(const StorePathSet & paths) { - StorePaths sorted; - StorePathSet visited, parents; - - std::function dfsVisit; - - dfsVisit = [&](const StorePath & path, const StorePath * parent) { - if (parents.count(path)) - throw BuildError("cycle detected in the references of '%s' from '%s'", - printStorePath(path), printStorePath(*parent)); - - if (!visited.insert(path).second) return; - parents.insert(path); - - StorePathSet references; - try { - references = queryPathInfo(path)->references; - } catch (InvalidPath &) { - } - - for (auto & i : references) - /* Don't traverse into paths that don't exist. That can - happen due to substitutes for non-existent paths. */ - if (i != path && paths.count(i)) - dfsVisit(i, &path); - - sorted.push_back(path); - parents.erase(path); - }; - - for (auto & i : paths) - dfsVisit(i, nullptr); - - std::reverse(sorted.begin(), sorted.end()); - - return sorted; + return topoSort(paths, + {[&](const StorePath & path) { + StorePathSet references; + try { + references = queryPathInfo(path)->references; + } catch (InvalidPath &) { + } + return references; + }}, + {[&](const StorePath & path, const StorePath & parent) { + return BuildError( + "cycle detected in the references of '%s' from '%s'", + printStorePath(path), + printStorePath(parent)); + }}); } diff --git a/src/libutil/topo-sort.hh b/src/libutil/topo-sort.hh new file mode 100644 index 000000000..7a68ff169 --- /dev/null +++ b/src/libutil/topo-sort.hh @@ -0,0 +1,40 @@ +#include "error.hh" + +namespace nix { + +template +std::vector topoSort(std::set items, + std::function(const T &)> getChildren, + std::function makeCycleError) +{ + std::vector sorted; + std::set visited, parents; + + std::function dfsVisit; + + dfsVisit = [&](const T & path, const T * parent) { + if (parents.count(path)) throw makeCycleError(path, *parent); + + if (!visited.insert(path).second) return; + parents.insert(path); + + std::set references = getChildren(path); + + for (auto & i : references) + /* Don't traverse into items that don't exist in our starting set. */ + if (i != path && items.count(i)) + dfsVisit(i, &path); + + sorted.push_back(path); + parents.erase(path); + }; + + for (auto & i : items) + dfsVisit(i, nullptr); + + std::reverse(sorted.begin(), sorted.end()); + + return sorted; +} + +} From e3a2154f5ac91a5cbab5d0715984972e1dd7d40d Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 31 Jul 2020 01:07:59 +0000 Subject: [PATCH 27/30] Fix indentation --- src/libfetchers/mercurial.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index aee42e136..3e76ffc4d 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -209,7 +209,7 @@ struct MercurialInputScheme : InputScheme }); if (auto res = getCache()->lookup(store, mutableAttrs)) { - auto rev2 = Hash::parseAny(getStrAttr(res->first, "rev"), htSHA1); + auto rev2 = Hash::parseAny(getStrAttr(res->first, "rev"), htSHA1); if (!input.getRev() || input.getRev() == rev2) { input.attrs.insert_or_assign("rev", rev2.gitRev()); return makeResult(res->first, std::move(res->second)); From 3cbee1e840ea1beff566555f1221b2791091e20c Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 1 Aug 2020 15:26:57 +0000 Subject: [PATCH 28/30] Convert to C-style comments --- src/libutil/hash.hh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index d4badbab3..ffc397ce0 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -42,9 +42,9 @@ struct Hash is not present, then the hash type must be specified in the string. */ static Hash parseAny(std::string_view s, std::optional type); - // hash type must be part of string + /* hash type must be part of string */ static Hash parseAnyPrefixed(std::string_view s); - // prefix parsed separately; non SRI hash + /* prefix parsed separately; non SRI hash */ static Hash parseNonSRIUnprefixed(std::string_view s, HashType type); static Hash parseSRI(std::string_view original); From bc165e28aee689a45535afda8012c9b63f87b24c Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 1 Aug 2020 15:32:20 +0000 Subject: [PATCH 29/30] Embelish documentation of new Hash functions --- src/libutil/hash.hh | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index ffc397ce0..00ce7bb6f 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -36,21 +36,26 @@ struct Hash /* Create a zero-filled hash object. */ Hash(HashType type); - /* Initialize the hash from a string representation, in the format + /* Parse the hash from a string representation in the format "[:]" or "-" (a Subresource Integrity hash expression). If the 'type' argument is not present, then the hash type must be specified in the string. */ static Hash parseAny(std::string_view s, std::optional type); - /* hash type must be part of string */ + + /* Parse a hash from a string representation like the above, except the + type prefix is mandatory is there is no separate arguement. */ static Hash parseAnyPrefixed(std::string_view s); - /* prefix parsed separately; non SRI hash */ + + /* Parse a plain hash that musst not have any prefix indicating the type. + The type is passed in to disambiguate. */ static Hash parseNonSRIUnprefixed(std::string_view s, HashType type); static Hash parseSRI(std::string_view original); private: - // type must be provided, s must not include prefix + /* The type must be provided, the string view must not include + prefix. `isSRI` helps disambigate the various base-* encodings. */ Hash(std::string_view s, HashType type, bool isSRI); public: From c4ada76e860a595e3f034b89f27374ce79513d9f Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 1 Aug 2020 16:22:50 +0000 Subject: [PATCH 30/30] Fix error message and avoid recalculation --- src/libfetchers/fetchers.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 08b83b0db..9c69fc564 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -203,8 +203,8 @@ std::optional Input::getNarHash() const if (auto s = maybeGetStrAttr(attrs, "narHash")) { auto hash = s->empty() ? Hash(htSHA256) : Hash::parseSRI(*s); if (hash.type != htSHA256) - throw UsageError("narHash must be specified with SRI notation"); - return newHashAllowEmpty(*s, htSHA256); + throw UsageError("narHash must use SHA-256"); + return hash; } return {}; }