GNU bug report logs - #63263
[PATCH] gexp: Stop generating unreadable builder scripts.

Previous Next

Full log

Message #8 received at 63263 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Christopher Baines <mail <at> cbaines.net>
Cc: 63263 <at> debbugs.gnu.org
Subject: Re: bug#63263: [PATCH] gexp: Stop generating unreadable builder
 scripts.
Date: Thu, 04 May 2023 14:47:23 +0200

Hi,

Christopher Baines <mail <at> 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.)

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.