Re: [PATCH] gnu: Add libosinfo.

[Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de>]

rennes@openmailbox.org writes:

> i attached libosinfo patch required for GNOME Boxes.

Thank you very much!  Below I’ll add a couple of comments.  I’m not sure
about a couple of things and I hope you can shed some light on these
issues.

> Considerations:
>
>   a) In the source i used 
> "https://fedorahosted.org/releases/l/i/libosinfo" instead 
> "mirror://gnome/sources/".

The project home page points to fedorahosted.org for downloading
releases.  I don’t even find the libosinfo sources on ftp.gnome.org, so
I think your choice is completely fine.

>   b) In native inputs, i used "vala" instead "glib:bin"; i follow the 
> README.

I don’t understand what you mean.  Vala and glib:bin are not the same
thing.  Following the README is a good idea.  If we don’t need
“glib:bin” then it’s the right thing not to add it.

> From 073a183499bd764b0b0efc246748638c6e4d3aeb Mon Sep 17 00:00:00 2001
> From: Rene Saavedra <rennes@openmailbox.org>
> Date: Sat, 13 Feb 2016 16:23:10 -0600
> Subject: [PATCH] gnu: Add libosinfo.

> * gnu/packages/gnome.scm (libosinfo): New variable.

OK.

> +
> +(define-public libosinfo
> +  (package
> +    (name "libosinfo")
> +    (version "0.3.0")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append "https://fedorahosted.org/releases/l/i/libosinfo/"
> +                            name "-" version ".tar.gz"))
> +       (sha256
> +       (base32
> +        "1g7g5hc4lhi4y0j3mbcj19hawlqkflni1zk4aggrx49fg5l392jk"))))

The indentation of the “(base32” expression is wrong; the opening
parenthesis should be aligned with the ‘s’ of “(sha256”.  (Don’t worry
about this too much — I can fix this before pushing if you don’t get it
right.)

> +    (build-system glib-or-gtk-build-system)

Is this necessary or can we use the simpler “gnu-build-system” instead?

> +    (native-inputs
> +     `(("check" ,check)
> +       ("intltool" ,intltool)
> +       ("libsoup" ,libsoup)
> +       ("pkg-config" ,pkg-config)
> +       ("vala" ,vala)
> +       ("wget" ,wget)))

Why are “wget” and “libsoup” native inputs?  In the build environment
there is no network access, so wget cannot be used to download anything.
Is libosinfo linked with libsoup?  If so, it should be a regular input,
not among the native-inputs.

> +    (inputs
> +     `(("libxslt" ,libxslt)))

Does it link with libxslt?  Or is it used at build time only (e.g. for
building manuals from XML sources)?

> +    (home-page "https://libosinfo.org")
> +    (synopsis "Library for managing information about operating systems")
> +    (description
> +     "libosinfo is a GObject based library API for managing information about
> +operating systems, hypervisors and the (virtual) hardware devices they can
> +support.  It includes a database containing device metadata and provides APIs
> +to match/identify optimal devices for deploying an operating system on a
> +hypervisor.  Via the magic of GObject Introspection, the API is available in all
> +common programming languages with demos for javascript (GJS/Seed) and python
> +(PyGObject).  Also provided are Vala bindings.")

I would cut “the magic of”.  Please also replace “javascript” with
“JavaScript” and “python” with “Python”.  Are the examples and the Vala
bindings actually installed?  I see that gobject-introspection is not
among the inputs, so I wonder if 

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

At least “tools/osinfo-query.c” has a license header that says it’s
released under GPLv2+.  It is better to make the license field hold a
list of “(license:gpl2+ license:lgpl2.1+)” with a comment above it that
explains what files are under GPL (the tools) and what files are LGPL
(the library).

Could you please send an updated patch (after clarifying the points
above if they are confusing/contentious)?

Thanks again for the patch!

~~ Ricardo