* 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 external index https://git.savannah.gnu.org/cgit/guix.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.