Hello Ludovic and Nils,

Thank you for review.

ludo@gnu.org (Ludovic Courtès) writes:

> Oleg Pykhalov <go.wigust@gmail.com> skribis:
>
>> * gnu/packages/cdrom.scm (cdrkit-libre): New variable.
>
> [...]
>
>> +    (source (origin
>> +              (method url-fetch)
>> +              (uri (string-append
>> +                    "https://repo.parabola.nu/other/cdrkit-libre/cdrkit-libre-"
>> +                    version ".tar.gz"))
>
> Like Nils I wonder: can you explain how it differs from cdrkit?  Could
> we use upstream cdrkit + snippet/patches instead of taking the source
> from Parabola’s repo?

Comparing Nix cdrkit with Parabola cdrkit-libre tarball not much differ.
--8<---------------cut here---------------start------------->8---
$ /tmp/local$ diff -r -u cdrkit-1.1.11-nix/  cdrkit-1.1.11-parabola/
Only in cdrkit-1.1.11-nix/doc/genisoimage: README.hfs_boot
Only in cdrkit-1.1.11-nix/genisoimage: apple_driver.8
Only in cdrkit-1.1.11-nix/genisoimage: apple_driver.c
--8<---------------cut here---------------end--------------->8---

I probably could get a diff for apple_driver.{8,c} files, but the the
upstream URI http://cdrkit.org/releases/cdrkit.tar.gz is dead.  I'm not
sure where is a better place to fetch the release tarball except from
another GNU/Linux distribution like Parabola.

>> +    (build-system cmake-build-system)
>> +    (native-inputs
>> +     `(("bzip2" ,bzip2)
>> +       ("libcap" ,libcap)
>> +       ("perl" ,perl)
>> +       ("zlib" ,zlib)))
>
> It may be that all of these should be ‘inputs’.  Could you check:
>
>   guix gc --references $(guix build cdrkit-libre)
--8<---------------cut here---------------start------------->8---
/gnu/store/5whrhayw7kispmd103xc05vmdvsdvhqf-cdrkit-libre-1.1.11
/gnu/store/7ifc22sh86zblnzamqimgmv06idyx69v-perl-5.26.1
/gnu/store/fxiwj2wpp11sif613axdax7gmwzsg6kp-zlib-1.2.11
/gnu/store/l4lr0f5cjd0nbsaaf8b5dmcw1a1yypr3-glibc-2.27
/gnu/store/pxf13xq984jvl2nk12wmsln39a1d8jwc-libcap-2.25
/gnu/store/qhxgdgyiyq2ilvh17fqfw0njpqlg4gsc-file-5.32
/gnu/store/rbrandv7anzjxqkr40d7fkanzssslk4b-bash-minimal-4.4.19
/gnu/store/rw1ilvs76rf2p4p0bi1fx3bffprwwb7g-bzip2-1.0.6
/gnu/store/vla5j7pbkpcp39lsdfsmz7m9azn48lr4-gcc-5.5.0-lib
--8<---------------cut here---------------end--------------->8---

> ?  If they show up here, then ‘inputs’.
>

Sure, I will do ‘s/native-inputs/inputs/g’.

>> +    (arguments
>> +     `(#:tests? #f ;no tests
>> +       #:phases
>> +       (modify-phases %standard-phases
>> +         (add-after 'install 'old-cdrecord
>> +           (lambda* (#:key outputs #:allow-other-keys)
>> +             (with-directory-excursion (string-append (assoc-ref outputs "out")
>> +                                                      "/bin")
>> +               (symlink "genisoimage" "mkisofs")
>> +               (symlink "wodim" "cdrecord")))))))
>
> Please return #t.

Oops, will do.

>> +    (home-page "http://cdrkit.org/")
>
> Like Nils wrote, is this the right URL?

It's a right URL, I think.  I converted recipe from Parabola, which
references the same URL [1].  Maybe I should use a Parabola repository
URL <https://repo.parabola.nu/other/cdrkit-libre/>?

>> +    (synopsis "Portable command-line CD/DVD recorder software, mostly
>> +compatible with cdrtools")
>
> “Command-line CD/DVD recorder” is enough IMO.

OK.

>> +    (description "Cdrkit is a suite of programs for recording CDs and DVDs,
>> +blanking CD-RW media, creating ISO-9660 filesystem images, extracting audio CD
>> +data, and more.")
>
> Here you can mention that it’s “mostly compatible …”.

Do you mean “creating more compatible ISO-9660 …”?

> Note: “file system” (two words).  :-)

OK.


[1]  https://git.parabola.nu/abslibre.git/tree/libre/cdrkit/PKGBUILD

Oleg.