unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* bug#26730: [PATCH] Fix bzip2 utilities
@ 2017-05-01  9:49 Christopher Baines
  2017-05-01 10:10 ` bug#26730: [PATCH 1/2] gnu: bzip2: Use 'modify-phases' syntax Christopher Baines
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Christopher Baines @ 2017-05-01  9:49 UTC (permalink / raw)
  To: 26730


[-- 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 --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#26730: [PATCH 1/2] gnu: bzip2: Use 'modify-phases' syntax.
  2017-05-01  9:49 bug#26730: [PATCH] Fix bzip2 utilities Christopher Baines
@ 2017-05-01 10:10 ` Christopher Baines
  2017-05-01 10:10   ` bug#26730: [PATCH 2/2] gnu: bzip2: Patch bzip2 utilities Christopher Baines
  2017-05-15 15:43   ` bug#26730: [PATCH 1/2] gnu: bzip2: Use 'modify-phases' syntax Marius Bakke
  2017-05-06 15:38 ` bug#26730: [PATCH] Fix bzip2 utilities Marius Bakke
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 16+ messages in thread
From: Christopher Baines @ 2017-05-01 10:10 UTC (permalink / raw)
  To: 26730

* 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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#26730: [PATCH 2/2] gnu: bzip2: Patch bzip2 utilities.
  2017-05-01 10:10 ` bug#26730: [PATCH 1/2] gnu: bzip2: Use 'modify-phases' syntax Christopher Baines
@ 2017-05-01 10:10   ` Christopher Baines
  2017-05-15 15:43   ` bug#26730: [PATCH 1/2] gnu: bzip2: Use 'modify-phases' syntax Marius Bakke
  1 sibling, 0 replies; 16+ messages in thread
From: Christopher Baines @ 2017-05-01 10:10 UTC (permalink / raw)
  To: 26730

* 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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#26730: [PATCH] Fix bzip2 utilities
  2017-05-01  9:49 bug#26730: [PATCH] Fix bzip2 utilities Christopher Baines
  2017-05-01 10:10 ` bug#26730: [PATCH 1/2] gnu: bzip2: Use 'modify-phases' syntax Christopher Baines
@ 2017-05-06 15:38 ` Marius Bakke
  2017-05-09 10:18   ` Ludovic Courtès
  2017-05-15  6:04 ` bug#26730: [PATCH] gnu: bzip2: Patch " Christopher Baines
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Marius Bakke @ 2017-05-06 15:38 UTC (permalink / raw)
  To: Christopher Baines, 26730

