[bug#49280] [PATCH 0/4] gnu: racket: Add racket-next. Bootstrap from C.

[Philip McGrath]

Hi!

I've been mostly offline for a bit, and Racket 8.2 was released today (a 
little ahead of schedule), so I will rework this patch series to just 
update to 8.2 and not deal with adding "-next" variants for now. I'll 
respond to here, though, to keep the discussion together.

On 7/8/21 5:25 PM, Ludovic Courtès wrote:
> Philip McGrath <philip@philipmcgrath.com> skribis:
>
> > * gnu/packages/racket.scm (racket-next-minimal,racket-next): New variables.
>
> [...]
>
> > +++ b/gnu/packages/racket.scm
> > @@ -23,6 +23,7 @@
> >     #:use-module ((guix licenses)
> >                   #:select (asl2.0 expat lgpl3+))
> >     #:use-module (guix packages)
> > +  #:use-module (guix base16)
>
> Leftover?

Yes, thanks!

> > +;;   - `racket-pkg-` should probably be the prefix for Racket packages
> > +;;     available as Guix packages, once we're able to build those.
> > +;;     More specifically, it should correspond
> > +;;     to packages registered in the catalog at https://pkgs.rackat-lang.org.
> > +;;     This is a social convention to manage the namespace, not a technical
> > +;;     limitation: Racket can use other catalogs (e.g. for pre-built packages
> > +;;     or packages pinned to specific versions), unregistered package source
> > +;;     urls, or purely local packages. But we also need a convention to
> > +;;     manage the namespace, so we should use this one. In practice,
> > +;;     all generally useful libre Racket packages are registered there.
> > +;;     We probably will need a clever encoding scheme to deal with the fact
> > +;;     that Racket package names can contain [A-Za-z_-], i.e. including "_",
> > +;;     which is not allowed in Guix package names.
>
> For this there’s already a documented convention (info "(guix) Package
> Naming"), although part of it is undocumented.  The prefix would rather
> be “racket-” to match what we do with other packages–“ghc-”, “ocaml-”,
> “guile-”, and so forth.

I wrote these as statements in the hope of eliciting any disagreement :)

The problem I see with using just “racket-” as the prefix is the 
potential for collisions, especially because Racket uses a lot of the 
namespace: for example, "_" is a useful example package for testing 
package issues, and I maintain the "_-exp" package. There don't seem to 
be Racket packages named "minimal" or "next" right now, but they seem 
reasonably likely to be used in the future, and Guix likewise may want 
to add packages that don't correspond directly to a single Racket-level 
package. (In fact, I think this may be necessary to build Racket 
packages with mutually recursive dependencies.) Other Racket package 
names that I think might be less confusing if prefixed with 
“racket-pkg-” include "base", "racket-lib", "unstable", "profile", 
"make", "data", "images", "compiler", "compatibility", "pkg-build", and 
"main-distribution".

But we don't need to resolve this now, and maybe actually implementing 
that support will clarify what issues really do or don't exist. I will 
just remove this whole comment for now, since I don't need to make a 
choice between "racket-next-minimal" and "racket-minimal-next".


> > +(define %pre-release-installers
> > +  "https://pre-release.racket-lang.org/installers/";)
> > +
> > +(define-public racket-next-minimal
> > +  (package
> > +    (inherit racket-minimal)
> > +    (name "racket-next-minimal")
> > +    (version "8.1.900")
> > +    (source
> > +     (origin
> > +       (inherit (package-source racket-minimal))
> > +       (sha256
> > +        (base32
> > +         "0dm849wvlaxpfgz2qmgy2kwdslyi515rxn1m1yff38lagbn21vxq"))
> > +       (uri (string-append %pre-release-installers
> > +                           "racket-minimal-src.tgz"))))))
> > +
> > +(define-public racket-next
> > +  (package
> > +    (inherit racket)
> > +    (name "racket-next")
> > +    (version (package-version racket-next-minimal))
> > +    (source
> > +     (origin
> > +       (inherit (package-source racket))
> > +       (sha256
> > +        (base32
> > +         "0ysvzgm0lx4b1p4k9balvcbvh2kapbfx91c9ls80ba062cd8y5qv"))
> > +       (uri (string-append %pre-release-installers
> > +                           "racket-src.tgz"))))))
>
> Do I get it right that *-src.tgz are not versioned?  That they’re
> updated in place regularly?
>
> In that case, we cannot refer to them in a package definition since the
> hash is bound to become stale.
>
> What we could do is refer to, say,
> <https://pre-release.racket-lang.org/installers/racket-8.1.900.1-src.tgz>.
> However, I suspect this file would vanish fairly quickly from the web
> site, which is not okay either.
>
> I’m not sure what a good solution would be.  WDYT?
>
> It may be that
> ‘--with-source=https://pre-release.racket-lang.org/installers/racket-src.tgz’
> would do the job for those who’re into that.

