about summary refs log tree commit diff
diff options
context:
space:
mode:
authorEelco Dolstra <edolstra@gmail.com>2018-02-03T09·04+0100
committerEelco Dolstra <edolstra@gmail.com>2018-02-03T09·08+0100
commit84722d67d2b09b5c28e1c2d9dd438ba592df2296 (patch)
tree81f9563b7bd1440209a7a706a8646a9ff0af9e71
parentde96daf54f3143753073314e0c0e23851c568633 (diff)
Remove nix-build --hash
Instead, if a fixed-output derivation produces has an incorrect output
hash, we now unconditionally move the outputs to the path
corresponding with the actual hash and register it as valid. Thus,
after correcting the hash in the Nix expression (e.g. in a fetchurl
call), the fixed-output derivation doesn't have to be built again.

It would still be good to have a command for reporting the actual hash
of a fixed-output derivation (instead of throwing an error), but
"nix-build --hash" didn't do that.
-rw-r--r--doc/manual/release-notes/rl-2.0.xml10
-rw-r--r--src/libstore/build.cc47
-rw-r--r--src/libstore/store-api.hh2
-rwxr-xr-xsrc/nix-build/nix-build.cc3
-rw-r--r--src/nix-store/nix-store.cc1
-rw-r--r--tests/fixed.sh13
6 files changed, 43 insertions, 33 deletions
diff --git a/doc/manual/release-notes/rl-2.0.xml b/doc/manual/release-notes/rl-2.0.xml
index 0d485c36928c..32cdb1d0cefc 100644
--- a/doc/manual/release-notes/rl-2.0.xml
+++ b/doc/manual/release-notes/rl-2.0.xml
@@ -99,11 +99,11 @@
   </listitem>
 
   <listitem>
-    <para>New build mode <command>nix-build --hash</command> that
-    builds a derivation, computes the hash of the output, and moves
-    the output to the store path corresponding to what a fixed-output
-    derivation with that hash would produce.
-    (Add docs and examples; see d367b8e7875161e655deaa96bf8a5dd0bcf8229e)</para>
+    <para>If a fixed-output derivation produces a result with an
+    incorrect hash, the output path will be moved to the location
+    corresponding to the actual hash and registered as valid. Thus, a
+    subsequent build of the fixed-output derivation with the correct
+    hash is unnecessary.</para>
   </listitem>
 
   <listitem>
diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index cca51f17ee26..d4b93b5104c1 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -1124,11 +1124,6 @@ void DerivationGoal::haveDerivation()
         return;
     }
 
-    /* Reject doing a hash build of anything other than a fixed-output
-       derivation. */
-    if (buildMode == bmHash && !drv->isFixedOutput())
-        throw Error("cannot do a hash build of non-fixed-output derivation '%1%'", drvPath);
-
     /* We are first going to try to create the invalid output paths
        through substitutes.  If that doesn't work, we'll build
        them. */
@@ -1320,9 +1315,7 @@ void DerivationGoal::inputsRealised()
     allPaths.insert(inputPaths.begin(), inputPaths.end());
 
     /* Is this a fixed-output derivation? */
-    fixedOutput = true;
-    for (auto & i : drv->outputs)
-        if (i.second.hash == "") fixedOutput = false;
+    fixedOutput = drv->isFixedOutput();
 
     /* Don't repeat fixed-output derivations since they're already
        verified by their output hash.*/
@@ -3019,6 +3012,8 @@ void DerivationGoal::registerOutputs()
     bool runDiffHook = settings.runDiffHook;
     bool keepPreviousRound = settings.keepFailed || runDiffHook;
 
+    std::exception_ptr delayedException;
+
     /* Check whether the output paths were created, and grep each
        output path to determine what other paths it references.  Also make all
        output paths read-only. */
@@ -3093,7 +3088,7 @@ void DerivationGoal::registerOutputs()
         /* Check that fixed-output derivations produced the right
            outputs (i.e., the content hash should match the specified
            hash). */
