Subject: Re: [bug#31999] [PATCH 6/7] gnu: Add libblockdev.

From: Marius Bakke <[email-masked]>
Date: Mon, 30 Jul 2018 00:56:14 +0200

Pierre Neidhardt <[email-masked]> writes:

> * gnu/package/disk.scm (libblockdev): New variable.

[...]

> +(define-public libblockdev
> +  (package
> +    (name "libblockdev")
> +    (version "2.17")
> +    (source (origin
> +              (method git-fetch)
> +              (uri (git-reference
> +                    (url "https://github.com/rhinstaller/libblockdev")
> +                    (commit (string-append version "-1"))))
> +              (file-name (string-append name "-" version "-checkout"))
> +              (sha256
> +               (base32
> +                "14f52cj2qcnm8i2zb57qfpdk3kij2gb3xgqkbvidmf6sjicq84z2"))))

This project also provides uploaded releases.  Please use those when
available.

Also, 2.18 is out!

> +    (build-system gnu-build-system)
> +    (native-inputs
> +     `(("python" ,python)
> +       ("gobject-introspection" ,gobject-introspection)
> +       ("automake" ,automake)
> +       ("autocont" ,autoconf)
> +       ("libtool" ,libtool)
> +       ("glib" ,glib)
> +       ("util-linux" ,util-linux)
> +       ("nss" ,nss)
> +       ("cryptsetup" ,cryptsetup)
> +       ("eudev" ,eudev)
> +       ("pkg-config" ,pkg-config)))

That's a lot of native dependencies!  I would assume glib, cryptsetup
and eudev at least were "normal" inputs, I guess they are needed for
tests?  Can you add a comment about it?

> +    (inputs
> +     `(("btrfs-progs" ,btrfs-progs)
> +       ("dosfstools" ,dosfstools)
> +       ("dmraid" ,dmraid)
> +       ("libbytesize" ,libbytesize)
> +       ("lvm2" ,lvm2)
> +       ("mdadm" ,mdadm)
> +       ("ndctl" ,ndctl)
> +       ("mdadm" ,mdadm)
> +       ("parted" ,parted)
> +       ("volume-key" ,volume-key)
> +       ;; ("xfsprogs" ,xfsprogs) ; TODO: Package?
> +       ("kmod" ,kmod)
> +       ))

These parenthesis feel lonely ;-)

> +     (arguments
> +     `(#:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'autogen
> +           (lambda _
> +             (invoke "sh" "autogen.sh"))))))

Try removing this phase here too, or replace 'bootstrap' :-)

> +    (home-page "https://github.com/rhinstaller/libblockdev")
> +    (synopsis "A library for manipulating block devices")

s/A library/C library/ to please `guix lint` :-)
 
> +    (description
> +     "libblockdev is a C library supporting GObject introspection for
> +manipulation of block devices. It has a plugin-based architecture where each
> +technology (like LVM, Btrfs, MD RAID, Swap,...) is implemented in a separate
> +plugin, possibly with multiple implementations (e.g. using LVM CLI or the new
> +LVM DBus API).")

Remember two spaces between sentences (guix lint should warn about this too).

Also s/DBus/D-Bus/

> +    (license license:lgpl2.1+)))

Most of the source files are GPL2+, actually.  Not a single source file
includes the LGPL header, though some are missing licensing information.

I'd write this as:

  ;; XXX: Copying says LGPL2.1, but the source files with license
  ;; information are GPL2+.
  (license license:gpl2+)))