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

From: Leo Prikler <[email-masked]>
Date: Tue, 16 Mar 2021 08:37:56 +0100

Hi,

Am Montag, den 15.03.2021, 18:53 -0400 schrieb Philip McGrath:
> -      (sha256 (base32
> "1synadgaycca39jfx525975ss9y0lkl516sdrc62wrrllamm8n21"))
> +      (sha256
> "16vjsik9rrzbabbhbxbaha51ppi3f9n8rk59pc6zdyffs0vziy4i")
You're inadvertently stripping away base32.

> -      (sha256 (base32
> "1q5i8pf4cdfjsj6r2k1rih7ljbfggyxdng2p2fvsgarzihpsin2i"))
> +      (sha256
> "01jnvw8qw33gnpzwrakwhsr05h6b609lm180jnspcrb7lds2p23d")
Same here.

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

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

> +         ;; put these where configure expects them to be
> +         (add-after 'unpack 'unpack-nanopass+stex
> +           (lambda* (#:key native-inputs inputs #:allow-other-keys)
> +             (let ((patch-source-shebangs
>                      (assoc-ref %standard-phases 'patch-source-
> shebangs)))
I don't think you need patch-source-shebangs directly after unpack. 
Those shebangs would be patched in their phase anyway – the only reason
we needed it before was because we delayed unpacking until configure.

> +               (setenv "CC" "gcc")
This should be determined via cc-for-target.

> +               (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
needed for chez to function?  (Even if it was, it would then need to be
part of the "out" output.)  I personally doubt this; if you need stex
itself for your purposes, you can try packaging it later.

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

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

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

Regards,
Leo