bug#26730: [PATCH] Fix bzip2 utilities

bug#26730: [PATCH] Fix bzip2 utilities

[Christopher Baines]

[-- Attachment #1.1: Type: text/plain, Size: 3995 bytes --]

The bzip2 package includes wrappers around diff, grep and less/more.
These shell scripts currently include /usr/bin (and sometimes /bin) on
the PATH, and therefore fail if any of the commands that they rely on
cannot be found.

By substituting /usr/bin (and /bin) for the appropriate package paths,
these scripts work much more reliably.

I had some issues with bzmore which uses more from util-linux, I think
there might be a circular dependency in the Guix package tree, as I get
a VM stack overflow when including util-linux as a input to bzip2.

Note that as the bzip2 package is used by so many other packages, any
non-superficial change to it probably means rebuilding the world.

Below is a script, and two invocations of it, showing first what happens
with the unaltered package, and then the package with the modifications
I describe above.


#!/bin/sh
set -x
bzip2 < /etc/shells > shells.bz2

bzcmp shells.bz2 shells.bz2
bzdiff shells.bz2 shells.bz2
bzegrep bash shells.bz2
bzfgrep bash shells.bz2
bzgrep bash shells.bz2
bzless shells.bz2
bzmore shells.bz2


→ PATH=/gnu/store/s3c442d075fc8a0q0nspc9jjsgjq613p-bzip2-1.0.6/bin
./test.sh
+ bzip2
+ bzcmp shells.bz2 shells.bz2
/gnu/store/s3c442d075fc8a0q0nspc9jjsgjq613p-bzip2-1.0.6/bin/bzcmp: line
16: sed: command not found
/gnu/store/s3c442d075fc8a0q0nspc9jjsgjq613p-bzip2-1.0.6/bin/bzcmp: line
40: mktemp: command not found
cannot create a temporary file
+ bzdiff shells.bz2 shells.bz2
/gnu/store/s3c442d075fc8a0q0nspc9jjsgjq613p-bzip2-1.0.6/bin/bzdiff: line
16: sed: command not found
/gnu/store/s3c442d075fc8a0q0nspc9jjsgjq613p-bzip2-1.0.6/bin/bzdiff: line
40: mktemp: command not found
cannot create a temporary file
+ bzegrep bash shells.bz2
/gnu/store/s3c442d075fc8a0q0nspc9jjsgjq613p-bzip2-1.0.6/bin/bzegrep:
line 11: sed: command not found
/gnu/store/s3c442d075fc8a0q0nspc9jjsgjq613p-bzip2-1.0.6/bin/bzegrep:
line 43: sed: command not found
/gnu/store/s3c442d075fc8a0q0nspc9jjsgjq613p-bzip2-1.0.6/bin/bzegrep:
line 63: grep: command not found
+ bzfgrep bash shells.bz2
/gnu/store/s3c442d075fc8a0q0nspc9jjsgjq613p-bzip2-1.0.6/bin/bzfgrep:
line 11: sed: command not found
/gnu/store/s3c442d075fc8a0q0nspc9jjsgjq613p-bzip2-1.0.6/bin/bzfgrep:
line 43: sed: command not found
/gnu/store/s3c442d075fc8a0q0nspc9jjsgjq613p-bzip2-1.0.6/bin/bzfgrep:
line 63: grep: command not found
+ bzgrep bash shells.bz2
/gnu/store/s3c442d075fc8a0q0nspc9jjsgjq613p-bzip2-1.0.6/bin/bzgrep: line
11: sed: command not found
/gnu/store/s3c442d075fc8a0q0nspc9jjsgjq613p-bzip2-1.0.6/bin/bzgrep: line
43: sed: command not found
/gnu/store/s3c442d075fc8a0q0nspc9jjsgjq613p-bzip2-1.0.6/bin/bzgrep: line
63: grep: command not found
+ bzless shells.bz2
/gnu/store/s3c442d075fc8a0q0nspc9jjsgjq613p-bzip2-1.0.6/bin/bzless: line
8: sed: command not found
------> shells.bz2 <------
/gnu/store/s3c442d075fc8a0q0nspc9jjsgjq613p-bzip2-1.0.6/bin/bzless: line
55: more: command not found
+ bzmore shells.bz2
/gnu/store/s3c442d075fc8a0q0nspc9jjsgjq613p-bzip2-1.0.6/bin/bzmore: line
8: sed: command not found
------> shells.bz2 <------
/gnu/store/s3c442d075fc8a0q0nspc9jjsgjq613p-bzip2-1.0.6/bin/bzmore: line
55: more: command not found


→ PATH=/gnu/store/i3zkljbz6ryqdlfc9gnpcjg964inp9l3-bzip2-1.0.6/bin
./test.sh
+ bzip2
+ bzcmp shells.bz2 shells.bz2
+ bzdiff shells.bz2 shells.bz2
+ bzegrep bash shells.bz2
/run/current-system/profile/bin/bash
/gnu/store/nylrl843mkfdwzz8cd5iabsib37vqc1j-bash-4.4.A/bin/bash
+ bzfgrep bash shells.bz2
/run/current-system/profile/bin/bash
/gnu/store/nylrl843mkfdwzz8cd5iabsib37vqc1j-bash-4.4.A/bin/bash
+ bzgrep bash shells.bz2
/run/current-system/profile/bin/bash
/gnu/store/nylrl843mkfdwzz8cd5iabsib37vqc1j-bash-4.4.A/bin/bash
+ bzless shells.bz2
------> shells.bz2 <------
+ bzmore shells.bz2
------> shells.bz2 <------
/gnu/store/i3zkljbz6ryqdlfc9gnpcjg964inp9l3-bzip2-1.0.6/bin/bzmore: line
55: more: command not found



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 858 bytes --]

