[PATCH 0/2] gnu: pciutils: Unbundle pci.ids and use latest.

Completely untested but I needed pci.ids for something and discovered pciutils bundles an old version so I made this patch as something of a suggestion. Looks like it would be a core-updates change.

[inputs]: Remove zlib. Add hwdata. pci.ids was never gzip'd anyway.

[arguments]: Don't install update-pciids script or it's man page. Don't

install bundled old pci.ids file. Copy pci.ids file from latest hwdata package.

---

gnu/packages/pciutils.scm | 25 ++++++++++++++++++-------

1 file changed, 18 insertions(+), 7 deletions(-)

---

gnu/packages/pciutils.scm | 29 +++++++++++++++++++++++++++++

1 file changed, 29 insertions(+)

Hi Brendan,

Thanks for this possibility, I've actually had to address something similar in https://issues.guix.gnu.org/53015where I made a pciutils variant for this purpose. The patch is unmerged currently though.

I think that would work short term for our packaging needs and, as you say, make this bigger change for core updates (or wherever it will best fit). I haven't reviewed this patch, but in this way I think any updates to hwdata will cause pciutils to rebuild, which has many dependents. I'm not sure what is best, maybe: have pciutils use a frozen version of the hwdata with the separate package being a standalone more up to date data?

By the way, I do see in pciutils that share/hwdata/pci.ids.gz though admittedly I never checked to see if that really is gziped. Packages that want a plain pci.ids did not like it though (maybe just from file name?).

Would be great to have others chime in on what the longer term fix would be, in the short term a pciutils variant and/or hwdata standalone package would be helpful.

John

Hi,

Brendan Tildesley <mail@brendan.scot> writes:

It seems to me perhaps a more proper fix would be to have hwdata used as

an input, replacing the bundled copy of the pci.ids file file, and then

letting pciutils proceed as usual? Disabling compression makes the

database about 4.3 times larger (1.2 MiB instead of 0.273 MiB).

I feel like applications parsing pci.ids themselves should probably rely

on the pciutils library API instead (libpci.so) to retrieve or filter

PCI identifiers without being concerned with the actual format of the

database.

What do others think?

Maxim

...

I mistakenly thought they were not gzip'd in the first place.

Should we have a separate output/package for each of the 3 files, and then

embed their paths as needed?

libosinfo also pulls in its own pci.ids. I searched nix and found there are

several more programs that refer to the files directly.

Hi Brendan and Maxim,

I'm aware of a couple of programs that want to use the plain pci.ids. True, they could use pciutils for that, but pci.ids is very simple, just a list of hardware ids paired with manufacturer/device names. While we can see if upstream for these programs will move to using pciutils, I could see reasons for not wanting the dependency or wanting more current info (especially in Guix where we would only update pciutils on core-updates).

So I would find the separate hwdata package useful for a few packages I have, like mangohud (pending review) and corectrl (not submitted yet). I have to check, but they might only want pci.ids and not anything in pciutils anyway. Also, hwdata provides more data than just pci.ids, which we don't have packaged for Guix as far as I know.

As for pciutils itself, I don't have any strong feelings on its pci.ids. Unbundling the included (old) info and using a hwdata package as input makes sense to me and is more flexible. On core-updates we can make that change for pciutils, where at some point we'd want to freeze what hwdata version it uses. Meanwhile, hwdata can be updated regularly (looks like monthly releases) for up-to-date data.

I think this would keep pciutils small (zipped pci.ids), unbundled/more up to date, and give us the hwdata as a useful package to use.

WDYT?

Hi John,

John Kehayias <john.kehayias@protonmail.com> writes:

I think your proposition makes a lot of sense; the handful of

applications wanting raw pci.ids can have it via hwdata, while our

pciutils package can take (a frozen/fixed variant of hwdata) as an input

to a get a newer yet compressed pci.ids.gz file.

Seems a win/win solution to me!

Thank you,

Maxim

Hi Maxim,

------- Original Message -------

On Tuesday, February 22nd, 2022 at 4:00 PM, Maxim Cournoyer wrote:

