Improve wrong format message with nix hash convert

We have the machinery to make a more informative error, telling the
user what format was actually encountered, and not just that it is not
the format that was requested.

Author: Ericson2314

src/libutil/hash.cc

@@ -182,8 +182,10 @@ Hash Hash::parseSRI(std::string_view original)
  *
  * @param resolveAlgo resolves the parsed type (or throws an error when it is not
  * possible.)
+ *
+ * @return the parsed hash and the format it was parsed from
  */
-static Hash parseAnyHelper(std::string_view rest, auto resolveAlgo)
+static std::pair<Hash, HashFormat> parseAnyHelper(std::string_view rest, auto resolveAlgo)
 {
     bool isSRI = false;
 
@@ -203,34 +205,45 @@ static Hash parseAnyHelper(std::string_view rest, auto resolveAlgo)
 
     HashAlgorithm algo = resolveAlgo(std::move(optParsedAlgo));
 
-    auto [decode, formatName] = [&]() -> DecodeNamePair {
+    auto [decode, formatName, format] = [&]() -> std::tuple<decltype(base16::decode) *, std::string_view, HashFormat> {
         if (isSRI) {
             /* In the SRI case, we always are using Base64. If the
                length is wrong, get an error later. */
-            return {base64::decode, "SRI"};
+            return {base64::decode, "SRI", HashFormat::SRI};
         } else {
             /* Otherwise, decide via the length of the hash (for the
                given algorithm) what base encoding it is. */
-            return baseExplicit(baseFromSize(rest, algo));
+            auto format = baseFromSize(rest, algo);
+            auto [decode, formatName] = baseExplicit(format);
+            return {decode, formatName, format};
         }
     }();
 
-    return parseLowLevel(rest, algo, {decode, formatName});
+    return {parseLowLevel(rest, algo, {decode, formatName}), format};
 }
 
 Hash Hash::parseAnyPrefixed(std::string_view original)
 {
-    return parseAnyHelper(original, [&](std::optional<HashAlgorithm> optParsedAlgo) {
-        // Either the string or user must provide the type, if they both do they
-        // must agree.
-        if (!optParsedAlgo)
-            throw BadHash("hash '%s' does not include a type", original);
+    return parseAnyHelper(
+               original,
+               [&](std::optional<HashAlgorithm> optParsedAlgo) {
+                   // Either the string or user must provide the type, if they both do they
+                   // must agree.
+                   if (!optParsedAlgo)
+                       throw BadHash("hash '%s' does not include a type", original);
 
-        return *optParsedAlgo;
-    });
+                   return *optParsedAlgo;
+               })
+        .first;
 }
 
 Hash Hash::parseAny(std::string_view original, std::optional<HashAlgorithm> optAlgo)
+{
+    return parseAnyReturningFormat(original, optAlgo).first;
+}
+
+std::pair<Hash, HashFormat>
+Hash::parseAnyReturningFormat(std::string_view original, std::optional<HashAlgorithm> optAlgo)
 {
     return parseAnyHelper(original, [&](std::optional<HashAlgorithm> optParsedAlgo) {
         // Either the string or user must provide the type, if they both do they

src/libutil/include/nix/util/hash.hh

@@ -79,6 +79,12 @@ struct Hash
      */
     static Hash parseAny(std::string_view s, std::optional<HashAlgorithm> optAlgo);
 
+    /**
+     * Like `parseAny`, but also returns the format the hash was parsed from.
+     */
+    static std::pair<Hash, HashFormat>
+    parseAnyReturningFormat(std::string_view s, std::optional<HashAlgorithm> optAlgo);
+
     /**
      * Parse a hash from a string representation like the above, except the
      * type prefix is mandatory is there is no separate argument.

src/nix/hash-convert.md

@@ -27,7 +27,7 @@ R""(
 
   ```console
   # nix hash convert --hash-algo sha256 --from nix32 ungWv48Bz+pBQUDeXa4iI7ADYaOWF3qctBD/YfIAFa0=
-  error: input hash 'ungWv48Bz+pBQUDeXa4iI7ADYaOWF3qctBD/YfIAFa0=' does not have the expected format '--from nix32'
+  error: input hash 'ungWv48Bz+pBQUDeXa4iI7ADYaOWF3qctBD/YfIAFa0=' has format 'base64', but '--from nix32' was specified
 
   # nix hash convert --hash-algo sha256 --from nix32 1b8m03r63zqhnjf7l5wnldhh7c134ap5vpj0850ymkq1iyzicy5s
   sha256-ungWv48Bz+pBQUDeXa4iI7ADYaOWF3qctBD/YfIAFa0=

src/nix/hash.cc

@@ -248,11 +248,13 @@ struct CmdHashConvert : Command
     void run() override
     {
         for (const auto & s : hashStrings) {
-            Hash h = from == HashFormat::SRI ? Hash::parseSRI(s) : Hash::parseAny(s, algo);
-            if (from && from != HashFormat::SRI
-                && h.to_string(*from, false) != (from == HashFormat::Base16 ? toLower(s) : s)) {
-                auto from_as_string = printHashFormat(*from);
-                throw BadHash("input hash '%s' does not have the expected format for '--from %s'", s, from_as_string);
+            auto [h, parsedFormat] = Hash::parseAnyReturningFormat(s, algo);
+            if (from && *from != parsedFormat) {
+                throw BadHash(
+                    "input hash '%s' has format '%s', but '--from %s' was specified",
+                    s,
+                    printHashFormat(parsedFormat),
+                    printHashFormat(*from));
             }
             logger->cout(h.to_string(to, to == HashFormat::SRI));
         }

tests/functional/hash-convert.sh

@@ -93,10 +93,10 @@ try3() {
     # Asserting input format fails.
     #
 
-    expectStderr 1 nix hash convert --hash-algo "$1" --from sri "$2" | grepQuiet "is not SRI"
-    expectStderr 1 nix hash convert --hash-algo "$1" --from nix32 "$2" | grepQuiet "input hash"
-    expectStderr 1 nix hash convert --hash-algo "$1" --from base16 "$3" | grepQuiet "input hash"
-    expectStderr 1 nix hash convert --hash-algo "$1" --from nix32 "$4" | grepQuiet "input hash"
+    expectStderr 1 nix hash convert --hash-algo "$1" --from sri "$2" | grepQuiet "'base16', but '--from sri'"
+    expectStderr 1 nix hash convert --hash-algo "$1" --from nix32 "$2" | grepQuiet "'base16', but '--from nix32'"
+    expectStderr 1 nix hash convert --hash-algo "$1" --from base16 "$3" | grepQuiet "'nix32', but '--from base16'"
+    expectStderr 1 nix hash convert --hash-algo "$1" --from nix32 "$4" | grepQuiet "'base64', but '--from nix32'"
 
     # Base-16 hashes can be in uppercase.
     nix hash convert --hash-algo "$1" --from base16 "$(echo "$2" | tr '[:lower:]' '[:upper:]')"