Subject: Re: [PATCH 08/11] gnu: Add chez-scheme-for-racket.

From: Philip McGrath <[email-masked]>
Date: Wed, 16 Feb 2022 14:58:53 -0500

Hi,

On 2/14/22 10:18, Liliana Marie Prikler wrote:
> Hi,
> 
> Am Sonntag, dem 13.02.2022 um 16:51 -0500 schrieb Philip McGrath:
>> [...]
>> * gnu/packages/chez-and-racket-bootstrap.scm (racket-vm-cgc):
>> (racket-vm-bc):
>> (racket-vm-cs):
>> (chez-scheme-for-racket-bootstrap-bootfiles):
>> (chez-scheme-for-racket): New variables.
> One patch per package is probably better.

That's fine; I'll change it. (I thought that's what I'd suggested in 
<https://issues.guix.gnu.org/53878#11>.)

> 
>> [...] One way of thinking about the
>> +;; bounary between the Racket VM and Racket programs is that the VM
>> implements
> boundary.

Thanks, I'll fix it.

> 
>> +;; the primitives accessed by the 'ffi/unsafe/vm' library. Another
>> perspective
>> +;; is that DrRacket's ``Open defining file''/``Jump to definition''
>> features
>> +;; can navigate into Racket programs, including into the
>> implementation of
>> +;; 'racket/base', but can not jump into the implementation of the
>> Racket VM
>> +;; itself. A third, related perspective is that Racket code is
>> usually
>> +;; installed with source files alongside compiled code (though this
>> is not
>> +;; mandatory), whereas the Racket VM is installed only in compiled
>> form.
>> [...]
>> +;; output. The function 'racket-vm-for-system' returns the
>> recomended Racket
>> +;; VM package for a given system.
> This is a very long comment.  Consider how much of it is actually
> necessary and how much of it is not (do I really need to know about the
> capabilities of DrRacket for instance?)

I guess it would be ok to cut the explanation of the distinction between 
VM primitives and the ``built in'' collections, since the bottom line is 
that the truly primitive primitives aren't useful without the main 
collections. I'm not sure what else to cut that wouldn't leave room for 
confusion.

>> +;; (Note: since the CGC variant is basically only for bootstrapping,
>> we
>> +;; often use "BC" to mean "3M", consistent with `(banner)` and the
>> +;; suffixes used on executables when more than one variant co-
>> exists.)
>> +;;
>> +;; One remaining bootstrapping limitation is that Racket's reader,
>> module
>> +;; system, and macro expander are implemented in Racket. For Racket
>> CS,
>> +;; they are compiled to R6RS libraries as discussed above. This note
>> from the
>> +;; README file applies to all such subsystems:
>> +;;
>> +;;     The Racket version must be practically the same as the
>> current Racket
>> +;;     verson, although it can be the Racket BC implementation
>> (instead of
>> +;;     the Racket CS implementation).
>> +;;
>> +;;     Unlike Chez Scheme boot files, the files generated in
>> "schemified"
>> +;;     are human-readable and -editable Scheme code. That provides a
>> way
>> +;;     out of bootstrapping black holes, even without BC.
>> +;;
>> +;; However, other Racket subsystems implemented in Racket for Racket
>> CS
>> +;; use older C implementations for Racket BC, whereas the reader,
>> expander,
>> +;; and module system were completely replaced with the Racket
>> implementation
>> +;;
>> +;; For Racket BC, the compiled "linklet" s-expressions (primitive
>> modules)
>> +;; are embeded in C as a static string constant. Eventually, they
>> are further
>> +;; compiled by the C-implemented Racket BC bytecode and JIT
>> compilers.
>> +;; (On platforms where Racket BC's JIT is not supported, yet another
>> compiler
>> +;; instead compiles the linklets to C code, but this is not a
>> bootstrapping
>> +;; issue.)
>> +;;
> Again, you want to be brief and understandable.  What does this mean in
> practise?  Do we have racket bootstrapped yet or is there still some
> magic hidden within?

This is just the comment that is currently at the top of 
"gnu/packages/racket.scm", but moved to this file because this is now 
going to be where the bootstrapping happens. Are there specific thing 
you want to cut?

On the current state of bootstrapping, almost everything is bootstrapped 
from C, but the "expander" subsystem (which includes the reader and the 
module system) is not currently bootstrappable, though it is readily 
auditable. I made another attempt at an explanation in this email: 
https://lists.gnu.org/archive/html/guix-devel/2021-08/msg00103.html I'd 
welcome suggestions to improve the explanation!

>>   (define unbundle-chez-submodules
>>     #~(begin
>>         (use-modules (guix build utils))
>>         (for-each (lambda (dir)
>> -                (when (directory-exists? dir)
>> -                  (delete-file-recursively dir)))
>> -              '("stex"
>> -                "nanopass"
>> -                "lz4"
>> -                "zlib"))))
>> +                  (when (directory-exists? dir)
>> +                    (delete-file-recursively dir)))
>> +                '("stex"
>> +                  "nanopass"
>> +                  "lz4"
>> +                  "zlib"))))
> As in one of your previous patches, you're mixing cosmetic changes with
> non-cosmetic ones.  This one could be prevented by correctly indenting
> it in the patch that introduces it.

Sorry, I missed this in a previous round of indentation fixing.

