GNU bug report logs - #26730
[PATCH] Fix bzip2 utilities

Previous Next

Full log

Message #29 received at 26730 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Christopher Baines <mail <at> cbaines.net>, 26730 <at> debbugs.gnu.org
Subject: Re: bug#26730: [PATCH 1/2] gnu: bzip2: Use 'modify-phases' syntax.
Date: Mon, 15 May 2017 17:43:06 +0200

[Message part 1 (text/plain, inline)]

Christopher Baines <mail <at> cbaines.net> writes:

> * gnu/packages/compression.scm (bzip2)[arguments]: Use 'modify-phases' syntax.

Thanks a lot for sorting this out! I only have a few nitpicks:

> +         (replace 'configure
> +           (lambda* (#:key target #:allow-other-keys)
> +             (if ,(%current-target-system)
> +                 ;; Cross-compilation: use the cross tools.
> +                 (substitute* (find-files "." "Makefile")
> +                   (("CC=.*$")
> +                    (string-append "CC = " target "-gcc\n"))
> +                   (("AR=.*$")
> +                    (string-append "AR = " target "-ar\n"))
> +                   (("RANLIB=.*$")
> +                    (string-append "RANLIB = " target "-ranlib\n"))
> +                   (("^all:(.*)test" _ prerequisites)
> +                    ;; Remove 'all' -> 'test' dependency.
> +                    (string-append "all:" prerequisites "\n"))))))

Noob question: What is returned here when (%current-target-system) is
false? Can we make it more explicit? We try to make sure all phases end
on a #t.

> +         (add-after 'install 'install-shared-lib
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (let* ((out    (assoc-ref outputs "out"))
> +                    (libdir (string-append out "/lib")))
> +               (for-each (lambda (file)
> +                           (let ((base (basename file)))
> +                             (format #t "installing `~a' to `~a'~%"
> +                                     base libdir)
> +                             (copy-file file
> +                                        (string-append libdir "/" base))))
> +                         (find-files "." "^libbz2\\.so"))))))

Similarly, if you send an updated patch, can you add a #t at the end of
this phase, since "for-each" has an unspecified return value? Otherwise
I can do so in a follow-up commit.

Apart from these "added-value" nitpicks LGTM. Tricky one!

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.