* Maintain the references/referers relation also for derivations.

  This simplifies garbage collection and `nix-store --query
  --requisites' since we no longer need to treat derivations
  specially.

* Better maintaining of the invariants, e.g., setReferences() can only
  be called on a valid/substitutable path.

8 files changed, 84 insertions, 82 deletions

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index e5089bdb45..b63488b8de 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -1135,8 +1135,7 @@ void DerivationGoal::computeClosure()
          i != drv.outputs.end(); ++i)
     {
         registerValidPath(txn, i->second.path,
-            contentHashes[i->second.path]);
-        setReferences(txn, i->second.path,
+            contentHashes[i->second.path],
             allReferences[i->second.path]);
     }
     txn.commit();
@@ -1494,7 +1493,7 @@ void SubstitutionGoal::finished()
 
     Transaction txn;
     createStoreTransaction(txn);
-    registerValidPath(txn, storePath, contentHash);
+    registerValidPath(txn, storePath, contentHash, references);
     txn.commit();
 
     outputLock->setDeletion(true);
diff --git a/src/libstore/build.hh b/src/libstore/build.hh
index 52e7c9b9d5..2c6bcf9f2d 100644
--- a/src/libstore/build.hh
+++ b/src/libstore/build.hh
@@ -28,23 +28,4 @@ Derivation derivationFromPath(const Path & drvPath);
 void computeFSClosure(const Path & storePath,
     PathSet & paths, bool flipDirection = false);
 
-/* Place in `paths' the set of paths that are required to `realise'
-   the given store path, i.e., all paths necessary for valid
-   deployment of the path.  For a derivation, this is the union of
-   requisites of the inputs, plus the derivation; for other store
-   paths, it is the set of paths in the FS closure of the path.  If
-   `includeOutputs' is true, include the requisites of the output
-   paths of derivations as well.
-
-   Note that this function can be used to implement three different
-   deployment policies:
-
-   - Source deployment (when called on a derivation).
-   - Binary deployment (when called on an output path).
-   - Source/binary deployment (when called on a derivation with
-     `includeOutputs' set to true).
-*/
-void storePathRequisites(const Path & storePath,
-    bool includeOutputs, PathSet & paths);
-
 #endif /* !__BUILD_H */
diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc
index 2d776fb748..e31333e777 100644
--- a/src/libstore/derivations.cc
+++ b/src/libstore/derivations.cc
@@ -14,8 +14,16 @@ Hash hashTerm(ATerm t)
 
 Path writeDerivation(const Derivation & drv, const string & name)
 {
+    PathSet references;
+    references.insert(drv.inputSrcs.begin(), drv.inputSrcs.end());
+    for (DerivationInputs::const_iterator i = drv.inputDrvs.begin();
+         i != drv.inputDrvs.end(); ++i)
+        references.insert(i->first);
+    /* Note that the outputs of a derivation are *not* references
+       (that can be missing (of course) and should not necessarily be
+       held during a garbage collection). */
     return addTextToStore(name + drvExtension,
-        atPrint(unparseDerivation(drv)));
+        atPrint(unparseDerivation(drv)), references);
 }
 
 
diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc
index 802e576512..b208791788 100644
--- a/src/libstore/misc.cc
+++ b/src/libstore/misc.cc
@@ -27,44 +27,3 @@ void computeFSClosure(const Path & storePath,
          i != references.end(); ++i)
         computeFSClosure(*i, paths, flipDirection);
 }
-
-
-void storePathRequisites(const Path & storePath,
-    bool includeOutputs, PathSet & paths)
-{
-    checkInterrupt();
-    
-    if (paths.find(storePath) != paths.end()) return;
-
-    if (isDerivation(storePath)) {
-
-        paths.insert(storePath);
-        
-        Derivation drv = derivationFromPath(storePath);
-
-        for (DerivationInputs::iterator i = drv.inputDrvs.begin();
-             i != drv.inputDrvs.end(); ++i)
-            /* !!! Maybe this is too strict, since it will include
-               *all* output paths of the input derivation, not just
-               the ones needed by this derivation. */
-            storePathRequisites(i->first, includeOutputs, paths);
-
-        for (PathSet::iterator i = drv.inputSrcs.begin();
-             i != drv.inputSrcs.end(); ++i)
-            storePathRequisites(*i, includeOutputs, paths);
-
-        if (includeOutputs) {
-
-            for (DerivationOutputs::iterator i = drv.outputs.begin();
-                 i != drv.outputs.end(); ++i)
-                if (isValidPath(i->second.path))
-                    storePathRequisites(i->second.path, includeOutputs, paths);
-
-        }
-        
-    }
-
-    else {
-        computeFSClosure(storePath, paths);
-    }
-}
diff --git a/src/libstore/store.cc b/src/libstore/store.cc
index 0dd4e1ed25..f5e7d2aa58 100644
--- a/src/libstore/store.cc
+++ b/src/libstore/store.cc
@@ -229,7 +229,7 @@ void canonicalisePathMetaData(const Path & path)
 }
 
 