bug#26730: [PATCH 1/2] gnu: bzip2: Use 'modify-phases' syntax.

[Christopher Baines]

* gnu/packages/compression.scm (bzip2)[arguments]: Use 'modify-phases' syntax.
---
 gnu/packages/compression.scm | 130 +++++++++++++++++++------------------------
 1 file changed, 58 insertions(+), 72 deletions(-)

diff --git a/gnu/packages/compression.scm b/gnu/packages/compression.scm
index 4793755c2..031ecaad4 100644
--- a/gnu/packages/compression.scm
+++ b/gnu/packages/compression.scm
@@ -207,84 +207,70 @@ file; as a result, it is often used in conjunction with \"tar\", resulting in
    (home-page "https://www.gnu.org/software/gzip/")))
 
 (define-public bzip2
-  (let ((build-shared-lib
-         ;; Build a shared library.
-         '(lambda* (#:key inputs #:allow-other-keys)
-            (patch-makefile-SHELL "Makefile-libbz2_so")
-            (zero? (system* "make" "-f" "Makefile-libbz2_so"))))
-        (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")))))
-        (set-cross-environment
-         '(lambda* (#:key target #:allow-other-keys)
-            (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"))))))
-    (package
-      (name "bzip2")
-      (version "1.0.6")
-      (source (origin
-               (method url-fetch)
-               (uri (string-append "http://www.bzip.org/" version "/bzip2-"
-                                   version ".tar.gz"))
-               (sha256
-                (base32
-                 "1kfrc7f0ja9fdn6j1y6yir6li818npy6217hvr3wzmnmzhs8z152"))))
-      (build-system gnu-build-system)
-      (arguments
-       `(#:modules ((guix build gnu-build-system)
-                    (guix build utils)
-                    (srfi srfi-1))
-         #:phases
-         ,(if (%current-target-system)
-
-              ;; Cross-compilation: use the cross tools.
-              `(alist-cons-before
-                'build 'build-shared-lib ,build-shared-lib
-                (alist-cons-after
-                 'install 'install-shared-lib ,install-shared-lib
-                 (alist-replace 'configure ,set-cross-environment
-                                %standard-phases)))
-
-              ;; Native compilation: build the shared library.
-              `(alist-cons-before
-                'build 'build-shared-lib ,build-shared-lib
-                (alist-cons-after
-                 'install 'install-shared-lib ,install-shared-lib
-                 (alist-delete 'configure %standard-phases))))
+  (package
+    (name "bzip2")
+    (version "1.0.6")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "http://www.bzip.org/" version "/bzip2-"
+                                  version ".tar.gz"))
+              (sha256
+               (base32
+                "1kfrc7f0ja9fdn6j1y6yir6li818npy6217hvr3wzmnmzhs8z152"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:modules ((guix build gnu-build-system)
+                  (guix build utils)
+                  (srfi srfi-1))
+       #:phases
+       (modify-phases %standard-phases
+         (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"))))))
+         (add-before 'build 'build-shared-lib
+           (lambda* (#:key inputs #:allow-other-keys)
+             (patch-makefile-SHELL "Makefile-libbz2_so")
+             (zero? (system* "make" "-f" "Makefile-libbz2_so"))))
+         (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"))))))
 
-         #:make-flags (list (string-append "PREFIX="
-                                           (assoc-ref %outputs "out")))
+       #:make-flags (list (string-append "PREFIX="
+                                         (assoc-ref %outputs "out")))
 
-         ;; Don't attempt to run the tests when cross-compiling.
-         ,@(if (%current-target-system)
-               '(#:tests? #f)
-               '())))
-      (synopsis "High-quality data compression program")
-      (description
-       "bzip2 is a freely available, patent free (see below), high-quality data
+       ;; Don't attempt to run the tests when cross-compiling.
+       ,@(if (%current-target-system)
+             '(#:tests? #f)
+             '())))
+    (synopsis "High-quality data compression program")
+    (description
+     "bzip2 is a freely available, patent free (see below), high-quality data
 compressor.  It typically compresses files to within 10% to 15% of the best
 available techniques (the PPM family of statistical compressors), whilst
 being around twice as fast at compression and six times faster at
 decompression.")
-      (license (license:non-copyleft "file://LICENSE"
-                                  "See LICENSE in the distribution."))
-      (home-page "http://www.bzip.org/"))))
+    (license (license:non-copyleft "file://LICENSE"
+                                   "See LICENSE in the distribution."))
+    (home-page "http://www.bzip.org/")))
 
 (define-public lbzip2
   (package
-- 
2.12.2

bug#26730: [PATCH 2/2] gnu: bzip2: Patch bzip2 utilities.

[Christopher Baines]

* gnu/packages/compression.scm (bzip2)[inputs]: Add grep, less, diffutils, sed
  and coreutils.
  [arguments]: Add patch-script phase.
---
 gnu/packages/compression.scm | 44 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/compression.scm b/gnu/packages/compression.scm
index 031ecaad4..fe32b95bd 100644
--- a/gnu/packages/compression.scm
+++ b/gnu/packages/compression.scm
@@ -46,6 +46,7 @@
   #:use-module (gnu packages backup)
   #:use-module (gnu packages base)
   #:use-module (gnu packages check)
+  #:use-module (gnu packages less)
   #:use-module (gnu packages perl)
   #:use-module (gnu packages pkg-config)
   #:use-module (gnu packages python)
@@ -218,6 +219,12 @@ file; as a result, it is often used in conjunction with \"tar\", resulting in
                (base32
                 "1kfrc7f0ja9fdn6j1y6yir6li818npy6217hvr3wzmnmzhs8z152"))))
     (build-system gnu-build-system)
+    (inputs
+     `(("grep" ,grep)
+       ("less" ,less)
+       ("diff" ,diffutils)
+       ("sed" ,sed)
+       ("coreutils" ,coreutils)))
     (arguments
      `(#:modules ((guix build gnu-build-system)
                   (guix build utils)
@@ -252,7 +259,42 @@ file; as a result, it is often used in conjunction with \"tar\", resulting in
                                      base libdir)
                              (copy-file file
                                         (string-append libdir "/" base))))
-                         (find-files "." "^libbz2\\.so"))))))
+                         (find-files "." "^libbz2\\.so")))))
+         (add-after 'install-shared-lib 'patch-scripts
+           (lambda* (#:key outputs inputs #:allow-other-keys)
+             (let* ((out       (assoc-ref outputs "out"))
+                    (grep      (assoc-ref inputs "grep"))
+                    (less      (assoc-ref inputs "less"))
+                    (diff      (assoc-ref inputs "diff"))
+                    (sed       (assoc-ref inputs "sed"))
+                    (coreutils (assoc-ref inputs "coreutils")))
+               (substitute* (string-append out "/bin/bzgrep")
+                 (("/usr/bin:\\$PATH")
+                  (string-join
+                   (list (string-append grep "/bin")
+                         (string-append out "/bin")
+                         (string-append sed "/bin"))
+                   ":")))
+               (substitute* (string-append out "/bin/bzmore")
+                 (("/usr/bin") ;; Don't remove $PATH, as if bzmore is to work,
+                               ;; more must be on the PATH in the
+                               ;; environment. util-linux, which contains more
+                               ;; is not included here as there is a potential
+                               ;; issues with circular dependencies.
+                  (string-join
+                   (list (string-append less "/bin")
+                         (string-append sed "/bin")
+                         (string-append out "/bin"))
+                   ":")))
+               (substitute* (string-append out "/bin/bzdiff")
+                 (("/usr/bin:/bin:\\$PATH")
+                  (string-join
+                   (list (string-append diff "/bin")
+                         (string-append coreutils "/bin")
+                         (string-append out "/bin")
+                         (string-append sed "/bin"))
+                   ":"))
+                 (("/bin/rm") "rm"))))))
 
        #:make-flags (list (string-append "PREFIX="
                                          (assoc-ref %outputs "out")))
-- 
2.12.2

bug#26730: [PATCH 1/2] gnu: bzip2: Use 'modify-phases' syntax.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 1982 bytes --]

Christopher Baines <mail@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!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26730: [PATCH] Fix bzip2 utilities

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 914 bytes --]

Christopher Baines <mail@cbaines.net> writes:

> The bzip2 package includes wrappers around diff, grep and less/more.
> These shell scripts currently include /usr/bin (and sometimes /bin) on
> the PATH, and therefore fail if any of the commands that they rely on
> cannot be found.
>
> By substituting /usr/bin (and /bin) for the appropriate package paths,
> these scripts work much more reliably.

Most of these dependencies are available in environments where the bz*
tools will be executed. I think it would be better to simply remove the
absolute /usr/bin and /bin references such that grep, sed etc
invocations are picked up from PATH instead.

The "xz*" equivalent tools seem to do that. The rationale being that
bzip2 is often needed early in bootstrapping, and adding those inputs
would complicate the dependency tree. Although I admittedly haven't
looked at how bzip2 is used in Guix bootstrap. Thoughts?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26730: [PATCH] Fix bzip2 utilities

[Ludovic Courtès]

Marius Bakke <mbakke@fastmail.com> skribis:

> Christopher Baines <mail@cbaines.net> writes:
>
>> The bzip2 package includes wrappers around diff, grep and less/more.
>> These shell scripts currently include /usr/bin (and sometimes /bin) on
>> the PATH, and therefore fail if any of the commands that they rely on
>> cannot be found.
>>
>> By substituting /usr/bin (and /bin) for the appropriate package paths,
>> these scripts work much more reliably.
>
> Most of these dependencies are available in environments where the bz*
> tools will be executed. I think it would be better to simply remove the
> absolute /usr/bin and /bin references such that grep, sed etc
> invocations are picked up from PATH instead.

Agreed.  Otherwise we could end up retaining references to the bootstrap
Bash and Coreutils, for instance (remember that bzip2 is built early on,
in (gnu packages commencement)).

Ludo’.

bug#26730: [PATCH] Fix bzip2 utilities

[Christopher Baines]

[-- Attachment #1.1: Type: text/plain, Size: 1043 bytes --]

On 09/05/17 11:18, Ludovic Courtès wrote:
> Marius Bakke <mbakke@fastmail.com> skribis:
> 
>> Christopher Baines <mail@cbaines.net> writes:
>>
>>> The bzip2 package includes wrappers around diff, grep and less/more.
>>> These shell scripts currently include /usr/bin (and sometimes /bin) on
>>> the PATH, and therefore fail if any of the commands that they rely on
>>> cannot be found.
>>>
>>> By substituting /usr/bin (and /bin) for the appropriate package paths,
>>> these scripts work much more reliably.
>>
>> Most of these dependencies are available in environments where the bz*
>> tools will be executed. I think it would be better to simply remove the
>> absolute /usr/bin and /bin references such that grep, sed etc
>> invocations are picked up from PATH instead.
> 
> Agreed.  Otherwise we could end up retaining references to the bootstrap
> Bash and Coreutils, for instance (remember that bzip2 is built early on,
> in (gnu packages commencement)).

Ok, I've sent a couple of updates for the 2nd patch.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 858 bytes --]

bug#26730: [PATCH] gnu: bzip2: Patch bzip2 utilities.

[Christopher Baines]

* gnu/packages/compression.scm (bzip2)[inputs]: Add grep, less, diffutils, sed
  and coreutils.
  [arguments]: Add patch-script phase.
---
 gnu/packages/compression.scm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/compression.scm b/gnu/packages/compression.scm
index 7d3a62e2f..829fd5053 100644
--- a/gnu/packages/compression.scm
+++ b/gnu/packages/compression.scm
@@ -252,7 +252,12 @@ file; as a result, it is often used in conjunction with \"tar\", resulting in
                                      base libdir)
                              (copy-file file
                                         (string-append libdir "/" base))))
-                         (find-files "." "^libbz2\\.so"))))))
+                         (find-files "." "^libbz2\\.so")))))
+         (add-after 'install-shared-lib 'patch-scripts
+           (lambda* (#:key outputs inputs #:allow-other-keys)
+             (let* ((out (assoc-ref outputs "out")))
+               (substitute* (string-append out "/bin/bzdiff")
+                 (("/bin/rm") "rm"))))))
 
        #:make-flags (list (string-append "PREFIX="
                                          (assoc-ref %outputs "out")))
-- 
2.13.0

bug#26730: [PATCH] gnu: bzip2: Patch bzip2 utilities.

[Christopher Baines]

* gnu/packages/compression.scm (bzip2)[arguments]: Add patch-script phase to
  remove absolute reference to /bin/rm.
---
 gnu/packages/compression.scm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/compression.scm b/gnu/packages/compression.scm
index 7d3a62e2f..829fd5053 100644
--- a/gnu/packages/compression.scm
+++ b/gnu/packages/compression.scm
@@ -252,7 +252,12 @@ file; as a result, it is often used in conjunction with \"tar\", resulting in
                                      base libdir)
                              (copy-file file
                                         (string-append libdir "/" base))))
-                         (find-files "." "^libbz2\\.so"))))))
+                         (find-files "." "^libbz2\\.so")))))
+         (add-after 'install-shared-lib 'patch-scripts
+           (lambda* (#:key outputs inputs #:allow-other-keys)
+             (let* ((out (assoc-ref outputs "out")))
+               (substitute* (string-append out "/bin/bzdiff")
+                 (("/bin/rm") "rm"))))))
 
        #:make-flags (list (string-append "PREFIX="
                                          (assoc-ref %outputs "out")))
-- 
2.13.0

bug#26730: [PATCH] gnu: bzip2: Patch bzip2 utilities.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 445 bytes --]

Christopher Baines <mail@cbaines.net> writes:

> * gnu/packages/compression.scm (bzip2)[arguments]: Add patch-script phase to
>   remove absolute reference to /bin/rm.

Thanks for catching this! (substitute* ...) has an unspecified return
value, so I'll add a #t at the end of this new phase before pushing.

Waiting for feedback on the modify-phases patch first. We still have
~two weeks until core-updates starts rolling again, so no rush ;-)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26730: [PATCH] gnu: bzip2: Patch bzip2 utilities.

[Christopher Baines]

[-- Attachment #1.1: Type: text/plain, Size: 633 bytes --]

On 15/05/17 16:46, Marius Bakke wrote:
> Christopher Baines <mail@cbaines.net> writes:
> 
>> * gnu/packages/compression.scm (bzip2)[arguments]: Add patch-script phase to
>>   remove absolute reference to /bin/rm.
> 
> Thanks for catching this! (substitute* ...) has an unspecified return
> value, so I'll add a #t at the end of this new phase before pushing.
> 
> Waiting for feedback on the modify-phases patch first. We still have
> ~two weeks until core-updates starts rolling again, so no rush ;-)

I've resent the patches with these changes, just in case that helps.

Thanks for your quick feedback,

Chris



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 858 bytes --]

bug#26730: [PATCH] gnu: bzip2: Patch bzip2 utilities.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 721 bytes --]

Christopher Baines <mail@cbaines.net> writes:

> On 15/05/17 16:46, Marius Bakke wrote:
>> Christopher Baines <mail@cbaines.net> writes:
>> 
>>> * gnu/packages/compression.scm (bzip2)[arguments]: Add patch-script phase to
>>>   remove absolute reference to /bin/rm.
>> 
>> Thanks for catching this! (substitute* ...) has an unspecified return
>> value, so I'll add a #t at the end of this new phase before pushing.
>> 
>> Waiting for feedback on the modify-phases patch first. We still have
>> ~two weeks until core-updates starts rolling again, so no rush ;-)
>
> I've resent the patches with these changes, just in case that helps.

Pushed to 'core-updates', thank you! Note: I squashed the first two.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26730: [PATCH] gnu: bzip2: Patch bzip2 utilities.

[Christopher Baines]

[-- Attachment #1.1: Type: text/plain, Size: 817 bytes --]

On 17/05/17 15:58, Marius Bakke wrote:
> Christopher Baines <mail@cbaines.net> writes:
> 
>> On 15/05/17 16:46, Marius Bakke wrote:
>>> Christopher Baines <mail@cbaines.net> writes:
>>>
>>>> * gnu/packages/compression.scm (bzip2)[arguments]: Add patch-script phase to
>>>>   remove absolute reference to /bin/rm.
>>>
>>> Thanks for catching this! (substitute* ...) has an unspecified return
>>> value, so I'll add a #t at the end of this new phase before pushing.
>>>
>>> Waiting for feedback on the modify-phases patch first. We still have
>>> ~two weeks until core-updates starts rolling again, so no rush ;-)
>>
>> I've resent the patches with these changes, just in case that helps.
> 
> Pushed to 'core-updates', thank you! Note: I squashed the first two.

Great, thanks for your review :)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 858 bytes --]

bug#26730: [PATCH 1/3] gnu: bzip2: Use 'modify-phases' syntax.

[Christopher Baines]

* gnu/packages/compression.scm (bzip2)[arguments]: Use 'modify-phases' syntax.
---
 gnu/packages/compression.scm | 130 +++++++++++++++++++------------------------
 1 file changed, 58 insertions(+), 72 deletions(-)

diff --git a/gnu/packages/compression.scm b/gnu/packages/compression.scm
index 11db2a66f..7d3a62e2f 100644
--- a/gnu/packages/compression.scm
+++ b/gnu/packages/compression.scm
@@ -207,84 +207,70 @@ file; as a result, it is often used in conjunction with \"tar\", resulting in
    (home-page "https://www.gnu.org/software/gzip/")))
 
 (define-public bzip2
-  (let ((build-shared-lib
-         ;; Build a shared library.
-         '(lambda* (#:key inputs #:allow-other-keys)
-            (patch-makefile-SHELL "Makefile-libbz2_so")
-            (zero? (system* "make" "-f" "Makefile-libbz2_so"))))
-        (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")))))
-        (set-cross-environment
-         '(lambda* (#:key target #:allow-other-keys)
-            (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"))))))
-    (package
-      (name "bzip2")
-      (version "1.0.6")
-      (source (origin
-               (method url-fetch)
-               (uri (string-append "http://www.bzip.org/" version "/bzip2-"
-                                   version ".tar.gz"))
-               (sha256
-                (base32
-                 "1kfrc7f0ja9fdn6j1y6yir6li818npy6217hvr3wzmnmzhs8z152"))))
-      (build-system gnu-build-system)
-      (arguments
-       `(#:modules ((guix build gnu-build-system)
-                    (guix build utils)
-                    (srfi srfi-1))
-         #:phases
-         ,(if (%current-target-system)
-
-              ;; Cross-compilation: use the cross tools.
-              `(alist-cons-before
-                'build 'build-shared-lib ,build-shared-lib
-                (alist-cons-after
-                 'install 'install-shared-lib ,install-shared-lib
-                 (alist-replace 'configure ,set-cross-environment
-                                %standard-phases)))
-
-              ;; Native compilation: build the shared library.
-              `(alist-cons-before
-                'build 'build-shared-lib ,build-shared-lib
-                (alist-cons-after
-                 'install 'install-shared-lib ,install-shared-lib
-                 (alist-delete 'configure %standard-phases))))
+  (package
+    (name "bzip2")
+    (version "1.0.6")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "http://www.bzip.org/" version "/bzip2-"
+                                  version ".tar.gz"))
+              (sha256
+               (base32
+                "1kfrc7f0ja9fdn6j1y6yir6li818npy6217hvr3wzmnmzhs8z152"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:modules ((guix build gnu-build-system)
+                  (guix build utils)
+                  (srfi srfi-1))
+       #:phases
+       (modify-phases %standard-phases
+         (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"))))))
+         (add-before 'build 'build-shared-lib
+           (lambda* (#:key inputs #:allow-other-keys)
+             (patch-makefile-SHELL "Makefile-libbz2_so")
+             (zero? (system* "make" "-f" "Makefile-libbz2_so"))))
+         (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"))))))
 
-         #:make-flags (list (string-append "PREFIX="
-                                           (assoc-ref %outputs "out")))
+       #:make-flags (list (string-append "PREFIX="
+                                         (assoc-ref %outputs "out")))
 
-         ;; Don't attempt to run the tests when cross-compiling.
-         ,@(if (%current-target-system)
-               '(#:tests? #f)
-               '())))
-      (synopsis "High-quality data compression program")
-      (description
-       "bzip2 is a freely available, patent free (see below), high-quality data
+       ;; Don't attempt to run the tests when cross-compiling.
+       ,@(if (%current-target-system)
+             '(#:tests? #f)
+             '())))
+    (synopsis "High-quality data compression program")
+    (description
+     "bzip2 is a freely available, patent free (see below), high-quality data
 compressor.  It typically compresses files to within 10% to 15% of the best
 available techniques (the PPM family of statistical compressors), whilst
 being around twice as fast at compression and six times faster at
 decompression.")
-      (license (license:non-copyleft "file://LICENSE"
-                                  "See LICENSE in the distribution."))
-      (home-page "http://www.bzip.org/"))))
+    (license (license:non-copyleft "file://LICENSE"
+                                   "See LICENSE in the distribution."))
+    (home-page "http://www.bzip.org/")))
 
 (define-public lbzip2
   (package
-- 
2.13.0

bug#26730: [PATCH 2/3] gnu: bzip2: Add explicit return value for 2 phases.

[Christopher Baines]

* gnu/packages/compression.scm (bzip2)[arguments]: Add explicit return values
  to the configure and build-shared-lib phases.
---
 gnu/packages/compression.scm | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/compression.scm b/gnu/packages/compression.scm
index 7d3a62e2f..0ce9d88b7 100644
--- a/gnu/packages/compression.scm
+++ b/gnu/packages/compression.scm
@@ -237,7 +237,8 @@ file; as a result, it is often used in conjunction with \"tar\", resulting in
                     (string-append "RANLIB = " target "-ranlib\n"))
                    (("^all:(.*)test" _ prerequisites)
                     ;; Remove 'all' -> 'test' dependency.
-                    (string-append "all:" prerequisites "\n"))))))
+                    (string-append "all:" prerequisites "\n")))
+                 #t)))
          (add-before 'build 'build-shared-lib
            (lambda* (#:key inputs #:allow-other-keys)
              (patch-makefile-SHELL "Makefile-libbz2_so")
@@ -252,7 +253,8 @@ file; as a result, it is often used in conjunction with \"tar\", resulting in
                                      base libdir)
                              (copy-file file
                                         (string-append libdir "/" base))))
-                         (find-files "." "^libbz2\\.so"))))))
+                         (find-files "." "^libbz2\\.so")))
+             #t)))
 
        #:make-flags (list (string-append "PREFIX="
                                          (assoc-ref %outputs "out")))
-- 
2.13.0

bug#26730: [PATCH 3/3] gnu: bzip2: Patch bzip2 utilities.

[Christopher Baines]

* gnu/packages/compression.scm (bzip2)[arguments]: Add patch-script phase to
  remove absolute reference to /bin/rm.
---
 gnu/packages/compression.scm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gnu/packages/compression.scm b/gnu/packages/compression.scm
index 0ce9d88b7..3d2adcda5 100644
--- a/gnu/packages/compression.scm
+++ b/gnu/packages/compression.scm
@@ -254,6 +254,12 @@ file; as a result, it is often used in conjunction with \"tar\", resulting in
                              (copy-file file
                                         (string-append libdir "/" base))))
                          (find-files "." "^libbz2\\.so")))
+             #t))
+         (add-after 'install-shared-lib 'patch-scripts
+           (lambda* (#:key outputs inputs #:allow-other-keys)
+             (let* ((out (assoc-ref outputs "out")))
+               (substitute* (string-append out "/bin/bzdiff")
+                 (("/bin/rm") "rm")))
              #t)))
 
        #:make-flags (list (string-append "PREFIX="
-- 
2.13.0