Early detection of derivations with unreadable builder scripts

Christopher Baines wrote on 8 Mar 2023 15:41

Recipients:(address . bug-guix@gnu.org)

Message-ID:87bkl3qovn.fsf@cbaines.net

Currently it's quite easy to end up with packages that have builder

scripts that can't be read by Guile.

This is part of the following builder script:

(cons "--enable-mpi-java" #<gexp gnu/packages/mpi.scm:233:24 7f366e0cd930>)

from: /gnu/store/yngxnpcs4s6y8acxf4nwx5pcpj0j6q6i-java-openmpi-4.1.4-builder

And when attempting to build that derivation, you get the following

error.

ice-9/read.scm:126:4: In procedure read-expr*:

/gnu/store/yngxnpcs4s6y8acxf4nwx5pcpj0j6q6i-java-openmpi-4.1.4-builder:1:3820: Unknown # object: "#<"

It would be nice if Guix could detect this category of problems and

raise an error at the time the derivation is created, rather than the

error occuring only when you build the derivation.

This would be helpful particularly for the Guix Data Service since

currently it ends up storing these useless derivations, often many times

since the builder includes some often changing string (7f366e0cd930 in

the example above), so this is a common cause of spurious changes

between revisions (as often noted on qa.guix.gnu.org).

Josselin Poiret wrote on 9 Mar 2023 21:51

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)

Message-ID:875yb9d4wo.fsf@jpoiret.xyz

Hi Chris,

Christopher Baines <mail@cbaines.net> writes:

Toggle quote (23 lines)> This is part of the following builder script:
>
>   (cons "--enable-mpi-java" #<gexp  gnu/packages/mpi.scm:233:24 7f366e0cd930>)
>
> from: /gnu/store/yngxnpcs4s6y8acxf4nwx5pcpj0j6q6i-java-openmpi-4.1.4-builder
>
> And when attempting to build that derivation, you get the following
> error.
>
>   ice-9/read.scm:126:4: In procedure read-expr*:
>   /gnu/store/yngxnpcs4s6y8acxf4nwx5pcpj0j6q6i-java-openmpi-4.1.4-builder:1:3820: Unknown # object: "#<"
>
>
> It would be nice if Guix could detect this category of problems and
> raise an error at the time the derivation is created, rather than the
> error occuring only when you build the derivation.
>
> This would be helpful particularly for the Guix Data Service since
> currently it ends up storing these useless derivations, often many times
> since the builder includes some often changing string (7f366e0cd930 in
> the example above), so this is a common cause of spurious changes
> between revisions (as often noted on qa.guix.gnu.org).

We could probably modify sexp->string, or the builder bind in
gexp->derivation so that the sexp is sanity-checked for non-printable
things (we could even work on a whitelist basis).  However, the
docstring of sexp->string talks about performance, and indeed "write" is
pure C code and very fast.  I'd be reluctant to introduce a performance
hit that would be too heavy here.

This particular example though was caused by non-gexp #:phase arguments,
so another option could be to sanity check sexps given to sexp->gexp,
but again, the docstring talks about performance, so I'm not sure what
we should do here.  In general, things written only with G-Exps should
work well, because you can't insert random stuff into them, but S-Exps
are more dangerous, hence why I think this option would be a better
middle ground.

Paging Ludo wrt. the performance cost of this (I can write a patch for
it adding a whitelist of what is allowed in a sexp->gexp sexp).

Best,
-- 
Josselin Poiret

Your comment

Commenting via the web interface is currently disabled.

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

To respond to this issue using the mumi CLI, first switch to it

mumi current 62051

Then, you may apply the latest patchset in this issue (with sign off)

mumi am -- -s

Or, compose a reply to this issue

mumi compose

Or, send patches to this issue

mumi send-email *.patch

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date