[bug#47153] [PATCH v2] gnu: chez-scheme: Update nanopass to 1.9.2.

[Leo Prikler]

Hi,

Am Dienstag, den 16.03.2021, 16:19 -0400 schrieb Philip McGrath:
> Hi,
> 
> On 3/16/21 3:37 AM, Leo Prikler wrote:
> > Am Montag, den 15.03.2021, 18:53 -0400 schrieb Philip McGrath:
> > > -      (sha256 (base32
> > > "1synadgaycca39jfx525975ss9y0lkl516sdrc62wrrllamm8n21"))
> > > +      (sha256
> > > "16vjsik9rrzbabbhbxbaha51ppi3f9n8rk59pc6zdyffs0vziy4i")
> > You're inadvertently stripping away base32.
> 
> I thought I'd read that explicitly calling base32 was redundant and
> no 
> longer recommended: is there a reason to keep it?
Without a reference to where you've read that, it'll be hard to verify
or falsify that claim.  Both master and core-updates regularly see
sha256/base32 is fancy syntax around hash-digest and hash-algo as far
as I understand, so I doubt you recall this correctly.

> > > -            (commit (string-append "v" version))))
> > > +            ;; This commit includes a fix for which we would
> > > +            ;; otherwise want to use a snippet.
> > > +            ;; When there's a new tagged release,
> > > +            ;; go back to using (string-append "v" version)
> > > +            (commit
> > > "54051494434a197772bf6ca5b4e6cf6be55f39a5")))
> > Could we then not cherry-pick this commit as a patch?  Or is there
> > more
> > needed?
> 
> We could, but the upstream history is simply v1.2.2 -> my patch ->
> Kent 
> Dybvig's merge commit accepting it. I thought doing it this way 
> clarified that it's not a Guix-specific patch that should stay
> around 
> indefinitely. Is there a reason to prefer cherry-picking it as a
> patch?
You'll probably hear differing opinions about that, and that's fine,
but my personal reason to prefer cherry-picking would be, that it makes
it very obvious, what changed from the base – that being the patch
modulo offset changes – and doesn't invite people to go out saying
"aha, but I found this commit and I like that more, let's take it".  In
other words, this is very subjective, but I believe we should stick as
close to releases as is reasonable.

> > > +                          ;; pre-built bootfiles for unsupported
> > > systems:
> > > +                          "boot/a6nt"
> > > +                          "boot/a6osx"
> > > +                          "boot/i3nt"
> > > +                          "boot/i3osx"
> > > +                          "boot/ta6nt"
> > > +                          "boot/ta6osx"
> > > +                          "boot/ti3nt"
> > > +                          "boot/ti3osx")))))))
> > What about pre-built bootfiles for supported systems?  Do we still
> > need
> > those?  If we so, I don't think it is right to delete anything in
> > boot;
> > if not we should delete it altogether.
> 
> Currently, you need a bootfile for your current system to bootstrap
> the 
> Chez compiler; once you have a running Chez, you can build bootfiles
> for 
> any system Chez supports (which is more systems than it includes in
> its 
> git repository for bootstrapping). The bootfiles you build will be 
> byte-for-byte identical to pre-built ones, if pre-built ones were 
> provided: Chez in fact builds them twice and errors if they aren't.
> So, 
> I'm not sure why we would want these files, which are over 20 MB and
> are 
> only useful for bootstrapping Chez on Windows or Mac OS. But if there
> is 
> a reason to want them, we can keep them!
I don't think we can necessarily trust the boot files in this
configuration.  "They are bit-for-bit identical" can also mean, that
the login() hack was successfully applied for all you know.  But
trusting trust aside, I don't think this is the right way of
"miraculously slashing" half of chez' bootstrap.  If you do want to
follow the direction of making the bootstrap explicit, how about a
chez-boot minipackage, that only keeps the relevant boot files for the
target system?  (This would of course need to be done in a separate
patch, which you can attach to this series, however.)

