From 28972864879a63c4110211d9852b692672869a2a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 18 Mar 2009 17:36:42 +0000 Subject: [PATCH] * Unify exportReferencesGraph and exportBuildReferencesGraph, and make sure that it works as expected when you pass it a derivation. That is, we have to make sure that all build-time dependencies are built, and that they are all in the input closure (otherwise remote builds might fail, for example). This is ensured at instantiation time by adding all derivations and their sources to inputDrvs and inputSrcs. --- scripts/nix-push.in | 6 +--- src/libexpr/primops.cc | 30 ++++++++++++-------- src/libstore/build.cc | 64 +++++++++++------------------------------- tests/export-graph.nix | 5 ++-- 4 files changed, 39 insertions(+), 66 deletions(-) diff --git a/scripts/nix-push.in b/scripts/nix-push.in index ba611465c9..38097f7401 100644 --- a/scripts/nix-push.in +++ b/scripts/nix-push.in @@ -140,11 +140,7 @@ while (scalar @tmp > 0) { my @tmp2 = @tmp[0..$n - 1]; @tmp = @tmp[$n..scalar @tmp - 1]; - # Note: we disable build hooks because of the impure path - # reference (see above). Even if that is fixed, using a hook - # probably wouldn't make that much sense; pumping lots of data - # around just to compress them won't gain that much. - my $pid = open(READ, "$binDir/nix-store --no-build-hook --realise @tmp2|") + my $pid = open(READ, "$binDir/nix-store --realise @tmp2|") or die "cannot run nix-store"; while () { chomp; diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 717f649c4e..34e7e80525 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -399,16 +399,29 @@ static Expr prim_derivationStrict(EvalState & state, const ATermVector & args) /* Everything in the context of the strings in the derivation attributes should be added as dependencies of the resulting derivation. */ - for (PathSet::iterator i = context.begin(); i != context.end(); ++i) { + foreach (PathSet::iterator, i, context) { Path path = *i; - bool buildDrv = true; + + /* Paths marked with `=' denote that the path of a derivation + is explicitly passed to the builder. Since that allows the + builder to gain access to every path in the dependency + graph of the derivation (including all outputs), all paths + in the graph must be added to this derivation's list of + inputs to ensure that they are available when the builder + runs. */ if (path.at(0) == '=') { - buildDrv = false; path = string(path, 1); + PathSet refs; computeFSClosure(path, refs); + foreach (PathSet::iterator, j, refs) { + drv.inputSrcs.insert(*j); + if (isDerivation(*j)) + drv.inputDrvs[*j] = singleton("out"); + } } + debug(format("derivation uses `%1%'") % path); assert(isStorePath(path)); - if (buildDrv && isDerivation(path)) + if (isDerivation(path)) drv.inputDrvs[path] = singleton("out"); else drv.inputSrcs.insert(path); @@ -484,7 +497,7 @@ static Expr prim_derivationStrict(EvalState & state, const ATermVector & args) outAttrs.set(toATerm("outPath"), makeAttrRHS(makeStr(outPath, singleton(drvPath)), makeNoPos())); outAttrs.set(toATerm("drvPath"), - makeAttrRHS(makeStr(drvPath, singleton(drvPath)), makeNoPos())); + makeAttrRHS(makeStr(drvPath, singleton("=" + drvPath)), makeNoPos())); return makeAttrs(outAttrs); } @@ -541,13 +554,6 @@ static Expr prim_storePath(EvalState & state, const ATermVector & args) Path path2 = toStorePath(path); if (!store->isValidPath(path2)) throw EvalError(format("store path `%1%' is not valid") % path2); - /* If this is a derivation, mark it so it doesn't get built; - i.e. we want the dependency as a "source" dependency. This is - to make nix-push work properly (we want it to create a NAR - archive of the derivation, not build the derivation as a - side-effect). The `=' is a special marker that gets stripped - off by prim_derivationStrict. */ - if (isDerivation(path2)) path2 = "=" + path2; context.insert(path2); return makeStr(path, context); } diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 6a62345f74..b275c9008e 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -935,8 +935,6 @@ void DerivationGoal::inputsRealised() /* Second, the input sources. */ foreach (PathSet::iterator, i, drv.inputSrcs) computeFSClosure(*i, inputPaths); - /* !!! if `*i' is a derivation, then add the closure of every - output in the dependency graph to `inputPaths'. */ debug(format("added input paths %1%") % showPaths(inputPaths)); @@ -1534,55 +1532,27 @@ void DerivationGoal::startBuilder() throw BuildError(format("`exportReferencesGraph' contains an invalid path `%1%'") % storePath); - /* Write closure info to `fileName'. */ - PathSet refs; - computeFSClosure(storePath, refs); - /* !!! in secure Nix, the writing should be done on the - build uid for security (maybe). */ - writeStringToFile(tmpDir + "/" + fileName, - makeValidityRegistration(refs, false, false)); - } + /* If there are derivations in the graph, then include their + outputs as well. This is useful if you want to do things + like passing all build-time dependencies of some path to a + derivation that builds a NixOS DVD image. */ + PathSet paths, paths2; + computeFSClosure(storePath, paths); + paths2 = paths; - // The same for derivations - // !!! urgh, cut&paste duplication - s = drv.env["exportBuildReferencesGraph"]; - ss = tokenizeString(s); - if (ss.size() % 2 != 0) - throw BuildError(format("odd number of tokens in `exportBuildReferencesGraph': `%1%'") % s); - for (Strings::iterator i = ss.begin(); i != ss.end(); ) { - string fileName = *i++; - checkStoreName(fileName); /* !!! abuse of this function */ - - /* Check that the store path is valid. */ - Path storePath = *i++; - if (!isInStore(storePath)) - throw BuildError(format("`exportBuildReferencesGraph' contains a non-store path `%1%'") - % storePath); - storePath = toStorePath(storePath); - if (!worker.store.isValidPath(storePath)) - throw BuildError(format("`exportBuildReferencesGraph' contains an invalid path `%1%'") - % storePath); + foreach (PathSet::iterator, j, paths2) { + if (isDerivation(*j)) { + Derivation drv = derivationFromPath(*j); + foreach (DerivationOutputs::iterator, k, drv.outputs) + computeFSClosure(k->second.path, paths); + } + } /* Write closure info to `fileName'. */ - PathSet refs1,refs; - computeFSClosure(storePath, refs1); - for (PathSet::iterator j = refs1.begin(); j != refs1.end() ; j++) { - refs.insert (*j); - if (isDerivation (*j)) { - Derivation deriv = derivationFromPath (*j); - for (DerivationOutputs::iterator k=deriv.outputs.begin(); - k != deriv.outputs.end(); k++) { - refs.insert(k->second.path); - - } - } - } - /* !!! in secure Nix, the writing should be done on the - build uid for security (maybe). */ writeStringToFile(tmpDir + "/" + fileName, - makeValidityRegistration(refs, false, false)); + makeValidityRegistration(paths, false, false)); } - + /* If `build-users-group' is not empty, then we have to build as one of the members of that group. */ @@ -2167,7 +2137,7 @@ SubstitutionGoal::SubstitutionGoal(const Path & storePath, Worker & worker) SubstitutionGoal::~SubstitutionGoal() { /* !!! Once we let substitution goals run under a build user, we - need to do use the setuid helper just as in ~DerivationGoal(). + need to use the setuid helper just as in ~DerivationGoal(). Idem for cancel. */ if (pid != -1) worker.childTerminated(pid); } diff --git a/tests/export-graph.nix b/tests/export-graph.nix index d65ad8c143..3a0b0368d6 100644 --- a/tests/export-graph.nix +++ b/tests/export-graph.nix @@ -4,12 +4,13 @@ rec { printRefs = '' + echo $exportReferencesGraph while read path; do read drv read nrRefs echo "$path has $nrRefs references" echo "$path" >> $out - for ((n = 0; n < $nrRefs; n++)); do read ref; echo "ref $ref"; done + for ((n = 0; n < $nrRefs; n++)); do read ref; echo "ref $ref"; test -e "$ref"; done done < refs ''; @@ -22,7 +23,7 @@ rec { buildGraph = mkDerivation { name = "dependencies"; builder = builtins.toFile "build-graph-builder" "${printRefs}"; - exportBuildReferencesGraph = ["refs" (import ./dependencies.nix).drvPath]; + exportReferencesGraph = ["refs" (import ./dependencies.nix).drvPath]; }; }