From 55b4715fd4c03e46501f123c5c9bc6072edf12a4 Mon Sep 17 00:00:00 2001 From: Ludovic Courtès Date: Tue, 6 Jun 2017 14:01:12 +0200 Subject: profiles: Represent propagated inputs as manifest entries. * guix/profiles.scm (package->manifest-entry): Turn DEPS into a list of manifest entries. (manifest->gexp)[entry->gexp]: Call 'entry->gexp' on DEPS. Bump version to 3. (sexp->manifest)[infer-dependency]: New procedure. Use it for versions 1 and 2. Parse version 3. (manifest-inputs)[entry->gexp]: New procedure. Adjust to 'dependencies' being a list of . * tests/profiles.scm ("packages->manifest, propagated inputs") ("read-manifest"): New fields. --- tests/profiles.scm | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) (limited to 'tests') diff --git a/tests/profiles.scm b/tests/profiles.scm index 093422792f..e8b1bb832c 100644 --- a/tests/profiles.scm +++ b/tests/profiles.scm @@ -288,6 +288,42 @@ (define (find-input name) (manifest-entry-search-paths (package->manifest-entry mpl))))) +(test-equal "packages->manifest, propagated inputs" + (map (match-lambda + ((label package) + (list (package-name package) (package-version package) + package))) + (package-propagated-inputs packages:guile-2.2)) + (map (lambda (entry) + (list (manifest-entry-name entry) + (manifest-entry-version entry) + (manifest-entry-item entry))) + (manifest-entry-dependencies + (package->manifest-entry packages:guile-2.2)))) + +(test-assertm "read-manifest" + (mlet* %store-monad ((manifest -> (packages->manifest + (list (package + (inherit %bootstrap-guile) + (native-search-paths + (package-native-search-paths + packages:guile-2.0)))))) + (drv (profile-derivation manifest + #:hooks '() + #:locales? #f)) + (out -> (derivation->output-path drv))) + (define (entry->sexp entry) + (list (manifest-entry-name entry) + (manifest-entry-version entry) + (manifest-entry-search-paths entry) + (manifest-entry-dependencies entry))) + + (mbegin %store-monad + (built-derivations (list drv)) + (let ((manifest2 (profile-manifest out))) + (return (equal? (map entry->sexp (manifest-entries manifest)) + (map entry->sexp (manifest-entries manifest2)))))))) + (test-assertm "etc/profile" ;; Make sure we get an 'etc/profile' file that at least defines $PATH. (mlet* %store-monad -- cgit v1.2.3 From b3a00885c0a420692ccc4c227252bb44619399d5 Mon Sep 17 00:00:00 2001 From: Ludovic Courtès Date: Tue, 6 Jun 2017 15:29:50 +0200 Subject: profiles: Manifest entries keep a reference to their parent entry. * guix/profiles.scm ()[parent]: New field. (package->manifest-entry): Add #:parent parameter. Fill out the 'parent' field of ; pass #:parent in recursive calls. * guix/profiles.scm (sexp->manifest)[sexp->manifest-entry]: New procedure. Use it for version 3. * tests/profiles.scm ("manifest-entry-parent"): New procedure. ("read-manifest")[entry->sexp]: Add 'manifest-entry-parent' to the result. --- guix/profiles.scm | 120 ++++++++++++++++++++++++++++++++--------------------- tests/profiles.scm | 12 +++++- 2 files changed, 83 insertions(+), 49 deletions(-) (limited to 'tests') diff --git a/guix/profiles.scm b/guix/profiles.scm index a66add3e07..c85d7ef5cb 100644 --- a/guix/profiles.scm +++ b/guix/profiles.scm @@ -68,6 +68,7 @@ (define-module (guix profiles) manifest-entry-item manifest-entry-dependencies manifest-entry-search-paths + manifest-entry-parent manifest-pattern manifest-pattern? @@ -157,7 +158,9 @@ (define-record-type* manifest-entry (dependencies manifest-entry-dependencies ; * (default '())) (search-paths manifest-entry-search-paths ; search-path-specification* - (default '()))) + (default '())) + (parent manifest-entry-parent ; promise (#f | ) + (default (delay #f)))) (define-record-type* manifest-pattern make-manifest-pattern @@ -175,21 +178,28 @@ (define (profile-manifest profile) (call-with-input-file file read-manifest) (manifest '())))) -(define* (package->manifest-entry package #:optional (output "out")) +(define* (package->manifest-entry package #:optional (output "out") + #:key (parent (delay #f))) "Return a manifest entry for the OUTPUT of package PACKAGE." - (let ((deps (map (match-lambda - ((label package) - (package->manifest-entry package)) - ((label package output) - (package->manifest-entry package output))) - (package-propagated-inputs package)))) - (manifest-entry - (name (package-name package)) - (version (package-version package)) - (output output) - (item package) - (dependencies (delete-duplicates deps)) - (search-paths (package-transitive-native-search-paths package))))) + ;; For each dependency, keep a promise pointing to its "parent" entry. + (letrec* ((deps (map (match-lambda + ((label package) + (package->manifest-entry package + #:parent (delay entry))) + ((label package output) + (package->manifest-entry package output + #:parent (delay entry)))) + (package-propagated-inputs package))) + (entry (manifest-entry + (name (package-name package)) + (version (package-version package)) + (output output) + (item package) + (dependencies (delete-duplicates deps)) + (search-paths + (package-transitive-native-search-paths package)) + (parent parent)))) + entry)) (define (packages->manifest packages) "Return a list of manifest entries, one for each item listed in PACKAGES. @@ -254,7 +264,7 @@ (define (infer-search-paths name version) (package-native-search-paths package) '()))) - (define (infer-dependency item) + (define (infer-dependency item parent) ;; Return a for ITEM. (let-values (((name version) (package-name->name+version @@ -262,7 +272,28 @@ (define (infer-dependency item) (manifest-entry (name name) (version version) - (item item)))) + (item item) + (parent parent)))) + + (define* (sexp->manifest-entry sexp #:optional (parent (delay #f))) + (match sexp + ((name version output path + ('propagated-inputs deps) + ('search-paths search-paths) + extra-stuff ...) + ;; For each of DEPS, keep a promise pointing to ENTRY. + (letrec* ((deps* (map (cut sexp->manifest-entry <> (delay entry)) + deps)) + (entry (manifest-entry + (name name) + (version version) + (output output) + (item path) + (dependencies deps*) + (search-paths (map sexp->search-path-specification + search-paths)) + (parent parent)))) + entry)))) (match sexp (('manifest ('version 0) @@ -291,13 +322,17 @@ (define (infer-dependency item) directories) ((directories ...) directories)))) - (manifest-entry - (name name) - (version version) - (output output) - (item path) - (dependencies (map infer-dependency deps)) - (search-paths (infer-search-paths name version))))) + (letrec* ((deps* (map (cute infer-dependency <> (delay entry)) + deps)) + (entry (manifest-entry + (name name) + (version version) + (output output) + (item path) + (dependencies deps*) + (search-paths + (infer-search-paths name version))))) + entry))) name version output path deps))) ;; Version 2 adds search paths and is slightly more verbose. @@ -309,35 +344,24 @@ (define (infer-dependency item) ...))) (manifest (map (lambda (name version output path deps search-paths) - (manifest-entry - (name name) - (version version) - (output output) - (item path) - (dependencies (map infer-dependency deps)) - (search-paths (map sexp->search-path-specification - search-paths)))) + (letrec* ((deps* (map (cute infer-dependency <> (delay entry)) + deps)) + (entry (manifest-entry + (name name) + (version version) + (output output) + (item path) + (dependencies deps*) + (search-paths + (map sexp->search-path-specification + search-paths))))) + entry)) name version output path deps search-paths))) ;; Version 3 represents DEPS as full-blown manifest entries. (('manifest ('version 3 minor-version ...) ('packages (entries ...))) - (letrec ((sexp->manifest-entry - (match-lambda - ((name version output path - ('propagated-inputs deps) - ('search-paths search-paths) - extra-stuff ...) - (manifest-entry - (name name) - (version version) - (output output) - (item path) - (dependencies (map sexp->manifest-entry deps)) - (search-paths (map sexp->search-path-specification - search-paths))))))) - - (manifest (map sexp->manifest-entry entries)))) + (manifest (map sexp->manifest-entry entries))) (_ (raise (condition (&message (message "unsupported manifest format"))))))) diff --git a/tests/profiles.scm b/tests/profiles.scm index e8b1bb832c..94759c05ef 100644 --- a/tests/profiles.scm +++ b/tests/profiles.scm @@ -301,6 +301,15 @@ (define (find-input name) (manifest-entry-dependencies (package->manifest-entry packages:guile-2.2)))) +(test-assert "manifest-entry-parent" + (let ((entry (package->manifest-entry packages:guile-2.2))) + (match (manifest-entry-dependencies entry) + ((dependencies ..1) + (and (every (lambda (parent) + (eq? entry (force parent))) + (map manifest-entry-parent dependencies)) + (not (force (manifest-entry-parent entry)))))))) + (test-assertm "read-manifest" (mlet* %store-monad ((manifest -> (packages->manifest (list (package @@ -316,7 +325,8 @@ (define (entry->sexp entry) (list (manifest-entry-name entry) (manifest-entry-version entry) (manifest-entry-search-paths entry) - (manifest-entry-dependencies entry))) + (manifest-entry-dependencies entry) + (force (manifest-entry-parent entry)))) (mbegin %store-monad (built-derivations (list drv)) -- cgit v1.2.3 From a654dc4bcf7c8e205bdefa1a1d5f23444dd22778 Mon Sep 17 00:00:00 2001 From: Ludovic Courtès Date: Wed, 7 Jun 2017 09:51:55 +0200 Subject: profiles: Catch and report collisions in the profile. * guix/profiles.scm (&profile-collision-error): New error condition. (manifest-transitive-entries, manifest-entry-lookup, lower-manifest-entry) (check-for-collisions): New procedures. (profile-derivation): Add call to 'check-for-collisions'. * guix/ui.scm (call-with-error-handling): Handle '&profile-collision-error'. * tests/profiles.scm ("collision", "collision of propagated inputs") ("no collision"): New tests. --- guix/profiles.scm | 113 ++++++++++++++++++++++++++++++++++++++++++++++++----- guix/ui.scm | 27 +++++++++++++ tests/profiles.scm | 66 +++++++++++++++++++++++++++++++ 3 files changed, 197 insertions(+), 9 deletions(-) (limited to 'tests') diff --git a/guix/profiles.scm b/guix/profiles.scm index c85d7ef5cb..9858ec7b35 100644 --- a/guix/profiles.scm +++ b/guix/profiles.scm @@ -35,6 +35,8 @@ (define-module (guix profiles) #:use-module (guix gexp) #:use-module (guix monads) #:use-module (guix store) + #:use-module (guix sets) + #:use-module (ice-9 vlist) #:use-module (ice-9 match) #:use-module (ice-9 regex) #:use-module (ice-9 ftw) @@ -51,6 +53,10 @@ (define-module (guix profiles) profile-error-profile &profile-not-found-error profile-not-found-error? + &profile-collistion-error + profile-collision-error? + profile-collision-error-entry + profile-collision-error-conflict &missing-generation-error missing-generation-error? missing-generation-error-generation @@ -58,6 +64,7 @@ (define-module (guix profiles) manifest make-manifest manifest? manifest-entries + manifest-transitive-entries ; FIXME: eventually make it internal manifest-entry @@ -130,6 +137,11 @@ (define-condition-type &profile-error &error (define-condition-type &profile-not-found-error &profile-error profile-not-found-error?) +(define-condition-type &profile-collision-error &error + profile-collision-error? + (entry profile-collision-error-entry) ; + (conflict profile-collision-error-conflict)) ; + (define-condition-type &missing-generation-error &profile-error missing-generation-error? (generation missing-generation-error-generation)) @@ -147,6 +159,23 @@ (define-record-type ;; Convenient alias, to avoid name clashes. (define make-manifest manifest) +(define (manifest-transitive-entries manifest) + "Return the entries of MANIFEST along with their propagated inputs, +recursively." + (let loop ((entries (manifest-entries manifest)) + (result '()) + (visited (set))) ;compare with 'equal?' + (match entries + (() + (reverse result)) + ((head . tail) + (if (set-contains? visited head) + (loop tail result visited) + (loop (append (manifest-entry-dependencies head) + tail) + (cons head result) + (set-insert head visited))))))) + (define-record-type* manifest-entry make-manifest-entry manifest-entry? @@ -178,6 +207,70 @@ (define (profile-manifest profile) (call-with-input-file file read-manifest) (manifest '())))) +(define (manifest-entry-lookup manifest) + "Return a lookup procedure for the entries of MANIFEST. The lookup +procedure takes two arguments: the entry name and output." + (define mapping + (let loop ((entries (manifest-entries manifest)) + (mapping vlist-null)) + (fold (lambda (entry result) + (vhash-cons (cons (manifest-entry-name entry) + (manifest-entry-output entry)) + entry + (loop (manifest-entry-dependencies entry) + result))) + mapping + entries))) + + (lambda (name output) + (match (vhash-assoc (cons name output) mapping) + ((_ . entry) entry) + (#f #f)))) + +(define* (lower-manifest-entry entry system #:key target) + "Lower ENTRY for SYSTEM and TARGET such that its 'item' field is a store +file name." + (let ((item (manifest-entry-item entry))) + (if (string? item) + (with-monad %store-monad + (return entry)) + (mlet %store-monad ((drv (lower-object item system + #:target target)) + (output -> (manifest-entry-output entry))) + (return (manifest-entry + (inherit entry) + (item (derivation->output-path drv output)))))))) + +(define* (check-for-collisions manifest system #:key target) + "Check whether the entries of MANIFEST conflict with one another; raise a +'&profile-collision-error' when a conflict is encountered." + (define lookup + (manifest-entry-lookup manifest)) + + (with-monad %store-monad + (foldm %store-monad + (lambda (entry result) + (match (lookup (manifest-entry-name entry) + (manifest-entry-output entry)) + ((? manifest-entry? second) ;potential conflict + (mlet %store-monad ((first (lower-manifest-entry entry system + #:target + target)) + (second (lower-manifest-entry second system + #:target + target))) + (if (string=? (manifest-entry-item first) + (manifest-entry-item second)) + (return result) + (raise (condition + (&profile-collision-error + (entry first) + (conflict second))))))) + (#f ;no conflict + (return result)))) + #t + (manifest-transitive-entries manifest)))) + (define* (package->manifest-entry package #:optional (output "out") #:key (parent (delay #f))) "Return a manifest entry for the OUTPUT of package PACKAGE." @@ -1116,15 +1209,17 @@ (define* (profile-derivation manifest When TARGET is true, it must be a GNU triplet, and the packages in MANIFEST are cross-built for TARGET." - (mlet %store-monad ((system (if system - (return system) - (current-system))) - (extras (if (null? (manifest-entries manifest)) - (return '()) - (sequence %store-monad - (map (lambda (hook) - (hook manifest)) - hooks))))) + (mlet* %store-monad ((system (if system + (return system) + (current-system))) + (ok? (check-for-collisions manifest system + #:target target)) + (extras (if (null? (manifest-entries manifest)) + (return '()) + (sequence %store-monad + (map (lambda (hook) + (hook manifest)) + hooks))))) (define inputs (append (filter-map (lambda (drv) (and (derivation? drv) diff --git a/guix/ui.scm b/guix/ui.scm index 889c9d0228..c141880316 100644 --- a/guix/ui.scm +++ b/guix/ui.scm @@ -476,6 +476,33 @@ (define (port-filename* port) (leave (G_ "generation ~a of profile '~a' does not exist~%") (missing-generation-error-generation c) (profile-error-profile c))) + ((profile-collision-error? c) + (let ((entry (profile-collision-error-entry c)) + (conflict (profile-collision-error-conflict c))) + (define (report-parent-entries entry) + (let ((parent (force (manifest-entry-parent entry)))) + (when (manifest-entry? parent) + (report-error (G_ " ... propagated from ~a@~a~%") + (manifest-entry-name parent) + (manifest-entry-version parent)) + (report-parent-entries parent)))) + + (report-error (G_ "profile contains conflicting entries for ~a:~a~%") + (manifest-entry-name entry) + (manifest-entry-output entry)) + (report-error (G_ " first entry: ~a@~a:~a ~a~%") + (manifest-entry-name entry) + (manifest-entry-version entry) + (manifest-entry-output entry) + (manifest-entry-item entry)) + (report-parent-entries entry) + (report-error (G_ " second entry: ~a@~a:~a ~a~%") + (manifest-entry-name conflict) + (manifest-entry-version conflict) + (manifest-entry-output conflict) + (manifest-entry-item conflict)) + (report-parent-entries conflict) + (exit 1))) ((nar-error? c) (let ((file (nar-error-file c)) (port (nar-error-port c))) diff --git a/tests/profiles.scm b/tests/profiles.scm index 94759c05ef..f731807e8c 100644 --- a/tests/profiles.scm +++ b/tests/profiles.scm @@ -35,6 +35,7 @@ (define-module (test-profiles) #:use-module (rnrs io ports) #:use-module (srfi srfi-1) #:use-module (srfi srfi-11) + #:use-module (srfi srfi-34) #:use-module (srfi srfi-64)) ;; Test the (guix profiles) module. @@ -334,6 +335,71 @@ (define (entry->sexp entry) (return (equal? (map entry->sexp (manifest-entries manifest)) (map entry->sexp (manifest-entries manifest2)))))))) +(test-equal "collision" + '(("guile-bootstrap" "2.0") ("guile-bootstrap" "42")) + (guard (c ((profile-collision-error? c) + (let ((entry1 (profile-collision-error-entry c)) + (entry2 (profile-collision-error-conflict c))) + (list (list (manifest-entry-name entry1) + (manifest-entry-version entry1)) + (list (manifest-entry-name entry2) + (manifest-entry-version entry2)))))) + (run-with-store %store + (mlet* %store-monad ((p0 -> (package + (inherit %bootstrap-guile) + (version "42"))) + (p1 -> (dummy-package "p1" + (propagated-inputs `(("p0" ,p0))))) + (manifest -> (packages->manifest + (list %bootstrap-guile p1))) + (drv (profile-derivation manifest + #:hooks '() + #:locales? #f))) + (return #f))))) + +(test-equal "collision of propagated inputs" + '(("guile-bootstrap" "2.0") ("guile-bootstrap" "42")) + (guard (c ((profile-collision-error? c) + (let ((entry1 (profile-collision-error-entry c)) + (entry2 (profile-collision-error-conflict c))) + (list (list (manifest-entry-name entry1) + (manifest-entry-version entry1)) + (list (manifest-entry-name entry2) + (manifest-entry-version entry2)))))) + (run-with-store %store + (mlet* %store-monad ((p0 -> (package + (inherit %bootstrap-guile) + (version "42"))) + (p1 -> (dummy-package "p1" + (propagated-inputs + `(("guile" ,%bootstrap-guile))))) + (p2 -> (dummy-package "p2" + (propagated-inputs + `(("guile" ,p0))))) + (manifest -> (packages->manifest (list p1 p2))) + (drv (profile-derivation manifest + #:hooks '() + #:locales? #f))) + (return #f))))) + +(test-assertm "no collision" + ;; Here we have an entry that is "lowered" (its 'item' field is a store file + ;; name) and another entry (its 'item' field is a package) that is + ;; equivalent. + (mlet* %store-monad ((p -> (dummy-package "p" + (propagated-inputs + `(("guile" ,%bootstrap-guile))))) + (guile (package->derivation %bootstrap-guile)) + (entry -> (manifest-entry + (inherit (package->manifest-entry + %bootstrap-guile)) + (item (derivation->output-path guile)))) + (manifest -> (manifest + (list entry + (package->manifest-entry p)))) + (drv (profile-derivation manifest))) + (return (->bool drv)))) + (test-assertm "etc/profile" ;; Make sure we get an 'etc/profile' file that at least defines $PATH. (mlet* %store-monad -- cgit v1.2.3 From afd06f605bf88a796acefc7ed598b43879346a6b Mon Sep 17 00:00:00 2001 From: Ludovic Courtès Date: Wed, 21 Jun 2017 16:50:59 +0200 Subject: environment: Disable profile collision checks. Reported by Efraim Flashner. This is a followup to a654dc4bcf7c8e205bdefa1a1d5f23444dd22778. * guix/profiles.scm (profile-derivation): Add #:allow-collisions? and honor it. * guix/scripts/environment.scm (inputs->profile-derivation): Pass #:allow-collisions? #f to 'profile-derivation'. * tests/guix-environment.sh: Test "guix environment guix". --- guix/profiles.scm | 10 ++++++++-- guix/scripts/environment.scm | 7 +++++++ tests/guix-environment.sh | 4 ++++ 3 files changed, 19 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/guix/profiles.scm b/guix/profiles.scm index 0c70975f7e..dcb5186c7a 100644 --- a/guix/profiles.scm +++ b/guix/profiles.scm @@ -1199,10 +1199,14 @@ (define* (profile-derivation manifest #:key (hooks %default-profile-hooks) (locales? #t) + (allow-collisions? #f) system target) "Return a derivation that builds a profile (aka. 'user environment') with the given MANIFEST. The profile includes additional derivations returned by the monadic procedures listed in HOOKS--such as an Info 'dir' file, etc. +Unless ALLOW-COLLISIONS? is true, a '&profile-collision-error' is raised if +entries in MANIFEST collide (for instance if there are two same-name packages +with a different version number.) When LOCALES? is true, the build is performed under a UTF-8 locale; this adds a dependency on the 'glibc-utf8-locales' package. @@ -1212,8 +1216,10 @@ (define* (profile-derivation manifest (mlet* %store-monad ((system (if system (return system) (current-system))) - (ok? (check-for-collisions manifest system - #:target target)) + (ok? (if allow-collisions? + (return #t) + (check-for-collisions manifest system + #:target target))) (extras (if (null? (manifest-entries manifest)) (return '()) (sequence %store-monad diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm index af69e2b730..0abc509a35 100644 --- a/guix/scripts/environment.scm +++ b/guix/scripts/environment.scm @@ -323,6 +323,13 @@ (define (inputs->profile-derivation inputs system bootstrap?) profile." (profile-derivation (packages->manifest inputs) #:system system + + ;; Packages can have conflicting inputs, or explicit + ;; inputs that conflict with implicit inputs (e.g., gcc, + ;; gzip, etc.). Thus, do not error out when we + ;; encounter collision. + #:allow-collisions? #t + #:hooks (if bootstrap? '() %default-profile-hooks) diff --git a/tests/guix-environment.sh b/tests/guix-environment.sh index 9115949123..bf5ca17fa5 100644 --- a/tests/guix-environment.sh +++ b/tests/guix-environment.sh @@ -105,6 +105,10 @@ else test $? = 42 fi +# Make sure we can build the environment of 'guix'. There may be collisions +# in its profile (e.g., for 'gzip'), but we have to accept them. +guix environment guix --bootstrap -n + if guile -c '(getaddrinfo "www.gnu.org" "80" AI_NUMERICSERV)' 2> /dev/null then # Compute the build environment for the initial GNU Make. -- cgit v1.2.3 From 1071f781d97509347144754b3248581cf7c6c1d5 Mon Sep 17 00:00:00 2001 From: Ludovic Courtès Date: Mon, 19 Jun 2017 17:39:24 +0200 Subject: daemon: '--listen' can be passed several times, can specify TCP endpoints. * nix/nix-daemon/guix-daemon.cc (DEFAULT_GUIX_PORT): New macro. (listen_options): New variable. (parse_opt): Push back '--listen' options to LISTEN_OPTIONS. (open_unix_domain_socket, open_inet_socket) (listening_sockets): New functions. (main): Use it. Pass SOCKETS to 'run'. * nix/nix-daemon/nix-daemon.cc (matchUser): Remove. (SD_LISTEN_FDS_START): Remove. (acceptConnection): New function. (daemonLoop): Rewrite to take a vector of file descriptors, to select(2) on them, and to call 'acceptConnection'. (run): Change to take a vector of file descriptors. * tests/guix-daemon.sh: Add test. --- doc/guix.texi | 55 +++++++- nix/nix-daemon/guix-daemon.cc | 152 +++++++++++++++++++++-- nix/nix-daemon/nix-daemon.cc | 283 +++++++++++++++++++----------------------- tests/guix-daemon.sh | 12 ++ 4 files changed, 327 insertions(+), 175 deletions(-) (limited to 'tests') diff --git a/doc/guix.texi b/doc/guix.texi index ee9f80ef4d..729ec081be 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -1258,12 +1258,47 @@ Assume @var{system} as the current system type. By default it is the architecture/kernel pair found at configure time, such as @code{x86_64-linux}. -@item --listen=@var{socket} -Listen for connections on @var{socket}, the file name of a Unix-domain -socket. The default socket is -@file{@var{localstatedir}/daemon-socket/socket}. This option is only -useful in exceptional circumstances, such as if you need to run several -daemons on the same machine. +@item --listen=@var{endpoint} +Listen for connections on @var{endpoint}. @var{endpoint} is interpreted +as the file name of a Unix-domain socket if it starts with +@code{/} (slash sign). Otherwise, @var{endpoint} is interpreted as a +host name or host name and port to listen to. Here are a few examples: + +@table @code +@item --listen=/gnu/var/daemon +Listen for connections on the @file{/gnu/var/daemon} Unix-domain socket, +creating it if needed. + +@item --listen=localhost +@cindex daemon, remote access +@cindex remote access to the daemon +@cindex daemon, cluster setup +@cindex clusters, daemon setup +Listen for TCP connections on the network interface corresponding to +@code{localhost}, on port 44146. + +@item --listen=128.0.0.42:1234 +Listen for TCP connections on the network interface corresponding to +@code{128.0.0.42}, on port 1234. +@end table + +This option can be repeated multiple times, in which case +@command{guix-daemon} accepts connections on all the specified +endpoints. Users can tell client commands what endpoint to connect to +by setting the @code{GUIX_DAEMON_SOCKET} environment variable +(@pxref{The Store, @code{GUIX_DAEMON_SOCKET}}). + +@quotation Note +The daemon protocol is @emph{unauthenticated and unencrypted}. Using +@code{--listen=@var{host}} is suitable on local networks, such as +clusters, where only trusted nodes may connect to the build daemon. In +other cases where remote access to the daemon is needed, we recommend +using Unix-domain sockets along with SSH. +@end quotation + +When @code{--listen} is omitted, @command{guix-daemon} listens for +connections on the Unix-domain socket located at +@file{@var{localstatedir}/daemon-socket/socket}. @end table @@ -3769,6 +3804,10 @@ These are for Unix-domain sockets. @file{/var/guix/daemon-socket/socket}. @item guix +@cindex daemon, remote access +@cindex remote access to the daemon +@cindex daemon, cluster setup +@cindex clusters, daemon setup These URIs denote connections over TCP/IP, without encryption nor authentication of the remote host. The URI must specify the host name and optionally a port number (by default port 44146 is used): @@ -3781,6 +3820,10 @@ This setup is suitable on local networks, such as clusters, where only trusted nodes may connect to the build daemon at @code{master.guix.example.org}. +The @code{--listen} option of @command{guix-daemon} can be used to +instruct it to listen for TCP connections (@pxref{Invoking guix-daemon, +@code{--listen}}). + @item ssh @cindex SSH access to build daemons These URIs allow you to connect to a remote daemon over diff --git a/nix/nix-daemon/guix-daemon.cc b/nix/nix-daemon/guix-daemon.cc index 0d9c33d1d2..7963358202 100644 --- a/nix/nix-daemon/guix-daemon.cc +++ b/nix/nix-daemon/guix-daemon.cc @@ -1,5 +1,6 @@ /* GNU Guix --- Functional package management for GNU Copyright (C) 2012, 2013, 2014, 2015, 2016, 2017 Ludovic Courtès + Copyright (C) 2006, 2010, 2012, 2014 Eelco Dolstra This file is part of GNU Guix. @@ -30,8 +31,12 @@ #include #include #include +#include +#include +#include #include #include +#include #include #include @@ -43,7 +48,7 @@ char **argvSaved; using namespace nix; /* Entry point in `nix-daemon.cc'. */ -extern void run (Strings args); +extern void run (const std::vector &); /* Command-line options. */ @@ -149,6 +154,12 @@ to live outputs") }, }; +/* Default port for '--listen' on TCP/IP. */ +#define DEFAULT_GUIX_PORT "44146" + +/* List of '--listen' options. */ +static std::list listen_options; + /* Convert ARG to a Boolean value, or throw an error if it does not denote a Boolean. */ static bool @@ -217,15 +228,7 @@ parse_opt (int key, char *arg, struct argp_state *state) settings.keepLog = false; break; case GUIX_OPT_LISTEN: - try - { - settings.nixDaemonSocketFile = canonPath (arg); - } - catch (std::exception &e) - { - fprintf (stderr, _("error: %s\n"), e.what ()); - exit (EXIT_FAILURE); - } + listen_options.push_back (arg); break; case GUIX_OPT_SUBSTITUTE_URLS: settings.set ("substitute-urls", arg); @@ -276,13 +279,134 @@ static const struct argp argp = guix_textdomain }; + +static int +open_unix_domain_socket (const char *file) +{ + /* Create and bind to a Unix domain socket. */ + AutoCloseFD fdSocket = socket (PF_UNIX, SOCK_STREAM, 0); + if (fdSocket == -1) + throw SysError (_("cannot create Unix domain socket")); + + createDirs (dirOf (file)); + + /* Urgh, sockaddr_un allows path names of only 108 characters. + So chdir to the socket directory so that we can pass a + relative path name. */ + if (chdir (dirOf (file).c_str ()) == -1) + throw SysError (_("cannot change current directory")); + Path fileRel = "./" + baseNameOf (file); + + struct sockaddr_un addr; + addr.sun_family = AF_UNIX; + if (fileRel.size () >= sizeof (addr.sun_path)) + throw Error (format (_("socket file name '%1%' is too long")) % fileRel); + strcpy (addr.sun_path, fileRel.c_str ()); + + unlink (file); + + /* Make sure that the socket is created with 0666 permission + (everybody can connect --- provided they have access to the + directory containing the socket). */ + mode_t oldMode = umask (0111); + int res = bind (fdSocket, (struct sockaddr *) &addr, sizeof addr); + umask (oldMode); + if (res == -1) + throw SysError (format (_("cannot bind to socket '%1%'")) % file); + + if (chdir ("/") == -1) /* back to the root */ + throw SysError (_("cannot change current directory")); + + if (listen (fdSocket, 5) == -1) + throw SysError (format (_("cannot listen on socket '%1%'")) % file); + + return fdSocket.borrow (); +} + +/* Return a listening socket for ADDRESS, which has the given LENGTH. */ +static int +open_inet_socket (const struct sockaddr *address, socklen_t length) +{ + AutoCloseFD fd = socket (address->sa_family, SOCK_STREAM, 0); + if (fd == -1) + throw SysError (_("cannot create TCP socket")); + + int res = bind (fd, address, length); + if (res == -1) + throw SysError (_("cannot bind TCP socket")); + + if (listen (fd, 5) == -1) + throw SysError (format (_("cannot listen on TCP socket"))); + + return fd.borrow (); +} + +/* Return a list of file descriptors of listening sockets. */ +static std::vector +listening_sockets (const std::list &options) +{ + std::vector result; + + if (options.empty ()) + { + /* Open the default Unix-domain socket. */ + auto fd = open_unix_domain_socket (settings.nixDaemonSocketFile.c_str ()); + result.push_back (fd); + return result; + } + + /* Open the user-specified sockets. */ + for (const std::string& option: options) + { + if (option[0] == '/') + { + /* Assume OPTION is the file name of a Unix-domain socket. */ + settings.nixDaemonSocketFile = canonPath (option); + int fd = + open_unix_domain_socket (settings.nixDaemonSocketFile.c_str ()); + result.push_back (fd); + } + else + { + /* Assume OPTIONS has the form "HOST" or "HOST:PORT". */ + auto colon = option.find_last_of (":"); + auto host = colon == std::string::npos + ? option : option.substr (0, colon); + auto port = colon == std::string::npos + ? DEFAULT_GUIX_PORT + : option.substr (colon + 1, option.size () - colon - 1); + + struct addrinfo *res, hints; + + memset (&hints, '\0', sizeof hints); + hints.ai_socktype = SOCK_STREAM; + hints.ai_flags = AI_NUMERICSERV | AI_ADDRCONFIG; + + int err = getaddrinfo (host.c_str(), port.c_str (), + &hints, &res); + + if (err != 0) + throw Error(format ("failed to look up '%1%': %2%") + % option % gai_strerror (err)); + + printMsg (lvlDebug, format ("listening on '%1%', port '%2%'") + % host % port); + + /* XXX: Pick the first result, RES. */ + result.push_back (open_inet_socket (res->ai_addr, + res->ai_addrlen)); + + freeaddrinfo (res); + } + } + + return result; +} int main (int argc, char *argv[]) { - static const Strings nothing; - setlocale (LC_ALL, ""); bindtextdomain (guix_textdomain, LOCALEDIR); textdomain (guix_textdomain); @@ -359,6 +483,8 @@ main (int argc, char *argv[]) argp_parse (&argp, argc, argv, 0, 0, 0); + auto sockets = listening_sockets (listen_options); + /* Effect all the changes made via 'settings.set'. */ settings.update (); @@ -402,7 +528,7 @@ using `--build-users-group' is highly recommended\n")); printMsg (lvlDebug, format ("listening on `%1%'") % settings.nixDaemonSocketFile); - run (nothing); + run (sockets); } catch (std::exception &e) { diff --git a/nix/nix-daemon/nix-daemon.cc b/nix/nix-daemon/nix-daemon.cc index 79580ffb48..3d8e909901 100644 --- a/nix/nix-daemon/nix-daemon.cc +++ b/nix/nix-daemon/nix-daemon.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -809,151 +810,87 @@ static void setSigChldAction(bool autoReap) } -bool matchUser(const string & user, const string & group, const Strings & users) +/* Accept a connection on FDSOCKET and fork a server process to process the + new connection. */ +static void acceptConnection(int fdSocket) { - if (find(users.begin(), users.end(), "*") != users.end()) - return true; - - if (find(users.begin(), users.end(), user) != users.end()) - return true; - - for (auto & i : users) - if (string(i, 0, 1) == "@") { - if (group == string(i, 1)) return true; - struct group * gr = getgrnam(i.c_str() + 1); - if (!gr) continue; - for (char * * mem = gr->gr_mem; *mem; mem++) - if (user == string(*mem)) return true; - } - - return false; -} - - -#define SD_LISTEN_FDS_START 3 - - -static void daemonLoop() -{ - if (chdir("/") == -1) - throw SysError("cannot change current directory"); - - /* Get rid of children automatically; don't let them become - zombies. */ - setSigChldAction(true); - - AutoCloseFD fdSocket; - - /* Handle socket-based activation by systemd. */ - if (getEnv("LISTEN_FDS") != "") { - if (getEnv("LISTEN_PID") != std::to_string(getpid()) || getEnv("LISTEN_FDS") != "1") - throw Error("unexpected systemd environment variables"); - fdSocket = SD_LISTEN_FDS_START; - } - - /* Otherwise, create and bind to a Unix domain socket. */ - else { - - /* Create and bind to a Unix domain socket. */ - fdSocket = socket(PF_UNIX, SOCK_STREAM, 0); - if (fdSocket == -1) - throw SysError("cannot create Unix domain socket"); - - string socketPath = settings.nixDaemonSocketFile; - - createDirs(dirOf(socketPath)); - - /* Urgh, sockaddr_un allows path names of only 108 characters. - So chdir to the socket directory so that we can pass a - relative path name. */ - if (chdir(dirOf(socketPath).c_str()) == -1) - throw SysError("cannot change current directory"); - Path socketPathRel = "./" + baseNameOf(socketPath); - - struct sockaddr_un addr; - addr.sun_family = AF_UNIX; - if (socketPathRel.size() >= sizeof(addr.sun_path)) - throw Error(format("socket path `%1%' is too long") % socketPathRel); - strcpy(addr.sun_path, socketPathRel.c_str()); - - unlink(socketPath.c_str()); - - /* Make sure that the socket is created with 0666 permission - (everybody can connect --- provided they have access to the - directory containing the socket). */ - mode_t oldMode = umask(0111); - int res = bind(fdSocket, (struct sockaddr *) &addr, sizeof(addr)); - umask(oldMode); - if (res == -1) - throw SysError(format("cannot bind to socket `%1%'") % socketPath); - - if (chdir("/") == -1) /* back to the root */ - throw SysError("cannot change current directory"); - - if (listen(fdSocket, 5) == -1) - throw SysError(format("cannot listen on socket `%1%'") % socketPath); - } - - closeOnExec(fdSocket); - - /* Loop accepting connections. */ - while (1) { - - try { - /* Important: the server process *cannot* open the SQLite - database, because it doesn't like forks very much. */ - assert(!store); - - /* Accept a connection. */ - struct sockaddr_un remoteAddr; - socklen_t remoteAddrLen = sizeof(remoteAddr); - - AutoCloseFD remote = accept(fdSocket, - (struct sockaddr *) &remoteAddr, &remoteAddrLen); - checkInterrupt(); - if (remote == -1) { - if (errno == EINTR) - continue; - else - throw SysError("accepting connection"); - } - - closeOnExec(remote); - - bool trusted = false; - pid_t clientPid = -1; + uid_t clientUid = (uid_t) -1; + gid_t clientGid = (gid_t) -1; + try { + /* Important: the server process *cannot* open the SQLite + database, because it doesn't like forks very much. */ + assert(!store); + + /* Accept a connection. */ + struct sockaddr_storage remoteAddr; + socklen_t remoteAddrLen = sizeof(remoteAddr); + + try_again: + AutoCloseFD remote = accept(fdSocket, + (struct sockaddr *) &remoteAddr, &remoteAddrLen); + checkInterrupt(); + if (remote == -1) { + if (errno == EINTR) + goto try_again; + else + throw SysError("accepting connection"); + } + + closeOnExec(remote); + + pid_t clientPid = -1; + bool trusted = false; + + /* Get the identity of the caller, if possible. */ + if (remoteAddr.ss_family == AF_UNIX) { #if defined(SO_PEERCRED) - /* Get the identity of the caller, if possible. */ - ucred cred; - socklen_t credLen = sizeof(cred); - if (getsockopt(remote, SOL_SOCKET, SO_PEERCRED, &cred, &credLen) == -1) - throw SysError("getting peer credentials"); + ucred cred; + socklen_t credLen = sizeof(cred); + if (getsockopt(remote, SOL_SOCKET, SO_PEERCRED, + &cred, &credLen) == -1) + throw SysError("getting peer credentials"); - clientPid = cred.pid; + clientPid = cred.pid; + clientUid = cred.uid; + clientGid = cred.gid; + trusted = clientUid == 0; struct passwd * pw = getpwuid(cred.uid); string user = pw ? pw->pw_name : std::to_string(cred.uid); - struct group * gr = getgrgid(cred.gid); - string group = gr ? gr->gr_name : std::to_string(cred.gid); - - Strings trustedUsers = settings.get("trusted-users", Strings({"root"})); - Strings allowedUsers = settings.get("allowed-users", Strings({"*"})); - - if (matchUser(user, group, trustedUsers)) - trusted = true; - - if (!trusted && !matchUser(user, group, allowedUsers)) - throw Error(format("user `%1%' is not allowed to connect to the Nix daemon") % user); - - printMsg(lvlInfo, format((string) "accepted connection from pid %1%, user %2%" - + (trusted ? " (trusted)" : "")) % clientPid % user); + printMsg(lvlInfo, + format((string) "accepted connection from pid %1%, user %2%") + % clientPid % user); #endif - - /* Fork a child to handle the connection. */ - startProcess([&]() { - fdSocket.close(); + } else { + char address_str[128]; + const char *result; + + if (remoteAddr.ss_family == AF_INET) { + struct sockaddr_in *addr = (struct sockaddr_in *) &remoteAddr; + struct in_addr inaddr = { addr->sin_addr }; + result = inet_ntop(AF_INET, &inaddr, + address_str, sizeof address_str); + } else if (remoteAddr.ss_family == AF_INET6) { + struct sockaddr_in6 *addr = (struct sockaddr_in6 *) &remoteAddr; + struct in6_addr inaddr = { addr->sin6_addr }; + result = inet_ntop(AF_INET6, &inaddr, + address_str, sizeof address_str); + } else { + result = NULL; + } + + if (result != NULL) { + printMsg(lvlInfo, + format("accepted connection from %1%") + % address_str); + } + } + + /* Fork a child to handle the connection. */ + startProcess([&]() { + close(fdSocket); /* Background the daemon. */ if (setsid() == -1) @@ -968,17 +905,11 @@ static void daemonLoop() strncpy(argvSaved[1], processName.c_str(), strlen(argvSaved[1])); } -#if defined(SO_PEERCRED) /* Store the client's user and group for this connection. This has to be done in the forked process since it is per - connection. */ - settings.clientUid = cred.uid; - settings.clientGid = cred.gid; -#else - /* Setting these to -1 means: do not change */ - settings.clientUid = (uid_t) -1; - settings.clientGid = (gid_t) -1; -#endif + connection. Setting these to -1 means: do not change. */ + settings.clientUid = clientUid; + settings.clientGid = clientGid; /* Handle the connection. */ from.fd = remote; @@ -988,23 +919,63 @@ static void daemonLoop() exit(0); }, false, "unexpected Nix daemon error: ", true); - } catch (Interrupted & e) { - throw; - } catch (Error & e) { - printMsg(lvlError, format("error processing connection: %1%") % e.msg()); - } + } catch (Interrupted & e) { + throw; + } catch (Error & e) { + printMsg(lvlError, format("error processing connection: %1%") % e.msg()); } } - -void run(Strings args) +static void daemonLoop(const std::vector& sockets) { - for (Strings::iterator i = args.begin(); i != args.end(); ) { - string arg = *i++; - if (arg == "--daemon") /* ignored for backwards compatibility */; + if (chdir("/") == -1) + throw SysError("cannot change current directory"); + + /* Get rid of children automatically; don't let them become + zombies. */ + setSigChldAction(true); + + /* Mark sockets as close-on-exec. */ + for(int fd: sockets) { + closeOnExec(fd); } - daemonLoop(); + /* Prepare the FD set corresponding to SOCKETS. */ + auto initializeFDSet = [&](fd_set *set) { + FD_ZERO(set); + for (int fd: sockets) { + FD_SET(fd, set); + } + }; + + /* Loop accepting connections. */ + while (1) { + fd_set readfds; + + initializeFDSet(&readfds); + int count = + select(*std::max_element(sockets.begin(), sockets.end()) + 1, + &readfds, NULL, NULL, + NULL); + if (count < 0) { + int err = errno; + if (err == EINTR) + continue; + throw SysError(format("select error: %1%") % strerror(err)); + } + + for (unsigned int i = 0; i < sockets.size(); i++) { + if (FD_ISSET(sockets[i], &readfds)) { + acceptConnection(sockets[i]); + } + } + } +} + + +void run(const std::vector& sockets) +{ + daemonLoop(sockets); } diff --git a/tests/guix-daemon.sh b/tests/guix-daemon.sh index 9186ffd585..7212e3eb68 100644 --- a/tests/guix-daemon.sh +++ b/tests/guix-daemon.sh @@ -81,6 +81,18 @@ guile -c " kill "$daemon_pid" +# Pass several '--listen' options, and make sure they are all honored. +guix-daemon --disable-chroot --listen="$socket" --listen="$socket-second" \ + --listen="localhost" --listen="localhost:9876" & +daemon_pid=$! + +for uri in "$socket" "$socket-second" \ + "guix://localhost" "guix://localhost:9876" +do + GUIX_DAEMON_SOCKET="$uri" guix build guile-bootstrap +done + +kill "$daemon_pid" # Check the failed build cache. -- cgit v1.2.3