diff mbox series

[bug#54069,1/2] gnu: Add hwdata.

Message ID 20220220095557.1497-1-mail@brendan.scot
State Accepted
Headers show
Series gnu: pciutils: Unbundle pci.ids and use latest. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

Brendan Tildesley Feb. 20, 2022, 9:55 a.m. UTC
* gnu/packages/pciutils.scm (hwdata): New variable.
---
 gnu/packages/pciutils.scm | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

Comments

Maxim Cournoyer Feb. 24, 2022, 3:32 a.m. UTC | #1
Hello,

Brendan Tildesley <mail@brendan.scot> 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 <me@tobias.gr>
>  ;;; Copyright © 2019 Mathieu Othacehe <m.othacehe@gmail.com>
>  ;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org>
> +;;; Copyright © 2022 Brendan Tildesley <mail@brendan.scot>
>  ;;;
>  ;;; 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
Brendan Tildesley Feb. 25, 2022, 4:44 a.m. UTC | #2
> On 02/24/2022 4:32 AM Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
> ...
> 
> 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---
> 
Note that it doesn't gzip them, and oui.txt is 8.4MB. Dependencies only tend
to need one or two of these files, lke usb.ids or pci.ids. Also the hwdata project
only updates these every couple months. We could define a package for each individual
file, then if for some reason all are needed in a directory, a "hwdata" union package
 could be created.
Also, this hwdata project is not fully up to date either but have 1-2 month old versions.
It also installs dist-blacklist.conf. Do we need that?
Maxim Cournoyer Feb. 26, 2022, 5 a.m. UTC | #3
Hi Brendan,

Brendan Tildesley <mail@brendan.scot> writes:

>> On 02/24/2022 4:32 AM Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>> ...
>> 
>> 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---
>> 
> Note that it doesn't gzip them, and oui.txt is 8.4MB. Dependencies only tend
> to need one or two of these files, lke usb.ids or pci.ids. Also the hwdata project
> only updates these every couple months. We could define a package for each individual
> file, then if for some reason all are needed in a directory, a "hwdata" union package
>  could be created.

Unless there's a good reason to split the package (which could be
multiple outputs instead of multiple packages), I'd leave it the way
upstream intended it; and there's no better place for inspiration than
[0] for that, as the Fedora package maintainer happens to also be the
maintainer of hwdata :-).

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

*If* we want to have our pciutils package use the hwdata pci.ids, we can
avoid depending on the bigger 'hwdata' package by copying hwdata's
pci.ids over its own, and then build as usual.

Does that make sense?

Thank you for looking into it!

Maxim
diff mbox series

Patch

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 <me@tobias.gr>
 ;;; Copyright © 2019 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org>
+;;; Copyright © 2022 Brendan Tildesley <mail@brendan.scot>
 ;;;
 ;;; 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"))))
+    (home-page "https://pci-ids.ucw.cz")
+    (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.")
+    (license (list license:gpl2+
+                   license:expat)))) ;; XFree86 1.0
+;;
 (define-public pciutils
   (package
     (name "pciutils")