-        if (i.second.hash != "") {
+        if (fixedOutput) {
 
             bool recursive; Hash h;
             i.second.parseHashInfo(recursive, h);
@@ -3109,27 +3104,34 @@ void DerivationGoal::registerOutputs()
             /* Check the hash. In hash mode, move the path produced by
                the derivation to its content-addressed location. */
             Hash h2 = recursive ? hashPath(h.type, actualPath).first : hashFile(h.type, actualPath);
-            if (buildMode == bmHash) {
-                Path dest = worker.store.makeFixedOutputPath(recursive, h2, drv->env["name"]);
-                printError(format("build produced path '%1%' with %2% hash '%3%'")
-                    % dest % printHashType(h.type) % printHash16or32(h2));
-                if (worker.store.isValidPath(dest))
-                    return;
+
+            Path dest = worker.store.makeFixedOutputPath(recursive, h2, drv->env["name"]);
+
+            if (h != h2) {
+
+                /* Throw an error after registering the path as
+                   valid. */
+                delayedException = std::make_exception_ptr(
+                    BuildError("fixed-output derivation produced path '%s' with %s hash '%s' instead of the expected hash '%s'",
+                        dest, printHashType(h.type), printHash16or32(h2), printHash16or32(h)));
+
                 Path actualDest = worker.store.toRealPath(dest);
+
+                if (worker.store.isValidPath(dest))
+                    std::rethrow_exception(delayedException);
+
                 if (actualPath != actualDest) {
                     PathLocks outputLocks({actualDest});
                     deletePath(actualDest);
                     if (rename(actualPath.c_str(), actualDest.c_str()) == -1)
                         throw SysError(format("moving '%1%' to '%2%'") % actualPath % dest);
                 }
+
                 path = dest;
                 actualPath = actualDest;
-            } else {
-                if (h != h2)
-                    throw BuildError(
-                        format("output path '%1%' has %2% hash '%3%' when '%4%' was expected")
-                        % path % i.second.hashAlgo % printHash16or32(h2) % printHash16or32(h));
             }
+            else
+                assert(path == dest);
 
             info.ca = makeFixedOutputCA(recursive, h2);
         }
@@ -3306,6 +3308,11 @@ void DerivationGoal::registerOutputs()
        paths referenced by each of them.  If there are cycles in the
        outputs, this will fail. */
     worker.store.registerValidPaths(infos);
+
+    /* In case of a fixed-output derivation hash mismatch, throw an
+       exception now that we have registered the output as valid. */
+    if (delayedException)
+        std::rethrow_exception(delayedException);
 }
 
 
diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh
index b21d3dd8a93b..70f23e1fcaf4 100644
--- a/src/libstore/store-api.hh
+++ b/src/libstore/store-api.hh
@@ -192,7 +192,7 @@ struct ValidPathInfo
 typedef list<ValidPathInfo> ValidPathInfos;
 
 
-enum BuildMode { bmNormal, bmRepair, bmCheck, bmHash };
+enum BuildMode { bmNormal, bmRepair, bmCheck };
 
 
 struct BuildResult
diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc
index 3d02276bf420..1581c282c75c 100755
--- a/src/nix-build/nix-build.cc
+++ b/src/nix-build/nix-build.cc
@@ -167,9 +167,6 @@ void mainWrapped(int argc, char * * argv)
             buildMode = bmRepair;
         }
 
-        else if (*arg == "--hash")
-            buildMode = bmHash;
-
         else if (*arg == "--run-env") // obsolete
             runEnv = true;
 
diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc
index f6f276dd1798..4fc3421c0dde 100644
--- a/src/nix-store/nix-store.cc
+++ b/src/nix-store/nix-store.cc
@@ -122,7 +122,6 @@ static void opRealise(Strings opFlags, Strings opArgs)
         if (i == "--dry-run") dryRun = true;
         else if (i == "--repair") buildMode = bmRepair;
         else if (i == "--check") buildMode = bmCheck;
-        else if (i == "--hash") buildMode = bmHash;
         else if (i == "--ignore-unknown") ignoreUnknown = true;
         else throw UsageError(format("unknown flag '%1%'") % i);
 
diff --git a/tests/fixed.sh b/tests/fixed.sh
index cac3f0be91b0..8f51403a7071 100644
--- a/tests/fixed.sh
+++ b/tests/fixed.sh
@@ -5,15 +5,22 @@ clearStore
 export IMPURE_VAR1=foo
 export IMPURE_VAR2=bar
 
+path=$(nix-store -q $(nix-instantiate fixed.nix -A good.0))
+
+echo 'testing bad...'
+nix-build fixed.nix -A bad --no-out-link && fail "should fail"
+
+# Building with the bad hash should produce the "good" output path as
+# a side-effect.
+[[ -e $path ]]
+nix path-info --json $path | grep fixed:md5:2qk15sxzzjlnpjk9brn7j8ppcd
+
 echo 'testing good...'
 nix-build fixed.nix -A good --no-out-link
 
 echo 'testing good2...'
 nix-build fixed.nix -A good2 --no-out-link
 
-echo 'testing bad...'
-nix-build fixed.nix -A bad --no-out-link && fail "should fail"
-
 echo 'testing reallyBad...'
 nix-instantiate fixed.nix -A reallyBad && fail "should fail"