Skip to contentSkip to conversation

libpciaccess: add dependency on hwdata

PATCH New Carlos Santos 4 participants 7 messages Jan 28, 2018 – Apr 28, 2018

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 libpciaccess: add dependency on hwdata

libpciaccess requires /usr/share/hwdata/pci.ids (or pci.ids.gz, if compiled with zlib support). That file is provided by the hwdata package, so add it as a run-time dependency.

---
 package/libpciaccess/Config.in | 2 ++
 1 file changed, 2 insertions(+)
package/libpciaccess/Config.in +2 -0
index 6cc983f577..381383e1f9 100644
@@ -1,4 +1,6 @@
 config BR2_PACKAGE_LIBPCIACCESS
 	bool "libpciaccess"
+	select BR2_PACKAGE_HWDATA
+	select BR2_PACKAGE_HWDATA_PCI_IDS
 	help
 	  X.Org libpciaccess
Signature · 1 line
2.14.3
#1 lore raw

PATCH 1/1 libpciaccess: add dependency on hwdata

Quoted 7 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Carlos" == Carlos Santos <[email protected]> writes:

libpciaccess requires /usr/share/hwdata/pci.ids (or pci.ids.gz, if compiled with zlib support). That file is provided by the hwdata package, so add it as a run-time dependency.

Is this really a hard dependency? Looking at src/common_device_name.c, this seems to only be used in populate_vendor(), which just returns without error if the file cannot be opened.

Signature · 1 line
Bye, Peter Korsgaard
#2 lore raw

PATCH 1/1 libpciaccess: add dependency on hwdata

Quoted 12 lines

From: "Peter Korsgaard" <[email protected]>
To: "Carlos Santos" <[email protected]>
Cc: "buildroot" <[email protected]>, "Bernd Kuhls" <[email protected]>
Sent: Thursday, March 29, 2018 11:05:17 AM
Subject: Re: [PATCH] libpciaccess: add dependency on hwdata

Quoted 5 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Carlos" == Carlos Santos <[email protected]> writes:

libpciaccess requires /usr/share/hwdata/pci.ids (or pci.ids.gz, if compiled with zlib support). That file is provided by the hwdata package, so add it as a run-time dependency.

Quoted 8 lines
Quoted 1 line

Is this really a hard dependency? Looking at src/common_device_name.c, this seems to only be used in populate_vendor(), which just returns without error if the file cannot be opened.

--
Bye, Peter Korsgaard

It is required by libvirt[1] to show device names when we use virtual machine manager to add hardware via PCI passthrough, otherwise only the PCI bus information (slot, port, function) is shown.

1. https://patchwork.ozlabs.org/patch/841613/

Signature · 4 lines
Carlos Santos (Casantos) - DATACOM, P&D
?The greatest triumph that modern PR can offer is the transcendent 
success of having your words and actions judged by your reputation, 
rather than the other way about.? ? Christopher Hitchens
#3 lore raw

PATCH 1/1 libpciaccess: add dependency on hwdata

Carlos, All,

On 2018-03-29 13:41 -0300, Carlos Santos spake thusly:

Quoted 18 lines
Quoted 18 lines

From: "Peter Korsgaard" <[email protected]>
To: "Carlos Santos" <[email protected]>
Cc: "buildroot" <[email protected]>, "Bernd Kuhls" <[email protected]>
Sent: Thursday, March 29, 2018 11:05:17 AM
Subject: Re: [PATCH] libpciaccess: add dependency on hwdata

Quoted 7 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Carlos" == Carlos Santos <[email protected]> writes:

libpciaccess requires /usr/share/hwdata/pci.ids (or pci.ids.gz, if compiled with zlib support). That file is provided by the hwdata package, so add it as a run-time dependency.

Is this really a hard dependency? Looking at src/common_device_name.c, this seems to only be used in populate_vendor(), which just returns without error if the file cannot be opened.

Quoted 6 lines
Quoted 2 lines

--
Bye, Peter Korsgaard

It is required by libvirt[1] to show device names when we use virtual machine manager to add hardware via PCI passthrough, otherwise only the PCI bus information (slot, port, function) is shown.

Then I would say that it is not _required_. It may be _needed_, but it is not mandatory; it just makes it a little bit more user-friendly.

Thus, I would say that we do not want to enforce this dependency, because it still works without it.

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#4 lore raw

PATCH 1/1 libpciaccess: add dependency on hwdata

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

Hi,

Quoted 9 lines
Quoted 3 lines

It is required by libvirt[1] to show device names when we use virtual machine manager to add hardware via PCI passthrough, otherwise only the PCI bus information (slot, port, function) is shown.

Then I would say that it is not _required_. It may be _needed_, but it is not mandatory; it just makes it a little bit more user-friendly.

Thus, I would say that we do not want to enforce this dependency, because it still works without it.

And if it IS required when using libvirt with pci passthrough, but not necessarily for other users of libpciaccess, then we can make libvirt select it.

In the general case for nice-to-have-but-not-absolutely-required dependencies we don't have a solution in Buildroot today except for a note in the help text. Upstream kconfig has recently added the imply keyword which could be used to implement this, but we would need to sync our kconfig with upstream for that.

Signature · 1 line
Bye, Peter Korsgaard
#5 lore raw

PATCH 1/1 libpciaccess: add dependency on hwdata

Quoted 25 lines

From: "Peter Korsgaard" <[email protected]>
To: "Yann E. MORIN" <[email protected]>
Cc: "Carlos Santos" <[email protected]>, "Bernd Kuhls" <[email protected]>, "buildroot" <[email protected]>
Sent: Thursday, March 29, 2018 4:05:28 PM
Subject: Re: [PATCH] libpciaccess: add dependency on hwdata

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

Hi,

Quoted 9 lines
Quoted 3 lines

It is required by libvirt[1] to show device names when we use virtual machine manager to add hardware via PCI passthrough, otherwise only the PCI bus information (slot, port, function) is shown.

Then I would say that it is not _required_. It may be _needed_, but it is not mandatory; it just makes it a little bit more user-friendly.

Thus, I would say that we do not want to enforce this dependency, because it still works without it.

And if it IS required when using libvirt with pci passthrough, but not necessarily for other users of libpciaccess, then we can make libvirt select it.

Quoted 5 lines

In the general case for nice-to-have-but-not-absolutely-required dependencies we don't have a solution in Buildroot today except for a note in the help text. Upstream kconfig has recently added the imply keyword which could be used to implement this, but we would need to sync our kconfig with upstream for that.

OK, I will move the selection o hwdata to libvirt in the next version of the path series.

Signature · 4 lines
Carlos Santos (Casantos) - DATACOM, P&D
?The greatest triumph that modern PR can offer is the transcendent 
success of having your words and actions judged by your reputation, 
rather than the other way about.? ? Christopher Hitchens
#6 lore raw

PATCH 1/1 libpciaccess: add dependency on hwdata

Hello,

Quoted 8 lines · Carlos Santos
Quoted 5 lines

In the general case for nice-to-have-but-not-absolutely-required dependencies we don't have a solution in Buildroot today except for a note in the help text. Upstream kconfig has recently added the imply keyword which could be used to implement this, but we would need to sync our kconfig with upstream for that.

OK, I will move the selection o hwdata to libvirt in the next version of the path series.

I don't know where we stand with the libvirt packaging, but following this feedback, I marked this patch as Rejected in patchwork.

Best regards,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com