[PATCH 0/4] Isolated inferiors.

OpenSubmitted by Christopher Baines.
Details
2 participants
  • Ludovic Courtès
  • Christopher Baines
Owner
unassigned
Severity
normal
C
C
Christopher Baines wrote on 24 Feb 2019 17:12
(address . guix-patches@gnu.org)
875zt9go87.fsf@cbaines.net
These patches form a prototype for Guix inferiors, that areisolated. Access to the inferior Guix is done through running a REPL asa separate process. These patches provide a way of launching that REPLin an isolated environment through Linux namespaces, providing someisolation from the wider system.
These patches should work, at least enough to get the derivations forpackages within the inferior Guix, as well as doing 'guix pull' withinthe inferior Guix.
They're not ready to be merged just yet though. I think some of theapproaches are a little odd (e.g. using (ice-9 popen) internals) andI've got no idea if the isolation is actually working properly.

Christopher Baines (4): utils: Add #:base-directory to call-with-temporary-directory. linux-container: Add 'start-child-in-container'. inferior: Add a shared-directory field to <inferior> inferior: Add 'open-inferior/container'.
gnu/build/linux-container.scm | 82 +++++++++++++++++++++++++++++++ guix/inferior.scm | 90 ++++++++++++++++++++++++++++++----- guix/utils.scm | 4 +- 3 files changed, 163 insertions(+), 13 deletions(-)
-----BEGIN PGP SIGNATURE-----
iQKTBAEBCgB9FiEEPonu50WOcg2XVOCyXiijOwuE9XcFAlxywlhfFIAAAAAALgAoaXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNFODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcACgkQXiijOwuE9XfDjg//W/v3E1k3KSBgq/cbEB3eDzShPBdOfQjXsm+8NtdZUtmifuzomzQPbSRbMdtFZzv1u91baWGc7OcArUujtDb7BJJFOdPLbaU+X5ypEsvdNHdndJTuPSefQS4VrAmbJFLi7vdgqguO8kp5UqT/mHoLDqNPcvCMYQFrlpF6hu1nIHGxtKyTX7TmGVODamLBFkHr1IF47Fy8+gahkz50jvW5bvc2kyUwXGFUU4xfB/shgKHuq7tZQDHbZwrzIwHEBT2Db4g6bJYb5XP0MpqBLaN9CSCCjaNUTo7Y4rxDxCiiOfkwLZtdugt1ghH7RnYApd9SMNf90VSjVJithX8Y/FtTCOsh+DVqPwEW0fVIDM0XZJ1a7V7JE3WKAy9+sNMX0AF4o9VxSSJXupAYb/vqXD3DhctY17VszZDVimkuyAvb3IAipdMiRe5rbQ0O8SSFXxmvQ+eQsSQ5YF5oq462DZmJ9yhkEXApIS3bwhWXqZw9gzxX8IPUWfAobVtcCpSYhsNDwyz5h0Iult+9rovwDBWu4DtsmRs7L1tykbvSNWOWhgFAqpTf+lx2V6J/F/XQe6dFuy26c3vH0xVbjSZsWRiqhBXBsRyHtiijI8ctZ5w2fbkzKH3F04st3yRvARqN+r66Zi207Y5HiSVGxZiYReFoS9i+RL9IJWcxdcMsPJDm0Dk==PrMq-----END PGP SIGNATURE-----
C
C
Christopher Baines wrote on 24 Feb 2019 17:18
[PATCH 1/4] utils: Add #:base-directory to call-with-temporary-directory.
(address . 34638@debbugs.gnu.org)
20190224161855.2632-1-mail@cbaines.net
This allows more easily creating temporary directories within a specificdirectory. This is motivated by using this in inferior-eval-with-store.
* guix/utils.scm (call-with-temporary-directory): Add optional keywordargument, base-directory.--- guix/utils.scm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Toggle diff (19 lines)diff --git a/guix/utils.scm b/guix/utils.scmindex ed1a418cca..abeb156f40 100644--- a/guix/utils.scm+++ b/guix/utils.scm@@ -620,10 +620,10 @@ call." (false-if-exception (close out)) (false-if-exception (delete-file template)))))) -(define (call-with-temporary-directory proc)+(define* (call-with-temporary-directory proc #:key base-directory) "Call PROC with a name of a temporary directory; close the directory and delete it when leaving the dynamic extent of this call."- (let* ((directory (or (getenv "TMPDIR") "/tmp"))+ (let* ((directory (or base-directory (getenv "TMPDIR") "/tmp")) (template (string-append directory "/guix-directory.XXXXXX")) (tmp-dir (mkdtemp! template))) (dynamic-wind-- 2.20.1
C
C
Christopher Baines wrote on 24 Feb 2019 17:18
[PATCH 3/4] inferior: Add a shared-directory field to <inferior>
(address . 34638@debbugs.gnu.org)
20190224161855.2632-3-mail@cbaines.net
--- guix/inferior.scm | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-)
Toggle diff (67 lines)diff --git a/guix/inferior.scm b/guix/inferior.scmindex 027418a98d..cf72454426 100644--- a/guix/inferior.scm+++ b/guix/inferior.scm@@ -97,14 +97,15 @@ ;; Inferior Guix process. (define-record-type <inferior>- (inferior pid socket close version packages table)+ (inferior pid socket close shared-directory version packages table) inferior?- (pid inferior-pid)- (socket inferior-socket)- (close inferior-close-socket) ;procedure- (version inferior-version) ;REPL protocol version- (packages inferior-package-promise) ;promise of inferior packages- (table inferior-package-table)) ;promise of vhash+ (pid inferior-pid)+ (socket inferior-socket)+ (close inferior-close-socket) ;procedure+ (shared-directory inferior-shared-directory)+ (version inferior-version) ;REPL protocol version+ (packages inferior-package-promise) ;promise of inferior packages+ (table inferior-package-table)) ;promise of vhash (define (inferior-pipe directory command) "Return an input/output pipe on the Guix instance in DIRECTORY. This runs@@ -136,7 +137,7 @@ it's an old Guix." ((@ (guix scripts repl) machine-repl)))))) pipe))) -(define* (port->inferior pipe #:optional (close close-port))+(define* (port->inferior pipe shared-directory #:optional (close close-port)) "Given PIPE, an input/output port, return an inferior that talks over PIPE. PIPE is closed with CLOSE when 'close-inferior' is called on the returned inferior."@@ -144,7 +145,8 @@ inferior." (match (read pipe) (('repl-version 0 rest ...)- (letrec ((result (inferior 'pipe pipe close (cons 0 rest)+ (letrec ((result (inferior 'pipe pipe close shared-directory+ (cons 0 rest) (delay (%inferior-packages result)) (delay (%inferior-package-table result))))) (inferior-eval '(use-modules (guix)) result)@@ -162,7 +164,7 @@ equivalent. Return #f if the inferior could not be launched." (define pipe (inferior-pipe directory command)) - (port->inferior pipe close-pipe))+ (port->inferior pipe #f close-pipe)) (define (close-inferior inferior) "Close INFERIOR."@@ -479,7 +481,8 @@ thus be the code of a one-argument procedure that accepts a store." ((client . address) (proxy client (store-connection-socket store)))) (close-port socket)- (read-inferior-response inferior)))))+ (read-inferior-response inferior)))+ #:base-directory (inferior-shared-directory inferior))) (define* (inferior-package-derivation store package #:optional-- 2.20.1
C
C
Christopher Baines wrote on 24 Feb 2019 17:18
[PATCH 4/4] inferior: Add 'open-inferior/container'.
(address . 34638@debbugs.gnu.org)
20190224161855.2632-4-mail@cbaines.net
--- guix/inferior.scm | 65 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+)
Toggle diff (92 lines)diff --git a/guix/inferior.scm b/guix/inferior.scmindex cf72454426..a5f773c147 100644--- a/guix/inferior.scm+++ b/guix/inferior.scm@@ -40,6 +40,9 @@ #:use-module (guix store) #:use-module (guix derivations) #:use-module (guix base32)+ #:use-module (gnu system file-systems)+ #:use-module (gnu build linux-container)+ #:use-module (guix build syscalls) #:use-module (gcrypt hash) #:autoload (guix cache) (maybe-remove-expired-cache-entries) #:autoload (guix ui) (show-what-to-build*)@@ -54,6 +57,7 @@ #:use-module ((rnrs bytevectors) #:select (string->utf8)) #:export (inferior? open-inferior+ open-inferior/container port->inferior close-inferior inferior-eval@@ -137,6 +141,67 @@ it's an old Guix." ((@ (guix scripts repl) machine-repl)))))) pipe))) +(define* (open-inferior/container store guix-store-item+ #:key+ (command "bin/guix")+ (share-host-network? #f)+ (extra-shared-directories '())+ (extra-environment-variables '()))+ (define requisite-store-items+ (requisites store (list guix-store-item)))++ (define shared-directory+ (mkdtemp! (string-append (or (getenv "TMPDIR") "/tmp")+ "/guix-inferior.XXXXXX")))++ (define mappings+ (append+ (map (lambda (dir)+ (file-system-mapping+ (source dir)+ (target dir)+ (writable? #f)))+ `(;; Share a directory, used in inferior-eval-with-store+ ,shared-directory+ ,@requisite-store-items+ ,@extra-shared-directories))+ (if share-host-network?+ %network-file-mappings+ '())))++ (define mounts+ (append %container-file-systems+ (map file-system-mapping->bind-mount+ mappings)))++ (define (inferior-pipe/container store+ guix-store-item+ shared-directory+ command)+ (start-child-in-container+ (list (string-append guix-store-item "/bin/guix")+ ;; TODO I'm not sure why "repl" is duplicated in the following+ ;; command+ "repl" "repl" "-t" "machine")+ #:read? #t+ #:write? #t+ #:mounts mounts+ #:namespaces (if share-host-network?+ (delq 'net %namespaces)+ %namespaces)+ #:extra-environment-variables+ `(;; Set HOME, so that the (guix profiles) module can be loaded, without it+ ;; trying to read from /etc/passwd+ "HOME=/tmp"+ ,@extra-environment-variables)))++ (port->inferior (inferior-pipe/container store+ guix-store-item+ shared-directory+ command)+ shared-directory+ close-pipe))+ (define* (port->inferior pipe shared-directory #:optional (close close-port)) "Given PIPE, an input/output port, return an inferior that talks over PIPE. PIPE is closed with CLOSE when 'close-inferior' is called on the returned-- 2.20.1
C
C
Christopher Baines wrote on 24 Feb 2019 17:18
[PATCH 2/4] linux-container: Add 'start-child-in-container'.
(address . 34638@debbugs.gnu.org)
20190224161855.2632-2-mail@cbaines.net
This new procedure is similar to open-pipe* in (ice-9 popen), but usingrun-container from (gnu build linux-container).
* gnu/build/linux-container.scm (start-child-in-container): New procedure.--- gnu/build/linux-container.scm | 82 +++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+)
Toggle diff (102 lines)diff --git a/gnu/build/linux-container.scm b/gnu/build/linux-container.scmindex 65e1325577..63c83902e4 100644--- a/gnu/build/linux-container.scm+++ b/gnu/build/linux-container.scm@@ -32,6 +32,7 @@ setgroups-supported? %namespaces run-container+ start-child-in-container call-with-container container-excursion container-excursion*))@@ -210,6 +211,87 @@ corresponds to the symbols in NAMESPACES." ('net CLONE_NEWNET)) namespaces))) +(define* (start-child-in-container command+ #:key read? write?+ (root 'temporary)+ (mounts '())+ (namespaces %namespaces)+ (host-uids 1)+ (extra-environment-variables '()))+ (define (with-root-directory f)+ (if (eq? root 'temporary)+ (call-with-temporary-directory f)+ (f root)))++ ;; (ice-9 popen) internals+ (define make-rw-port (@@ (ice-9 popen) make-rw-port))+ (define pipe-guardian (@@ (ice-9 popen) pipe-guardian))+ (define make-pipe-info (@@ (ice-9 popen) make-pipe-info))++ ;; car is the inport port, cdr is the output port. You write to the output+ ;; port, and read from the input port.+ (define child-to-parent-pipe+ (if read?+ (pipe)+ #f))++ (define parent-to-child-pipe+ (if write?+ (pipe)+ #f))++ (define (run-program)+ (when read?+ (match child-to-parent-pipe+ ((input-port . output-port)+ ;; close the output part of the child-to-parent-pipe, as this is used+ ;; by the parent process+ (close-port input-port)++ ;; Make the input part of the child-to-parent-pipe the standard+ ;; output of this process+ (dup2 (fileno output-port) 1))))++ (when write?+ (match parent-to-child-pipe+ ((input-port . output-port)+ ;; close the input part of the parent-to-child-pipe, as this is used+ ;; by the parent processs+ (close-port output-port)++ ;; Make the output part of the parent-to-child-pipe the standard+ ;; input of this process+ (dup2 (fileno input-port) 0))))++ ;; TODO Maybe close all file descriptors, as start_child in Guile does?++ (for-each putenv extra-environment-variables)++ (apply execlp command))++ (with-root-directory+ (lambda (root)+ (let ((pid (run-container root mounts namespaces host-uids run-program)))+ ;; Catch SIGINT and kill the container process.+ (sigaction SIGINT+ (lambda (signum)+ (false-if-exception+ (kill pid SIGKILL))))++ (let* ((read-port (and=> child-to-parent-pipe car))+ (write-port (and=> parent-to-child-pipe cdr))++ (port (or (and read-port write-port+ (make-rw-port read-port write-port))+ read-port+ write-port))+ (pipe-info (make-pipe-info pid)))++ (pipe-guardian pipe-info)+ (%set-port-property! port 'popen-pipe-info pipe-info)++ port)))))+ (define (run-container root mounts namespaces host-uids thunk) "Run THUNK in a new container process and return its PID. ROOT specifies the root directory for the container. MOUNTS is a list of <file-system>-- 2.20.1
L
L
Ludovic Courtès wrote on 14 Mar 2019 20:35
Re: [bug#34638] [PATCH 0/4] Isolated inferiors.
(name . Christopher Baines)(address . mail@cbaines.net)(address . 34638@debbugs.gnu.org)
878sxhi6zq.fsf@gnu.org
Hello!
Christopher Baines <mail@cbaines.net> skribis:
Toggle quote (10 lines)> These patches form a prototype for Guix inferiors, that are> isolated. Access to the inferior Guix is done through running a REPL as> a separate process. These patches provide a way of launching that REPL> in an isolated environment through Linux namespaces, providing some> isolation from the wider system.>> These patches should work, at least enough to get the derivations for> packages within the inferior Guix, as well as doing 'guix pull' within> the inferior Guix.
This is really cool.
When we do this kind of thing (like also the “Compute Guix derivation”trampoline used by ‘guix pull’), it reminds me of what the Nix peoplecall “recursive Nix”—the ability for a derivation’s build process tocompute other derivation. If we had that, then basically what you’redoing might just as well be a derivation.
BTW, thinking about it, for the Guix Data Service, would‘gexp->derivation-in-inferior’ be of any use? This is used, forexample, to compute the package cache when running ‘guix pull’. Ithink it’s good enough if all you want is to extract basic filemeta-data, but it’s no good if you also want to extract packagederivations and the likes.
Or we could have a new store back-end that computes derivations inmemory and eventually spits a Nar…
I’m just thinking out loud!
Thanks,Ludo’.
L
L
Ludovic Courtès wrote on 14 Mar 2019 19:17
Re: [bug#34638] [PATCH 2/4] linux-container: Add 'start-child-in-container'.
(name . Christopher Baines)(address . mail@cbaines.net)(address . 34638@debbugs.gnu.org)
87lg1hiam0.fsf@gnu.org
Hello!
Christopher Baines <mail@cbaines.net> skribis:
Toggle quote (5 lines)> This new procedure is similar to open-pipe* in (ice-9 popen), but using> run-container from (gnu build linux-container).>> * gnu/build/linux-container.scm (start-child-in-container): New procedure.
[...]
+(define* (start-child-in-container command
Toggle quote (7 lines)> + #:key read? write?> + (root 'temporary)> + (mounts '())> + (namespaces %namespaces)> + (host-uids 1)> + (extra-environment-variables '()))
We could even call that ‘open-pipe/container’, for clarity.
Toggle quote (10 lines)> + (define (with-root-directory f)> + (if (eq? root 'temporary)> + (call-with-temporary-directory f)> + (f root)))> +> + ;; (ice-9 popen) internals> + (define make-rw-port (@@ (ice-9 popen) make-rw-port))> + (define pipe-guardian (@@ (ice-9 popen) pipe-guardian))> + (define make-pipe-info (@@ (ice-9 popen) make-pipe-info))
So this is the funky part. ;-)
What if we did something like:
(call-with-container mounts (lambda () ;; Somehow act as a proxy between the output process ;; and the one spawned by ‘open-pipe*’. (open-pipe* …)))
? Would that work?
That’s create an extra process, but if it works, it’s probably safer anda lesser maintenance burden.
Now, I think that Guile should expose some of the popen internalssomehow so we can do things like you did, but that’s another story.
Ludo’.
C
C
Christopher Baines wrote on 19 Apr 2019 16:04
[PATCH v2 3/4] inferior: Add a shared-directory field to <inferior>
(address . 34638@debbugs.gnu.org)
20190419140427.15183-3-mail@cbaines.net
--- guix/inferior.scm | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-)
Toggle diff (67 lines)diff --git a/guix/inferior.scm b/guix/inferior.scmindex 63c95141d7..6d18ab90e9 100644--- a/guix/inferior.scm+++ b/guix/inferior.scm@@ -97,14 +97,15 @@ ;; Inferior Guix process. (define-record-type <inferior>- (inferior pid socket close version packages table)+ (inferior pid socket close shared-directory version packages table) inferior?- (pid inferior-pid)- (socket inferior-socket)- (close inferior-close-socket) ;procedure- (version inferior-version) ;REPL protocol version- (packages inferior-package-promise) ;promise of inferior packages- (table inferior-package-table)) ;promise of vhash+ (pid inferior-pid)+ (socket inferior-socket)+ (close inferior-close-socket) ;procedure+ (shared-directory inferior-shared-directory)+ (version inferior-version) ;REPL protocol version+ (packages inferior-package-promise) ;promise of inferior packages+ (table inferior-package-table)) ;promise of vhash (define (inferior-pipe directory command) "Return an input/output pipe on the Guix instance in DIRECTORY. This runs@@ -136,7 +137,7 @@ it's an old Guix." ((@ (guix scripts repl) machine-repl)))))) pipe))) -(define* (port->inferior pipe #:optional (close close-port))+(define* (port->inferior pipe shared-directory #:optional (close close-port)) "Given PIPE, an input/output port, return an inferior that talks over PIPE. PIPE is closed with CLOSE when 'close-inferior' is called on the returned inferior."@@ -144,7 +145,8 @@ inferior." (match (read pipe) (('repl-version 0 rest ...)- (letrec ((result (inferior 'pipe pipe close (cons 0 rest)+ (letrec ((result (inferior 'pipe pipe close shared-directory+ (cons 0 rest) (delay (%inferior-packages result)) (delay (%inferior-package-table result))))) (inferior-eval '(use-modules (guix)) result)@@ -162,7 +164,7 @@ equivalent. Return #f if the inferior could not be launched." (define pipe (inferior-pipe directory command)) - (port->inferior pipe close-pipe))+ (port->inferior pipe #f close-pipe)) (define (close-inferior inferior) "Close INFERIOR."@@ -479,7 +481,8 @@ thus be the code of a one-argument procedure that accepts a store." ((client . address) (proxy client (store-connection-socket store)))) (close-port socket)- (read-inferior-response inferior)))))+ (read-inferior-response inferior)))+ #:base-directory (inferior-shared-directory inferior))) (define* (inferior-package-derivation store package #:optional-- 2.21.0
C
C
Christopher Baines wrote on 19 Apr 2019 16:04
[PATCH v2 2/4] linux-container: Add 'start-child-in-container'.
(address . 34638@debbugs.gnu.org)
20190419140427.15183-2-mail@cbaines.net
This new procedure is similar to open-pipe* in (ice-9 popen), but usingrun-container from (gnu build linux-container).
* gnu/build/linux-container.scm (start-child-in-container): New procedure.--- gnu/build/linux-container.scm | 83 +++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+)
Toggle diff (103 lines)diff --git a/gnu/build/linux-container.scm b/gnu/build/linux-container.scmindex 3d7b52f098..88b00e00f6 100644--- a/gnu/build/linux-container.scm+++ b/gnu/build/linux-container.scm@@ -32,6 +32,7 @@ setgroups-supported? %namespaces run-container+ start-child-in-container call-with-container container-excursion container-excursion*))@@ -213,6 +214,88 @@ corresponds to the symbols in NAMESPACES." ('net CLONE_NEWNET)) namespaces))) +(define* (start-child-in-container command+ #:key read? write?+ (root 'temporary)+ (mounts '())+ (namespaces %namespaces)+ (host-uids 1)+ (extra-environment-variables '()))+ (define (with-root-directory f)+ (if (eq? root 'temporary)+ (call-with-temporary-directory f)+ (f root)))++ (define (make-rw-port read-port write-port)+ (make-soft-port+ (vector+ (lambda (c) (write-char c write-port))+ (lambda (s) (display s write-port))+ (lambda () (force-output write-port))+ (lambda () (read-char read-port))+ (lambda () (close-port read-port) (close-port write-port)))+ "r+"))++ ;; car is the inport port, cdr is the output port. You write to the output+ ;; port, and read from the input port.+ (define child-to-parent-pipe+ (if read?+ (pipe)+ #f))++ (define parent-to-child-pipe+ (if write?+ (pipe)+ #f))++ (define (run-program)+ (when read?+ (match child-to-parent-pipe+ ((input-port . output-port)+ ;; close the output part of the child-to-parent-pipe, as this is used+ ;; by the parent process+ (close-port input-port)++ ;; Make the input part of the child-to-parent-pipe the standard+ ;; output of this process+ (dup2 (fileno output-port) 1))))++ (when write?+ (match parent-to-child-pipe+ ((input-port . output-port)+ ;; close the input part of the parent-to-child-pipe, as this is used+ ;; by the parent processs+ (close-port output-port)++ ;; Make the output part of the parent-to-child-pipe the standard+ ;; input of this process+ (dup2 (fileno input-port) 0))))++ ;; TODO Maybe close all file descriptors, as start_child in Guile does?++ (for-each putenv extra-environment-variables)++ (apply execlp command))++ (with-root-directory+ (lambda (root)+ (let ((pid (run-container root mounts namespaces host-uids run-program)))+ ;; Catch SIGINT and kill the container process.+ (sigaction SIGINT+ (lambda (signum)+ (false-if-exception+ (kill pid SIGKILL))))++ (let* ((read-port (and=> child-to-parent-pipe car))+ (write-port (and=> parent-to-child-pipe cdr))++ (port (or (and read-port write-port+ (make-rw-port read-port write-port))+ read-port+ write-port)))++ (values port pid))))))+ (define* (run-container root mounts namespaces host-uids thunk #:key (guest-uid 0) (guest-gid 0)) "Run THUNK in a new container process and return its PID. ROOT specifies-- 2.21.0
C
C
Christopher Baines wrote on 19 Apr 2019 16:04
[PATCH v2 1/4] utils: Add #:base-directory to call-with-temporary-directory.
(address . 34638@debbugs.gnu.org)
20190419140427.15183-1-mail@cbaines.net
This allows more easily creating temporary directories within a specificdirectory. This is motivated by using this in inferior-eval-with-store.
* guix/utils.scm (call-with-temporary-directory): Add optional keywordargument, base-directory.--- guix/utils.scm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Toggle diff (19 lines)diff --git a/guix/utils.scm b/guix/utils.scmindex ed1a418cca..abeb156f40 100644--- a/guix/utils.scm+++ b/guix/utils.scm@@ -620,10 +620,10 @@ call." (false-if-exception (close out)) (false-if-exception (delete-file template)))))) -(define (call-with-temporary-directory proc)+(define* (call-with-temporary-directory proc #:key base-directory) "Call PROC with a name of a temporary directory; close the directory and delete it when leaving the dynamic extent of this call."- (let* ((directory (or (getenv "TMPDIR") "/tmp"))+ (let* ((directory (or base-directory (getenv "TMPDIR") "/tmp")) (template (string-append directory "/guix-directory.XXXXXX")) (tmp-dir (mkdtemp! template))) (dynamic-wind-- 2.21.0
C
C
Christopher Baines wrote on 19 Apr 2019 16:04
[PATCH v2 4/4] inferior: Add 'open-inferior/container'.
(address . 34638@debbugs.gnu.org)
20190419140427.15183-4-mail@cbaines.net
--- guix/inferior.scm | 76 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+)
Toggle diff (110 lines)diff --git a/guix/inferior.scm b/guix/inferior.scmindex 6d18ab90e9..8238c7fb38 100644--- a/guix/inferior.scm+++ b/guix/inferior.scm@@ -19,6 +19,7 @@ (define-module (guix inferior) #:use-module (srfi srfi-9) #:use-module (srfi srfi-9 gnu)+ #:use-module (srfi srfi-11) #:use-module ((guix utils) #:select (%current-system source-properties->location@@ -40,6 +41,9 @@ #:use-module (guix store) #:use-module (guix derivations) #:use-module (guix base32)+ #:use-module (gnu system file-systems)+ #:use-module (gnu build linux-container)+ #:use-module (guix build syscalls) #:use-module (gcrypt hash) #:autoload (guix cache) (maybe-remove-expired-cache-entries) #:autoload (guix ui) (show-what-to-build*)@@ -54,6 +58,7 @@ #:use-module ((rnrs bytevectors) #:select (string->utf8)) #:export (inferior? open-inferior+ open-inferior/container port->inferior close-inferior inferior-eval@@ -137,6 +142,77 @@ it's an old Guix." ((@ (guix scripts repl) machine-repl)))))) pipe))) +(define* (open-inferior/container store guix-store-item+ #:key+ (command "bin/guix")+ (share-host-network? #f)+ (extra-shared-directories '())+ (extra-environment-variables '()))+ (define requisite-store-items+ (requisites store (list guix-store-item)))++ (define shared-directory+ (mkdtemp! (string-append (or (getenv "TMPDIR") "/tmp")+ "/guix-inferior.XXXXXX")))++ (define mappings+ (append+ (map (lambda (dir)+ (file-system-mapping+ (source dir)+ (target dir)+ (writable? #f)))+ `(;; Share a directory, used in inferior-eval-with-store+ ,shared-directory+ ,@requisite-store-items+ ,@extra-shared-directories))+ (if share-host-network?+ %network-file-mappings+ '())))++ (define mounts+ (append %container-file-systems+ (map file-system-mapping->bind-mount+ mappings)))++ (define (inferior-pipe/container store+ guix-store-item+ shared-directory+ command)+ (start-child-in-container+ (list (string-append guix-store-item "/bin/guix")+ ;; TODO I'm not sure why "repl" is duplicated in the following+ ;; command+ "repl" "repl" "-t" "machine")+ #:read? #t+ #:write? #t+ #:mounts mounts+ #:namespaces (if share-host-network?+ (delq 'net %namespaces)+ %namespaces)+ #:extra-environment-variables+ `(;; Set HOME, so that the (guix profiles) module can be loaded, without it+ ;; trying to read from /etc/passwd+ "HOME=/tmp"+ ,@extra-environment-variables)))++ (let*-values+ (((pipe pid)+ (inferior-pipe/container store+ guix-store-item+ shared-directory+ command))+ ((close-inferior-pipe)+ (lambda (pipe*)+ (unless (eq? pipe pipe*)+ (error "wrong pipe being closed"))+ (close-port pipe)+ (cdr (waitpid pid)))))++ (port->inferior pipe+ shared-directory+ close-inferior-pipe)))+ (define* (port->inferior pipe shared-directory #:optional (close close-port)) "Given PIPE, an input/output port, return an inferior that talks over PIPE. PIPE is closed with CLOSE when 'close-inferior' is called on the returned-- 2.21.0
C
C
Christopher Baines wrote on 19 Apr 2019 16:16
Re: [bug#34638] [PATCH 2/4] linux-container: Add 'start-child-in-container'.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 34638@debbugs.gnu.org)
87pnpit707.fsf@cbaines.net
Ludovic Courtès <ludo@gnu.org> writes:
Toggle quote (21 lines)> Hello!>> Christopher Baines <mail@cbaines.net> skribis:>>> This new procedure is similar to open-pipe* in (ice-9 popen), but using>> run-container from (gnu build linux-container).>>>> * gnu/build/linux-container.scm (start-child-in-container): New procedure.>> [...]>> +(define* (start-child-in-container command>> + #:key read? write?>> + (root 'temporary)>> + (mounts '())>> + (namespaces %namespaces)>> + (host-uids 1)>> + (extra-environment-variables '()))>> We could even call that ‘open-pipe/container’, for clarity.
I've made some changes (see below) that move this a little further awayfrom open-pipe in terms of behaviour now.
Toggle quote (28 lines)>> + (define (with-root-directory f)>> + (if (eq? root 'temporary)>> + (call-with-temporary-directory f)>> + (f root)))>> +>> + ;; (ice-9 popen) internals>> + (define make-rw-port (@@ (ice-9 popen) make-rw-port))>> + (define pipe-guardian (@@ (ice-9 popen) pipe-guardian))>> + (define make-pipe-info (@@ (ice-9 popen) make-pipe-info))>> So this is the funky part. ;-)>> What if we did something like:>> (call-with-container mounts> (lambda ()> ;; Somehow act as a proxy between the output process> ;; and the one spawned by ‘open-pipe*’.> (open-pipe* …)))>> ? Would that work?>> That’s create an extra process, but if it works, it’s probably safer and> a lesser maintenance burden.>> Now, I think that Guile should expose some of the popen internals> somehow so we can do things like you did, but that’s another story.
I'm hesitant to try that, as the additional process in the middle seemsa bit awkward to me.
I've made another pass over the code, removed all the uses of (ice-9popen) internals, and sent another set of patches. For the make-rw-portfunction, I just copied that over. The pipe-guardian isn't being usednow, and instead of returning a <pipe-info> record, the port and pid arereturned instead. This works with the inferior use case, as the closefunction provided to port->inferior does the right thing, closing theport and then waiting for the child process to exit, just like popen.
I'm still more interested in getting something working than it beingperfect in any particular way, but let me know what you think.
Thanks,
Chris
-----BEGIN PGP SIGNATURE-----
iQKTBAEBCgB9FiEEPonu50WOcg2XVOCyXiijOwuE9XcFAly52EhfFIAAAAAALgAoaXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNFODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcACgkQXiijOwuE9XfZTw/6A8MgyDllG42QeJT5f/qXG9mee6q8edCLQHBTytq9Mzv3RRr20uJ4jIozxc8AYRsTO8niCgU6CquAIbz1eR/koz4xo753qlo1grQYj/M71VpI5lQkukyPhcX6ZcCZPDXWxrz/wiYlbhVB813B6rP8DLo7wxK9StB3ybQk4kkI8+lR9b69nteyXa/BYoV3L8CG+jiuvyRokTcNytpj3GJjv+Xiy9hOVPe9B1Oken76kIpqmc14kV2341Ih8o+T29vuHS+0dlTz5ovVtvGp5QIK5tKBR0jZingiN60tSCqgrY7AouEGMAM+V1S4Z+QLGqk+boLKhFIZgseT3Tn66OIiO12J7SPPyLhyBLU8dUa3r8oYFVlmQFWVsiK7RDLTot0C2+MDOooCtA+Kam33IAppD1DZY3AImbMut7ZzXVwTOMdaCgzRLP8acn3XlZlURvieJkzAiQFDIKYnicbqnDFdXoH8tm997tb98giu8Ruf7YiaT1aHM5u2iGsTHh1uFEWnlaW+Y09fLFdsPhJBH4ovoESvT1TuxZqbYF3FtRTdhlQ4daUYx49Zf04+5fNdzX79mFPUXygc0PZ+v8+mQ4vwOUQS02giBS63igBWseb6ObzR/gA/mjLwRJ+wqYb7A9gUURe9aXC47AoL92lxFL+xfmTAPDFyi4LAMS1WQFtaEuk==0lHn-----END PGP SIGNATURE-----
L
L
Ludovic Courtès wrote on 26 Mar 2020 10:22
Re: [bug#34638] [PATCH v2 1/4] utils: Add #:base-directory to call-with-temporary-directory.
(name . Christopher Baines)(address . mail@cbaines.net)(address . 34638@debbugs.gnu.org)
87lfnneah8.fsf@gnu.org
Hello,
Christopher Baines <mail@cbaines.net> skribis:
Toggle quote (6 lines)> This allows more easily creating temporary directories within a specific> directory. This is motivated by using this in inferior-eval-with-store.>> * guix/utils.scm (call-with-temporary-directory): Add optional keyword> argument, base-directory.
LGTM.
L
L
Ludovic Courtès wrote on 26 Mar 2020 10:28
Re: [bug#34638] [PATCH v2 2/4] linux-container: Add 'start-child-in-container'.
(name . Christopher Baines)(address . mail@cbaines.net)(address . 34638@debbugs.gnu.org)
87ftdvea8d.fsf@gnu.org
Christopher Baines <mail@cbaines.net> skribis:
Toggle quote (5 lines)> This new procedure is similar to open-pipe* in (ice-9 popen), but using> run-container from (gnu build linux-container).>> * gnu/build/linux-container.scm (start-child-in-container): New procedure.
[...]
Toggle quote (8 lines)> +(define* (start-child-in-container command> + #:key read? write?> + (root 'temporary)> + (mounts '())> + (namespaces %namespaces)> + (host-uids 1)> + (extra-environment-variables '()))
Please add a docstring. :-)
I’d change (extra-environment-variables '()) to:
(environment-variables (environ))
I always find it too hard to reason about “extra” thing; it’s just moreconvenient as an interface to specify the whole thing rather than a listof “extras”.
Toggle quote (2 lines)> + (apply execlp command))
To provide a correct argv[0] by default, you should probably change itto:
(match command ((program arguments ...) (execlp program program arguments)))
(That’ll also address a comment of yours in one of the subsequentpatches.)
Could you add a test to ‘tests/containers.scm’?
Thanks,Ludo’.
L
L
Ludovic Courtès wrote on 26 Mar 2020 10:30
Re: [bug#34638] [PATCH v2 3/4] inferior: Add a shared-directory field to <inferior>
(name . Christopher Baines)(address . mail@cbaines.net)(address . 34638@debbugs.gnu.org)
87blojea3y.fsf@gnu.org
Christopher Baines <mail@cbaines.net> skribis:
Toggle quote (4 lines)> ---> guix/inferior.scm | 25 ++++++++++++++-----------> 1 file changed, 14 insertions(+), 11 deletions(-)
Commit log please. :-)
Toggle quote (5 lines)> + (pid inferior-pid)> + (socket inferior-socket)> + (close inferior-close-socket) ;procedure> + (shared-directory inferior-shared-directory)
Please add a margin comment like “#f | directory”.
Toggle quote (6 lines)> -(define* (port->inferior pipe #:optional (close close-port))> +(define* (port->inferior pipe shared-directory #:optional (close close-port))> "Given PIPE, an input/output port, return an inferior that talks over PIPE.> PIPE is closed with CLOSE when 'close-inferior' is called on the returned> inferior."
Make ‘shared-directory’ a keyword argument?
(Otherwise there’s a user in (guix ssh) that needs to be updated.)
Toggle quote (7 lines)> ((client . address)> (proxy client (store-connection-socket store))))> (close-port socket)> - (read-inferior-response inferior)))))> + (read-inferior-response inferior)))> + #:base-directory (inferior-shared-directory inferior)))
What if ‘inferior-shared-directory’ returns #f?
Otherwise LGTM.
Ludo’.
L
L
Ludovic Courtès wrote on 26 Mar 2020 10:32
Re: [bug#34638] [PATCH v2 4/4] inferior: Add 'open-inferior/container'.
(name . Christopher Baines)(address . mail@cbaines.net)(address . 34638@debbugs.gnu.org)
877dz7ea1g.fsf@gnu.org
Christopher Baines <mail@cbaines.net> skribis:
Toggle quote (4 lines)> ---> guix/inferior.scm | 76 +++++++++++++++++++++++++++++++++++++++++++++++> 1 file changed, 76 insertions(+)
[...]
Toggle quote (7 lines)> +(define* (open-inferior/container store guix-store-item> + #:key> + (command "bin/guix")> + (share-host-network? #f)> + (extra-shared-directories '())> + (extra-environment-variables '()))
Please add a docstring. Same comment as before regarding “extras”. :-)
Toggle quote (6 lines)> + (start-child-in-container> + (list (string-append guix-store-item "/bin/guix")> + ;; TODO I'm not sure why "repl" is duplicated in the following> + ;; command> + "repl" "repl" "-t" "machine")
This is the argv[0] issue mentioned earlier.
I think it’s not really feasible to write a test for this one, or atleast I don’t see how.
Otherwise LGTM, thanks!
Ludo’.
C
C
Christopher Baines wrote on 28 Mar 2020 12:26
Re: [bug#34638] [PATCH v2 2/4] linux-container: Add 'start-child-in-container'.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 34638@debbugs.gnu.org)
87mu80sosv.fsf@cbaines.net
Ludovic Courtès <ludo@gnu.org> writes:
Toggle quote (27 lines)> Christopher Baines <mail@cbaines.net> skribis:>>> This new procedure is similar to open-pipe* in (ice-9 popen), but using>> run-container from (gnu build linux-container).>>>> * gnu/build/linux-container.scm (start-child-in-container): New procedure.>> [...]>>> +(define* (start-child-in-container command>> + #:key read? write?>> + (root 'temporary)>> + (mounts '())>> + (namespaces %namespaces)>> + (host-uids 1)>> + (extra-environment-variables '()))>> Please add a docstring. :-)>> I’d change (extra-environment-variables '()) to:>> (environment-variables (environ))>> I always find it too hard to reason about “extra” thing; it’s just more> convenient as an interface to specify the whole thing rather than a list> of “extras”.
I had a go at this, but I think trying to copy the environment variablesfrom the host Guix to the inferior one caused problems, at least thisbacktrace appears when calling open-inferior/container and I'm guessingit comes from the inferior guix.
I think calling it environment-variables and having it be '() is OK, theonly change I can see being made elsewhere is thatopen-inferior/container adds HOME=/tmp, and that's just to avoid issueswith (guix profiles).
Does that make sense?

Backtrace: 6 (apply-smob/1 #<catch-closure 7f77e0a889a0>)In ice-9/boot-9.scm: 705:2 5 (call-with-prompt ("prompt") #<procedure 7f77e0a9f560 at ice-9/eval.scm:330:13 ()> #<procedure default-prompt-handler (k proc)>)In ice-9/eval.scm: 619:8 4 (_ #(#(#<directory (guile-user) 7f77e0717140>))) 293:34 3 (_ #(#(#<directory (guile-user) 7f77e0717140>) ("/gnu/store/ain1rvg7vrrcr85v0fgpyjm8k2sflxpz-guix-1.0.1-15.0984481/bin/.guix-real" "repl" "-t" "machi?"))) 159:9 2 (_ #(#(#<directory (guile-user) 7f77e0717140>) ("/gnu/store/ain1rvg7vrrcr85v0fgpyjm8k2sflxpz-guix-1.0.1-15.0984481/bin/.guix-real" "repl" "-t" "machi?")))In ice-9/boot-9.scm: 2803:6 1 (resolve-interface _ #:select _ #:hide _ #:prefix _ #:renamer _ #:version _)In unknown file: 0 (scm-error misc-error #f "~A ~S" ("no code for module" (guix ui)) #f)
-----BEGIN PGP SIGNATURE-----
iQKTBAEBCgB9FiEEPonu50WOcg2XVOCyXiijOwuE9XcFAl5/NHBfFIAAAAAALgAoaXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNFODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcACgkQXiijOwuE9XfXhxAAp4e9MN7waIdKAiqb+wTQZrwvNzzPutFMWJiao99q+TMI7EEoIwPwBJBiV/+cG769CtrTSvRpYi2/BjCaeFVNwGbXcXFzSMsFbFzZ9WFDkPQXI5IzU4UxbQ8/JssHQqEr+OiSVsgjcLQxYleXT8UOB9ejoLRvIiQ86QJoDDMOj8ZminlLzu9yeMR8sHojdMM79D2NvSbaiKEpzgOaWsoPKvP0B8oqm7qWyGXYsSql8Iz5/8XbCuWlwyVilftYl/8FCAfS5Emtm25+L4iRN1oTphfaD/QIa3JrPDoc/eewYS+NKsyuIGBUELwOwnIyxbbW+hEoYz35v0mT6K4YJTJiPdTCmK/N+9MAco9cQyOYkL0zBB/ZtNoNgyXJOE1r4w3dkEuN0fFEjiRH9MtPcgw5ZdsWKaqzznW9qV7fTIuo5NiRY+2wf7Zcc1WsGg6ORRnmOq89wy73fEOQUVygcxgT8T3pD6TuQ0wkOo2zfFs/QSlEZzz/QB/8iiL8D8YPhpvFwi437pGkl403BOKn8/ssCVLUWjFCV0RmWNLOVtLjaIOAU+c0DfCkPGD8mdSLOwqa7eEcZAHVO705ZIp7Lm6gYPEJ/Jk7iEK/bMBpNjAexLW7NYqWEdnalUtvlE5W/Qphf48ARQswn6qivbDsiVw0yfIZqc3fbb1XpA2wtLobovE==ThB4-----END PGP SIGNATURE-----
L
L
Ludovic Courtès wrote on 28 Mar 2020 13:20
(name . Christopher Baines)(address . mail@cbaines.net)(address . 34638@debbugs.gnu.org)
877dz44qo7.fsf@gnu.org
Hi!
Christopher Baines <mail@cbaines.net> skribis:
Toggle quote (41 lines)> Ludovic Courtès <ludo@gnu.org> writes:>>> Christopher Baines <mail@cbaines.net> skribis:>>>>> This new procedure is similar to open-pipe* in (ice-9 popen), but using>>> run-container from (gnu build linux-container).>>>>>> * gnu/build/linux-container.scm (start-child-in-container): New procedure.>>>> [...]>>>>> +(define* (start-child-in-container command>>> + #:key read? write?>>> + (root 'temporary)>>> + (mounts '())>>> + (namespaces %namespaces)>>> + (host-uids 1)>>> + (extra-environment-variables '()))>>>> Please add a docstring. :-)>>>> I’d change (extra-environment-variables '()) to:>>>> (environment-variables (environ))>>>> I always find it too hard to reason about “extra” thing; it’s just more>> convenient as an interface to specify the whole thing rather than a list>> of “extras”.>> I had a go at this, but I think trying to copy the environment variables> from the host Guix to the inferior one caused problems, at least this> backtrace appears when calling open-inferior/container and I'm guessing> it comes from the inferior guix.>> I think calling it environment-variables and having it be '() is OK, the> only change I can see being made elsewhere is that> open-inferior/container adds HOME=/tmp, and that's just to avoid issues> with (guix profiles).>> Does that make sense?
Ah yes, defaulting to the empty list is even better.
Thanks,Ludo’.
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send email to 34638@debbugs.gnu.org