Add a flag ‘--check’ to verify build determinism

The flag ‘--check’ to ‘nix-store -r’ or ‘nix-build’ will cause Nix to
redo the build of a derivation whose output paths are already valid.
If the new output differs from the original output, an error is
printed.  This makes it easier to test if a build is deterministic.
(Obviously this cannot catch all sources of non-determinism, but it
catches the most common one, namely the current time.)

For example:

  $ nix-build '<nixpkgs>' -A patchelf
  ...
  $ nix-build '<nixpkgs>' -A patchelf --check
  error: derivation `/nix/store/1ipvxsdnbhl1rw6siz6x92s7sc8nwkkb-patchelf-0.6' may not be deterministic: hash mismatch in output `/nix/store/4pc1dmw5xkwmc6q3gdc9i5nbjl4dkjpp-patchelf-0.6.drv'

The --check build fails if not all outputs are valid.  Thus the first
call to nix-build is necessary to ensure that all outputs are valid.

The current outputs are left untouched: the new outputs are either put
in a chroot or diverted to a different location in the store using
hash rewriting.
This commit is contained in:
Eelco Dolstra 2014-02-18 01:01:14 +01:00
parent 4ec626a286
commit 1aa19b24b2
9 changed files with 97 additions and 55 deletions

View file

@ -121,6 +121,10 @@ for (my $n = 0; $n < scalar @ARGV; $n++) {
push @instArgs, $arg; push @instArgs, $arg;
} }
elsif ($arg eq "--check") {
push @buildArgs, $arg;
}
elsif ($arg eq "--run-env") { # obsolete elsif ($arg eq "--run-env") { # obsolete
$runEnv = 1; $runEnv = 1;
} }

View file

