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

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

Hi,

Philip McGrath <philip@philipmcgrath.com> skribis:

> This commit bootstraps the Racket compiler and runtime system from source,
> including Racket CS as well as both variants of Racket BC. (One remaining
> limitation is discussed in comments added to gnu/packages/racket.scm.)
>
> In the process, it moves to building minimal Racket from the Git repository,
> rather than the packaged source tarballs. The Git repository is slightly
> better as the ``corresponding source'':
>
>  1. A few packages especially closely tied to the Racket core implementation
>     (like "compiler-lib", "base", and "racket-doc") are developed in the
>     same Git repository. Having them use the same Guix origin, too, will
>     help to keep them in sync.
>
>  2. The top-level Makefile in the Git repository is an important
>     ``script[] used to control compilation and installation.''
>     In particular, it cooperates with the "distro-build" package to
>     create the source tarballs and installers for a Racket distribution.
>     (Racket supports a notion of custom distributions.)
>
>  3. It is ``the preferred form ... for making modifications'' to the core
>     Racket implementation.
>
> Racket releases are tagged in the Git repository (e.g. "v8.1"). At the
> beginning of each release cycle, a branch is created to stabalize a version
> for extra testing. Active development happens on the "master" branch.
>
> * gnu/packages/racket-next-minimal-sh-via-rktio.patch: New file, coppied
> from racket-sh-via-rktio.patch to accomodate an extra directory layer.
> When racket-next-minimal becomes racket-minimal, this version will be
> the only one needed.
> * gnu/local.mk (dist_patch_DATA): Add it.
> * gnu/local/racket.scm (cfg-flag:sh-for-rktio, cfg-flag:enable-lt,
> cfg-flag:enable-racket, unpack-nanopass+stex,
> %main-repo-main-distribution-pkgs): New private variables.
> * gnu/local/racket.scm (racket-next-minimal)[source]: Use Git.
> [source](snippet): Unbundle nanopass, stex, and libffi.
> [inputs]: List explicitly.
> [native-inputs]: Use racket-next-bootstrap-bootfiles, plus its
> dependencies (for Chez, plus a Racket for bootstrappig).
> [arguments]: Revise extensively.
> * gnu/local/racket.scm (racket-next-minimal-bc-3m,
> racket-next-minimal-bc-cgc): New packages, hidden at least for now.
> (racket-next-bootstrap-chez-bootfiles): Another new package, but this one
> is especially likely to stay hidden.
> * gnu/local/racket.scm (racket-next)[origin](snippet): Unbundle packages
> developed in the main Git repository, but leave their links.rktd and
> pkgs.rktd entries in place.
> [native-inputs]: Add the main Racket Git repository.
> [arguments](#:phases): Adjust 'unpack-packages to also unpack package
> sources from the main Racket Git repository.

Exciting!

[...]

> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://github.com/racket/racket")
> +             (commit "0874b76de4f147ada46607857d8acf8445a1073d")))
>         (sha256
>          (base32
> -         "0dm849wvlaxpfgz2qmgy2kwdslyi515rxn1m1yff38lagbn21vxq"))
> -       (uri (string-append %pre-release-installers
> -                           "racket-minimal-src.tgz"))))))
> +         "0gy6rwyrpaij5k5pcyiif821b4vffqiaxg1vpg4iykw2c5ypfp43"))
> +       (file-name
> +        (git-file-name name version))
> +       (patches
> +        (search-patches
> +         "racket-next-minimal-sh-via-rktio.patch"))

Please keep these on a single line, as in:

  (file-name (git-file-name name version))

> +         (replace 'configure
> +           (let ((inner (assq-ref %standard-phases 'configure)))
> +             (lambda args
> +               (chdir "racket/src")
> +               (apply inner args))))

I’d find it clearer like this:

  (add-before 'configure 'change-directory
    (lambda _
      (chdir "racket/src")))

> +         (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’ …”).

> +(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?

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

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.

> +     (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" …))

(It matters notably because ‘guix style’ recognizes it:
<https://issues.guix.gnu.org/49169>.)

> +     (synopsis "Minimal Racket with the BC [3M] runtime system")
> +     (description "The Racket BC (``before Chez'' or ``bytecode'') implementation was the default before Racket 8.0. It uses a compiler written in C targeting architecture-independent bytecode, plus a JIT compiler on most platforms. Racket BC has a different C API than the newer runtune system (Racket CS) supports a slightly different set of architectures than the current runtime system, Racket CS (based on ``Chez Scheme'').
> +
> +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).  :-)

Please also check what ‘guix lint’ thinks!

> +(define-public racket-next-minimal-bc-cgc
> +  (package/inherit racket-next-minimal-bc-3m
> +    (name "racket-next-minimal-bc-cgc")
> +    (native-inputs
> +     (filter (match-lambda
> +               (("racket" . _)
> +                #f)
> +               (_
> +                #t))
> +             (package-native-inputs racket-next-minimal-bc-3m)))

Rather: (alist-delete "racket" (package-native-inputs racket-next-minimal-bc-3m))

> +     (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.

Thanks for all this!

Ludo’.