From b682fae9d93037af65282e02b88a9ce34129026e Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Mon, 2 Feb 2009 17:24:10 +0000
Subject: [PATCH] * Build hooks: use nix-store --import.  This prevents a
 redundant   scan for runtime dependencies (i.e. the local machine shouldn't
 do a   scan that the remote machine has already done).  Also pipe directly  
 into `nix-store --import': don't use a temporary file.

---
 scripts/build-remote.pl.in  | 15 ++-------------
 src/libstore/build.cc       | 27 +++++++++++++++++++--------
 src/libstore/local-store.cc |  9 ++++++++-
 3 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/scripts/build-remote.pl.in b/scripts/build-remote.pl.in
index 53cf53644..e821b4071 100755
--- a/scripts/build-remote.pl.in
+++ b/scripts/build-remote.pl.in
@@ -192,7 +192,7 @@ my $buildFlags = "--max-silent-time $maxSilentTime";
 # connection dies.  Without it, the remote process might continue to
 # run indefinitely (that is, until it next tries to write to
 # stdout/stderr).
-if (system("ssh -tt $sshOpts $hostName 'nix-store -rvvK $buildFlags $drvPath'") != 0) {
+if (system("ssh -tt $sshOpts $hostName 'nix-store --realise -K $buildFlags $drvPath > /dev/null'") != 0) {
     # If we couldn't run ssh or there was an ssh problem (indicated by
     # exit code 255), then we return exit code 1; otherwise we assume
     # that the builder failed, which we indicated to Nix using exit
@@ -209,17 +209,6 @@ foreach my $output (split '\n', $outputs) {
     my $maybeSignRemote = "";
     $maybeSignRemote = "--sign" if $UID != 0;
     
-    system("ssh $sshOpts $hostName 'nix-store --export $maybeSignRemote $output' > dump") == 0
+    system("ssh $sshOpts $hostName 'nix-store --export $maybeSignRemote $output' | @bindir@/nix-store --import > /dev/null") == 0
 	or die "cannot copy $output from $hostName: $?";
-
-    # This doesn't work yet, since the caller has a lock on the output
-    # path.  We should move towards lock-free invocation of build
-    # hooks and substitutes.
-    #system("nix-store --import < dump") == 0
-    #    or die "cannot import $output: $?";
-
-    # Hack: skip the first 8 bytes (the nix-store --export next
-    # archive marker).  The archive follows.
-    system("(dd bs=1 count=8 of=/dev/null && cat) < dump | nix-store --restore $output") == 0
-	or die "cannot restore $output: $?";
 }
diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index 6b1bc55f9..d44dcf0ff 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -1240,6 +1240,12 @@ DerivationGoal::HookReply DerivationGoal::tryBuildHook()
 
             initChild();
 
+            string s;
+            foreach (DerivationOutputs::const_iterator, i, drv.outputs)
+                s += i->second.path + " ";
+            if (setenv("NIX_HELD_LOCKS", s.c_str(), 1))
+                throw SysError("setting an environment variable");
+
             execl(buildHook.c_str(), buildHook.c_str(),
                 (worker.canBuildMore() ? (string) "1" : "0").c_str(),
                 thisSystem.c_str(),
@@ -1945,13 +1951,21 @@ void DerivationGoal::computeClosure()
 {
     map<Path, PathSet> allReferences;
     map<Path, Hash> contentHashes;
-    
+
+    /* When using a build hook, the build hook can register the output
+       as valid (by doing `nix-store --import').  If so we don't have
+       to do anything here. */
+    if (usingBuildHook) {
+        bool allValid = true;
+        foreach (DerivationOutputs::iterator, i, drv.outputs)
+            if (!worker.store.isValidPath(i->second.path)) allValid = false;
+        if (allValid) return;
+    }
+        
     /* 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. */
-    for (DerivationOutputs::iterator i = drv.outputs.begin(); 
-         i != drv.outputs.end(); ++i)
-    {
+    foreach (DerivationOutputs::iterator, i, drv.outputs) {
         Path path = i->second.path;
         if (!pathExists(path)) {
             throw BuildError(
@@ -2043,14 +2057,11 @@ void DerivationGoal::computeClosure()
        paths referenced by each of them.  !!! this should be
        atomic so that either all paths are registered as valid, or
        none are. */
-    for (DerivationOutputs::iterator i = drv.outputs.begin(); 
-         i != drv.outputs.end(); ++i)
-    {
+    foreach (DerivationOutputs::iterator, i, drv.outputs)
         worker.store.registerValidPath(i->second.path,
             contentHashes[i->second.path],
             allReferences[i->second.path],
             drvPath);
-    }
 
     /* It is now safe to delete the lock files, since all future
        lockers will see that the output paths are valid; they will not
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index 159f28c91..1d0c68cb8 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -924,7 +924,14 @@ Path LocalStore::importPath(bool requireSignature, Source & source)
 
     if (!isValidPath(dstPath)) {
 
-        PathLocks outputLock(singleton<PathSet, Path>(dstPath));
+        PathLocks outputLock;
+
+        /* Lock the output path.  But don't lock if we're being called
+           from a build hook (whose parent process already acquired a
+           lock on this path). */
+        Strings locksHeld = tokenizeString(getEnv("NIX_HELD_LOCKS"));
+        if (find(locksHeld.begin(), locksHeld.end(), dstPath) == locksHeld.end())
+            outputLock.lockPaths(singleton<PathSet, Path>(dstPath));
 
         if (!isValidPath(dstPath)) {