From 3c92ea399d717dc45b3fa91424c0dadc0239ebf2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sat, 2 Aug 2008 12:54:35 +0000 Subject: [PATCH] * Make nix-env --dry-run print the paths to be substituted correctly again. (After the previous substituter mechanism refactoring I didn't update the code that obtains the references of substitutable paths.) This required some refactoring: the substituter programs are now kept running and receive/respond to info requests via stdin/stdout. --- scripts/copy-from-other-stores.pl.in | 71 +++++----- scripts/download-using-manifests.pl.in | 77 +++++----- src/libstore/build.cc | 189 ++++++------------------- src/libstore/local-store.cc | 117 ++++++++++++--- src/libstore/local-store.hh | 23 +++ src/libstore/misc.cc | 10 +- src/libstore/remote-store.cc | 13 +- src/libstore/remote-store.hh | 5 +- src/libstore/store-api.cc | 7 - src/libstore/store-api.hh | 19 ++- src/libutil/util.cc | 22 ++- src/libutil/util.hh | 4 + tests/substituter.sh | 27 ++-- tests/substituter2.sh | 26 ++-- 14 files changed, 338 insertions(+), 272 deletions(-) diff --git a/scripts/copy-from-other-stores.pl.in b/scripts/copy-from-other-stores.pl.in index 74efbd7459..00de3ff999 100644 --- a/scripts/copy-from-other-stores.pl.in +++ b/scripts/copy-from-other-stores.pl.in @@ -2,6 +2,9 @@ use strict; use File::Basename; +use IO::Handle; + +STDOUT->autoflush(1); my @remoteStoresAll = split ':', ($ENV{"NIX_OTHER_STORES"} or ""); @@ -33,42 +36,46 @@ sub findStorePath { } -if ($ARGV[0] eq "--query-paths") { - foreach my $store (@remoteStores) { - opendir DIR, "$store/var/nix/db/info" or next; - print "@storedir@/$_\n" foreach readdir DIR; - closedir DIR; - } -} +if ($ARGV[0] eq "--query") { + while () { + my $cmd = $_; chomp $cmd; -elsif ($ARGV[0] eq "--query-info") { - shift @ARGV; - - foreach my $storePath (@ARGV) { - - (my $infoFile) = findStorePath $storePath; - next unless $infoFile; - - my $deriver = ""; - my @references = (); - - open INFO, "<$infoFile" or die "cannot read info file $infoFile\n"; - while () { - chomp; - #print STDERR "GOT $_\n"; - /^([\w-]+): (.*)$/ or die "bad info file"; - my $key = $1; - my $value = $2; - if ($key eq "Deriver") { $deriver = $value; } - elsif ($key eq "References") { @references = split ' ', $value; } + if ($cmd eq "have") { + my $storePath = ; chomp $storePath; + (my $infoFile) = findStorePath $storePath; + print STDOUT ($infoFile ? "1\n" : "0\n"); } - close INFO; - print "$storePath\n"; - print "$deriver\n"; - print scalar @references, "\n"; - print "$_\n" foreach @references; + elsif ($cmd eq "info") { + my $storePath = ; chomp $storePath; + (my $infoFile) = findStorePath $storePath; + if (!$infoFile) { + print "0\n"; + next; # not an error + } + print "1\n"; + + my $deriver = ""; + my @references = (); + + open INFO, "<$infoFile" or die "cannot read info file $infoFile\n"; + while () { + chomp; + /^([\w-]+): (.*)$/ or die "bad info file"; + my $key = $1; + my $value = $2; + if ($key eq "Deriver") { $deriver = $value; } + elsif ($key eq "References") { @references = split ' ', $value; } + } + close INFO; + + print "$deriver\n"; + print scalar @references, "\n"; + print "$_\n" foreach @references; + } + + else { die "unknown command `$cmd'"; } } } diff --git a/scripts/download-using-manifests.pl.in b/scripts/download-using-manifests.pl.in index c0b822b912..e862a2d9fb 100644 --- a/scripts/download-using-manifests.pl.in +++ b/scripts/download-using-manifests.pl.in @@ -5,27 +5,18 @@ use readmanifest; use POSIX qw(strftime); use File::Temp qw(tempdir); +STDOUT->autoflush(1); + my $manifestDir = "@localstatedir@/nix/manifests"; my $logFile = "@localstatedir@/log/nix/downloads"; -# Create a temporary directory. -my $tmpDir = tempdir("nix-download.XXXXXX", CLEANUP => 1, TMPDIR => 1) - or die "cannot create a temporary directory"; - -chdir $tmpDir or die "cannot change to `$tmpDir': $!"; - -my $tmpNar = "$tmpDir/nar"; -my $tmpNar2 = "$tmpDir/nar2"; - - # Load all manifests. my %narFiles; my %localPaths; my %patches; for my $manifest (glob "$manifestDir/*.nixmanifest") { -# print STDERR "reading $manifest\n"; if (readManifest($manifest, \%narFiles, \%localPaths, \%patches) < 3) { print STDERR "you have an old-style manifest `$manifest'; please delete it\n"; exit 1; @@ -35,34 +26,40 @@ for my $manifest (glob "$manifestDir/*.nixmanifest") { # Parse the arguments. -if ($ARGV[0] eq "--query-paths") { - foreach my $storePath (keys %narFiles) { print "$storePath\n"; } - foreach my $storePath (keys %localPaths) { print "$storePath\n"; } - exit 0; -} +if ($ARGV[0] eq "--query") { -elsif ($ARGV[0] eq "--query-info") { - shift @ARGV; - foreach my $storePath (@ARGV) { - my $info; - if (defined $narFiles{$storePath}) { - $info = @{$narFiles{$storePath}}[0]; + while () { + my $cmd = $_; chomp $cmd; + + if ($cmd eq "have") { + my $storePath = ; chomp $storePath; + print STDOUT ((defined $narFiles{$storePath} or defined $localPaths{$storePath}) + ? "1\n" : "0\n"); } - elsif (defined $localPaths{$storePath}) { - $info = @{$localPaths{$storePath}}[0]; - } - else { - next; # not an error - } - print "$storePath\n"; - print "$info->{deriver}\n"; - my @references = split " ", $info->{references}; - my $count = scalar @references; - print "$count\n"; - foreach my $reference (@references) { - print "$reference\n"; + + elsif ($cmd eq "info") { + my $storePath = ; chomp $storePath; + my $info; + if (defined $narFiles{$storePath}) { + $info = @{$narFiles{$storePath}}[0]; + } + elsif (defined $localPaths{$storePath}) { + $info = @{$localPaths{$storePath}}[0]; + } + else { + print "0\n"; + next; # not an error + } + print "1\n"; + print "$info->{deriver}\n"; + my @references = split " ", $info->{references}; + print scalar @references, "\n"; + print "$_\n" foreach @references; } + + else { die "unknown command `$cmd'"; } } + exit 0; } @@ -75,6 +72,16 @@ die unless scalar @ARGV == 2; my $targetPath = $ARGV[1]; +# Create a temporary directory. +my $tmpDir = tempdir("nix-download.XXXXXX", CLEANUP => 1, TMPDIR => 1) + or die "cannot create a temporary directory"; + +chdir $tmpDir or die "cannot change to `$tmpDir': $!"; + +my $tmpNar = "$tmpDir/nar"; +my $tmpNar2 = "$tmpDir/nar2"; + + open LOGFILE, ">>$logFile" or die "cannot open log file $logFile"; my $date = strftime ("%F %H:%M:%S UTC", gmtime (time)); diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 1d624723f9..22f1c64ae3 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -184,11 +184,6 @@ private: /* Goals waiting for a build slot. */ WeakGoals wantingToBuild; - /* Goals waiting for info from substituters (using --query-info), - and the info they're (collectively) waiting for. */ - WeakGoals waitingForInfo; - map requestedInfo; - /* Child processes currently running. */ Children children; @@ -243,11 +238,6 @@ public: call is made to childTerminate(..., true). */ void waitForChildTermination(GoalPtr goal); - /* Put `goal' to sleep until the top-level loop has run `sub' to - get info about `storePath' (with --query-info). We combine - substituter invocations to reduce overhead. */ - void waitForInfo(GoalPtr goal, Path sub, Path storePath); - /* Wait for any goal to finish. Pretty indiscriminate way to wait for some resource that some other goal is holding. */ void waitForAnyGoal(GoalPtr goal); @@ -257,12 +247,6 @@ public: /* Wait for input to become available. */ void waitForInput(); - -private: - - /* Process the pending paths in requestedInfo and wake up the - goals in waitingForInfo. */ - void getInfo(); }; @@ -2042,12 +2026,12 @@ void DerivationGoal::initChild() } /* Close all other file descriptors. */ - int maxFD = 0; - maxFD = sysconf(_SC_OPEN_MAX); - for (int fd = 0; fd < maxFD; ++fd) - if (fd != STDIN_FILENO && fd != STDOUT_FILENO && fd != STDERR_FILENO - && (!inHook || (fd != 3 && fd != 4))) - close(fd); /* ignore result */ + set exceptions; + if (inHook) { + exceptions.insert(3); + exceptions.insert(4); + } + closeMostFDs(exceptions); } @@ -2118,10 +2102,8 @@ private: /* The current substituter. */ Path sub; - /* Path info returned by the substituter's --query-info operation. */ - bool infoOkay; - PathSet references; - Path deriver; + /* Path info returned by the substituter's query info operation. */ + SubstitutablePathInfo info; /* Pipe for the substitute's standard output/error. */ Pipe logPipe; @@ -2205,6 +2187,42 @@ void SubstitutionGoal::init() return; } + if (!worker.store.querySubstitutablePathInfo(storePath, info)) { + printMsg(lvlError, + format("path `%1%' is required, but there is no substituter that knows anything about it") + % storePath); + amDone(ecFailed); + return; + } + + /* To maintain the closure invariant, we first have to realise the + paths referenced by this one. */ + foreach (PathSet::iterator, i, info.references) + if (*i != storePath) /* ignore self-references */ + addWaitee(worker.makeSubstitutionGoal(*i)); + + if (waitees.empty()) /* to prevent hang (no wake-up event) */ + referencesValid(); + else + state = &SubstitutionGoal::referencesValid; +} + + +void SubstitutionGoal::referencesValid() +{ + trace("all references realised"); + + if (nrFailed > 0) { + printMsg(lvlError, + format("some references of path `%1%' could not be realised") % storePath); + amDone(ecFailed); + return; + } + + foreach (PathSet::iterator, i, info.references) + if (*i != storePath) /* ignore self-references */ + assert(worker.store.isValidPath(*i)); + subs = substituters; tryNext(); @@ -2228,51 +2246,6 @@ void SubstitutionGoal::tryNext() sub = subs.front(); subs.pop_front(); - infoOkay = false; - state = &SubstitutionGoal::gotInfo; - worker.waitForInfo(shared_from_this(), sub, storePath); -} - - -void SubstitutionGoal::gotInfo() -{ - trace("got info"); - - if (!infoOkay) { - tryNext(); - return; - } - - /* To maintain the closure invariant, we first have to realise the - paths referenced by this one. */ - for (PathSet::iterator i = references.begin(); - i != references.end(); ++i) - if (*i != storePath) /* ignore self-references */ - addWaitee(worker.makeSubstitutionGoal(*i)); - - if (waitees.empty()) /* to prevent hang (no wake-up event) */ - referencesValid(); - else - state = &SubstitutionGoal::referencesValid; -} - - -void SubstitutionGoal::referencesValid() -{ - trace("all references realised"); - - if (nrFailed > 0) { - printMsg(lvlError, - format("some references of path `%1%' could not be realised") % storePath); - amDone(ecFailed); - return; - } - - for (PathSet::iterator i = references.begin(); - i != references.end(); ++i) - if (*i != storePath) /* ignore self-references */ - assert(worker.store.isValidPath(*i)); - state = &SubstitutionGoal::tryToRun; worker.waitForBuildSlot(shared_from_this()); } @@ -2413,7 +2386,7 @@ void SubstitutionGoal::finished() Hash contentHash = hashPath(htSHA256, storePath); worker.store.registerValidPath(storePath, contentHash, - references, deriver); + info.references, info.deriver); outputLock->setDeletion(true); @@ -2608,14 +2581,6 @@ void Worker::waitForChildTermination(GoalPtr goal) } -void Worker::waitForInfo(GoalPtr goal, Path sub, Path storePath) -{ - debug("wait for info"); - requestedInfo[sub].insert(storePath); - waitingForInfo.insert(goal); -} - - void Worker::waitForAnyGoal(GoalPtr goal) { debug("wait for any goal"); @@ -2623,68 +2588,6 @@ void Worker::waitForAnyGoal(GoalPtr goal) } -void Worker::getInfo() -{ - for (map::iterator i = requestedInfo.begin(); - i != requestedInfo.end(); ++i) - { - Path sub = i->first; - PathSet paths = i->second; - - while (!paths.empty()) { - - /* Run the substituter for at most 100 paths at a time to - prevent command line overflows. */ - PathSet paths2; - while (!paths.empty() && paths2.size() < 100) { - paths2.insert(*paths.begin()); - paths.erase(paths.begin()); - } - - /* Ask the substituter for the references and deriver of - the paths. */ - debug(format("running `%1%' to get info about `%2%'") % sub % showPaths(paths2)); - Strings args; - args.push_back("--query-info"); - args.insert(args.end(), paths2.begin(), paths2.end()); - string res = runProgram(sub, false, args); - std::istringstream str(res); - - while (true) { - ValidPathInfo info = decodeValidPathInfo(str); - if (info.path == "") break; - - /* !!! inefficient */ - for (WeakGoals::iterator k = waitingForInfo.begin(); - k != waitingForInfo.end(); ++k) - { - GoalPtr goal = k->lock(); - if (goal) { - SubstitutionGoal * goal2 = dynamic_cast(goal.get()); - if (goal2->storePath == info.path) { - goal2->references = info.references; - goal2->deriver = info.deriver; - goal2->infoOkay = true; - wakeUp(goal); - } - } - } - } - } - } - - for (WeakGoals::iterator k = waitingForInfo.begin(); - k != waitingForInfo.end(); ++k) - { - GoalPtr goal = k->lock(); - if (goal) wakeUp(goal); - } - - requestedInfo.clear(); - waitingForInfo.clear(); // !!! have we done them all? -} - - void Worker::run(const Goals & _topGoals) { for (Goals::iterator i = _topGoals.begin(); @@ -2711,8 +2614,6 @@ void Worker::run(const Goals & _topGoals) if (topGoals.empty()) break; - getInfo(); - /* Wait for input. */ if (!children.empty()) waitForInput(); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index d886ba5586..6926c49370 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -83,6 +83,13 @@ LocalStore::~LocalStore() { try { flushDelayedUpdates(); + + foreach (RunningSubstituters::iterator, i, runningSubstituters) { + i->second.toBuf.reset(); + i->second.to.reset(); + i->second.pid.wait(true); + } + } catch (...) { ignoreException(); } @@ -367,8 +374,6 @@ ValidPathInfo LocalStore::queryPathInfo(const Path & path) std::map::iterator lookup = pathInfoCache.find(path); if (lookup != pathInfoCache.end()) return lookup->second; - //printMsg(lvlError, "queryPathInfo: " + path); - /* Read the info file. */ Path infoFile = infoFileFor(path); if (!pathExists(infoFile)) @@ -467,33 +472,105 @@ Path LocalStore::queryDeriver(const Path & path) } -PathSet LocalStore::querySubstitutablePaths() +void LocalStore::startSubstituter(const Path & substituter, RunningSubstituter & run) { - if (!substitutablePathsLoaded) { - for (Paths::iterator i = substituters.begin(); i != substituters.end(); ++i) { - debug(format("running `%1%' to find out substitutable paths") % *i); - Strings args; - args.push_back("--query-paths"); - Strings ss = tokenizeString(runProgram(*i, false, args), "\n"); - for (Strings::iterator j = ss.begin(); j != ss.end(); ++j) { - if (!isStorePath(*j)) - throw Error(format("`%1%' returned a bad substitutable path `%2%'") - % *i % *j); - substitutablePaths.insert(*j); - } + if (run.pid != -1) return; + + debug(format("starting substituter program `%1%'") % substituter); + + Pipe toPipe, fromPipe; + + toPipe.create(); + fromPipe.create(); + + run.pid = fork(); + + switch (run.pid) { + + case -1: + throw SysError("unable to fork"); + + case 0: /* child */ + try { + fromPipe.readSide.close(); + toPipe.writeSide.close(); + if (dup2(toPipe.readSide, STDIN_FILENO) == -1) + throw SysError("dupping stdin"); + if (dup2(fromPipe.writeSide, STDOUT_FILENO) == -1) + throw SysError("dupping stdout"); + closeMostFDs(set()); + execl(substituter.c_str(), substituter.c_str(), "--query", NULL); + throw SysError(format("executing `%1%'") % substituter); + } catch (std::exception & e) { + std::cerr << "error: " << e.what() << std::endl; } - substitutablePathsLoaded = true; + quickExit(1); } - return substitutablePaths; + /* Parent. */ + + toPipe.readSide.close(); + fromPipe.writeSide.close(); + + run.toBuf = boost::shared_ptr(new stdio_filebuf(toPipe.writeSide.borrow(), std::ios_base::out)); + run.to = boost::shared_ptr(new std::ostream(&*run.toBuf)); + + run.fromBuf = boost::shared_ptr(new stdio_filebuf(fromPipe.readSide.borrow(), std::ios_base::in)); + run.from = boost::shared_ptr(new std::istream(&*run.fromBuf)); } bool LocalStore::hasSubstitutes(const Path & path) { - if (!substitutablePathsLoaded) - querySubstitutablePaths(); - return substitutablePaths.find(path) != substitutablePaths.end(); + foreach (Paths::iterator, i, substituters) { + RunningSubstituter & run(runningSubstituters[*i]); + startSubstituter(*i, run); + + *run.to << "have\n" << path << "\n" << std::flush; + + string s; + + int res; + getline(*run.from, s); + if (!string2Int(s, res)) abort(); + + if (res) return true; + } + + return false; +} + + +bool LocalStore::querySubstitutablePathInfo(const Path & path, + SubstitutablePathInfo & info) +{ + foreach (Paths::iterator, i, substituters) { + RunningSubstituter & run(runningSubstituters[*i]); + startSubstituter(*i, run); + + *run.to << "info\n" << path << "\n" << std::flush; + + string s; + + int res; + getline(*run.from, s); + if (!string2Int(s, res)) abort(); + + if (res) { + getline(*run.from, info.deriver); + int nrRefs; + getline(*run.from, s); + if (!string2Int(s, nrRefs)) abort(); + while (nrRefs--) { + Path p; getline(*run.from, p); + info.references.insert(p); + } + info.downloadSize = 0; + return true; + } + } + + return false; } diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 1957b9f579..f096bdd855 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -3,6 +3,8 @@ #include +#include + #include "store-api.hh" #include "util.hh" @@ -34,11 +36,26 @@ struct OptimiseStats }; +typedef __gnu_cxx::stdio_filebuf stdio_filebuf; + + +struct RunningSubstituter +{ + Pid pid; + boost::shared_ptr toBuf, fromBuf; + boost::shared_ptr to; + boost::shared_ptr from; +}; + + class LocalStore : public StoreAPI { private: bool substitutablePathsLoaded; PathSet substitutablePaths; + + typedef std::map RunningSubstituters; + RunningSubstituters runningSubstituters; public: @@ -65,6 +82,9 @@ public: PathSet querySubstitutablePaths(); bool hasSubstitutes(const Path & path); + + bool querySubstitutablePathInfo(const Path & path, + SubstitutablePathInfo & info); Path addToStore(const Path & srcPath, bool fixed = false, bool recursive = false, string hashAlgo = "", @@ -147,6 +167,9 @@ private: void tryToDelete(const GCOptions & options, GCResults & results, const PathSet & livePaths, const PathSet & tempRootsClosed, PathSet & done, const Path & path); + + void startSubstituter(const Path & substituter, + RunningSubstituter & runningSubstituter); }; diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 4b192ec9a0..acf9346d4b 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -1,5 +1,6 @@ #include "misc.hh" #include "store-api.hh" +#include "local-store.hh" #include @@ -79,10 +80,13 @@ void queryMissing(const PathSet & targets, else { if (store->isValidPath(p)) continue; - if (store->hasSubstitutes(p)) + SubstitutablePathInfo info; + if (dynamic_cast(store.get())->querySubstitutablePathInfo(p, info)) { willSubstitute.insert(p); - // XXX call the substituters - // store->queryReferences(p, todo); + todo.insert(info.references.begin(), info.references.end()); + } + /* Not substitutable and not buildable; should we flag + this? */ } } } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 274196a2b6..b7e60043d6 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -213,6 +213,13 @@ bool RemoteStore::hasSubstitutes(const Path & path) } +bool RemoteStore::querySubstitutablePathInfo(const Path & path, + SubstitutablePathInfo & info) +{ + throw Error("not implemented"); +} + + Hash RemoteStore::queryPathHash(const Path & path) { writeInt(wopQueryPathHash, to); @@ -256,12 +263,6 @@ Path RemoteStore::queryDeriver(const Path & path) } -PathSet RemoteStore::querySubstitutablePaths() -{ - throw Error("not implemented"); -} - - Path RemoteStore::addToStore(const Path & _srcPath, bool fixed, bool recursive, string hashAlgo, PathFilter & filter) { diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index c40feeaa47..969c8f9b62 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -37,10 +37,11 @@ public: Path queryDeriver(const Path & path); - PathSet querySubstitutablePaths(); - bool hasSubstitutes(const Path & path); + bool querySubstitutablePathInfo(const Path & path, + SubstitutablePathInfo & info); + Path addToStore(const Path & srcPath, bool fixed = false, bool recursive = false, string hashAlgo = "", PathFilter & filter = defaultPathFilter); diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 1fb7326335..9b578eac7d 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -17,13 +17,6 @@ GCOptions::GCOptions() } -bool StoreAPI::hasSubstitutes(const Path & path) -{ - PathSet paths = querySubstitutablePaths(); - return paths.find(path) != paths.end(); -} - - bool isInStore(const Path & path) { return path[0] == '/' diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index b2a2dc7a53..9645237e04 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -88,6 +88,14 @@ struct GCResults }; +struct SubstitutablePathInfo +{ + Path deriver; + PathSet references; + unsigned long long downloadSize; /* 0 = unknown or inapplicable */ +}; + + class StoreAPI { public: @@ -117,12 +125,13 @@ public: no deriver has been set. */ virtual Path queryDeriver(const Path & path) = 0; - /* Query the set of substitutable paths. */ - virtual PathSet querySubstitutablePaths() = 0; + /* Query whether a path has substitutes. */ + virtual bool hasSubstitutes(const Path & path) = 0; - /* More efficient variant if we just want to know if a path has - substitutes. */ - virtual bool hasSubstitutes(const Path & path); + /* Query the references, deriver and download size of a + substitutable path. */ + virtual bool querySubstitutablePathInfo(const Path & path, + SubstitutablePathInfo & info) = 0; /* Copy the contents of a path to the store and register the validity the resulting path. The resulting path is returned. diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 4f6c367da8..c28f4e1bb7 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -579,7 +579,14 @@ AutoCloseFD::AutoCloseFD(int fd) AutoCloseFD::AutoCloseFD(const AutoCloseFD & fd) { - abort(); + /* Copying a AutoCloseFD isn't allowed (who should get to close + it?). But as a edge case, allow copying of closed + AutoCloseFDs. This is necessary due to tiresome reasons + involving copy constructor use on default object values in STL + containers (like when you do `map[value]' where value isn't in + the map yet). */ + this->fd = fd.fd; + if (this->fd != -1) abort(); } @@ -832,7 +839,7 @@ string runProgram(Path program, bool searchPath, const Strings & args) pipe.readSide.close(); if (dup2(pipe.writeSide, STDOUT_FILENO) == -1) - throw SysError("dupping from-hook write side"); + throw SysError("dupping stdout"); std::vector cargs; /* careful with c_str()! */ cargs.push_back(program.c_str()); @@ -868,6 +875,17 @@ string runProgram(Path program, bool searchPath, const Strings & args) } +void closeMostFDs(const set & exceptions) +{ + int maxFD = 0; + maxFD = sysconf(_SC_OPEN_MAX); + for (int fd = 0; fd < maxFD; ++fd) + if (fd != STDIN_FILENO && fd != STDOUT_FILENO && fd != STDERR_FILENO + && exceptions.find(fd) == exceptions.end()) + close(fd); /* ignore result */ +} + + void quickExit(int status) { #ifdef __CYGWIN__ diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 5d28a8308c..6c67e450b5 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -245,6 +245,10 @@ void killUser(uid_t uid); string runProgram(Path program, bool searchPath = false, const Strings & args = Strings()); +/* Close all file descriptors except stdin, stdout, stderr, and those + listed in the given set. Good practice in child processes. */ +void closeMostFDs(const set & exceptions); + /* Wrapper around _exit() on Unix and ExitProcess() on Windows. (On Cygwin, _exit() doesn't seem to do the right thing.) */ void quickExit(int status); diff --git a/tests/substituter.sh b/tests/substituter.sh index 96fb95b91f..b244b30e83 100755 --- a/tests/substituter.sh +++ b/tests/substituter.sh @@ -1,14 +1,25 @@ #! /bin/sh -e echo substituter args: $* >&2 -if test $1 = "--query-paths"; then - cat $TEST_ROOT/sub-paths -elif test $1 = "--query-info"; then - shift - for i in in $@; do - echo $i - echo "" # deriver - echo 0 # nr of refs +if test $1 = "--query"; then + while read cmd; do + echo FOO $cmd >&2 + if test "$cmd" = "have"; then + read path + if grep -q "$path" $TEST_ROOT/sub-paths; then + echo 1 + else + echo 0 + fi + elif test "$cmd" = "info"; then + read path + echo 1 + echo "" # deriver + echo 0 # nr of refs + else + echo "bad command $cmd" + exit 1 + fi done elif test $1 = "--substitute"; then mkdir $2 diff --git a/tests/substituter2.sh b/tests/substituter2.sh index 1bcf65a54d..401e7b7bef 100755 --- a/tests/substituter2.sh +++ b/tests/substituter2.sh @@ -1,14 +1,24 @@ #! /bin/sh -e echo substituter2 args: $* >&2 -if test $1 = "--query-paths"; then - cat $TEST_ROOT/sub-paths -elif test $1 = "--query-info"; then - shift - for i in in $@; do - echo $i - echo "" # deriver - echo 0 # nr of refs +if test $1 = "--query"; then + while read cmd; do + if test "$cmd" = "have"; then + read path + if grep -q "$path" $TEST_ROOT/sub-paths; then + echo 1 + else + echo 0 + fi + elif test "$cmd" = "info"; then + read path + echo 1 + echo "" # deriver + echo 0 # nr of refs + else + echo "bad command $cmd" + exit 1 + fi done elif test $1 = "--substitute"; then exit 1