Great! Brendan do you want to update your patches if you agree with this approach? I will test hwdata later for my mangohud patch, but since it is a simple copy install, I don't anticipate any problems.

Oh, and if I understand correctly, keeping pciutils with a pci.ids.gz would avoid a profile collision with hwdata's pci.ids (so you can have both pciutils and the extra data of hwdata), right? Which would also be good.

Thanks everyone,

John

Hi,

John Kehayias <john.kehayias@protonmail.com> writes:

Indeed, I hadn't thought about this but it seems a nice property.

Maxim

I can report that the hwdata package installs for me, and a pending package (mangohud) that wanted the pci.ids file is happy with just hwdata; pciutils is no longer needed.

I did not do a review of the package definition other than this testing, but it worked and was useful for me.

Hello,

Brendan Tildesley <mail@brendan.scot> writes:

A Makefile is provided and has an install target; we should use it. It

installs the following files:

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:

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

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

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?

---

gnu/packages/pciutils.scm | 41 +++++++++++++++++++++++++++++++++++++++

1 file changed, 41 insertions(+)

[inputs]: Add hwdata:pci for latest pci.ids file. Remove zlib since the ids

files aren't gzipped.

[arguments]: Don't install update-pciids script or it's man page. Don't

install bundled old pci.ids file, use hwdata instead.

---

gnu/packages/pciutils.scm | 15 +++++++++------

1 file changed, 9 insertions(+), 6 deletions(-)

[arguments]: Use Gexp style. Use hwdata package for latest usb.ids and

pci.ids.

[inputs]: Add hwdata:pci, hwdata:usb. remove pci.ids, usb.ids origins.

Use new input style.

---

gnu/packages/virtualization.scm | 64 ++++++++++++++-------------------

1 file changed, 26 insertions(+), 38 deletions(-)

[arguments]: Remove zlib support since pci.ids isn't gzipped. Use Gexp.

[inputs]: Add hwdata:pci, remove zlib, pciutils.

[native-inputs]: Remove pkg-config since zlib isn't used.

---

gnu/packages/xorg.scm | 31 ++++++-------------------------

1 file changed, 6 insertions(+), 25 deletions(-)

Hi Brendan,

Brendan Tildesley <mail@brendan.scot> writes:

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

Brendan Tildesley schreef op za 26-02-2022 om 15:32 [+1100]:

To allow for package transformations, I recommend using

this-package-input or the like instead of hardcoding a particular

package:

#~(list

(string-append

"-Dwith-usb-ids-path="

#$(this-package-input "hwdata:usb") "/share/hwdata/usb.ids"))

[...])

it might also be possible to use 'search-input-file' instead, avoiding

input labels (untested):

#~(let* ((inputs #$(input-tuples->gexp (package-inputs this-package)))

(usb.ids (search-input-file inputs "/share/hwdata/usb.ids"))

(pci.ids (search-input-file inputs "/share/hwdata/pci.ids")))

(list (string-append "-Dwith-usb-ids-path=" usb.ids) [...]))

Greetings,

Maxime.

Hi Brendan,

Brendan Tildesley <mail@brendan.scot> writes:

Thank you for refreshing this series. It's starting to look good!

For pciutils though, what I had in mind was more like this:

1. copy hwdata's pci.ids *over* pciutils one after unpack. This means

hwdata can be a simple native-input and no run time reference gets

registered.

2. This would leave the rest of the build unaltered, so that our

pciutils package could still be smaller thanks to the gzip'd pci.ids.gz

(generated at build time from the version copied from hwdawa).

Does this makes sense?

Thanks again,

Maxim

Hello Brendan,

Brendan Tildesley <mail@brendan.scot> writes:

I've removed the above extraneous import,

expound the description a bit to provide guidance about output

usage, e.g.:

and pushed this first commit as 7f8bc02fd8.

Thanks!

Maxim

I got strange errors trying that and couldn't figure out how to make it work.

There are no examples of input-tuples->gexp being used in a package definition.

This works:

#~(list