>> +;;
>> +;; Racket VM:
>> +;;
>> +
>> +(define (racket-vm-common-configure-flags)
>> +  ;; under a lambda extraction to avoid evaluating bash-minimal too
>> early
>> +  #~`(,@(cond
>> +         ((false-if-exception
>> +           (search-input-file %build-inputs "/bin/libtool"))
>> +          => (lambda (libtool)
>> +               (list (string-append "--enable-lt=" libtool))))
>> +         (else
>> +          '()))
>> +      ,@(cond
>> +         ((false-if-exception
>> +           (search-input-file %build-inputs "/opt/racket-
>> vm/bin/racket"))
> Did we have /opt/racket... before?  We should probably avoid such
> paths.

We did not have "opt/racket-vm/" before---adding it was sort of the 
point of this patch series.

Is the reason to avoid it a dislike for "opt", or something else?

An ``in place'' build of Racket is not meant to be unpacked directly 
into some PREFIX where it will coexist with other software. The build is 
vaguely FHS-like in that e.g. it includes a "bin" directory, but it also 
e.g. has a "collects" directory at the top level, and it puts other 
things in paths like "etc/config.rktd" rather than "etc/racket/config.rktd".

Subdirectories of "opt" often have that sort of layout, so it seemed 
like a reasonable place to put it. I considered just embracing Guix not 
being tied to FHS and putting it directly in the store output, but in 
build-side code it turned out to be useful to be able to use 
`search-input-{file,directory}` without potentially confusing the 
in-place VM with an intermediate, potentially tethered layer.

If the question is, ``why do we want an in-place build?'', I can go into 
as much depth as you want, but it makes the build-side code easier to 
reason about, it's more compatible with Racket tools e.g. for 
cross-compilation, and it should help to reduce closure sizes by letting 
us build packages with non-tethered intermediate layers.

>> +          => (lambda (racket)
>> +               (list (string-append "--enable-racket=" racket))))
>> +         (else
>> +          '()))
>> +      ,(string-append "CPPFLAGS=-DGUIX_RKTIO_PATCH_BIN_SH="
>> +                      #$(file-append bash-minimal "/bin/sh"))
>> +      "--disable-strip"
>> +      "--enable-origtree")
> For the record, why do you need double quoting here?  Would ungexp-
> splicing extract this too soon?

I'm not 100% sure I'm following your question correctly, but yes, there 
were problems with referencing `bash-minimal` too early.

(An alternative I considered was to add these arguments in a wrapper 
around gnu-build-system's configure phase, so that #:configure-flags 
would be only for arguments that variants want to override, or plausibly 
might.)

> 
>> +(define-public racket-vm-cgc
>> +  ;; Eventually, it may make sense for some vm packages to not be
>> hidden,
>> +  ;; but this one is especially likely to remain hidden.
>> +  (hidden-package
>> +   (package
>> +     (name "racket-vm-cgc")
>> +     (version "8.4")
>> +     ;; ^ Remember to also update the version of
>> +     ;;   chez-scheme-for-racket-bootstrap-bootfiles
>> +     (source
>> +      (origin
>> +        (method git-fetch)
>> +        (uri (git-reference
>> +              (url "https://github.com/racket/racket")
>> +              (commit (string-append "v" version))))
>> +        (sha256
>> +         (base32
>> "1vpl66gdgc8rnldmn8rmb7ar9l057jqjvgpfn29k57i3c5skr8s6"))
>> +        (file-name (git-file-name "racket" version))
>> +        (patches (search-patches "racket-minimal-sh-via-rktio.patch"
>> +                                 ;; Remove by Racket 8.5:
>> +                                 "racket-enable-scheme-
>> backport.patch"))
>> +        (modules '((guix build utils)))
>> +        (snippet
>> +         #~(begin
>> +             ;; Unbundle Chez submodules.
>> +             (with-directory-excursion "racket/src/ChezScheme"
>> +               #$unbundle-chez-submodules)
>> +             ;; Unbundle libffi.
>> +             (delete-file-recursively
>> "racket/src/bc/foreign/libffi")))))
>> +     (inputs (list ncurses ;; <- common to all variants (for
>> #%terminal)
>> +                   bash-minimal ;; <- common to all variants (for
>> `system`)
>> +                   libffi)) ;; <- only for BC variants
>> +     (native-inputs (list libtool)) ;; <- only for BC variants
>> +     (outputs '("out" "debug"))
>> +     (build-system gnu-build-system)
>> +     (arguments
>> +      (list
>> +       #:configure-flags
>> +       #~(cons "--enable-cgcdefault"
>> +               #$(racket-vm-common-configure-flags))
>> +       ;; Tests are in packages like racket-test-core and
>> +       ;; main-distribution-test that aren't part of the main
>> +       ;; distribution.
>> +       #:tests? #f
>> +       ;; Upstream recommends #:out-of-source?, and it does
>> +       ;; help with debugging, but it confuses `install-license-
>> files`.
>> +       #:modules '((ice-9 match)
>> +                   (ice-9 regex)
>> +                   (guix build gnu-build-system)
>> +                   (guix build utils))
>> +       #:strip-directories #~'("opt/racket-vm/bin"
>> +                               "opt/racket-vm/lib")
>> +       #:phases
>> +       #~(let ()
>> +           (define* ((wrap-racket-vm-outputs phase) . args)
>> +             (apply
>> +              phase
>> +              (let loop ((args args))
>> +                (match args
>> +                  ((#:outputs outputs . args)
>> +                   `(#:outputs
>> +                     ,(let loop ((outputs outputs))
>> +                        (match outputs
>> +                          ((("out" . out) . outputs)
>> +                           `(("out" . ,(string-append out
>> "/opt/racket-vm/"))
>> +                             ,@outputs))
>> +                          ((other . outputs)
>> +                           (cons other (loop outputs)))))
>> +                     ,@args))
>> +                  ((arg . args)
>> +                   (cons arg (loop args)))))))
> Why?
> 

Why what? Why 'wrap-racket-vm-outputs'? The wrapped phases don't have 
keywords like #:strip-directories, so adjusting their #:output argument 
is the only way to tell them where to find the files they need to 
operate on.

-Philip