[-- 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 --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#26730: [PATCH] Fix bzip2 utilities
  2017-05-06 15:38 ` bug#26730: [PATCH] Fix bzip2 utilities Marius Bakke
@ 2017-05-09 10:18   ` Ludovic Courtès
  2017-05-15  6:08     ` Christopher Baines
  0 siblings, 1 reply; 16+ messages in thread
From: Ludovic Courtès @ 2017-05-09 10:18 UTC (permalink / raw)
  To: Marius Bakke; +Cc: 26730

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#26730: [PATCH] gnu: bzip2: Patch bzip2 utilities.
  2017-05-01  9:49 bug#26730: [PATCH] Fix bzip2 utilities Christopher Baines
  2017-05-01 10:10 ` bug#26730: [PATCH 1/2] gnu: bzip2: Use 'modify-phases' syntax Christopher Baines
  2017-05-06 15:38 ` bug#26730: [PATCH] Fix bzip2 utilities Marius Bakke
@ 2017-05-15  6:04 ` Christopher Baines
  2017-05-15  6:07 ` Christopher Baines
  2017-05-16 20:36 ` bug#26730: [PATCH 1/3] gnu: bzip2: Use 'modify-phases' syntax Christopher Baines
  4 siblings, 0 replies; 16+ messages in thread
From: Christopher Baines @ 2017-05-15  6:04 UTC (permalink / raw)
  To: 26730

* 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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#26730: [PATCH] gnu: bzip2: Patch bzip2 utilities.
  2017-05-01  9:49 bug#26730: [PATCH] Fix bzip2 utilities Christopher Baines
                   ` (2 preceding siblings ...)
  2017-05-15  6:04 ` bug#26730: [PATCH] gnu: bzip2: Patch " Christopher Baines
@ 2017-05-15  6:07 ` Christopher Baines
  2017-05-15 15:46   ` Marius Bakke
  2017-05-16 20:36 ` bug#26730: [PATCH 1/3] gnu: bzip2: Use 'modify-phases' syntax Christopher Baines
  4 siblings, 1 reply; 16+ messages in thread
From: Christopher Baines @ 2017-05-15  6:07 UTC (permalink / raw)
  To: 26730

* 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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#26730: [PATCH] Fix bzip2 utilities
  2017-05-09 10:18   ` Ludovic Courtès
@ 2017-05-15  6:08     ` Christopher Baines
  0 siblings, 0 replies; 16+ messages in thread
From: Christopher Baines @ 2017-05-15  6:08 UTC (permalink / raw)
  To: Ludovic Courtès, Marius Bakke; +Cc: 26730


[-- 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 --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#26730: [PATCH 1/2] gnu: bzip2: Use 'modify-phases' syntax.
  2017-05-01 10:10 ` bug#26730: [PATCH 1/2] gnu: bzip2: Use 'modify-phases' syntax Christopher Baines
  2017-05-01 10:10   ` bug#26730: [PATCH 2/2] gnu: bzip2: Patch bzip2 utilities Christopher Baines
@ 2017-05-15 15:43   ` Marius Bakke
  1 sibling, 0 replies; 16+ messages in thread
From: Marius Bakke @ 2017-05-15 15:43 UTC (permalink / raw)
  To: Christopher Baines, 26730

[-- 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 --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#26730: [PATCH] gnu: bzip2: Patch bzip2 utilities.
  2017-05-15  6:07 ` Christopher Baines
@ 2017-05-15 15:46   ` Marius Bakke
  2017-05-16 20:41     ` Christopher Baines
  0 siblings, 1 reply; 16+ messages in thread
From: Marius Bakke @ 2017-05-15 15:46 UTC (permalink / raw)
  To: Christopher Baines, 26730

[-- 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 --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#26730: [PATCH 1/3] gnu: bzip2: Use 'modify-phases' syntax.
  2017-05-01  9:49 bug#26730: [PATCH] Fix bzip2 utilities Christopher Baines
                   ` (3 preceding siblings ...)
  2017-05-15  6:07 ` Christopher Baines
@ 2017-05-16 20:36 ` Christopher Baines
  2017-05-16 20:36   ` bug#26730: [PATCH 2/3] gnu: bzip2: Add explicit return value for 2 phases Christopher Baines
  2017-05-16 20:36   ` bug#26730: [PATCH 3/3] gnu: bzip2: Patch bzip2 utilities Christopher Baines
  4 siblings, 2 replies; 16+ messages in thread
From: Christopher Baines @ 2017-05-16 20:36 UTC (permalink / raw)
  To: 26730

* 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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#26730: [PATCH 2/3] gnu: bzip2: Add explicit return value for 2 phases.
  2017-05-16 20:36 ` bug#26730: [PATCH 1/3] gnu: bzip2: Use 'modify-phases' syntax Christopher Baines
@ 2017-05-16 20:36   ` Christopher Baines
  2017-05-16 20:36   ` bug#26730: [PATCH 3/3] gnu: bzip2: Patch bzip2 utilities Christopher Baines
  1 sibling, 0 replies; 16+ messages in thread
From: Christopher Baines @ 2017-05-16 20:36 UTC (permalink / raw)
  To: 26730

* 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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#26730: [PATCH 3/3] gnu: bzip2: Patch bzip2 utilities.
  2017-05-16 20:36 ` bug#26730: [PATCH 1/3] gnu: bzip2: Use 'modify-phases' syntax Christopher Baines
  2017-05-16 20:36   ` bug#26730: [PATCH 2/3] gnu: bzip2: Add explicit return value for 2 phases Christopher Baines
@ 2017-05-16 20:36   ` Christopher Baines
  1 sibling, 0 replies; 16+ messages in thread
From: Christopher Baines @ 2017-05-16 20:36 UTC (permalink / raw)
  To: 26730

* 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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#26730: [PATCH] gnu: bzip2: Patch bzip2 utilities.
  2017-05-15 15:46   ` Marius Bakke
@ 2017-05-16 20:41     ` Christopher Baines
  2017-05-17 14:58       ` Marius Bakke
  0 siblings, 1 reply; 16+ messages in thread
From: Christopher Baines @ 2017-05-16 20:41 UTC (permalink / raw)
  To: Marius Bakke, 26730


[-- 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 --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#26730: [PATCH] gnu: bzip2: Patch bzip2 utilities.
  2017-05-16 20:41     ` Christopher Baines
@ 2017-05-17 14:58       ` Marius Bakke
  2017-05-18 18:46         ` Christopher Baines
  0 siblings, 1 reply; 16+ messages in thread
From: Marius Bakke @ 2017-05-17 14:58 UTC (permalink / raw)
  To: Christopher Baines, 26730-done

[-- 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 --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#26730: [PATCH] gnu: bzip2: Patch bzip2 utilities.
  2017-05-17 14:58       ` Marius Bakke
@ 2017-05-18 18:46         ` Christopher Baines
  0 siblings, 0 replies; 16+ messages in thread
From: Christopher Baines @ 2017-05-18 18:46 UTC (permalink / raw)
  To: Marius Bakke, 26730


[-- 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 --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2017-05-18 18:47 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-01  9:49 bug#26730: [PATCH] Fix bzip2 utilities Christopher Baines
2017-05-01 10:10 ` bug#26730: [PATCH 1/2] gnu: bzip2: Use 'modify-phases' syntax Christopher Baines
2017-05-01 10:10   ` bug#26730: [PATCH 2/2] gnu: bzip2: Patch bzip2 utilities Christopher Baines
2017-05-15 15:43   ` bug#26730: [PATCH 1/2] gnu: bzip2: Use 'modify-phases' syntax Marius Bakke
2017-05-06 15:38 ` bug#26730: [PATCH] Fix bzip2 utilities Marius Bakke
2017-05-09 10:18   ` Ludovic Courtès
2017-05-15  6:08     ` Christopher Baines
2017-05-15  6:04 ` bug#26730: [PATCH] gnu: bzip2: Patch " Christopher Baines
2017-05-15  6:07 ` Christopher Baines
2017-05-15 15:46   ` Marius Bakke
2017-05-16 20:41     ` Christopher Baines
2017-05-17 14:58       ` Marius Bakke
2017-05-18 18:46         ` Christopher Baines
2017-05-16 20:36 ` bug#26730: [PATCH 1/3] gnu: bzip2: Use 'modify-phases' syntax Christopher Baines
2017-05-16 20:36   ` bug#26730: [PATCH 2/3] gnu: bzip2: Add explicit return value for 2 phases Christopher Baines
2017-05-16 20:36   ` bug#26730: [PATCH 3/3] gnu: bzip2: Patch bzip2 utilities Christopher Baines

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).