Don't keep "disabled" substituters running

For instance, it's pointless to keep copy-from-other-stores running if
there are no other stores, or download-using-manifests if there are no
manifests.  This also speeds things up because we don't send queries
to those substituters.

9 files changed, 46 insertions, 7 deletions

diff --git a/perl/lib/Nix/Manifest.pm b/perl/lib/Nix/Manifest.pm
index ed43900b5c53..50e354c0c5bd 100644
--- a/perl/lib/Nix/Manifest.pm
+++ b/perl/lib/Nix/Manifest.pm
@@ -227,6 +227,9 @@ sub writeManifest {
 sub updateManifestDB {
     my $manifestDir = $Nix::Config::manifestDir;
 
+    my @manifests = glob "$manifestDir/*.nixmanifest";
+    return undef if scalar @manifests == 0;
+
     mkpath($manifestDir);
 
     unlink "$manifestDir/cache.sqlite"; # remove obsolete cache
@@ -311,7 +314,7 @@ EOF
     # unless we've already done so on a previous run.
     my %seen;
 
-    for my $manifestLink (glob "$manifestDir/*.nixmanifest") {
+    for my $manifestLink (@manifests) {
         my $manifest = Cwd::abs_path($manifestLink);
         next unless -f $manifest;
         my $timestamp = lstat($manifest)->mtime;
diff --git a/scripts/copy-from-other-stores.pl.in b/scripts/copy-from-other-stores.pl.in
index 9ed7e4cc22c0..7a7c30cdfb23 100755
--- a/scripts/copy-from-other-stores.pl.in
+++ b/scripts/copy-from-other-stores.pl.in
@@ -16,6 +16,9 @@ foreach my $dir (@remoteStoresAll) {
     push @remoteStores, glob($dir);
 }
 
+exit if scalar @remoteStores == 0;
+print "\n";
+
 
 $ENV{"NIX_REMOTE"} = "";
 
diff --git a/scripts/download-from-binary-cache.pl.in b/scripts/download-from-binary-cache.pl.in
index 10444dc61bc8..abd1f7b712c4 100644
--- a/scripts/download-from-binary-cache.pl.in
+++ b/scripts/download-from-binary-cache.pl.in
@@ -199,10 +199,6 @@ sub getAvailableCaches {
     return if $gotCaches;
     $gotCaches = 1;
 
-    return if
-        ($Nix::Config::config{"use-binary-caches"} // "true") eq "false" ||
-        ($Nix::Config::config{"untrusted-use-binary-caches"} // "true") eq "false";
-
     sub strToList {
         my ($s) = @_;
         return map { s/\/+$//; $_ } split(/ /, $s);
@@ -543,6 +539,13 @@ sub downloadBinary {
 }
 
 
+# Bail out right away if binary caches are disabled.
+exit 0 if
+    ($Nix::Config::config{"use-binary-caches"} // "true") eq "false" ||
+    ($Nix::Config::config{"untrusted-use-binary-caches"} // "true") eq "false";
+print "\n";
+flush STDOUT;
+
 initCache();
 
 
diff --git a/scripts/download-using-manifests.pl.in b/scripts/download-using-manifests.pl.in
index ecd0b489316a..0471a9e1fac4 100755
--- a/scripts/download-using-manifests.pl.in
+++ b/scripts/download-using-manifests.pl.in
@@ -22,6 +22,8 @@ my $curl = "$Nix::Config::curl --fail --location --insecure";
 
 # Open the manifest cache and update it if necessary.
 my $dbh = updateManifestDB();
+exit 0 unless defined $dbh; # exit if there are no manifests
+print "\n";
 
 
 # $hashCache->{$algo}->{$path} yields the $algo-hash of $path.
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index 13c70fbf5713..ab5ab70d0737 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -317,8 +317,10 @@ LocalStore::~LocalStore()
 {
     try {
         foreach (RunningSubstituters::iterator, i, runningSubstituters) {
+            if (i->second.disabled) continue;
             i->second.to.close();
             i->second.from.close();
+            i->second.error.close();
             i->second.pid.wait(true);
         }
     } catch (...) {
@@ -998,7 +1000,7 @@ void LocalStore::setSubstituterEnv()
 
 void LocalStore::startSubstituter(const Path & substituter, RunningSubstituter & run)
 {
-    if (run.pid != -1) return;
+    if (run.disabled || run.pid != -1) return;
 
     debug(format("starting substituter program `%1%'") % substituter);
 
@@ -1039,6 +1041,23 @@ void LocalStore::startSubstituter(const Path & substituter, RunningSubstituter &
     run.to = toPipe.writeSide.borrow();
     run.from = run.fromBuf.fd = fromPipe.readSide.borrow();
     run.error = errorPipe.readSide.borrow();
+
+    toPipe.readSide.close();
+    fromPipe.writeSide.close();
+    errorPipe.writeSide.close();
+
+    /* The substituter may exit right away if it's disabled in any way
+       (e.g. copy-from-other-stores.pl will exit if no other stores
+       are configured). */
+    try {
+        getLineFromSubstituter(run);
+    } catch (EndOfFile & e) {
+        run.to.close();
+        run.from.close();
+        run.error.close();
+        run.disabled = true;
+        if (run.pid.wait(true) != 0) throw;
+    }
 }
 
 
@@ -1052,6 +1071,8 @@ string LocalStore::getLineFromSubstituter(RunningSubstituter & run)
     if (run.fromBuf.hasData()) goto haveData;
 
     while (1) {
+        checkInterrupt();
+
         fd_set fds;
         FD_ZERO(&fds);
         FD_SET(run.from, &fds);
@@ -1072,7 +1093,7 @@ string LocalStore::getLineFromSubstituter(RunningSubstituter & run)
                 if (errno == EINTR) continue;
                 throw SysError("reading from substituter's stderr");
             }
-            if (n == 0) throw Error(format("substituter `%1%' died unexpectedly") % run.program);
+            if (n == 0) throw EndOfFile(format("substituter `%1%' died unexpectedly") % run.program);
             err.append(buf, n);
             string::size_type p;
             while ((p = err.find('\n')) != string::npos) {
@@ -1114,6 +1135,7 @@ PathSet LocalStore::querySubstitutablePaths(const PathSet & paths)
         if (res.size() == paths.size()) break;
         RunningSubstituter & run(runningSubstituters[*i]);
         startSubstituter(*i, run);
+        if (run.disabled) continue;
         string s = "have ";
         foreach (PathSet::const_iterator, j, paths)
             if (res.find(*j) == res.end()) { s += *j; s += " "; }
@@ -1137,6 +1159,7 @@ void LocalStore::querySubstitutablePathInfos(const Path & substituter,
 {
     RunningSubstituter & run(runningSubstituters[substituter]);
     startSubstituter(substituter, run);
+    if (run.disabled) return;
 
     string s = "info ";
     foreach (PathSet::const_iterator, i, paths)
diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index 4b132972e5ad..d3b210e3f62b 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -48,6 +48,8 @@ struct RunningSubstituter
     Pid pid;
     AutoCloseFD to, from, error;
     FdSource fromBuf;
+    bool disabled;
+    RunningSubstituter() : disabled(false) { };
 };
 
 
diff --git a/src/libutil/util.cc b/src/libutil/util.cc
index e67568b1a78d..b1231e45574f 100644
--- a/src/libutil/util.cc
+++ b/src/libutil/util.cc
@@ -790,6 +790,7 @@ void Pid::kill()
 
 int Pid::wait(bool block)
 {
+    assert(pid != -1);
     while (1) {
         int status;
         int res = waitpid(pid, &status, block ? 0 : WNOHANG);
diff --git a/tests/substituter.sh b/tests/substituter.sh
index 885655760e05..9aab295de87b 100755
--- a/tests/substituter.sh
+++ b/tests/substituter.sh
@@ -1,4 +1,5 @@
 #! /bin/sh -e
+echo
 echo substituter args: $* >&2
 
 if test $1 = "--query"; then
diff --git a/tests/substituter2.sh b/tests/substituter2.sh
index 34b2c0eafaf8..5d1763599c25 100755
--- a/tests/substituter2.sh
+++ b/tests/substituter2.sh
@@ -1,4 +1,5 @@
 #! /bin/sh -e
+echo
 echo substituter2 args: $* >&2
 
 if test $1 = "--query"; then