This is also a good catch!

For now, I will avoid the problem by just not dealing with "-next" variants.

For posterity: while working on this patch series before the release, I 
faced a similar issue, because the "snapshot" builds explicitly are not 
retained indefinitely. As a work-around, I based my work on snapshots 
from Northwestern University (as opposed to the University of Utah), 
because they retain one snapshot per week for a few months. For the 
longer term, rather than using the tarballs directly, I used them to 
produce patch files, which I checked into Guix. Since minimal Racket 
could be build from Git, I could restrict the patch to main-distribution 
Racket package sources, which kept the size manageable.

Something analogous would probably work for release candidates, but the 
right long-term solution is for Guix to be able to build Racket packages 
directly, so we don't have to rely on particular snapshot bundles.


On 7/8/21 5:43 PM, Ludovic Courtès wrote:
> I’d find it clearer like this:
>
>    (add-before 'configure 'change-directory
>      (lambda _
>        (chdir "racket/src")))

Ah, that's nice.

>
>> +         (add-after 'install 'remove-pkgs-directory
>> +           ;; otherwise, e.g., `raco pkg show` will try and fail to
>> +           ;; create a lock file
>> +           (lambda* (#:key outputs #:allow-other-keys)
>> +             ;; rmdir because we want an error if it isn't empty
>> +             (rmdir (string-append (assoc-ref outputs "out")
>> +                                   "/share/racket/pkgs"))
>> +             #t)))))
>
> Please write full sentences with a bit more context (“Remove package
> directory, otherwise ‘raco pkg show’ …”).

Will do.

>> +(define-public racket-next-minimal-bc-3m
>> +  (hidden-package
>> +   (package/inherit racket-next-minimal
>> +     (name "racket-next-minimal-bc-3m")
>
> This is “-next” because it’s targeting 8.1, which is not released yet,
> right?

Correct, but 8.2 (8.1 was released in May). Now that it's been released, 
the name would be "racket-minimal-bc-3m".

> Since it’s only used for bootstrapping, perhaps use ‘define’ instead of
> ‘define-public’ and remove the call to ‘hidden-package’.

In addition to bootstrapping, there are three reasons I know of to want 
Racket BC:

 1. The BC and CS implementations have different C APIs, so some
    low-level code may support BC but not CS. But this isn't usually a
    good reason. Racket packages should support both implementations.
    Embedding applications ideally would also be portable: if it's
    only feasible to support one implementation, it should be CS.

 2. Comparing the BC and CS implementations can be useful for testing
    and debugging, both for packages that use the FFI and when hacking
    on the Racket runtime system itself.

 3. Most importantly, BC supports some architectures that CS does not.

In particular, Racket CS does not (yet) support ppc64le, which Racket BC 
does support. The recommendation to packagers, and what Debian does, is
to explicitly use BC on platforms without CS support: 
https://github.com/racket/racket/issues/3773#issuecomment-832935403

I'm not sure what the most idiomatic way to do this is in Guix.

(Just for the record, Racket CS also supports platforms which Racket BC 
supports only partially---without the JIT, places, or futures---or does 
not support at all. One motivation of Racket CS was to make porting 
easier in general.)

>
> It should also be (package (inherit …) …) rather than (package/inherit
> …).  The latter is only useful when defining variants of a package (same
> version, same code) where the same security updates would apply.

I don't think I understand this very well. Setting aside “-next”-related 
issues, a given commit in the Racket source repository will be used to 
build CGC, 3M, and CS (the default) variants with the same version---at 
least in the Racket senses of “version” and “variant”. It's possible 
that there could be a VM-specific security issue, but usually a bug in 
Racket, security-related or otherwise, will affect all three variants.

>> +     (inputs
>> +      `(("libffi" ,libffi) ;; <- only for BC variants
>> +        ,@(filter (match-lambda
>> +                    ((label . _)
>> +                     (not (member label
>> +                                  '("zlib" "zlib:static"
>> +                                    "lz4" "lz4:static")))))
>> +                  (package-inputs racket-next-minimal))))
>
> Please use this more common idiom:
>
>    ,@(fold alist-delete (package-inputs racket-next-minimal) '("zlib" …))

Thanks, I was looking for something like `alist-delete` but didn't find it.

>> +This packackage is the normal implementation of Racket BC with a 
precise garbage collector, 3M (``Moving Memory Mana
>          ^
> Typo here, and lines too long (here and in other places).  :-)

Thanks, usually I have Emacs set up to catch that.

>> +     (license (package-license chez-scheme)))))
>
> You cannot do that since here since potentially we could end up with
> circular top-level references from these two modules.
>
> Instead, restate what the license is.

Ok, I'd been lulled into complacency by the implicitly thunked fields.

- Philip