unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Add Asunder
@ 2016-12-17  9:20 Chris Marusich
  2016-12-17  9:20 ` [PATCH] gnu: Add asunder Chris Marusich
  0 siblings, 1 reply; 13+ messages in thread
From: Chris Marusich @ 2016-12-17  9:20 UTC (permalink / raw)
  To: guix-devel

The following email contains a patch for Asunder.  I've tested it and verified
that it works in GuixSD for ripping a CD to OGG files.  (Note that you must be
able to read the CD-ROM device file, e.g., by being a member of the cdrom
group.)

- Chris

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

* [PATCH] gnu: Add asunder.
  2016-12-17  9:20 Add Asunder Chris Marusich
@ 2016-12-17  9:20 ` Chris Marusich
  2016-12-17  9:46   ` John Darrington
                     ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Chris Marusich @ 2016-12-17  9:20 UTC (permalink / raw)
  To: guix-devel

* gnu/packages/cdrom.scm (asunder): New variable.
---
 gnu/packages/cdrom.scm | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/gnu/packages/cdrom.scm b/gnu/packages/cdrom.scm
index 829156a7c..b78382ec6 100644
--- a/gnu/packages/cdrom.scm
+++ b/gnu/packages/cdrom.scm
@@ -28,14 +28,18 @@
   #:use-module ((guix licenses) #:select (lgpl2.1+ gpl2 gpl2+ gpl3+))
   #:use-module (guix build-system cmake)
   #:use-module (guix build-system gnu)
+  #:use-module (guix build-system glib-or-gtk)
   #:use-module (gnu packages)
   #:use-module (gnu packages acl)
+  #:use-module (gnu packages audio)
   #:use-module (gnu packages bison)
   #:use-module (gnu packages compression)
   #:use-module (gnu packages flex)
   #:use-module (gnu packages gettext)
   #:use-module (gnu packages gtk)
+  #:use-module (gnu packages glib)
   #:use-module (gnu packages man)
+  #:use-module (gnu packages mp3)
   #:use-module (gnu packages ncurses)
   #:use-module (gnu packages elf)
   #:use-module (gnu packages pkg-config)
@@ -395,3 +399,43 @@ for bootable CD-ROMs.
 Image data is written to standard output by default and all other
 information is written to standard error.")
     (license gpl2+)))
+
+(define-public asunder
+  (package
+    (name "asunder")
+    (version "2.8")
+    (home-page "http://www.littlesvr.ca/asunder/index.php")
+    (source (origin
+              (method url-fetch)
+              (uri
+               (string-append "http://www.littlesvr.ca/asunder/releases/asunder-"
+                              version
+                              ".tar.bz2"))
+              (sha256
+               (base32
+                "1nq9kd4rd4k2kibf57gdbm0zw2gxa234vvvdhxkm8g5bhx5h3iyq"))))
+    (build-system glib-or-gtk-build-system)
+    ;; Asunder fails to build when built outside of its source directory.
+    (arguments `(#:out-of-source? #f))
+    (native-inputs `(("intltool" ,intltool)
+                     ("pkg-config" ,pkg-config)))
+    (inputs `(("gtk+-2" ,gtk+-2)
+              ("glib" ,glib)
+              ("libcddb" ,libcddb)
+              ("cdparanoia" ,cdparanoia)
+              ("lame" ,lame)
+              ("vorbis-tools" ,vorbis-tools)
+              ("flac" ,flac)
+              ("opus" ,opus)
+              ("wavpack" ,wavpack)
+              ("libmpcdec" ,libmpcdec)))
+    (synopsis "Graphical Audio CD ripper and encoder")
+    ;; Asunder can also encode to AAC using neroAacEnc and to Monkey's audio
+    ;; using mac, but we do not include those libraries as they are non-free.
+    (description
+     "Asunder is a graphical Audio CD ripper and encoder.  It can save audio
+tracks as WAV, MP3, Ogg Vorbis, FLAC, Opus, Wavpack, and Musepack.  It can use
+CDDB to name and tag each track automatically, and it allows for each track to
+be by a different artist.  Asunder can encode to multiple formats in one
+session, and it can create M3U playlists.")
+    (license gpl2)))
-- 
2.11.0

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

