From 22144afa8d9f8968da351618a1347072a93bd8aa Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 20 Jun 2013 11:55:15 +0200 Subject: [PATCH] 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. --- perl/lib/Nix/Manifest.pm | 5 ++++- scripts/copy-from-other-stores.pl.in | 3 +++ scripts/download-from-binary-cache.pl.in | 11 ++++++---- scripts/download-using-manifests.pl.in | 2 ++ src/libstore/local-store.cc | 27 ++++++++++++++++++++++-- src/libstore/local-store.hh | 2 ++ src/libutil/util.cc | 1 + tests/substituter.sh | 1 + tests/substituter2.sh | 1 + 9 files changed, 46 insertions(+), 7 deletions(-) diff --git a/perl/lib/Nix/Manifest.pm b/perl/lib/Nix/Manifest.pm index ed43900b5..50e354c0c 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 9ed7e4cc2..7a7c30cdf 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 10444dc61..abd1f7b71 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 ecd0b4893..0471a9e1f 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 13c70fbf5..ab5ab70d0 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 4b132972e..d3b210e3f 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 e67568b1a..b1231e455 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 885655760..9aab295de 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 34b2c0eaf..5d1763599 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