@ -196,9 +196,6 @@ struct Child
typedef map<pid_t, Child> Children; typedef map<pid_t, Child> Children;
enum BuildMode { bmNormal, bmRepair };
/* The worker class. */ /* The worker class. */
class Worker class Worker
{ {
@ -764,15 +761,15 @@ private:
/* Hash rewriting. */ /* Hash rewriting. */
HashRewrites rewritesToTmp, rewritesFromTmp; HashRewrites rewritesToTmp, rewritesFromTmp;
PathSet redirectedOutputs; typedef map<Path, Path> RedirectedOutputs;
RedirectedOutputs redirectedOutputs;
BuildMode buildMode; BuildMode buildMode;
/* If we're repairing without a chroot, there may be outputs that /* If we're repairing without a chroot, there may be outputs that
are valid but corrupt. So we redirect these outputs to are valid but corrupt. So we redirect these outputs to
temporary paths. This contains the mapping from outputs to
temporary paths. */ temporary paths. */
map<Path, Path> redirectedBadOutputs; PathSet redirectedBadOutputs;
/* Set of inodes seen during calls to canonicalisePathMetaData() /* Set of inodes seen during calls to canonicalisePathMetaData()
for this build's outputs. This needs to be shared between for this build's outputs. This needs to be shared between
@ -1028,10 +1025,17 @@ void DerivationGoal::outputsSubstituted()
return; return;
} }
if (checkPathValidity(false, buildMode == bmRepair).size() == 0) { unsigned int nrInvalid = checkPathValidity(false, buildMode == bmRepair).size();
if (buildMode == bmRepair) repairClosure(); else amDone(ecSuccess); if (buildMode == bmNormal && nrInvalid == 0) {
amDone(ecSuccess);
return; return;
} }
if (buildMode == bmRepair && nrInvalid == 0) {
repairClosure();
return;
}
if (buildMode == bmCheck && nrInvalid > 0)
throw Error(format("some outputs of `%1%' are not valid, so checking is not possible") % drvPath);
/* Otherwise, at least one of the output paths could not be /* Otherwise, at least one of the output paths could not be
produced using a substitute. So we have to build instead. */ produced using a substitute. So we have to build instead. */
@ -1119,8 +1123,7 @@ void DerivationGoal::inputsRealised()
if (nrFailed != 0) { if (nrFailed != 0) {
printMsg(lvlError, printMsg(lvlError,
format("cannot build derivation `%1%': " format("cannot build derivation `%1%': %2% dependencies couldn't be built")
"%2% dependencies couldn't be built")
% drvPath % nrFailed); % drvPath % nrFailed);
amDone(ecFailed); amDone(ecFailed);
return; return;
@ -1246,7 +1249,8 @@ void DerivationGoal::tryToBuild()
now hold the locks on the output paths, no other process can now hold the locks on the output paths, no other process can
build this derivation, so no further checks are necessary. */ build this derivation, so no further checks are necessary. */
validPaths = checkPathValidity(true, buildMode == bmRepair); validPaths = checkPathValidity(true, buildMode == bmRepair);
if (validPaths.size() == drv.outputs.size()) { assert(buildMode != bmCheck || validPaths.size() == drv.outputs.size());
if (buildMode != bmCheck && validPaths.size() == drv.outputs.size()) {
debug(format("skipping build of derivation `%1%', someone beat us to it") % drvPath); debug(format("skipping build of derivation `%1%', someone beat us to it") % drvPath);
outputLocks.setDeletion(true); outputLocks.setDeletion(true);
amDone(ecSuccess); amDone(ecSuccess);
@ -1254,7 +1258,8 @@ void DerivationGoal::tryToBuild()
} }
missingPaths = outputPaths(drv.outputs); missingPaths = outputPaths(drv.outputs);
foreach (PathSet::iterator, i, validPaths) missingPaths.erase(*i); if (buildMode != bmCheck)
foreach (PathSet::iterator, i, validPaths) missingPaths.erase(*i);
/* If any of the outputs already exist but are not valid, delete /* If any of the outputs already exist but are not valid, delete
them. */ them. */
@ -1262,7 +1267,7 @@ void DerivationGoal::tryToBuild()
Path path = i->second.path; Path path = i->second.path;
if (worker.store.isValidPath(path)) continue; if (worker.store.isValidPath(path)) continue;
if (!pathExists(path)) continue; if (!pathExists(path)) continue;
debug(format("removing unregistered path `%1%'") % path); debug(format("removing invalid path `%1%'") % path);
deletePath(path); deletePath(path);
} }
@ -1273,8 +1278,9 @@ void DerivationGoal::tryToBuild()
if (pathFailed(i->second.path)) return; if (pathFailed(i->second.path)) return;
/* Don't do a remote build if the derivation has the attribute /* Don't do a remote build if the derivation has the attribute
`preferLocalBuild' set. */ `preferLocalBuild' set. Also, check and repair modes are only
bool buildLocally = willBuildLocally(drv); supported for local builds. */
bool buildLocally = buildMode != bmNormal || willBuildLocally(drv);
/* Is the build hook willing to accept this job? */ /* Is the build hook willing to accept this job? */
if (!buildLocally) { if (!buildLocally) {
@ -1414,27 +1420,34 @@ void DerivationGoal::buildDone()
/* Move paths out of the chroot for easier debugging of /* Move paths out of the chroot for easier debugging of
build failures. */ build failures. */
if (useChroot) if (useChroot && buildMode == bmNormal)
foreach (PathSet::iterator, i, missingPaths) foreach (PathSet::iterator, i, missingPaths)
if (pathExists(chrootRootDir + *i)) if (pathExists(chrootRootDir + *i))
rename((chrootRootDir + *i).c_str(), i->c_str()); rename((chrootRootDir + *i).c_str(), i->c_str());
if (WIFEXITED(status) && WEXITSTATUS(status) == childSetupFailed) if (WIFEXITED(status) && WEXITSTATUS(status) == childSetupFailed)
throw Error(format("failed to set up the build environment for `%1%'") % drvPath); throw Error(format("failed to set up the build environment for `%1%'") % drvPath);
if (diskFull) if (diskFull)
printMsg(lvlError, "note: build failure may have been caused by lack of free disk space"); printMsg(lvlError, "note: build failure may have been caused by lack of free disk space");
throw BuildError(format("builder for `%1%' %2%") throw BuildError(format("builder for `%1%' %2%")
% drvPath % statusToString(status)); % drvPath % statusToString(status));
} }
/* Delete redirected outputs (when doing hash rewriting). */
foreach (PathSet::iterator, i, redirectedOutputs)
deletePath(*i);
/* Compute the FS closure of the outputs and register them as /* Compute the FS closure of the outputs and register them as
being valid. */ being valid. */
registerOutputs(); registerOutputs();
if (buildMode == bmCheck) {
amDone(ecSuccess);
return;
}
/* Delete unused redirected outputs (when doing hash rewriting). */
foreach (RedirectedOutputs::iterator, i, redirectedOutputs)
if (pathExists(i->second)) deletePath(i->second);
/* Delete the chroot (if we were using one). */ /* Delete the chroot (if we were using one). */
autoDelChroot.reset(); /* this runs the destructor */ autoDelChroot.reset(); /* this runs the destructor */
@ -1589,7 +1602,10 @@ int childEntry(void * arg)
void DerivationGoal::startBuilder() void DerivationGoal::startBuilder()
{ {
startNest(nest, lvlInfo, format(buildMode == bmRepair ? "repairing path(s) %1%" : "building path(s) %1%") % showPaths(missingPaths)); startNest(nest, lvlInfo, format(
buildMode == bmRepair ? "repairing path(s) %1%" :
buildMode == bmCheck ? "checking path(s) %1%" :
"building path(s) %1%") % showPaths(missingPaths));
/* Right platform? */ /* Right platform? */
if (!canBuildLocally(drv.platform)) { if (!canBuildLocally(drv.platform)) {
@ -1873,16 +1889,17 @@ void DerivationGoal::startBuilder()
contents of the new outputs to replace the dummy strings contents of the new outputs to replace the dummy strings
with the actual hashes. */ with the actual hashes. */
if (validPaths.size() > 0) if (validPaths.size() > 0)
//throw Error(format("derivation `%1%' is blocked by its output path(s) %2%") % drvPath % showPaths(validPaths));
foreach (PathSet::iterator, i, validPaths) foreach (PathSet::iterator, i, validPaths)
redirectedOutputs.insert(addHashRewrite(*i)); addHashRewrite(*i);
/* If we're repairing, then we don't want to delete the /* If we're repairing, then we don't want to delete the
corrupt outputs in advance. So rewrite them as well. */ corrupt outputs in advance. So rewrite them as well. */
if (buildMode == bmRepair) if (buildMode == bmRepair)
foreach (PathSet::iterator, i, missingPaths) foreach (PathSet::iterator, i, missingPaths)
if (worker.store.isValidPath(*i) && pathExists(*i)) if (worker.store.isValidPath(*i) && pathExists(*i)) {
redirectedBadOutputs[*i] = addHashRewrite(*i); addHashRewrite(*i);
redirectedBadOutputs.insert(*i);
}
} }
@ -2166,28 +2183,35 @@ void DerivationGoal::registerOutputs()
Path path = i->second.path; Path path = i->second.path;
if (missingPaths.find(path) == missingPaths.end()) continue; if (missingPaths.find(path) == missingPaths.end()) continue;
Path actualPath = path;
if (useChroot) { if (useChroot) {
if (pathExists(chrootRootDir + path)) { actualPath = chrootRootDir + path;
if (pathExists(actualPath)) {
/* Move output paths from the chroot to the Nix store. */ /* Move output paths from the chroot to the Nix store. */
if (buildMode == bmRepair) if (buildMode == bmRepair)
replaceValidPath(path, chrootRootDir + path); replaceValidPath(path, actualPath);
else else
if (rename((chrootRootDir + path).c_str(), path.c_str()) == -1) if (buildMode != bmCheck && rename(actualPath.c_str(), path.c_str()) == -1)
throw SysError(format("moving build output `%1%' from the chroot to the Nix store") % path); throw SysError(format("moving build output `%1%' from the chroot to the Nix store") % path);
} }
if (buildMode != bmCheck) actualPath = path;
} else { } else {
Path redirected; Path redirected = redirectedOutputs[path];
if (buildMode == bmRepair && (redirected = redirectedBadOutputs[path]) != "" && pathExists(redirected)) if (buildMode == bmRepair
&& redirectedBadOutputs.find(path) != redirectedBadOutputs.end()
&& pathExists(redirected))
replaceValidPath(path, redirected); replaceValidPath(path, redirected);
if (buildMode == bmCheck)
actualPath = redirected;
} }
struct stat st; struct stat st;
if (lstat(path.c_str(), &st) == -1) { if (lstat(actualPath.c_str(), &st) == -1) {
if (errno == ENOENT) if (errno == ENOENT)
throw BuildError( throw BuildError(
format("builder for `%1%' failed to produce output path `%2%'") format("builder for `%1%' failed to produce output path `%2%'")
% drvPath % path); % drvPath % path);
throw SysError(format("getting attributes of path `%1%'") % path); throw SysError(format("getting attributes of path `%1%'") % actualPath);
} }
#ifndef __CYGWIN__ #ifndef __CYGWIN__
@ -2208,15 +2232,15 @@ void DerivationGoal::registerOutputs()
/* Canonicalise first. This ensures that the path we're /* Canonicalise first. This ensures that the path we're
rewriting doesn't contain a hard link to /etc/shadow or rewriting doesn't contain a hard link to /etc/shadow or
something like that. */ something like that. */
canonicalisePathMetaData(path, buildUser.enabled() ? buildUser.getUID() : -1, inodesSeen); canonicalisePathMetaData(actualPath, buildUser.enabled() ? buildUser.getUID() : -1, inodesSeen);
/* FIXME: this is in-memory. */ /* FIXME: this is in-memory. */
StringSink sink; StringSink sink;
dumpPath(path, sink); dumpPath(actualPath, sink);
deletePath(path); deletePath(actualPath);
sink.s = rewriteHashes(sink.s, rewritesFromTmp); sink.s = rewriteHashes(sink.s, rewritesFromTmp);
StringSource source(sink.s); StringSource source(sink.s);
restorePath(path, source); restorePath(actualPath, source);
rewritten = true; rewritten = true;
} }
@ -2237,12 +2261,11 @@ void DerivationGoal::registerOutputs()
execute permission. */ execute permission. */
if (!S_ISREG(st.st_mode) || (st.st_mode & S_IXUSR) != 0) if (!S_ISREG(st.st_mode) || (st.st_mode & S_IXUSR) != 0)
throw BuildError( throw BuildError(
format("output path `%1% should be a non-executable regular file") format("output path `%1% should be a non-executable regular file") % path);
% path);
} }
/* Check the hash. */ /* Check the hash. */
Hash h2 = recursive ? hashPath(ht, path).first : hashFile(ht, path); Hash h2 = recursive ? hashPath(ht, actualPath).first : hashFile(ht, actualPath);
if (h != h2) if (h != h2)
throw BuildError( throw BuildError(
format("output path `%1%' should have %2% hash `%3%', instead has `%4%'") format("output path `%1%' should have %2% hash `%3%', instead has `%4%'")
@ -2251,7 +2274,7 @@ void DerivationGoal::registerOutputs()
/* Get rid of all weird permissions. This also checks that /* Get rid of all weird permissions. This also checks that
all files are owned by the build user, if applicable. */ all files are owned by the build user, if applicable. */
canonicalisePathMetaData(path, canonicalisePathMetaData(actualPath,
buildUser.enabled() && !rewritten ? buildUser.getUID() : -1, inodesSeen); buildUser.enabled() && !rewritten ? buildUser.getUID() : -1, inodesSeen);
/* For this output path, find the references to other paths /* For this output path, find the references to other paths
@ -2259,7 +2282,15 @@ void DerivationGoal::registerOutputs()
time. The hash is stored in the database so that we can time. The hash is stored in the database so that we can
verify later on whether nobody has messed with the store. */ verify later on whether nobody has messed with the store. */
HashResult hash; HashResult hash;
PathSet references = scanForReferences(path, allPaths, hash); PathSet references = scanForReferences(actualPath, allPaths, hash);
if (buildMode == bmCheck) {
ValidPathInfo info = worker.store.queryPathInfo(path);
if (hash.first != info.hash)
throw Error(format("derivation `%2%' may not be deterministic: hash mismatch in output `%1%'") % drvPath % path);
continue;
}
contentHashes[path] = hash; contentHashes[path] = hash;
/* For debugging, print out the referenced and unreferenced /* For debugging, print out the referenced and unreferenced
@ -2290,6 +2321,8 @@ void DerivationGoal::registerOutputs()
worker.store.markContentsGood(path); worker.store.markContentsGood(path);
} }
if (buildMode == bmCheck) return;
/* Register each output path as valid, and register the sets of /* Register each output path as valid, and register the sets of
paths referenced by each of them. If there are cycles in the paths referenced by each of them. If there are cycles in the
outputs, this will fail. */ outputs, this will fail. */
@ -2457,6 +2490,7 @@ Path DerivationGoal::addHashRewrite(const Path & path)
assert(path.size() == p.size()); assert(path.size() == p.size());
rewritesToTmp[h1] = h2; rewritesToTmp[h1] = h2;
rewritesFromTmp[h2] = h1; rewritesFromTmp[h2] = h1;
redirectedOutputs[path] = p;
return p; return p;
} }
@ -3212,7 +3246,7 @@ unsigned int Worker::exitStatus()
////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////
void LocalStore::buildPaths(const PathSet & drvPaths, bool repair) void LocalStore::buildPaths(const PathSet & drvPaths, BuildMode buildMode)
{ {
startNest(nest, lvlDebug, startNest(nest, lvlDebug,
format("building %1%") % showPaths(drvPaths)); format("building %1%") % showPaths(drvPaths));
@ -3223,9 +3257,9 @@ void LocalStore::buildPaths(const PathSet & drvPaths, bool repair)
foreach (PathSet::const_iterator, i, drvPaths) { foreach (PathSet::const_iterator, i, drvPaths) {
DrvPathWithOutputs i2 = parseDrvPathWithOutputs(*i); DrvPathWithOutputs i2 = parseDrvPathWithOutputs(*i);
if (isDerivation(i2.first)) if (isDerivation(i2.first))
goals.insert(worker.makeDerivationGoal(i2.first, i2.second, repair ? bmRepair : bmNormal)); goals.insert(worker.makeDerivationGoal(i2.first, i2.second, buildMode));
else else
goals.insert(worker.makeSubstitutionGoal(*i, repair)); goals.insert(worker.makeSubstitutionGoal(*i, buildMode));
} }
worker.run(goals); worker.run(goals);

View file

@ -150,7 +150,7 @@ public:
Paths importPaths(bool requireSignature, Source & source); Paths importPaths(bool requireSignature, Source & source);
void buildPaths(const PathSet & paths, bool repair = false); void buildPaths(const PathSet & paths, BuildMode buildMode);
void ensurePath(const Path & path); void ensurePath(const Path & path);

View file

@ -447,9 +447,9 @@ Paths RemoteStore::importPaths(bool requireSignature, Source & source)
} }
void RemoteStore::buildPaths(const PathSet & drvPaths, bool repair) void RemoteStore::buildPaths(const PathSet & drvPaths, BuildMode buildMode)
{ {
if (repair) throw Error("repairing is not supported when building through the Nix daemon"); if (buildMode != bmNormal) throw Error("repairing or checking is not supported when building through the Nix daemon");
openConnection(); openConnection();
writeInt(wopBuildPaths, to); writeInt(wopBuildPaths, to);
if (GET_PROTOCOL_MINOR(daemonVersion) >= 13) if (GET_PROTOCOL_MINOR(daemonVersion) >= 13)

View file

@ -65,7 +65,7 @@ public:
Paths importPaths(bool requireSignature, Source & source); Paths importPaths(bool requireSignature, Source & source);
void buildPaths(const PathSet & paths, bool repair = false); void buildPaths(const PathSet & paths, BuildMode buildMode);
void ensurePath(const Path & path); void ensurePath(const Path & path);

View file

@ -98,6 +98,9 @@ struct ValidPathInfo
typedef list<ValidPathInfo> ValidPathInfos; typedef list<ValidPathInfo> ValidPathInfos;
enum BuildMode { bmNormal, bmRepair, bmCheck };
class StoreAPI class StoreAPI
{ {
public: public:
@ -190,7 +193,7 @@ public:
output paths can be created by running the builder, after output paths can be created by running the builder, after
recursively building any sub-derivations. For inputs that are recursively building any sub-derivations. For inputs that are
not derivations, substitute them. */ not derivations, substitute them. */
virtual void buildPaths(const PathSet & paths, bool repair = false) = 0; virtual void buildPaths(const PathSet & paths, BuildMode buildMode = bmNormal) = 0;
/* Ensure that a path is valid. If it is not currently valid, it /* Ensure that a path is valid. If it is not currently valid, it
may be made valid by running a substitute (if defined for the may be made valid by running a substitute (if defined for the

View file

@ -714,7 +714,7 @@ static void opSet(Globals & globals,
PathSet paths = singleton<PathSet>(drv.queryDrvPath()); PathSet paths = singleton<PathSet>(drv.queryDrvPath());
printMissing(*store, paths); printMissing(*store, paths);
if (globals.dryRun) return; if (globals.dryRun) return;
store->buildPaths(paths, globals.state.repair); store->buildPaths(paths, globals.state.repair ? bmRepair : bmNormal);
} }
else { else {
printMissing(*store, singleton<PathSet>(drv.queryOutPath())); printMissing(*store, singleton<PathSet>(drv.queryOutPath()));

View file

@ -38,7 +38,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
drvsToBuild.insert(i->queryDrvPath()); drvsToBuild.insert(i->queryDrvPath());
debug(format("building user environment dependencies")); debug(format("building user environment dependencies"));
store->buildPaths(drvsToBuild, state.repair); store->buildPaths(drvsToBuild, state.repair ? bmRepair : bmNormal);
/* Construct the whole top level derivation. */ /* Construct the whole top level derivation. */
PathSet references; PathSet references;
@ -125,7 +125,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
/* Realise the resulting store expression. */ /* Realise the resulting store expression. */
debug("building user environment"); debug("building user environment");
store->buildPaths(singleton<PathSet>(topLevelDrv), state.repair); store->buildPaths(singleton<PathSet>(topLevelDrv), state.repair ? bmRepair : bmNormal);
/* Switch the current user environment to the output path. */ /* Switch the current user environment to the output path. */
PathLocks lock; PathLocks lock;

View file

@ -104,12 +104,13 @@ static PathSet realisePath(const Path & path, bool build = true)
static void opRealise(Strings opFlags, Strings opArgs) static void opRealise(Strings opFlags, Strings opArgs)
{ {
bool dryRun = false; bool dryRun = false;
bool repair = false; BuildMode buildMode = bmNormal;
bool ignoreUnknown = false; bool ignoreUnknown = false;
foreach (Strings::iterator, i, opFlags) foreach (Strings::iterator, i, opFlags)
if (*i == "--dry-run") dryRun = true; if (*i == "--dry-run") dryRun = true;
else if (*i == "--repair") repair = true; else if (*i == "--repair") buildMode = bmRepair;
else if (*i == "--check") buildMode = bmCheck;
else if (*i == "--ignore-unknown") ignoreUnknown = true; else if (*i == "--ignore-unknown") ignoreUnknown = true;
else throw UsageError(format("unknown flag `%1%'") % *i); else throw UsageError(format("unknown flag `%1%'") % *i);
@ -137,7 +138,7 @@ static void opRealise(Strings opFlags, Strings opArgs)
if (dryRun) return; if (dryRun) return;
/* Build all paths at the same time to exploit parallelism. */ /* Build all paths at the same time to exploit parallelism. */
store->buildPaths(PathSet(paths.begin(), paths.end()), repair); store->buildPaths(PathSet(paths.begin(), paths.end()), buildMode);
if (!ignoreUnknown) if (!ignoreUnknown)
foreach (Paths::iterator, i, paths) { foreach (Paths::iterator, i, paths) {