Package: guix-patches;
Reported by: Philip McGrath <philip <at> philipmcgrath.com>
Date: Tue, 8 Feb 2022 15:14:01 UTC
Severity: normal
Tags: patch
Merged with 53997
Done: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Philip McGrath <philip <at> philipmcgrath.com> To: 53878 <at> debbugs.gnu.org, Liliana Marie Prikler <liliana.prikler <at> gmail.com> Subject: [bug#53878] Fwd: [PATCH v7 00/24] Update Racket to 8.4. Adjust Chez Scheme packages. Date: Wed, 27 Apr 2022 16:03:19 -0400
(Hopefully I've made the necessary debbugs incantations for this to go
through this time.)
-------- Forwarded Message --------
Subject: Re: [PATCH v7 00/24] Update Racket to 8.4. Adjust Chez Scheme
packages.
Date: Wed, 27 Apr 2022 15:52:49 -0400
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 53878 <at> debbugs.gnu.org
Hi,
On 3/4/22 17:59, Liliana Marie Prikler wrote:
> Hi Philip,
>
> Am Sonntag, dem 27.02.2022 um 16:28 -0500 schrieb Philip McGrath:
>> Rather than debate it, I'm sending a v7 with the change to patch
>> 03/24 that Liliana requested in
>> <https://issues.guix.gnu.org/53878#187>.
> Sorry for the delay. I cleaned up some of your commits and their
> messages, but apart from that pushed v7 without major changes. I'm
> marking this as done now; if you feel I've made a mistake somewhere,
> don't hesitate to reopen.
>
While getting ready for the upcoming Racket 8.5 release, I noticed some
differences between my issue 53878 v7 patch series and the commits as
applied to Guix that I hadn't noticed at the time. I don't want to make
a big a deal out of it---I'm sure everyone was acting in good faith, and
it isn't enormously consequential in the grand scheme of things---but it
took me by surprise.
Considering a diff between my v7 and the series as merged:
> diff --cc gnu/packages/racket.scm
> index 8d44241414,471a11dd48..0000000000
> --- a/gnu/packages/racket.scm
> +++ b/gnu/packages/racket.scm
> @@@ -248,7 -248,10 +248,14 @@@
> ,(string-append "CPPFLAGS=-DGUIX_RKTIO_PATCH_BIN_SH="
> #$(file-append bash-minimal "/bin/sh"))
> "--disable-strip"
> ++<<<<<<< HEAD
> + "--enable-origtree"))
> ++=======
> + ;; XXX: origtree layout is required by some other packages down the
> + ;; bootstrap chain. Remove these flags as soon as we can do without them.
> + "--enable-origtree"
> + ,(string-append "--prefix=" #$output "/opt/racket-vm")))
> ++>>>>>>> 992ed3b4ce20335ca61df0d29bfd02495dee87e6
>
This comment was what first gave me pause: I was concerned it meant
something had gone, so I went looking through the Git blame to find when
it was added, and Git blamed me.
I think the comment is not quite right, factually (or, at least, is only
true for very specific definitions of "required" and "bootstrap chain"),
and, more significantly, I disagree that it should be a goal to "remove
these flags": IMO, adding these flags brought us closer to being able to
build the Racket VM, Racket packages, and Racket installation layers in
a well-organized fashion.
> (define-public racket-vm-cgc
> ;; Eventually, it may make sense for some vm packages to not be hidden,
> @@@ -282,69 -285,36 +289,102 @@@
> #:strip-directories #~'("opt/racket-vm/bin"
> "opt/racket-vm/lib")
> #:phases
> ++<<<<<<< HEAD
> + #~(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)))))))
> + (modify-phases %standard-phases
> + (add-before 'configure 'initialize-config.rktd
> + (lambda* (#:key inputs #:allow-other-keys)
> + (define (write-racket-hash alist)
> + ;; inside must use dotted pair notation
> + (display "#hash(")
> + (for-each (match-lambda
> + ((k . v)
> + (format #t "(~s . ~s)" k v)))
> + alist)
> + (display ")\n"))
> + (define maybe-release-catalog
> + (let ((v #$(package-version this-package)))
> + (if (string-match "^[0-9]+\\.[0-9]+($|\\.[0-8][0-9]*$)"
> + v)
> + `(,(string-append
> + "https://download.racket-lang.org/releases/"
> + v
> + "/catalog/"))
> + '())))
> + (mkdir-p "racket/etc")
> + (with-output-to-file "racket/etc/config.rktd"
> + (lambda ()
> + (write-racket-hash
> + `((build-stamp . "")
> + (catalogs ,@maybe-release-catalog
> + #f)))))))
> + (add-before 'configure 'chdir
> + (lambda _
> + (chdir "racket/src")))
> + (replace 'configure
> + (wrap-racket-vm-outputs
> + (assoc-ref %standard-phases 'configure)))
> + (replace 'patch-shebangs
> + (wrap-racket-vm-outputs
> + (assoc-ref %standard-phases 'patch-shebangs)))
> + (replace 'validate-runpath
> + (wrap-racket-vm-outputs
> + (assoc-ref %standard-phases 'validate-runpath)))
> + (replace 'make-dynamic-linker-cache
> + (wrap-racket-vm-outputs
> + (assoc-ref %standard-phases 'make-dynamic-linker-cache)))
> + (replace 'patch-dot-desktop-files
> + (wrap-racket-vm-outputs
> + (assoc-ref %standard-phases 'patch-dot-desktop-files)))))))
> ++=======
> + #~(modify-phases %standard-phases
> + (add-before 'configure 'initialize-config.rktd
> + (lambda* (#:key inputs #:allow-other-keys)
> + (define (write-racket-hash alist)
> + ;; inside must use dotted pair notation
> + (display "#hash(")
> + (for-each (match-lambda
> + ((k . v)
> + (format #t "(~s . ~s)" k v)))
> + alist)
> + (display ")\n"))
> + (define maybe-release-catalog
> + (let ((v #$(package-version this-package)))
> + (if (string-match "^[0-9]+\\.[0-9]+($|\\.[0-8][0-9]*$)"
> + v)
> + `(,(string-append
> + "https://download.racket-lang.org/releases/"
> + v
> + "/catalog/"))
> + '())))
> + (mkdir-p "racket/etc")
> + (with-output-to-file "racket/etc/config.rktd"
> + (lambda ()
> + (write-racket-hash
> + `((build-stamp . "")
> + (catalogs ,@maybe-release-catalog
> + #f)))))))
> + (add-before 'configure 'chdir
> + (lambda _
> + (chdir "racket/src"))))))
> ++>>>>>>> 992ed3b4ce20335ca61df0d29bfd02495dee87e6
> (home-page "https://racket-lang.org")
> (synopsis "Old Racket implementation used for bootstrapping")
> (description "This variant of the Racket BC (``before Chez'' or
> @@@ -599,6 -569,7 +639,10 @@@ DrRacket IDE, are not included."
More concretely, removing `wrap-racket-vm-outputs` here without a
replacement means that the phases in question did not run on the
relevant files/directories. For example, this interaction illustrates
that the patches as applied don't generate a `ld.so.cache` for Racket:
```
philip <at> bastet:~$ echo v7
v7
philip <at> bastet:~$ ls $(guix time-machine
--url=https://gitlab.com/philip1/guix-patches.git
--disable-authentication
--branch=racket-chez-refactor-guix-issue-53878-v7 -- build -e "(@ (gnu
packages racket) racket-vm-cs)" 2>/dev/null | tail -n 1)/opt/racket-vm/etc
config.rktd ld.so.cache
philip <at> bastet:~$ echo as applied
as applied
philip <at> bastet:~$ ls $(guix time-machine
--commit=992ed3b4ce20335ca61df0d29bfd02495dee87e6 -- build -e "(@ (gnu
packages racket) racket-vm-cs)" 2>/dev/null | tail -n 1)/opt/racket-vm/etc
config.rktd
philip <at> bastet:~$
```
I thought we had discussed the reason for this e.g. in
<https://issues.guix.gnu.org/53878#32>.
> (inherit racket-minimal)
> (name "racket")
> (source #f)
> ++<<<<<<< HEAD
> ++=======
> + (native-inputs (list racket-minimal)) ; XXX: conservative estimate, untested
> ++>>>>>>> 992ed3b4ce20335ca61df0d29bfd02495dee87e6
> (inputs
> (list
> cairo
(For completeness, this was also a change: it really isn't a big deal,
but much more will be needed to be able to cross-compile Racket code,
and I think it would be best addressed in the context of a
'racket-build-system'.)
I certainly don't want anything that would further burden reviewers or
slow down the process (this patch series having taken 7 revisions and
almost a month as it is). For me, at least, it would have been easier to
notice these changes if they had been in their own commit (or commits),
rather than mixed in with changes that had been revised and discussed
several times.
-Philip
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.