(string-append "-Dwith-usb-ids-path="

(search-input-file %build-inputs "/share/hwdata/usb.ids"))

(string-append "-Dwith-pci-ids-path="

(search-input-file %build-inputs "/share/hwdata/pci.ids")))

I thought %build-inputs was not a recommended thing to use though?

Backtrace:

In guix/store.scm:

1385:9 19 (map/accumulate-builds #<store-connection 256.99 7f3a3…> …)

1320:8 18 (call-with-build-handler #<procedure 7f3a22f4b360 at g…> …)

In guix/scripts/build.scm:

582:16 17 (_ #<package libosinfo@1.9.0 /home/b/code/guix/gnu/pack…>)

571:24 16 (_ #<package libosinfo@1.9.0 /home/b/code/guix/gnu/pac…> …)

In guix/packages.scm:

1260:17 15 (supported-package? #<package libosinfo@1.9.0 /home/b/…> …)

In guix/memoization.scm:

101:0 14 (_ #<hash-table 7f3a2538e7a0 0/31> #<package libosinfo…> …)

In guix/packages.scm:

1238:37 13 (_)

1498:16 12 (package->bag _ _ _ #:graft? _)

1603:43 11 (thunk)

In ice-9/eval.scm:

191:35 10 (_ #(#(#<directory (gnu packages virtualization) 7f…>) …))

173:47 9 (_ #(#(#<directory (gnu packages virtualization) 7f…>) …))

213:29 8 (_ #(#(#<directory (gnu packages virtualization) 7f…>) …))

155:9 7 (_ #(#(#<directory (gnu packages virtualization) 7f…>) …))

202:35 6 (_ #(#(#<directory (gnu packages virtualization) 7f…>) …))

In guix/gexp.scm:

1886:4 5 (input-tuples->gexp (("glib" #<package glib@2.70…> …) …) …)

In srfi/srfi-1.scm:

586:17 4 (map1 (("glib" #<package glib@2.70.2 gnu/packages…> …) …))

In guix/gexp.scm:

1886:9 3 (_ _)

In ice-9/boot-9.scm:

1685:16 2 (raise-exception _ #:continuable? _)

1685:16 1 (raise-exception _ #:continuable? _)

1685:16 0 (raise-exception _ #:continuable? _)

ice-9/boot-9.scm:1685:16: In procedure raise-exception:

Throw to key `match-error' with args `("match" "no matching pattern" ("glib" #<package glib@2.70.2 gnu/packages/glib.scm:180 7f3a24fdbdc0> "bin"))'.

Brendan Tildesley schreef op zo 27-02-2022 om 02:37 [+0100]:

It is. Anyway, I found a solution, see attachement.

Maybe the 'gexp-input-compiler' could be moved into (guix gexp)

and 'this-package-input-list'/'this-package-native-input-list' could

be moved into (guix packages) (+ documented in the manual)?

CC'ing ludo for the proposed new gexpology.

Greetings,

Maxime.

Maxime Devos schreef op zo 27-02-2022 om 11:46 [+0100]:

Oops forgot the attachement

(use-modules (guix packages) (gnu packages) (gnu packages gtk) (gnu packages glib) (guix gexp)

(gnu packages gnome) (gnu packages xml) (gnu packages virtualization)

(gnu packages pkg-config) (guix git-download) (guix build-system gnu)

(guix build-system meson) (gnu packages compression) (ice-9 match)

(guix monads) (guix store) (guix derivations)

((guix licenses) #:prefix license:))

(define <gexp-input> (@@ (guix gexp) <gexp-input>))

(define-gexp-compiler gexp-input-compiler <gexp-input>

compiler => (lambda (input system target)

(lower-object (gexp-input-thing input) system

#:target

(and (not (gexp-input-native? input)) target)))

expander => (lambda (object lowered output)

(derivation->output-path lowered (gexp-input-output object))))

(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 gnu-build-system)

(outputs '("out" "iab" "oui" "pci" "pnp" "usb"))

(native-inputs (list gzip))

(arguments

;; Tests require pciutils, python, podman. Disable to avoid recursive dep.

(list

#:tests? #f

#:configure-flags #~(list (string-append "--datadir=" #$output "/share"))

#:phases

#~(modify-phases %standard-phases

(replace 'install

(lambda _

(install-file "iab.txt" (string-append #$output:iab "/share/hwdata"))

(install-file "oui.txt" (string-append #$output:oui "/share/hwdata"))

(install-file "pci.ids" (string-append #$output:pci "/share/hwdata"))

(install-file "pnp.ids" (string-append #$output:pnp "/share/hwdata"))

(install-file "usb.ids" (string-append #$output:usb "/share/hwdata")))))))

(home-page "https://github.com/vcrhonek/hwdata")

(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.")

(license (list license:gpl2+

license:expat)))) ;XFree86 1.0

(define* (package-input-list->build-input-list inputs native?)

"Expand INPUTS, a list of input tuples, into a list of name/<gexp-input>

tulples."

#~'#$(map (match-lambda

((name object output)

#~(#$name . #$(gexp-input object output #:native? native?)))

((name object) #~(#$name . #$object)))

inputs))

(define-syntax this-package-input-list

(identifier-syntax

(package-input-list->build-input-list

(append (package-inputs this-package)

(package-propagated-inputs this-package))

#false)))

(define-syntax this-package-native-input-list

(identifier-syntax

(package-input-list->build-input-list

(package-native-inputs this-package) #true)))

(define libosinfo-new

(package

(inherit (specification->package "libosinfo"))

(arguments

(list #:configure-flags

#~(let* ((inputs #$this-package-native-input-list)

(usb.ids (search-input-file inputs "/share/hwdata/usb.ids"))

(pci.ids (search-input-file inputs "/share/hwdata/pci.ids")))

(list (string-append "-Dwith-usb-ids-path=" usb.ids)

(string-append "-Dwith-pci-ids-path=" pci.ids)))

#:phases

#~(modify-phases %standard-phases

(add-after 'unpack 'patch-osinfo-path

(lambda* (#:key inputs #:allow-other-keys)

(substitute* "osinfo/osinfo_loader.c"

(("path = DATA_DIR.*")

(string-append "path = \"" (search-input-directory inputs "/share/osinfo") "\";"))))))))

(inputs

(list libsoup-minimal-2 libxml2 libxslt osinfo-db))

(native-inputs

(list `(,glib "bin")

gobject-introspection

gtk-doc/stable

vala

intltool

pkg-config

`(,hwdata "pci")

`(,hwdata "usb")))))

libosinfo-new

Maxim Cournoyer schreef op za 26-02-2022 om 18:14 [-0500]:

Given that this is pure data, WDYT of adding #:target #false to

'arguments', such that ‘cross-compiling’ hwdata is free, by

making the derivation for cross-compiling hwdata the same

as the derivation for compiling hwdata natively?

Greetings,

Maxime.

Brendan Tildesley schreef op za 26-02-2022 om 15:32 [+1100]:

I would put these in 'inputs', since they are kept in the closure,

and I'd like to eventually automatically implicitely set #:allowed-

references to make sure only (non-native) inputs make their way into

the references, to catch some cross-compilation bugs early.

It doesn't truly matter here though, since hwdata is architecture-

independent.

Greetings,

Maxime.

Hello!

Maxime Devos <maximedevos@telenet.be> skribis:

I think this is equivalent to:

#~(let* ((inputs #$(input-tuples->gexp (package-inputs this-package)))

…)

…)

Am I right?

However, that looks a bit far-fetched to me.

Can’t we instead do:

#~(let* ((usb.ids (string-append #$hwdata:usb "/share/hwdata/usb.ids"))

(pci.ids (string-append #$hwdata:pci "/share/hwdata/pci.ids")))

…)

?

Ludo’.

Ludovic Courtès schreef op zo 27-02-2022 om 22:23 [+0100]:

We could, and that's what the original v2 patch did.

However, this prevents package transformations:

(package

(inherit libosinfo)

(native-inputs

;; not sure about the syntax

(modify-inputs (package-inputs libosinfo)

(replace "hwdata:usb"

hwdata-with-some-new-fancy-hardware "usb"))))

Greetings,

Maxime.

Ludovic Courtès schreef op zo 27-02-2022 om 22:23 [+0100]:

That's what I suggested initially:

However, that lead to errors -- strings like #<package foo [...]>

appeared in the builder.

Greetings,

Maxime.

Hi!

Maxime Devos <maximedevos@telenet.be> skribis:

Right, but maybe that’s good enough?…

Otherwise, we can do:

#~(let* ((usb.ids (string-append

(ungexp (this-package-inputs "hwdata") "usb")

…)))

…)

[...]

Hmm weird.

Thanks,

Ludo’.

I have used the ungexp suggestion Ludo made. I don't have any stance

on the gexp-ology business, so I just did that.

Feel free to change it however one wishes.

the hwata updates can go to master, but the rest should be considered for

core-updates i think?

I have fixed up the pciutils definiton a bit, where I had previously

removed the installation of a man page by mistake.

Implemented the #:target #f suggestion in hwdata, and fixed a small

mistake.

I was away from the computer for a week so I hope I haven't forgotten

to consider all feedback.

Updated hwdata to 357.

cross-compiling free, as explained by Maxime in bug#54069.

---

gnu/packages/pciutils.scm | 1 +

1 file changed, 1 insertion(+)

here which is unused. Remove it.

---

gnu/packages/pciutils.scm | 1 -

1 file changed, 1 deletion(-)

---

gnu/packages/pciutils.scm | 4 ++--

1 file changed, 2 insertions(+), 2 deletions(-)

[inputs]: Add hwdata:pci for latest pci.ids file.

[arguments]: Don't install update-pciids script or it's man page. Don't

install bundled old pci.ids file, use hwdata instead. Copy the pci.ids

over so it can be gzipped.

---

gnu/packages/pciutils.scm | 18 +++++++++++++++---

1 file changed, 15 insertions(+), 3 deletions(-)

this-package-input for osinfo-db to support package-transformations.

---

gnu/packages/virtualization.scm | 4 +++-

1 file changed, 3 insertions(+), 1 deletion(-)

[arguments]: Use Gexp style. Use hwdata package for latest usb.ids and

pci.ids.

[inputs]: Add hwdata:pci, hwdata:usb. remove pci.ids, usb.ids origins.

Use new input style.

---

gnu/packages/virtualization.scm | 68 +++++++++++++++------------------

1 file changed, 30 insertions(+), 38 deletions(-)

[arguments]: Remove zlib support since pci.ids isn't gzipped. Use Gexp.

[inputs]: Add hwdata:pci, remove zlib, pciutils.

[native-inputs]: Remove pkg-config since zlib isn't used.

---

gnu/packages/xorg.scm | 34 +++++++++-------------------------

1 file changed, 9 insertions(+), 25 deletions(-)

Hi Brendan,

Brendan Tildesley <mail@brendan.scot> writes:

While that simplify things a bit, it's not really required since the

pci.ids.gz from pciutils will now be generated from hwdata:pci, right?

I think I'd rather it stay that way for now.

Thanks,

Maxim

Hi Brendan,

Brendan Tildesley <mail@brendan.scot> writes:

The GNU changelog idea is to stick to changes in the commit message; if

explanations are needed they can go in the code as comments.

Alternatively, you can also put explanations as a paragraph between the

commit summary (first line) and the changelog.

^ not needed

I used search-input-file, making sure to use (or native-inputs inputs)

as its first argument, so that it works even when cross-compiling.

^ not needed

I moved hwdata to native-inputs,

And pushed as 387a9772c2571d4375d6c3682c7572d12e7ec5b2 to the

core-updates branch.

Thank you!

Maxim

Hi!

Brendan Tildesley <mail@brendan.scot> writes:

I've squashed the modified the libosinfo patches a little bit, updated

it to 1.10.0, and made some other small adjustments, and pushed the

hwdata and libosinfo patches as

4b3b873344eb78a4cd96742e4035b39cbec79577.

Thank you!

Closing.

Maxim