Make outputHashAlgo accept "nar", stay in sync

Now that we have a few things identifying content address methods by
name, we should be consistent about it.

Move up the `parseHashAlgoOpt` for tidiness too.

Discussed this change for consistency's sake as part of #8876

Co-authored-by: Eelco Dolstra <edolstra@gmail.com>

doc/manual/src/language/advanced-attributes.md

@@ -203,12 +203,17 @@ Derivations can declare some infrequently used optional attributes.
 
         This is the default.
 
-      - `"recursive"`\
-        The hash is computed over the NAR archive dump of the output
+      - `"recursive"` or `"nar"`\
+        The hash is computed over the [NAR archive](@docroot@/glossary.md#gloss-nar) dump of the output
         (i.e., the result of [`nix-store --dump`](@docroot@/command-ref/nix-store/dump.md)). In
         this case, the output can be anything, including a directory
         tree.
 
+        `"recursive"` is the traditional way of indicating this,
+        and is supported since 2005 (virtually the entire history of Nix).
+        `"nar"` is more clear, and consistent with other parts of Nix (such as the CLI),
+        however support for it is only added in Nix version 2.21.
+
     The `outputHash` attribute, finally, must be a string containing
     the hash in either hexadecimal or base-32 notation. (See the
     [`nix-hash` command](../command-ref/nix-hash.md) for information

src/libexpr/primops.cc

@@ -1108,7 +1108,7 @@ drvName, Bindings * attrs, Value & v)
     bool contentAddressed = false;
     bool isImpure = false;
     std::optional<std::string> outputHash;
-    std::string outputHashAlgo;
+    std::optional<HashAlgorithm> outputHashAlgo;
     std::optional<ContentAddressMethod> ingestionMethod;
 
     StringSet outputs;
@@ -1120,15 +1120,18 @@ drvName, Bindings * attrs, Value & v)
         vomit("processing attribute '%1%'", key);
 
         auto handleHashMode = [&](const std::string_view s) {
-            if (s == "recursive") ingestionMethod = FileIngestionMethod::Recursive;
-            else if (s == "flat") ingestionMethod = FileIngestionMethod::Flat;
-            else if (s == "text") {
-                experimentalFeatureSettings.require(Xp::DynamicDerivations);
-                ingestionMethod = TextIngestionMethod {};
-            } else
+            if (s == "recursive") {
+                // back compat, new name is "nar"
+                ingestionMethod = FileIngestionMethod::Recursive;
+            } else try {
+                ingestionMethod = ContentAddressMethod::parse(s);
+            } catch (UsageError &) {
                 state.error<EvalError>(
                     "invalid value '%s' for 'outputHashMode' attribute", s
                 ).atPos(v).debugThrow();
+            }
+            if (ingestionMethod == TextIngestionMethod {})
+                experimentalFeatureSettings.require(Xp::DynamicDerivations);
         };
 
         auto handleOutputs = [&](const Strings & ss) {
@@ -1204,7 +1207,7 @@ drvName, Bindings * attrs, Value & v)
                     else if (i->name == state.sOutputHash)
                         outputHash = state.forceStringNoCtx(*i->value, pos, context_below);
                     else if (i->name == state.sOutputHashAlgo)
-                        outputHashAlgo = state.forceStringNoCtx(*i->value, pos, context_below);
+                        outputHashAlgo = parseHashAlgoOpt(state.forceStringNoCtx(*i->value, pos, context_below));
                     else if (i->name == state.sOutputHashMode)
                         handleHashMode(state.forceStringNoCtx(*i->value, pos, context_below));
                     else if (i->name == state.sOutputs) {
@@ -1222,7 +1225,7 @@ drvName, Bindings * attrs, Value & v)
                     if (i->name == state.sBuilder) drv.builder = std::move(s);
                     else if (i->name == state.sSystem) drv.platform = std::move(s);
                     else if (i->name == state.sOutputHash) outputHash = std::move(s);
-                    else if (i->name == state.sOutputHashAlgo) outputHashAlgo = std::move(s);
+                    else if (i->name == state.sOutputHashAlgo) outputHashAlgo = parseHashAlgoOpt(s);
                     else if (i->name == state.sOutputHashMode) handleHashMode(s);
                     else if (i->name == state.sOutputs)
                         handleOutputs(tokenizeString<Strings>(s));
@@ -1306,7 +1309,7 @@ drvName, Bindings * attrs, Value & v)
                 "multiple outputs are not supported in fixed-output derivations"
             ).atPos(v).debugThrow();
 
-        auto h = newHashAllowEmpty(*outputHash, parseHashAlgoOpt(outputHashAlgo));
+        auto h = newHashAllowEmpty(*outputHash, outputHashAlgo);
 
         auto method = ingestionMethod.value_or(FileIngestionMethod::Flat);
 
@@ -1326,7 +1329,7 @@ drvName, Bindings * attrs, Value & v)
             state.error<EvalError>("derivation cannot be both content-addressed and impure")
                 .atPos(v).debugThrow();
 
-        auto ha = parseHashAlgoOpt(outputHashAlgo).value_or(HashAlgorithm::SHA256);
+        auto ha = outputHashAlgo.value_or(HashAlgorithm::SHA256);
         auto method = ingestionMethod.value_or(FileIngestionMethod::Recursive);
 
         for (auto & i : outputs) {

tests/functional/fixed.nix

@@ -57,11 +57,13 @@ rec {
     outputHashMode = "flat";
   };
 
-  # Test for building two derivations in parallel that produce the
+  # Test for building derivations in parallel that produce the
   # same output path because they're fixed-output derivations.
   parallelSame = [
     (f2 "foo" ./fixed.builder2.sh "recursive" "md5" "3670af73070fa14077ad74e0f5ea4e42")
     (f2 "bar" ./fixed.builder2.sh "recursive" "md5" "3670af73070fa14077ad74e0f5ea4e42")
+    (f2 "foo" ./fixed.builder2.sh "nar" "md5" "3670af73070fa14077ad74e0f5ea4e42")
+    (f2 "bar" ./fixed.builder2.sh "nar" "md5" "3670af73070fa14077ad74e0f5ea4e42")
   ];
 
 }