[bug#63263] [PATCH] gexp: Stop generating unreadable builder scripts.

["Ludovic Courtès" <ludo@gnu.org>]

Hi,

Christopher Baines <mail@cbaines.net> skribis:

> In Guile, it's possible to produce output from write that can't be read, and
> this applies to the code staged through g-expressions for derivations.  This
> commit detects this early when the derivation is being created, rather than
> leaving the error to happen when the derivation is built.
>
> This is important as it means that tools like guix lint will indicate that
> there's a problem, hopefully reducing the number of broken derivations in
> Guix.
>
> * guix/gexp.scm (gexp->derivation): Check that the builder script can be read.

Calling ‘read’ on every generated sexp is definitely not something we
should do, performance-wise.

Commit 24ab804ce11fe12ff49cd144a3d9c4bfcf55b41c addressed that to some
extent.  It works in examples like this:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,lower (computed-file "foo" #~(list #$(current-module)))
While executing meta-command:
ERROR:
  1. &gexp-input-error: #<directory (guile-user) 7f26d5918c80>
--8<---------------cut here---------------end--------------->8---

… where ‘current-module’ returns a non-serializable object.

I think the problem you’re trying to address that we frequently
encounter is old-style packages that end up splicing gexps inside sexps,
as in:

  (package
    ;; …
    (arguments `(#:phases (modify-phases whatever ,#~doh!))))

Is that right?

The problem here is that ‘sexp->gexp’, which was added precisely as an
optimization for build systems¹, does not check the sexp it’s given.
Example:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,lower (computed-file "foo" (sexp->gexp `(list a b c ,(current-module))))
$19 = #<derivation /gnu/store/j5rgrmdzk4mic67zkal4759bcm5xbk1c-foo.drv =>  7f26baf56be0>
scheme@(guile-user)> (sexp->gexp `(list a b c ,(current-module)))
$20 = #<gexp (list a b c #<directory (guile-user) 7f26d5918c80>) 7f26bbf2f090>
--8<---------------cut here---------------end--------------->8---

Oops!

It would be tempting to change ‘sexp->gexp’ to traverse the sexp in
search of non-serializable things… but that’d defeat the whole point of
‘sexp->gexp’.

How about a linter instead, with the understanding that use of sexps in
packages is vanishing?  Perhaps coupled with a ‘guix style’ automatic
rewriter.

Thanks,
Ludo’.

¹ Packages would get long lists/trees in their ‘arguments’ field.
  Traversing them in search of lowerable objects is costly, and
  ‘sexp->gexp’ was introduced precisely do we don’t have to traverse the
  sexp.  (Gexps are designed so that no such traversal is necessary at
  run time.)