-static bool isValidPathTxn(const Path & path, const Transaction & txn)
+static bool isValidPathTxn(const Transaction & txn, const Path & path)
 {
     string s;
     return nixDB.queryString(txn, dbValidPaths, path, s);
@@ -238,13 +238,29 @@ static bool isValidPathTxn(const Path & path, const Transaction & txn)
 
 bool isValidPath(const Path & path)
 {
-    return isValidPathTxn(path, noTxn);
+    return isValidPathTxn(noTxn, path);
+}
+
+
+static Substitutes readSubstitutes(const Transaction & txn,
+    const Path & srcPath);
+
+
+static bool isRealisablePath(const Transaction & txn, const Path & path)
+{
+    return isValidPathTxn(txn, path)
+        || readSubstitutes(txn, path).size() > 0;
 }
 
 
 void setReferences(const Transaction & txn, const Path & storePath,
     const PathSet & references)
 {
+    if (!isRealisablePath(txn, storePath))
+        throw Error(
+            format("cannot set references for path `%1%' which is invalid and has no substitutes")
+            % storePath);
+    
     nixDB.setStrings(txn, dbReferences, storePath,
         Paths(references.begin(), references.end()));
 
@@ -265,8 +281,8 @@ void setReferences(const Transaction & txn, const Path & storePath,
 void queryReferences(const Path & storePath, PathSet & references)
 {
     Paths references2;
-    //    if (!isValidPath(storePath))
-    //        throw Error(format("path `%1%' is not valid") % storePath);
+    if (!isRealisablePath(noTxn, storePath))
+        throw Error(format("path `%1%' is not valid") % storePath);
     nixDB.queryStrings(noTxn, dbReferences, storePath, references2);
     references.insert(references2.begin(), references2.end());
 }
@@ -275,8 +291,8 @@ void queryReferences(const Path & storePath, PathSet & references)
 void queryReferers(const Path & storePath, PathSet & referers)
 {
     Paths referers2;
-    //    if (!isValidPath(storePath))
-    //        throw Error(format("path `%1%' is not valid") % storePath);
+    if (!isRealisablePath(noTxn, storePath))
+        throw Error(format("path `%1%' is not valid") % storePath);
     nixDB.queryStrings(noTxn, dbReferers, storePath, referers2);
     referers.insert(referers2.begin(), referers2.end());
 }
@@ -370,7 +386,7 @@ void clearSubstitutes()
 
 
 void registerValidPath(const Transaction & txn,
-    const Path & _path, const Hash & hash)
+    const Path & _path, const Hash & hash, const PathSet & references)
 {
     Path path(canonPath(_path));
     assertStorePath(path);
@@ -380,11 +396,11 @@ void registerValidPath(const Transaction & txn,
     debug(format("registering path `%1%'") % path);
     nixDB.setString(txn, dbValidPaths, path, "sha256:" + printHash(hash));
 
+    setReferences(txn, path, references);
+    
     /* Check that all referenced paths are also valid. */
-    Paths references;
-    nixDB.queryStrings(txn, dbReferences, path, references);
-    for (Paths::iterator i = references.begin(); i != references.end(); ++i)
-        if (!isValidPathTxn(*i, txn))
+    for (PathSet::iterator i = references.begin(); i != references.end(); ++i)
+        if (!isValidPathTxn(txn, *i))
             throw Error(format("cannot register path `%1%' as valid, since its reference `%2%' is invalid")
                 % path % *i);
 }
@@ -451,7 +467,7 @@ Path addToStore(const Path & _srcPath)
             canonicalisePathMetaData(dstPath);
             
             Transaction txn(nixDB);
-            registerValidPath(txn, dstPath, h);
+            registerValidPath(txn, dstPath, h, PathSet());
             txn.commit();
         }
 
@@ -462,7 +478,8 @@ Path addToStore(const Path & _srcPath)
 }
 
 
-Path addTextToStore(const string & suffix, const string & s)
+Path addTextToStore(const string & suffix, const string & s,
+    const PathSet & references)
 {
     Hash hash = hashString(htSHA256, s);
 
@@ -483,7 +500,8 @@ Path addTextToStore(const string & suffix, const string & s)
             canonicalisePathMetaData(dstPath);
             
             Transaction txn(nixDB);
-            registerValidPath(txn, dstPath, hashPath(htSHA256, dstPath));
+            registerValidPath(txn, dstPath,
+                hashPath(htSHA256, dstPath), references);
             txn.commit();
         }
 
diff --git a/src/libstore/store.hh b/src/libstore/store.hh
index 20f50cbfb1..968786305e 100644
--- a/src/libstore/store.hh
+++ b/src/libstore/store.hh
@@ -57,7 +57,7 @@ void clearSubstitutes();
    of the file system contents of the path.  The hash must be a
    SHA-256 hash. */
 void registerValidPath(const Transaction & txn,
-    const Path & path, const Hash & hash);
+    const Path & path, const Hash & hash, const PathSet & references);
 
 /* Throw an exception if `path' is not directly in the Nix store. */
 void assertStorePath(const Path & path);