* Re: [PATCH] gnu: Add asunder.
  2016-12-17  9:20 ` [PATCH] gnu: Add asunder Chris Marusich
@ 2016-12-17  9:46   ` John Darrington
  2016-12-17 16:14   ` Hartmut Goebel
  2016-12-17 18:56   ` Leo Famulari
  2 siblings, 0 replies; 13+ messages in thread
From: John Darrington @ 2016-12-17  9:46 UTC (permalink / raw)
  To: Chris Marusich; +Cc: guix-devel

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

On Sat, Dec 17, 2016 at 01:20:40AM -0800, Chris Marusich wrote:

     +(define-public asunder
     +  (package
     +    (name "asunder")
     +    (version "2.8")
     +    (home-page "http://www.littlesvr.ca/asunder/index.php")
     +    (source (origin
     +              (method url-fetch)

It's unueual to have the home-page field here.  Most package definitions place it just
before the synopsis.

J'

-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* Re: [PATCH] gnu: Add asunder.
  2016-12-17  9:20 ` [PATCH] gnu: Add asunder Chris Marusich
  2016-12-17  9:46   ` John Darrington
@ 2016-12-17 16:14   ` Hartmut Goebel
  2016-12-17 18:56   ` Leo Famulari
  2 siblings, 0 replies; 13+ messages in thread
From: Hartmut Goebel @ 2016-12-17 16:14 UTC (permalink / raw)
  To: guix-devel

Am 17.12.2016 um 10:20 schrieb Chris Marusich:
> +    (home-page "http://www.littlesvr.ca/asunder/index.php")

I'd leave of the "index.php" here. Its uselass and ugly :-)

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          | h.goebel@crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |

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

* Re: [PATCH] gnu: Add asunder.
  2016-12-17  9:20 ` [PATCH] gnu: Add asunder Chris Marusich
  2016-12-17  9:46   ` John Darrington
  2016-12-17 16:14   ` Hartmut Goebel
@ 2016-12-17 18:56   ` Leo Famulari
  2016-12-18  9:04     ` Chris Marusich
  2 siblings, 1 reply; 13+ messages in thread
From: Leo Famulari @ 2016-12-17 18:56 UTC (permalink / raw)
  To: Chris Marusich; +Cc: guix-devel

On Sat, Dec 17, 2016 at 01:20:40AM -0800, Chris Marusich wrote:
> * gnu/packages/cdrom.scm (asunder): New variable.

Thanks!

> +    (home-page "http://www.littlesvr.ca/asunder/index.php")

I agree with the other reviewers about the home-page.

> +    (inputs `(("gtk+-2" ,gtk+-2)
> +              ("glib" ,glib)
> +              ("libcddb" ,libcddb)
> +              ("cdparanoia" ,cdparanoia)
> +              ("lame" ,lame)
> +              ("vorbis-tools" ,vorbis-tools)
> +              ("flac" ,flac)
> +              ("opus" ,opus)
> +              ("wavpack" ,wavpack)
> +              ("libmpcdec" ,libmpcdec)))

I checked the references of the built package:
$ guix gc --references $(./pre-inst-env guix build asunder)
/gnu/store/1jh6z732id8w014i66abm2h2iivkwk8v-gdk-pixbuf+svg-2.34.0
/gnu/store/46kis1wxzqfk3yysaz6ds0pc7w195a3b-gtk+-2.24.31
/gnu/store/4glf79v1r1l4k7by4vf1lfldq7n0yafw-libcddb-1.3.2
/gnu/store/7m55pyfv0rm19rqrrr7xx0c2irsvaq3y-pango-1.40.1
/gnu/store/cd92wyv466fcfnavx3wcjcr8qln4ycix-atk-2.20.0
/gnu/store/cdi08kw7r6r684w8mk0xq0dkgpjhfpmd-gcc-4.9.4-lib
/gnu/store/hmc1jiyr29mk9cl2d9j0jwf0dim1q76g-freetype-2.6.3
/gnu/store/iwgi9001dmmihrjg4rqhd6pa6788prjw-glibc-2.24
/gnu/store/n56vagr3zv7ildvh9b7mzvk7h4bgwjpf-shared-mime-info-1.7
/gnu/store/p56ai0sj3bbh8hdqc9qigdp91gj73brp-glib-2.48.2
/gnu/store/pkv2qqgprp4zxcqfspwwx81qm9lng0da-fontconfig-2.12.1
/gnu/store/qkw4zrwfybxww8f56nkb6hggxambk89b-bash-4.4.0
/gnu/store/y30l675nz41p87pmvzscqhhyhik626na-cairo-1.14.6
/gnu/store/zfm6sgn6mpv8yqfjwk23i7hnjib2rc30-asunder-2.8

Several inputs are missing from this list, so they will be garbage
collected when the user runs `guix gc`. Can you look into that?

> +    ;; Asunder can also encode to AAC using neroAacEnc and to Monkey's audio
> +    ;; using mac, but we do not include those libraries as they are non-free.

This comment isn't necessary, in my opinion.

> +    (description
> +     "Asunder is a graphical Audio CD ripper and encoder.  It can save audio

Does audio need to be capitalized here?

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

* Re: [PATCH] gnu: Add asunder.
  2016-12-17 18:56   ` Leo Famulari
@ 2016-12-18  9:04     ` Chris Marusich
  2016-12-23  8:16       ` Chris Marusich
  0 siblings, 1 reply; 13+ messages in thread
From: Chris Marusich @ 2016-12-18  9:04 UTC (permalink / raw)
  To: Leo Famulari; +Cc: guix-devel


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

Hi everyone,

Thank you for the review!  Please find a new patch attached.

Hartmut Goebel <h.goebel@crazy-compilers.com> writes:

> Am 17.12.2016 um 10:20 schrieb Chris Marusich:
>> +    (home-page "http://www.littlesvr.ca/asunder/index.php")
>
> I'd leave of the "index.php" here. Its uselass and ugly :-)

Good point.  I've changed this, but I've left the trailing slash to
avoid an unnecessary redirect.

John Darrington <john@darrington.wattle.id.au> writes:

>      +(define-public asunder
>      +  (package
>      +    (name "asunder")
>      +    (version "2.8")
>      +    (home-page "http://www.littlesvr.ca/asunder/index.php")
>      +    (source (origin
>      +              (method url-fetch)
>
> It's unueual to have the home-page field here.  Most package
> definitions place it just
> before the synopsis.

OK - I've moved it.

Leo Famulari <leo@famulari.name> writes:

>> +    (inputs `(("gtk+-2" ,gtk+-2)
>> +              ("glib" ,glib)
>> +              ("libcddb" ,libcddb)
>> +              ("cdparanoia" ,cdparanoia)
>> +              ("lame" ,lame)
>> +              ("vorbis-tools" ,vorbis-tools)
>> +              ("flac" ,flac)
>> +              ("opus" ,opus)
>> +              ("wavpack" ,wavpack)
>> +              ("libmpcdec" ,libmpcdec)))
>
> I checked the references of the built package:
> $ guix gc --references $(./pre-inst-env guix build asunder)
> /gnu/store/1jh6z732id8w014i66abm2h2iivkwk8v-gdk-pixbuf+svg-2.34.0
> /gnu/store/46kis1wxzqfk3yysaz6ds0pc7w195a3b-gtk+-2.24.31
> /gnu/store/4glf79v1r1l4k7by4vf1lfldq7n0yafw-libcddb-1.3.2
> /gnu/store/7m55pyfv0rm19rqrrr7xx0c2irsvaq3y-pango-1.40.1
> /gnu/store/cd92wyv466fcfnavx3wcjcr8qln4ycix-atk-2.20.0
> /gnu/store/cdi08kw7r6r684w8mk0xq0dkgpjhfpmd-gcc-4.9.4-lib
> /gnu/store/hmc1jiyr29mk9cl2d9j0jwf0dim1q76g-freetype-2.6.3
> /gnu/store/iwgi9001dmmihrjg4rqhd6pa6788prjw-glibc-2.24
> /gnu/store/n56vagr3zv7ildvh9b7mzvk7h4bgwjpf-shared-mime-info-1.7
> /gnu/store/p56ai0sj3bbh8hdqc9qigdp91gj73brp-glib-2.48.2
> /gnu/store/pkv2qqgprp4zxcqfspwwx81qm9lng0da-fontconfig-2.12.1
> /gnu/store/qkw4zrwfybxww8f56nkb6hggxambk89b-bash-4.4.0
> /gnu/store/y30l675nz41p87pmvzscqhhyhik626na-cairo-1.14.6
> /gnu/store/zfm6sgn6mpv8yqfjwk23i7hnjib2rc30-asunder-2.8
>
> Several inputs are missing from this list, so they will be garbage
> collected when the user runs `guix gc`. Can you look into that?

Great catch.  This was a problem.  The reason I didn't realize it was
because I tested by installing Asunder into my existing profile, which
contains the necessary tools already, and Asunder was finding the tools
via my PATH environment variable.  I've fixed this issue by wrapping the
asunder executable with wrap-program.  I've verified (by using "guix
environment" with the --pure option) that with this latest patch,
Asunder can successfully rip an audio CD and encode to MP3, FLAC, OGG
Vorbis, OPUS, and WavPack.  I've also verified that the inputs which
previously were missing in the references are now included.

The remaining encodings supported by Asunder - Musepack, Monkey's Audio,
and AAC - don't work because (1) we haven't packaged the Musepack
encoder yet (I think), and (2) the latter two are not distributed under
a free license.

>> + ;; Asunder can also encode to AAC using neroAacEnc and to Monkey's
>> audio
>> + ;; using mac, but we do not include those libraries as they are
>> non-free.
>
> This comment isn't necessary, in my opinion.

OK.  I've removed it.

>> +    (description
>> + "Asunder is a graphical Audio CD ripper and encoder.  It can save
>> audio
>
> Does audio need to be capitalized here?

No, it doesn't.  I've decapitalized it.

-- 
Chris

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-gnu-Add-asunder.patch --]
[-- Type: text/x-patch, Size: 4042 bytes --]

From 0e4998622ac076d57b6df492727d7cebbabc290b Mon Sep 17 00:00:00 2001
From: Chris Marusich <cmmarusich@gmail.com>
Date: Sat, 17 Dec 2016 01:06:48 -0800
Subject: [PATCH] gnu: Add asunder.

* gnu/packages/cdrom.scm (asunder): New variable.
---
 gnu/packages/cdrom.scm | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/gnu/packages/cdrom.scm b/gnu/packages/cdrom.scm
index 829156a7c..20937dc8f 100644
--- a/gnu/packages/cdrom.scm
+++ b/gnu/packages/cdrom.scm
@@ -28,14 +28,19 @@
   #:use-module ((guix licenses) #:select (lgpl2.1+ gpl2 gpl2+ gpl3+))
   #:use-module (guix build-system cmake)
   #:use-module (guix build-system gnu)
+  #:use-module (guix build-system glib-or-gtk)
+  #:use-module (guix gexp)
   #:use-module (gnu packages)
   #:use-module (gnu packages acl)
+  #:use-module (gnu packages audio)
   #:use-module (gnu packages bison)
   #:use-module (gnu packages compression)
   #:use-module (gnu packages flex)
   #:use-module (gnu packages gettext)
   #:use-module (gnu packages gtk)
+  #:use-module (gnu packages glib)
   #:use-module (gnu packages man)
+  #:use-module (gnu packages mp3)
   #:use-module (gnu packages ncurses)
   #:use-module (gnu packages elf)
   #:use-module (gnu packages pkg-config)
@@ -395,3 +400,56 @@ for bootable CD-ROMs.
 Image data is written to standard output by default and all other
 information is written to standard error.")
     (license gpl2+)))
+
+(define-public asunder
+  (package
+    (name "asunder")
+    (version "2.8")
+    (source (origin
+              (method url-fetch)
+              (uri
+               (string-append "http://www.littlesvr.ca/asunder/releases/asunder-"
+                              version
+                              ".tar.bz2"))
+              (sha256
+               (base32
+                "1nq9kd4rd4k2kibf57gdbm0zw2gxa234vvvdhxkm8g5bhx5h3iyq"))))
+    (build-system glib-or-gtk-build-system)
+    (arguments
+     '(#:out-of-source? #f
+       #:phases (modify-phases %standard-phases
+                  (add-after 'install 'wrap
+                    (lambda* (#:key inputs outputs #:allow-other-keys)
+                      (let ((program (string-append (assoc-ref outputs "out")
+                                                    "/bin/asunder")))
+                        (define (bin-directory input-name)
+                          (string-append (assoc-ref inputs input-name) "/bin"))
+                        (wrap-program program
+                          `("PATH" ":" prefix
+                            ,(map bin-directory (list "cdparanoia"
+                                                      "lame"
+                                                      "vorbis-tools"
+                                                      "flac"
+                                                      "opus-tools"
+                                                      "wavpack"))))))))))
+    (native-inputs `(("intltool" ,intltool)
+                     ("pkg-config" ,pkg-config)))
+    ;; TODO: Add the necessary packages for Musepack encoding.
+    (inputs `(("gtk+-2" ,gtk+-2)
+              ("glib" ,glib)
+              ("libcddb" ,libcddb)
+              ("cdparanoia" ,cdparanoia)
+              ("lame" ,lame)
+              ("vorbis-tools" ,vorbis-tools)
+              ("flac" ,flac)
+              ("opus-tools" ,opus-tools)
+              ("wavpack" ,wavpack)))
+    (home-page "http://www.littlesvr.ca/asunder/")
+    (synopsis "Graphical audio CD ripper and encoder")
+    (description
+     "Asunder is a graphical audio CD ripper and encoder.  It can save audio
+tracks as WAV, MP3, Ogg Vorbis, FLAC, Opus, Wavpack, and Musepack.  It can use
+CDDB to name and tag each track automatically, and it allows for each track to
+be by a different artist.  Asunder can encode to multiple formats in one
+session, and it can create M3U playlists.")
+    (license gpl2)))
-- 
2.11.0


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

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

* Re: [PATCH] gnu: Add asunder.
  2016-12-18  9:04     ` Chris Marusich
@ 2016-12-23  8:16       ` Chris Marusich
  2016-12-23 16:36         ` Leo Famulari
  0 siblings, 1 reply; 13+ messages in thread
From: Chris Marusich @ 2016-12-23  8:16 UTC (permalink / raw)
  To: Leo Famulari; +Cc: guix-devel

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

Hi Leo,

I've responded to the feedback from you, Hartmut, and John.  How does
the latest patch look?  It's available here:

https://lists.gnu.org/archive/html/guix-devel/2016-12/msg00707.html

-- 
Chris

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

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

* Re: [PATCH] gnu: Add asunder.
  2016-12-23  8:16       ` Chris Marusich
@ 2016-12-23 16:36         ` Leo Famulari
  2016-12-24  2:11           ` Chris Marusich
  0 siblings, 1 reply; 13+ messages in thread
From: Leo Famulari @ 2016-12-23 16:36 UTC (permalink / raw)
  To: Chris Marusich; +Cc: guix-devel

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

On Fri, Dec 23, 2016 at 12:16:24AM -0800, Chris Marusich wrote:
> Hi Leo,
> 
> I've responded to the feedback from you, Hartmut, and John.  How does
> the latest patch look?  It's available here:
> 
> https://lists.gnu.org/archive/html/guix-devel/2016-12/msg00707.html

Thanks for the reminder! I pushed as
426e6083ae9d4569005dab8edf948485e5979171.

I think it would be useful to figure out if it's possible to avoid the
wrapper somehow, but I didn't look into this closely.

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

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

* Re: [PATCH] gnu: Add asunder.
  2016-12-23 16:36         ` Leo Famulari
@ 2016-12-24  2:11           ` Chris Marusich
  2016-12-24  7:07             ` John Darrington
  0 siblings, 1 reply; 13+ messages in thread
From: Chris Marusich @ 2016-12-24  2:11 UTC (permalink / raw)
  To: Leo Famulari; +Cc: guix-devel

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

Leo Famulari <leo@famulari.name> writes:

> On Fri, Dec 23, 2016 at 12:16:24AM -0800, Chris Marusich wrote:
>> Hi Leo,
>> 
>> I've responded to the feedback from you, Hartmut, and John.  How does
>> the latest patch look?  It's available here:
>> 
>> https://lists.gnu.org/archive/html/guix-devel/2016-12/msg00707.html
>
> Thanks for the reminder! I pushed as
> 426e6083ae9d4569005dab8edf948485e5979171.
>
> I think it would be useful to figure out if it's possible to avoid the
> wrapper somehow, but I didn't look into this closely.

I'm sure we could avoid the wrapper by patching the source, but why
would that be better?  The wrapper is a simple and robust solution, and
in this case I can't see any drawbacks to using it.

For context, Asunder assumes that various tools will be made available
via the PATH environment variable.  It refuses to function in certain
cases when a tool that it needs can't be found in the PATH.  I'm sure we
could patch this mechanism, but it seems simpler to just create a
wrapper that puts the directories containing the tools onto the PATH.

-- 
Chris

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

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

* Re: [PATCH] gnu: Add asunder.
  2016-12-24  2:11           ` Chris Marusich
@ 2016-12-24  7:07             ` John Darrington
  2016-12-25  2:03               ` Chris Marusich
  0 siblings, 1 reply; 13+ messages in thread
From: John Darrington @ 2016-12-24  7:07 UTC (permalink / raw)
  To: Chris Marusich; +Cc: guix-devel

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

On Fri, Dec 23, 2016 at 06:11:44PM -0800, Chris Marusich wrote:
     Leo Famulari <leo@famulari.name> writes:
     
     > On Fri, Dec 23, 2016 at 12:16:24AM -0800, Chris Marusich wrote:
     >> Hi Leo,
     >> 
     >> I've responded to the feedback from you, Hartmut, and John.  How does
     >> the latest patch look?  It's available here:
     >> 
     >> https://lists.gnu.org/archive/html/guix-devel/2016-12/msg00707.html
     >
     > Thanks for the reminder! I pushed as
     > 426e6083ae9d4569005dab8edf948485e5979171.
     >
     > I think it would be useful to figure out if it's possible to avoid the
     > wrapper somehow, but I didn't look into this closely.
     
     I'm sure we could avoid the wrapper by patching the source, but why
     would that be better?  The wrapper is a simple and robust solution, and
     in this case I can't see any drawbacks to using it.
     
     For context, Asunder assumes that various tools will be made available
     via the PATH environment variable.  It refuses to function in certain
     cases when a tool that it needs can't be found in the PATH.  I'm sure we
     could patch this mechanism, but it seems simpler to just create a
     wrapper that puts the directories containing the tools onto the PATH.
     

FWIW, I think wrappers are  bit of a nasty solution and should be avoided
if feasible.  Sometimes however there is no reasonable way to avoid them.
It sounds as if this might be such as case.  J'


-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* Re: [PATCH] gnu: Add asunder.
  2016-12-24  7:07             ` John Darrington
@ 2016-12-25  2:03               ` Chris Marusich
  2016-12-25  7:48                 ` John Darrington
  0 siblings, 1 reply; 13+ messages in thread
From: Chris Marusich @ 2016-12-25  2:03 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel

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

John Darrington <john@darrington.wattle.id.au> writes:

> On Fri, Dec 23, 2016 at 06:11:44PM -0800, Chris Marusich wrote:
>      Leo Famulari <leo@famulari.name> writes:
>      
>      > On Fri, Dec 23, 2016 at 12:16:24AM -0800, Chris Marusich wrote:
>      >> Hi Leo,
>      >> 
>      >> I've responded to the feedback from you, Hartmut, and John.  How does
>      >> the latest patch look?  It's available here:
>      >> 
>      >> https://lists.gnu.org/archive/html/guix-devel/2016-12/msg00707.html
>      >
>      > Thanks for the reminder! I pushed as
>      > 426e6083ae9d4569005dab8edf948485e5979171.
>      >
>      > I think it would be useful to figure out if it's possible to avoid the
>      > wrapper somehow, but I didn't look into this closely.
>      
>      I'm sure we could avoid the wrapper by patching the source, but why
>      would that be better?  The wrapper is a simple and robust solution, and
>      in this case I can't see any drawbacks to using it.
>      
>      For context, Asunder assumes that various tools will be made available
>      via the PATH environment variable.  It refuses to function in certain
>      cases when a tool that it needs can't be found in the PATH.  I'm sure we
>      could patch this mechanism, but it seems simpler to just create a
>      wrapper that puts the directories containing the tools onto the PATH.
>      
>
> FWIW, I think wrappers are  bit of a nasty solution and should be avoided
> if feasible.  Sometimes however there is no reasonable way to avoid them.
> It sounds as if this might be such as case.  J'

Why is the wrapper not good here?  What would be a better solution?

Here's why I think the wrapper produced by 'wrap-program' is a good
solution in this case:

* The wrapper script allows us to package the software without modifying
its source.  As previously explained, Asunder is currently written under
the assumption that the tools it requires will be made available via the
PATH environment variable.

* The wrapper script guarantees "complete deployment" of Asunder (i.e.,
no missing dependencies).  This is because the wrapper script contains
references to the components in the store that provide the command-line
tools that Asunder requires.

* The wrapper script requires less work than patching Asunder.

* The wrapper script is more robust than any patch we might attempt to
  apply to Asunder's source code.

This is a good argument for using a wrapper script in this case.  And I
believe these points apply to any component, like Asunder, which is
written under the assumption that tools will be made available via
PATH.  I also am willing to believe there are cases where the wrapper
script is undesirable, but I don't think this is one of them.

If there's a better way to package Asunder, I'm happy to do it.
However, I haven't heard of any concrete alternatives, or any concrete
explanations of why a wrapper is undesirable.  Until then, I think using
a wrapper like this to package components like Asunder is the best way.

-- 
Chris

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

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

* Re: [PATCH] gnu: Add asunder.
  2016-12-25  2:03               ` Chris Marusich
@ 2016-12-25  7:48                 ` John Darrington
  2016-12-25 20:03                   ` Leo Famulari
  0 siblings, 1 reply; 13+ messages in thread
From: John Darrington @ 2016-12-25  7:48 UTC (permalink / raw)
  To: Chris Marusich; +Cc: guix-devel

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

On Sat, Dec 24, 2016 at 06:03:45PM -0800, Chris Marusich wrote:

     Why is the wrapper not good here?  What would be a better solution?
     
     Here's why I think the wrapper produced by 'wrap-program' is a good
     solution in this case:
     
     * The wrapper script allows us to package the software without modifying
     its source.  As previously explained, Asunder is currently written under
     the assumption that the tools it requires will be made available via the
     PATH environment variable.
     
     * The wrapper script guarantees "complete deployment" of Asunder (i.e.,
     no missing dependencies).  This is because the wrapper script contains
     references to the components in the store that provide the command-line
     tools that Asunder requires.
     
     * The wrapper script requires less work than patching Asunder.
     
     * The wrapper script is more robust than any patch we might attempt to
       apply to Asunder's source code.
     
     This is a good argument for using a wrapper script in this case.  And I
     believe these points apply to any component, like Asunder, which is
     written under the assumption that tools will be made available via
     PATH.  I also am willing to believe there are cases where the wrapper
     script is undesirable, but I don't think this is one of them.
     
     If there's a better way to package Asunder, I'm happy to do it.
     However, I haven't heard of any concrete alternatives, or any concrete
     explanations of why a wrapper is undesirable.  Until then, I think using
     a wrapper like this to package components like Asunder is the best way.
     

I think all your arguments are valid, and I am not disagreeing with you.
I am merely presenting a counter argument for you to consider.

My problem with wrappers is that 1) it makes it difficult for a users
to override environment variables, if they want to for some reason. 2)
programs which the user expects to be a binary are in fact an shell 
script, adding a layer of confusion.

Just my $0.02.

J'


-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* Re: [PATCH] gnu: Add asunder.
  2016-12-25  7:48                 ` John Darrington
@ 2016-12-25 20:03                   ` Leo Famulari
  0 siblings, 0 replies; 13+ messages in thread
From: Leo Famulari @ 2016-12-25 20:03 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel

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

On Sun, Dec 25, 2016 at 08:48:35AM +0100, John Darrington wrote:
> On Sat, Dec 24, 2016 at 06:03:45PM -0800, Chris Marusich wrote:
> 
>      Why is the wrapper not good here?  What would be a better solution?
>      
>      Here's why I think the wrapper produced by 'wrap-program' is a good
>      solution in this case:
>      
>      * The wrapper script allows us to package the software without modifying
>      its source.  As previously explained, Asunder is currently written under
>      the assumption that the tools it requires will be made available via the
>      PATH environment variable.
>      
>      * The wrapper script guarantees "complete deployment" of Asunder (i.e.,
>      no missing dependencies).  This is because the wrapper script contains
>      references to the components in the store that provide the command-line
>      tools that Asunder requires.
>      
>      * The wrapper script requires less work than patching Asunder.
>      
>      * The wrapper script is more robust than any patch we might attempt to
>        apply to Asunder's source code.
>      
>      This is a good argument for using a wrapper script in this case.  And I
>      believe these points apply to any component, like Asunder, which is
>      written under the assumption that tools will be made available via
>      PATH.  I also am willing to believe there are cases where the wrapper
>      script is undesirable, but I don't think this is one of them.

Okay, thanks for explanation. A wrapper does indeed seem appropriate in
this case.

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

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

end of thread, other threads:[~2016-12-25 20:03 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-17  9:20 Add Asunder Chris Marusich
2016-12-17  9:20 ` [PATCH] gnu: Add asunder Chris Marusich
2016-12-17  9:46   ` John Darrington
2016-12-17 16:14   ` Hartmut Goebel
2016-12-17 18:56   ` Leo Famulari
2016-12-18  9:04     ` Chris Marusich
2016-12-23  8:16       ` Chris Marusich
2016-12-23 16:36         ` Leo Famulari
2016-12-24  2:11           ` Chris Marusich
2016-12-24  7:07             ` John Darrington
2016-12-25  2:03               ` Chris Marusich
2016-12-25  7:48                 ` John Darrington
2016-12-25 20:03                   ` Leo Famulari

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