> > > +               (apply invoke
> > > +                      "./configure"
> > > +                      flags)
> > This can very likely be one line.  Also, it should probably be done
> > via
> > configure-flags.
> > 
> > > [outputs]: Add "stex"
> > I don't think an stex output is justified in and of itself, or is
> > it
> 
> Oops, I'd actually removed the "stex" output, but I missed it in the 
> commit message.
I do think I saw it in the actual commit as well, but I might be
mistaken about that.

> > > +                    (flags (if (assoc-ref inputs "ncurses")
> > > +                               flags
> > > +                               (cons "--disable-curses"
> > > +                                     flags)))
> > > +                    (flags (if (assoc-ref inputs "libx11")
> > > +                               flags
> > > +                               (cons "--disable-x11"
> > > +                                     flags))))
> > These will probably be detected by the build system itself, there's
> > no
> > need for you to add them.  If not, then it's up to the packager of
> > other variants to specify them, not you.
> 
> IIRC, Chez's build system errors without these flags if the library 
> isn't found. Also, I intend to be "the packager of the other 
> variants"—my primary motivation for sending these patches is to reuse
> as 
> much as possible for Racket's fork of Chez, rather than all of the
> messy 
> copy-pasting we're doing now. But see below …
Fair enough, but either way you should just cons them onto #:configure-
flags where applicable.

> > > +                    (flags (list
> > > +                            (string-append "--installprefix="
> > > out)
> > > +                            (string-append "ZLIB=" zlib-static
> > > "/lib/libz.a")
> > > +                            (string-append "LZ4=" lz4-static
> > > "/lib/liblz4.a")
> > > +                            "--nogzip-man-pages" ;; guix will do
> > > it
> > > +                            "--threads"))
> > This should be done via #:configure-flags.
> > 
> > Now that I think of it…
> > 
> > > +         (replace 'configure
> > I don't think this is needed at all.  At most, you should filter
> > the
> > incompatible flags from configure-flags in some way, but you might
> > also
> > want to patch configure, so that it swallows them.
> 
> I'm not enthusiastic about the idea of maintaining such a patch for
> two 
> variants of this custom configure script which accept different sets
> of 
> flags, but neither of which accepts *any* of the flags that would be 
> added by the 'configure phase from %standard-phases. That procedure 
> offers no way to interpose between its adding the flags and its
> invoking 
> the configure script, so I don't see a good alternative to replacing
> it. 
> In fact, I'd modeled my implementation on the one from %standard-
> phases, 
> which similarly has a big `let*` expression adding implicit phases
> based 
> on the inputs and outputs.
> 
> If you think it's important to support #:configure-flags, I could
> change 
> this implementation to accept them, in which case I'd be ok with
> leaving 
> out handling of --disable-curses and --disable-x11. But I think
> anyone 
> packaging a variant of Chez will need to be aware of its
> idiosyncratic 
> configure script.
Perhaps I was a little too harsh.  If you need to replace 'configure to
not let gnu-build-system's own flags pass into this idiosyncratic
script, you are of course allowed to do that.  I just think storing
configure flags inside the argument that's named like that is a better
choice, than an ad-hoc construction.  Whatever you see inside that
standard-phase is done precisely because storing it as an argument
would not be an option.

> > > Refactor 'install-doc' phase into 'build+install-stex' and
> > > 'build+install-doc'
> > 'build+install' implies, that it should actually be two phases,
> > build
> > and install.  I already talked about install-stex, so you should
> > only
> > refactor install-doc insofar as it is needed to accommodate changes
> > in
> > the build system.
> 
> It could be two phases, but the current install-doc phase does, in
> fact, 
> also build the docs, in addition to installing them. (It actually 
> neglects to install the HTML release notes but installs two copies
> of 
> the PDF user's guide.) I'm ok with calling it install-doc if you
> prefer. 
> I guess I could even split it into two phases, but that seems like
> it 
> would just make things more complicated for no obvious benefit.
That is irrelevant, `make install' is not prohibited from building
stuff.

> As I mentioned, build+install-stex actually just "installs" stex to
> a 
> temporary directory right now. I think I could probably skip it and
> rely 
> on Chez's on-the-fly compilation, but I didn't see a problem with
> doing 
> it this way.
Sounds like it might work.

Regards,
Leo