Re: [PATCH] gnu: Add asunder.

[Chris Marusich]

Hi everyone,

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

Hartmut Goebel <address@hidden> 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 <address@hidden> 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 <address@hidden> 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

0001-gnu-Add-asunder.patch
Description: Text Data

signature.asc
Description: PGP signature