Subject: Re: bug#54069: [PATCH 0/2] gnu: pciutils: Unbundle pci.ids and use latest.

From: Maxim Cournoyer <[email-masked]>
Date: Wed, 23 Feb 2022 22:32:28 -0500

Hello,

Brendan Tildesley <[email-masked]> writes:

> * gnu/packages/pciutils.scm (hwdata): New variable.
> ---
>  gnu/packages/pciutils.scm | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/gnu/packages/pciutils.scm b/gnu/packages/pciutils.scm
> index b6b1e3ad85..416328bda2 100644
> --- a/gnu/packages/pciutils.scm
> +++ b/gnu/packages/pciutils.scm
> @@ -4,6 +4,7 @@
>  ;;; Copyright © 2018 Tobias Geerinckx-Rice <[email-masked]>
>  ;;; Copyright © 2019 Mathieu Othacehe <[email-masked]>
>  ;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <[email-masked]>
> +;;; Copyright © 2022 Brendan Tildesley <[email-masked]>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -25,7 +26,9 @@ (define-module (gnu packages pciutils)
>    #:use-module (guix download)
>    #:use-module ((guix licenses) #:prefix license:)
>    #:use-module (guix utils)
> +  #:use-module (guix git-download)
>    #:use-module (guix build-system gnu)
> +  #:use-module (guix build-system copy)
>    #:use-module (gnu packages)
>    #:use-module (gnu packages compression)
>    #:use-module (gnu packages pkg-config)
> @@ -33,6 +36,32 @@ (define-module (gnu packages pciutils)
>    #:use-module (gnu packages linux)
>    #:use-module (gnu packages base))
>  
> +(define-public hwdata
> +  (package
> +    (name "hwdata")
> +    (version "0.356")
> +    (source (origin
> +              (method git-fetch)
> +              (uri (git-reference
> +                    (url "https://github.com/vcrhonek/hwdata")
> +                    (commit (string-append "v" version))))
> +              (file-name (git-file-name name version))
> +              (sha256
> +               (base32
> +                "0m04d93dwiplwp9v74nhnc0hyi2n007mylkg8f0frb46z5qjrpl3"))))
> +    (build-system copy-build-system)
> +    (arguments
> +     `(#:install-plan '(("pci.ids" "share/hwdata/pci.ids")
> +                        ("pnp.ids" "share/hwdata/pnp.ids")
> +                        ("usb.ids" "share/hwdata/usb.ids"))))

A Makefile is provided and has an install target; we should use it.  It
installs the following files:

--8<---------------cut here---------------start------------->8---
IDFILES = pci.ids usb.ids oui.txt iab.txt pnp.ids
--8<---------------cut here---------------end--------------->8---

> +    (home-page "https://pci-ids.ucw.cz")

The home page should rather be "https://github.com/vcrhonek/hwdata";
this project aggregates various databases that are sourced not only from
the pciutils project (see:
https://github.com/vcrhonek/hwdata/blob/master/Makefile#L99).

> +    (synopsis "Hardware identification databases")
> +    (description "Hardware databases pci.ids, pnp.ids, and usb.ids including
> +all known ID's used in PCI devices: ID's of vendors, devices, subsystems and
> +device classes.")

I'd take the summary and description from Fedora, which reads better to
me [0]:

synopsis: "Hardware identification and configuration data"
	
description: "@code{hwdata} contains various hardware identification and
configuration data, such as the @file{pci.ids} and @file{usb.ids}
databases."

[0]  https://src.fedoraproject.org/rpms/hwdata/blob/rawhide/f/hwdata.spec

> +    (license (list license:gpl2+
> +                   license:expat)))) ;; XFree86 1.0
> +;;

Nit-pick: inline comments should be ";some comment" (typically no
capitalization although XFree86 is OK and no space between the single
semicolon and first comment word).

Also some trailing ';;' was added by mistake.

Could you make the changes along those lines and resend a v2 for the
first patch?

Thanks,

Maxim