[PATCH] system: Remove spec->file-system.

[PATCH] system: Remove spec->file-system.

[David Craven]

* gnu/system/file-systems.scm (spec->file-system): Remove variable.
* gnu/system/linux-container.scm (container-script): Refactor.
---
 gnu/system/file-systems.scm    | 11 -----------
 gnu/system/linux-container.scm |  6 ++----
 2 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/gnu/system/file-systems.scm b/gnu/system/file-systems.scm
index 4cc1221..b51d57f 100644
--- a/gnu/system/file-systems.scm
+++ b/gnu/system/file-systems.scm
@@ -40,7 +40,6 @@
             file-system-dependencies
 
             file-system->spec
-            spec->file-system
             specification->file-system-mapping
             uuid
 
@@ -108,16 +107,6 @@ initrd code."
     (($ <file-system> device title mount-point type flags options _ _ check?)
      (list device title mount-point type flags options check?))))
 
-(define (spec->file-system sexp)
-  "Deserialize SEXP, a list, to the corresponding <file-system> object."
-  (match sexp
-    ((device title mount-point type flags options check?)
-     (file-system
-       (device device) (title title)
-       (mount-point mount-point) (type type)
-       (flags flags) (options options)
-       (check? check?)))))
-
 (define (specification->file-system-mapping spec writable?)
   "Read the SPEC and return the corresponding <file-system-mapping>.  SPEC is
 a string of the form \"SOURCE\" or \"SOURCE=TARGET\".  The former specifies
diff --git a/gnu/system/linux-container.scm b/gnu/system/linux-container.scm
index 24e61c3..0146df1 100644
--- a/gnu/system/linux-container.scm
+++ b/gnu/system/linux-container.scm
@@ -81,8 +81,7 @@ MAPPINGS is a list of <file-system> objects that specify the files/directories
 that will be shared with the host system."
   (let* ((os           (containerized-operating-system os mappings))
          (file-systems (filter file-system-needed-for-boot?
-                               (operating-system-file-systems os)))
-         (specs        (map file-system->spec file-systems)))
+                               (operating-system-file-systems os))))
 
     (mlet* %store-monad ((os-drv (operating-system-derivation
                                   os
@@ -94,10 +93,9 @@ that will be shared with the host system."
                                   (gnu build linux-container)))
           #~(begin
               (use-modules (gnu build linux-container)
-                           (gnu system file-systems) ;spec->file-system
                            (guix build utils))
 
-              (call-with-container (map spec->file-system '#$specs)
+              (call-with-container #$file-systems
                 (lambda ()
                   (setenv "HOME" "/root")
                   (setenv "TMPDIR" "/tmp")
-- 
2.9.0

Re: [PATCH] system: Remove spec->file-system.

[David Craven]

The reason I'm suggesting to remove this function, is because it seems
like any case one would want to use it, one should not be using it.
There is a separation of gnu/system and gnu/build and a
spec->file-system function encourages people to try using the
<file-system> in the build part. I think that it's a "code smell" as
the one instance where it was used is dubious.

Re: [PATCH] system: Remove spec->file-system.

[Ludovic Courtès]

David Craven <david@craven.ch> skribis:

> * gnu/system/file-systems.scm (spec->file-system): Remove variable.
> * gnu/system/linux-container.scm (container-script): Refactor.

[...]

> --- a/gnu/system/linux-container.scm
> +++ b/gnu/system/linux-container.scm
> @@ -81,8 +81,7 @@ MAPPINGS is a list of <file-system> objects that specify the files/directories
>  that will be shared with the host system."
>    (let* ((os           (containerized-operating-system os mappings))
>           (file-systems (filter file-system-needed-for-boot?
> -                               (operating-system-file-systems os)))
> -         (specs        (map file-system->spec file-systems)))
> +                               (operating-system-file-systems os))))
>  
>      (mlet* %store-monad ((os-drv (operating-system-derivation
>                                    os
> @@ -94,10 +93,9 @@ that will be shared with the host system."
>                                    (gnu build linux-container)))
>            #~(begin
>                (use-modules (gnu build linux-container)
> -                           (gnu system file-systems) ;spec->file-system
>                             (guix build utils))
>  
> -              (call-with-container (map spec->file-system '#$specs)
> +              (call-with-container #$file-systems

AFAICS that doesn’t work because <file-system> and records in general
are not automatically marshalled/unmarshalled when staging code.  Thus,
the above would leave to an invalid on-disk s-expression like this:

  (begin
    (use-modules …)
    (call-with-container (#<<file-system> …> #<<file-system> …>)
      …))

Manually calling ‘file-system->spec’ on the host side, and then
‘spec->file-system’ on the build side is a way to explicitly
marshall/unmarshall the record (I mentioned it as a current limitation
of gexps in my Scheme Workshop talk).

Makes sense?

Thanks,
Ludo’.

Re: [PATCH] system: Remove spec->file-system.

[David Craven]

Hi Ludo,

Ah yes, that makes sense. Thank you for explaining. I think I'm
understanding the general design pattern better:

Build side code that uses a record from gnu/system is a gexp in
gnu/system. This gexp is passed to a function in gnu/build so that
gnu/build itself doesn't need to import gnu/system and can avoid
recursive imports.

David

Re: [PATCH] system: Remove spec->file-system.

[Ludovic Courtès]

Hi!

David Craven <david@craven.ch> skribis:

> Ah yes, that makes sense. Thank you for explaining. I think I'm
> understanding the general design pattern better:
>
> Build side code that uses a record from gnu/system is a gexp in
> gnu/system. This gexp is passed to a function in gnu/build so that
> gnu/build itself doesn't need to import gnu/system and can avoid
> recursive imports.

One of the goals of using Scheme on both sides is code reuse.

However, things like (guix build utils) are inherently “build side”,
hence the name.  In addition, it wouldn’t make sense to pull (guix
store), (guix packages), and friends on the “build side”, because these
things cannot be used there since the build environment doesn’t give
access to the daemon (IOW, a build process cannot talk to the daemon,
it’s not recursive.)

Thus, the (guix build …) module space also serves as a way to
distinguish things and avoid pulling all of the Guix modules unwillingly
on the build side.  It introduces some sort of a frontier between the
build side and the host side (info "(guix) G-Expressions").

But borders are unbearable and sometimes there’s useful stuff that we
want to use freely on both sides, like this <file-system> record type.

:-)

Ludo’.