@@ -97,7 +97,8 @@ Path addToStore(const Path & srcPath);
 
 /* Like addToStore, but the contents written to the output path is a
    regular file containing the given string. */
-Path addTextToStore(const string & suffix, const string & s);
+Path addTextToStore(const string & suffix, const string & s,
+    const PathSet & references);
 
 /* Delete a value from the nixStore directory. */
 void deleteFromStore(const Path & path);
diff --git a/src/nix-env/main.cc b/src/nix-env/main.cc
index b64bc281af..3cb6b02c1f 100644
--- a/src/nix-env/main.cc
+++ b/src/nix-env/main.cc
@@ -205,7 +205,8 @@ void createUserEnv(EvalState & state, const DrvInfos & drvs,
 
     /* Also write a copy of the list of inputs to the store; we need
        it for future modifications of the environment. */
-    Path inputsFile = addTextToStore("env-inputs", atPrint(inputs2));
+    Path inputsFile = addTextToStore("env-inputs", atPrint(inputs2),
+        PathSet() /* !!! incorrect */);
 
     Expr topLevel = makeCall(envBuilder, makeAttrs(ATmakeList3(
         makeBind(toATerm("system"),
diff --git a/src/nix-store/main.cc b/src/nix-store/main.cc
index ed93f20657..ea8d398f1a 100644
--- a/src/nix-store/main.cc
+++ b/src/nix-store/main.cc
@@ -74,6 +74,41 @@ static void opAdd(Strings opFlags, Strings opArgs)
 }
 
 
+/* Place in `paths' the set of paths that are required to `realise'
+   the given store path, i.e., all paths necessary for valid
+   deployment of the path.  For a derivation, this is the union of
+   requisites of the inputs, plus the derivation; for other store
+   paths, it is the set of paths in the FS closure of the path.  If
+   `includeOutputs' is true, include the requisites of the output
+   paths of derivations as well.
+
+   Note that this function can be used to implement three different
+   deployment policies:
+
+   - Source deployment (when called on a derivation).
+   - Binary deployment (when called on an output path).
+   - Source/binary deployment (when called on a derivation with
+     `includeOutputs' set to true).
+*/
+static void storePathRequisites(const Path & storePath,
+    bool includeOutputs, PathSet & paths)
+{
+    computeFSClosure(storePath, paths);
+
+    if (includeOutputs) {
+        for (PathSet::iterator i = paths.begin();
+             i != paths.end(); ++i)
+            if (isDerivation(*i)) {
+                Derivation drv = derivationFromPath(*i);
+                for (DerivationOutputs::iterator j = drv.outputs.begin();
+                     j != drv.outputs.end(); ++j)
+                    if (isValidPath(j->second.path))
+                        computeFSClosure(j->second.path, paths);
+            }
+    }
+}
+
+
 static Path maybeUseOutput(const Path & storePath, bool useOutput, bool forceRealise)
 {
     if (forceRealise) realisePath(storePath);
@@ -221,7 +256,7 @@ static void opValidPath(Strings opFlags, Strings opArgs)
     createStoreTransaction(txn);
     for (Strings::iterator i = opArgs.begin();
          i != opArgs.end(); ++i)
-        registerValidPath(txn, *i, hashPath(htSHA256, *i));
+        registerValidPath(txn, *i, hashPath(htSHA256, *i), PathSet());
     txn.commit();
 }