aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLudovic Courtès <ludo@gnu.org>2020-12-01 15:01:40 +0100
committerLudovic Courtès <ludo@gnu.org>2020-12-08 22:30:08 +0100
commit79c6614f58a57b985daf8940766319e440311db0 (patch)
treeadbcd3c475f7820c31f4c73ed5f78beb566b8b40
parent2e308238ad09e9527f9b8c3ec4a90b6b05a93367 (diff)
downloadguix-79c6614f58a57b985daf8940766319e440311db0.tar
guix-79c6614f58a57b985daf8940766319e440311db0.tar.gz
daemon: Use 'Agent' to spawn 'guix substitute --query'.
* nix/libstore/local-store.hh (RunningSubstituter): Remove. (LocalStore)[runningSubstituter]: Change to unique_ptr<Agent>. [setSubstituterEnv, didSetSubstituterEnv]: Remove. [getLineFromSubstituter, getIntLineFromSubstituter]: Take an 'Agent'. * nix/libstore/local-store.cc (LocalStore::~LocalStore): Remove reference to 'runningSubstituter'. (LocalStore::setSubstituterEnv, LocalStore::startSubstituter): Remove. (LocalStore::getLineFromSubstituter): Adjust to 'run' being an 'Agent'. (LocalStore::querySubstitutablePaths): Spawn substituter agent if needed. Adjust to 'Agent' interface. (LocalStore::querySubstitutablePathInfos): Likewise. * nix/libstore/build.cc (SubstitutionGoal::tryToRun): Remove call to 'setSubstituterEnv' and add 'setenv' call for "_NIX_OPTIONS" instead. (SubstitutionGoal::finished): Remove 'readLine' call for 'dummy'. * guix/scripts/substitute.scm (%allow-unauthenticated-substitutes?): Remove second argument to 'make-parameter'. (process-query): Call 'warn-about-missing-authentication' when (%allow-unauthenticated-substitutes?) is #t. (guix-substitute): Wrap body in 'parameterize'. Set 'guix-warning-port' too. No longer exit when 'substitute-urls' returns the empty list. No longer print newline initially. * tests/substitute.scm (test-quit): Parameterize 'current-error-port' to account for the port changes in 'guix-substitute'.
-rwxr-xr-xguix/scripts/substitute.scm132
-rw-r--r--nix/libstore/build.cc6
-rw-r--r--nix/libstore/local-store.cc159
-rw-r--r--nix/libstore/local-store.hh22
-rw-r--r--tests/substitute.scm3
5 files changed, 110 insertions, 212 deletions
diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm
index feae2df9cb..5e392eaa8b 100755
--- a/guix/scripts/substitute.scm
+++ b/guix/scripts/substitute.scm
@@ -124,11 +124,7 @@ disabled!~%"))
;; purposes, and should be avoided otherwise.
(make-parameter
(and=> (getenv "GUIX_ALLOW_UNAUTHENTICATED_SUBSTITUTES")
- (cut string-ci=? <> "yes"))
- (lambda (value)
- (when value
- (warn-about-missing-authentication))
- value)))
+ (cut string-ci=? <> "yes"))))
(define %narinfo-ttl
;; Number of seconds during which cached narinfo lookups are considered
@@ -893,6 +889,9 @@ authorized substitutes."
(define (valid? obj)
(valid-narinfo? obj acl))
+ (when (%allow-unauthenticated-substitutes?)
+ (warn-about-missing-authentication))
+
(match (string-tokenize command)
(("have" paths ..1)
;; Return the subset of PATHS available in CACHE-URLS.
@@ -1139,68 +1138,67 @@ default value."
((= string->number number) (> number 0))
(_ #f)))
- (mkdir-p %narinfo-cache-directory)
- (maybe-remove-expired-cache-entries %narinfo-cache-directory
- cached-narinfo-files
- #:entry-expiration
- cached-narinfo-expiration-time
- #:cleanup-period
- %narinfo-expired-cache-entry-removal-delay)
- (check-acl-initialized)
-
- ;; Starting from commit 22144afa in Nix, we are allowed to bail out directly
- ;; when we know we cannot substitute, but we must emit a newline on stdout
- ;; when everything is alright.
- (when (null? (substitute-urls))
- (exit 0))
-
- ;; Say hello (see above.)
- (newline)
- (force-output (current-output-port))
-
- ;; Sanity-check SUBSTITUTE-URLS so we can provide a meaningful error message.
- (for-each validate-uri (substitute-urls))
-
- ;; Attempt to install the client's locale so that messages are suitably
- ;; translated. LC_CTYPE must be a UTF-8 locale; it's the case by default so
- ;; don't change it.
- (match (or (find-daemon-option "untrusted-locale")
- (find-daemon-option "locale"))
- (#f #f)
- (locale (false-if-exception (setlocale LC_MESSAGES locale))))
-
- (catch 'system-error
- (lambda ()
- (set-thread-name "guix substitute"))
- (const #t)) ;GNU/Hurd lacks 'prctl'
-
- (with-networking
- (with-error-handling ; for signature errors
- (match args
- (("--query")
- (let ((acl (current-acl)))
- (let loop ((command (read-line)))
- (or (eof-object? command)
- (begin
- (process-query command
- #:cache-urls (substitute-urls)
- #:acl acl)
- (loop (read-line)))))))
- (("--substitute" store-path destination)
- ;; Download STORE-PATH and add store it as a Nar in file DESTINATION.
- ;; Specify the number of columns of the terminal so the progress
- ;; report displays nicely.
- (parameterize ((current-terminal-columns (client-terminal-columns)))
- (process-substitution store-path destination
- #:cache-urls (substitute-urls)
- #:acl (current-acl)
- #:print-build-trace? print-build-trace?)))
- ((or ("-V") ("--version"))
- (show-version-and-exit "guix substitute"))
- (("--help")
- (show-help))
- (opts
- (leave (G_ "~a: unrecognized options~%") opts))))))
+ ;; The daemon's agent code opens file descriptor 4 for us and this is where
+ ;; stderr should go.
+ (parameterize ((current-error-port (match args
+ (("--query") (fdopen 4 "wl"))
+ (_ (current-error-port)))))
+ ;; Redirect diagnostics to file descriptor 4 as well.
+ (guix-warning-port (current-error-port))
+
+ (mkdir-p %narinfo-cache-directory)
+ (maybe-remove-expired-cache-entries %narinfo-cache-directory
+ cached-narinfo-files
+ #:entry-expiration
+ cached-narinfo-expiration-time
+ #:cleanup-period
+ %narinfo-expired-cache-entry-removal-delay)
+ (check-acl-initialized)
+
+ ;; Sanity-check SUBSTITUTE-URLS so we can provide a meaningful error
+ ;; message.
+ (for-each validate-uri (substitute-urls))
+
+ ;; Attempt to install the client's locale so that messages are suitably
+ ;; translated. LC_CTYPE must be a UTF-8 locale; it's the case by default
+ ;; so don't change it.
+ (match (or (find-daemon-option "untrusted-locale")
+ (find-daemon-option "locale"))
+ (#f #f)
+ (locale (false-if-exception (setlocale LC_MESSAGES locale))))
+
+ (catch 'system-error
+ (lambda ()
+ (set-thread-name "guix substitute"))
+ (const #t)) ;GNU/Hurd lacks 'prctl'
+
+ (with-networking
+ (with-error-handling ; for signature errors
+ (match args
+ (("--query")
+ (let ((acl (current-acl)))
+ (let loop ((command (read-line)))
+ (or (eof-object? command)
+ (begin
+ (process-query command
+ #:cache-urls (substitute-urls)
+ #:acl acl)
+ (loop (read-line)))))))
+ (("--substitute" store-path destination)
+ ;; Download STORE-PATH and store it as a Nar in file DESTINATION.
+ ;; Specify the number of columns of the terminal so the progress
+ ;; report displays nicely.
+ (parameterize ((current-terminal-columns (client-terminal-columns)))
+ (process-substitution store-path destination
+ #:cache-urls (substitute-urls)
+ #:acl (current-acl)
+ #:print-build-trace? print-build-trace?)))
+ ((or ("-V") ("--version"))
+ (show-version-and-exit "guix substitute"))
+ (("--help")
+ (show-help))
+ (opts
+ (leave (G_ "~a: unrecognized options~%") opts)))))))
;;; Local Variables:
;;; eval: (put 'with-timeout 'scheme-indent-function 1)
diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index 8413819114..7e9ab3f39c 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -2986,8 +2986,6 @@ void SubstitutionGoal::tryToRun()
if (pathExists(destPath))
deletePath(destPath);
- worker.store.setSubstituterEnv();
-
/* Fill in the arguments. */
Strings args;
args.push_back("guix");
@@ -2999,6 +2997,9 @@ void SubstitutionGoal::tryToRun()
/* Fork the substitute program. */
pid = startProcess([&]() {
+ /* Communicate substitute-urls & co. to 'guix substitute'. */
+ setenv("_NIX_OPTIONS", settings.pack().c_str(), 1);
+
commonChildInit(logPipe);
if (dup2(outPipe.writeSide, STDOUT_FILENO) == -1)
@@ -3041,7 +3042,6 @@ void SubstitutionGoal::finished()
logPipe.readSide.close();
/* Get the hash info from stdout. */
- string dummy = readLine(outPipe.readSide);
string expectedHashStr = statusOk(status) ? readLine(outPipe.readSide) : "";
outPipe.readSide.close();
diff --git a/nix/libstore/local-store.cc b/nix/libstore/local-store.cc
index 8c479002ec..4219573a56 100644
--- a/nix/libstore/local-store.cc
+++ b/nix/libstore/local-store.cc
@@ -57,7 +57,6 @@ void checkStoreNotSymlink()
LocalStore::LocalStore(bool reserveSpace)
- : didSetSubstituterEnv(false)
{
schemaPath = settings.nixDBPath + "/schema";
@@ -183,21 +182,6 @@ LocalStore::LocalStore(bool reserveSpace)
LocalStore::~LocalStore()
{
try {
- if (runningSubstituter) {
- RunningSubstituter &i = *runningSubstituter;
- if (!i.disabled) {
- i.to.close();
- i.from.close();
- i.error.close();
- if (i.pid != -1)
- i.pid.wait(true);
- }
- }
- } catch (...) {
- ignoreException();
- }
-
- try {
if (fdTempRoots != -1) {
fdTempRoots.close();
unlink(fnTempRoots.c_str());
@@ -796,96 +780,31 @@ Path LocalStore::queryPathFromHashPart(const string & hashPart)
});
}
-
-void LocalStore::setSubstituterEnv()
-{
- if (didSetSubstituterEnv) return;
-
- /* Pass configuration options (including those overridden with
- --option) to substituters. */
- setenv("_NIX_OPTIONS", settings.pack().c_str(), 1);
-
- didSetSubstituterEnv = true;
-}
-
-
-void LocalStore::startSubstituter(RunningSubstituter & run)
-{
- if (run.disabled || run.pid != -1) return;
-
- debug(format("starting substituter program `%1% substitute'")
- % settings.guixProgram);
-
- Pipe toPipe, fromPipe, errorPipe;
-
- toPipe.create();
- fromPipe.create();
- errorPipe.create();
-
- setSubstituterEnv();
-
- run.pid = startProcess([&]() {
- if (dup2(toPipe.readSide, STDIN_FILENO) == -1)
- throw SysError("dupping stdin");
- if (dup2(fromPipe.writeSide, STDOUT_FILENO) == -1)
- throw SysError("dupping stdout");
- if (dup2(errorPipe.writeSide, STDERR_FILENO) == -1)
- throw SysError("dupping stderr");
- execl(settings.guixProgram.c_str(), "guix", "substitute", "--query", NULL);
- throw SysError(format("executing `%1%'") % settings.guixProgram);
- });
-
- 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;
- }
-}
-
-
/* Read a line from the substituter's stdout, while also processing
its stderr. */
-string LocalStore::getLineFromSubstituter(RunningSubstituter & run)
+string LocalStore::getLineFromSubstituter(Agent & run)
{
string res, err;
- /* We might have stdout data left over from the last time. */
- if (run.fromBuf.hasData()) goto haveData;
-
while (1) {
checkInterrupt();
fd_set fds;
FD_ZERO(&fds);
- FD_SET(run.from, &fds);
- FD_SET(run.error, &fds);
+ FD_SET(run.fromAgent.readSide, &fds);
+ FD_SET(run.builderOut.readSide, &fds);
/* Wait for data to appear on the substituter's stdout or
stderr. */
- if (select(run.from > run.error ? run.from + 1 : run.error + 1, &fds, 0, 0, 0) == -1) {
+ if (select(std::max(run.fromAgent.readSide, run.builderOut.readSide) + 1, &fds, 0, 0, 0) == -1) {
if (errno == EINTR) continue;
throw SysError("waiting for input from the substituter");
}
/* Completely drain stderr before dealing with stdout. */
- if (FD_ISSET(run.error, &fds)) {
+ if (FD_ISSET(run.builderOut.readSide, &fds)) {
char buf[4096];
- ssize_t n = read(run.error, (unsigned char *) buf, sizeof(buf));
+ ssize_t n = read(run.builderOut.readSide, (unsigned char *) buf, sizeof(buf));
if (n == -1) {
if (errno == EINTR) continue;
throw SysError("reading from substituter's stderr");
@@ -903,23 +822,20 @@ string LocalStore::getLineFromSubstituter(RunningSubstituter & run)
}
/* Read from stdout until we get a newline or the buffer is empty. */
- else if (run.fromBuf.hasData() || FD_ISSET(run.from, &fds)) {
- haveData:
- do {
- unsigned char c;
- run.fromBuf(&c, 1);
- if (c == '\n') {
- if (!err.empty()) printMsg(lvlError, "substitute: " + err);
- return res;
- }
- res += c;
- } while (run.fromBuf.hasData());
+ else if (FD_ISSET(run.fromAgent.readSide, &fds)) {
+ unsigned char c;
+ readFull(run.fromAgent.readSide, (unsigned char *) &c, 1);
+ if (c == '\n') {
+ if (!err.empty()) printMsg(lvlError, "substitute: " + err);
+ return res;
+ }
+ res += c;
}
}
}
-template<class T> T LocalStore::getIntLineFromSubstituter(RunningSubstituter & run)
+template<class T> T LocalStore::getIntLineFromSubstituter(Agent & run)
{
string s = getLineFromSubstituter(run);
T res;
@@ -935,27 +851,26 @@ PathSet LocalStore::querySubstitutablePaths(const PathSet & paths)
if (!settings.useSubstitutes || paths.empty()) return res;
if (!runningSubstituter) {
- std::unique_ptr<RunningSubstituter>fresh(new RunningSubstituter);
+ const Strings args = { "substitute", "--query" };
+ const std::map<string, string> env = { { "_NIX_OPTIONS", settings.pack() } };
+ std::unique_ptr<Agent> fresh(new Agent(settings.guixProgram, args, env));
runningSubstituter.swap(fresh);
}
- RunningSubstituter & run = *runningSubstituter;
- startSubstituter(run);
-
- if (!run.disabled) {
- string s = "have ";
- foreach (PathSet::const_iterator, j, paths)
- if (res.find(*j) == res.end()) { s += *j; s += " "; }
- writeLine(run.to, s);
- while (true) {
- /* FIXME: we only read stderr when an error occurs, so
- substituters should only write (short) messages to
- stderr when they fail. I.e. they shouldn't write debug
- output. */
- Path path = getLineFromSubstituter(run);
- if (path == "") break;
- res.insert(path);
- }
+ Agent & run = *runningSubstituter;
+
+ string s = "have ";
+ foreach (PathSet::const_iterator, j, paths)
+ if (res.find(*j) == res.end()) { s += *j; s += " "; }
+ writeLine(run.toAgent.writeSide, s);
+ while (true) {
+ /* FIXME: we only read stderr when an error occurs, so
+ substituters should only write (short) messages to
+ stderr when they fail. I.e. they shouldn't write debug
+ output. */
+ Path path = getLineFromSubstituter(run);
+ if (path == "") break;
+ res.insert(path);
}
return res;
@@ -967,18 +882,18 @@ void LocalStore::querySubstitutablePathInfos(PathSet & paths, SubstitutablePathI
if (!settings.useSubstitutes) return;
if (!runningSubstituter) {
- std::unique_ptr<RunningSubstituter>fresh(new RunningSubstituter);
+ const Strings args = { "substitute", "--query" };
+ const std::map<string, string> env = { { "_NIX_OPTIONS", settings.pack() } };
+ std::unique_ptr<Agent> fresh(new Agent(settings.guixProgram, args, env));
runningSubstituter.swap(fresh);
}
- RunningSubstituter & run = *runningSubstituter;
- startSubstituter(run);
- if (run.disabled) return;
+ Agent & run = *runningSubstituter;
string s = "info ";
foreach (PathSet::const_iterator, i, paths)
if (infos.find(*i) == infos.end()) { s += *i; s += " "; }
- writeLine(run.to, s);
+ writeLine(run.toAgent.writeSide, s);
while (true) {
Path path = getLineFromSubstituter(run);
diff --git a/nix/libstore/local-store.hh b/nix/libstore/local-store.hh
index 2e48cf03e6..57d15bac7e 100644
--- a/nix/libstore/local-store.hh
+++ b/nix/libstore/local-store.hh
@@ -38,21 +38,11 @@ struct OptimiseStats
};
-struct RunningSubstituter
-{
- Pid pid;
- AutoCloseFD to, from, error;
- FdSource fromBuf;
- bool disabled;
- RunningSubstituter() : disabled(false) { };
-};
-
-
class LocalStore : public StoreAPI
{
private:
/* The currently running substituter or empty. */
- std::unique_ptr<RunningSubstituter> runningSubstituter;
+ std::unique_ptr<Agent> runningSubstituter;
Path linksDir;
@@ -178,8 +168,6 @@ public:
void markContentsGood(const Path & path);
- void setSubstituterEnv();
-
void createUser(const std::string & userName, uid_t userId);
private:
@@ -213,8 +201,6 @@ private:
/* Cache for pathContentsGood(). */
std::map<Path, bool> pathContentsGoodCache;
- bool didSetSubstituterEnv;
-
/* The file to which we write our temporary roots. */
Path fnTempRoots;
AutoCloseFD fdTempRoots;
@@ -262,11 +248,9 @@ private:
void removeUnusedLinks(const GCState & state);
- void startSubstituter(RunningSubstituter & runningSubstituter);
-
- string getLineFromSubstituter(RunningSubstituter & run);
+ string getLineFromSubstituter(Agent & run);
- template<class T> T getIntLineFromSubstituter(RunningSubstituter & run);
+ template<class T> T getIntLineFromSubstituter(Agent & run);
Path createTempDirInStore();
diff --git a/tests/substitute.scm b/tests/substitute.scm
index 6560612c40..bd5b6305b0 100644
--- a/tests/substitute.scm
+++ b/tests/substitute.scm
@@ -47,7 +47,8 @@ it writes to GUIX-WARNING-PORT a messages that matches ERROR-RX."
(test-equal name
'(1 #t)
(let ((error-output (open-output-string)))
- (parameterize ((guix-warning-port error-output))
+ (parameterize ((current-error-port error-output)
+ (guix-warning-port error-output))
(catch 'quit
(lambda ()
exp