Subject: [bug#58231] [PATCH 2/2] packages: Raise an exception for invalid 'license' values.

From: Philip McGrath <[email-masked]>
Date: Sat, 15 Oct 2022 18:12:19 -0400

Hi,

On Monday, October 10, 2022 10:52:16 AM EDT Ludovic Courtès wrote:
> Hi,
> 
> zimoun <[email-masked]> skribis:
> > On sam., 01 oct. 2022 at 18:20, Ludovic Courtès <[email-masked]> wrote:
> >>  ;; A package.
> >>  (define-record-type* <package>
> >>    package make-package
> >> @@ -574,7 +607,8 @@ (define-record-type* <package>
> >>              (sanitize validate-texinfo))          ; one-line description
> >>    (description package-description
> >>                 (sanitize validate-texinfo))       ; one or two
> >>                 paragraphs
> >> -  (license package-license)                       ; (list of) <license>
> >> +  (license package-license                        ; (list of) <license>
> >> +           (sanitize validate-license))
> >>    (home-page package-home-page)
> >>    (supported-systems package-supported-systems    ; list of strings
> >>    
> >>                       (default %supported-systems))
> > 
> > This change is the core, IIUC.  The question is: does it make sense to
> > have something similar for all the fields?
> > 
> > For instance, the fields ’name’ and ’verson’ are expected to be ’string’
> > and could be similarly checked?
> 
> I think eventually we should have contracts rather than home-made type
> checks like this (Cc’ing Philip).
> 

Definitely agreed that contracts are The Right Thing, though `string?` by
itself would in fact be a contract.

I'm planning to get back to trying to write a basic `(guix contracts)` library
once I've finished my current project, a `racket-build-system` (for which I've
finally started writing code, as opposed to planning and shaving many
yaks: https://gitlab.com/philip1/guix-racket-experiment/).

I had started a fairly direct port of Racket's contract system, though with the
intention of starting out with as little as possible. I think I'd more or less
finished the representation of "blame" objects.

However, one thing that gave me pause was some of the advice I got on
<https://racket.discourse.group/t/advice-on-implementing-a-contract-system/832/2>.
Matthias Felleisen wrote:

> I will also say that as much as I like ho  [higher-order] contracts and use them
> extensively,
> making everything a function is bad for any future optimization.
> 
> Michael B[allantyne] Cameron M and I are thinking of designing a hosted contract DSL
> on top, via Michael’s special-purpose expanders, and experimenting with a
> classical optimizer, perhaps even using Leif’s nano-passes (adapted for
> syntax of course).

Robby Findler, the mastermind of Racket's contract system, later added in part:

> If I were to get to redo racket/contract, the one big change I'd make is to
> design a (macro-based) DSL for contracts instead of going with a
> combinator-based approach. Although you won't need to do it for a
> minimum-viable product, the opportunity to, at a later date, look at an
> entire contract at compile time and generate better code for it is probably
> going to be useful.
> 
> [...]
> 
> That said, you definitely want to allow arbitrary predicates to just be
> contracts without having to type a lot of stuff, as that's turned out to be
> super useful.
>
> [...]

This came as a surprise, though the explanations made sense. In Racket, I have a
reasonably good sense of how I would implement the kind of DSL they describe,
either using the framework Ballantyne, King, and Felleisen set out in
https://dl.acm.org/doi/abs/10.1145/3428297 (open-access) or manually along the
lines Alexis King explains in a series of blog posts:

 1. https://lexi-lambda.github.io/blog/2018/04/15/reimplementing-hackett-s-type-language-expanding-to-custom-core-forms-in-racket/
 2. https://lexi-lambda.github.io/blog/2018/09/13/custom-core-forms-in-racket-part-ii-generalizing-to-arbitrary-expressions-and-internal-definitions/
 3. https://lexi-lambda.github.io/blog/2018/10/06/macroexpand-anywhere-with-local-apply-transformer/

However, I have no idea how to implement such a thing in Guile, and I think it
would be arduous, perhaps even impossible, without `local-expand` and some of
the other features from "Macros that Work Together: Compile-Time Bindings,
Partial Expansion, and Definition Contexts"
(<https://www.cs.utah.edu/plt/publications/jfp12-draft-fcdf.pdf>) and related
papers.

I'm also wary of shifting the scope from a minimum viable contract library to
what would amount to a research project trying to improve upon the
state-of-the-art contract system.

So, despite this advice, I still tentatively think I'd start by doing
following the Racket contract library very closely. Still, it reinforces my
view that we should start small and initially keep the library as
`(guix contracts)` or something (though I don't think it should use any Guix
specific in its implementation), rather than trying to make a general-purpose
library, so we could change things fairly freely as we get a more concrete
sense of Guix's needs. (In contrast, Racket tries to maintain an extremely
high level of source compatibility with even twenty-year-old code.)

> However, as you write, we have to pay attention to performance in the
> case of packages as it could quickly become too expensive.  While I
> think it could make sense to have contracts for all the fields of
> service configuration records, I think we’ll have to do much less for
> <package> fields.
> 

I'm a little unsure about the specific performance hack in this patch, though.
In either Racket or Chez Scheme, I would expect the compiler constant-fold an
application of a record-type predicate to a known constant identifier. It
should be able to do this in all of the scenarios where a macro could
interpose, and possibly in additional scenarios made visible through previous
inlining etc. in the compiler. So I would expect handling special cases in a
macro to introduce compile-time cost with no run-time benefit. I would expect
this to be covered by "The Guaranteed Optimization Clause of the Macro Writer's
Bill of Rights": https://www.youtube.com/watch?v=LIEX3tUliHw

From Andy Wingo's message at
https://lists.gnu.org/archive/html/guix-devel/2022-03/msg00230.html
I've been hoping roughly the same intuitions apply to Guile records, but maybe
that's not true? For example, I'm not sure that Guile allows assignment to
imported identifiers for which no assignment appeared in their exporting
modules (but the chaos that would ensue from
`(set! license:gpl3.0+ "not a license")` seems like a great example of why
that's such a useful rule).

-Philip