[Buildroot] [PATCH v4 01/10] package/wlroots: add hwdata and hwdata_pnp_ids as a dependency

[Thomas Petazzoni]

Hello,

On Fri, 22 Dec 2023 18:02:40 +0100
"Yann E. MORIN" <yann.morin.1998 at free.fr> wrote:

> Thanks for the reproducer. :-) However, Thomas was also wondering why
> this was never caught in our autobuilders. Do you have an idea why it
> was never caught? Since the autobuilders did not help, we have no idea
> when this started to occur, so we have no idea whether we need to
> backport the fix. Having an explanation why the autobuilder did not
> catch the issue will help decide whether this needs to be backported.
> 
> Second, hwdata does not provide a library or headers, just data files
> that describe the VID/PID mappings to human-readable names; those are
> supposed to be used at runtime to do the conversion (e.g. lspci or lsusb
> do that).
> 
> And as you report a build failure, it means those are used at build
> time, which is weird.
> 
> So, what does wlroots do?
> 
> In fact, the build scans the pnp.ids and generates a C file with a big
> switch-case mapping all the PNP ids to their vendor string, and thus
> hwdata is not needed on the target, but is needed at build time.
> 
> So, it means we end up with the hwdata on the target for nothing. The
> default config will install about 2.1MiB, which is arguably not too much
> if one builds a system with wlroots (the stack is already big). Still,
> this is noticeable.
> 
> Of course, this is not your fault! :-) But it would have been good to
> provide this info in the commit log, even as a terse sentence, to avoid
> the maintainers trying to understand this (i.e. try to understand the
> reason why the data files are needed at build time rather than are
> runtime).
> 
> Third and finally, hwdata is not mandatory in wlroots; it is only
> required for the DRM backend, so it should not be unconditional. Except
> that in Buildroot, we do enable the DRM backend unconditionally, so
> hwdata indeed becomes an unconditional dependency.
> 
> So yes, it does indeed take some time to come up with the underlying
> reasons for why a change is needed, and to digest them into a commit
> log. But if submitters don't do that work on their end, it means the
> maintainers have do it, and that means even more time is spent reviewing
> and replying to each patch, which makes the patch queue grow, and
> ultimately makes everyone grumpy, submitters first.

Thanks for the additional research. So let me fill in the blanks, which
I could find with some further research. Looking at
http://autobuild.buildroot.net/index.php?symbols[BR2_PACKAGE_WLROOTS]=y&step=250,
we had no fully successful build of a configuration that includes
wlroots since 2022-05-05, at which time we built Buildroot 27c672e5,
which had wlroots 0.15.1. We now have wlroots 0.16.2 in Buildroot.

Turns out that upstream commit
"eec95e3d5e1a4f2e13b1f6b34cc287475ca57daf backend/drm: use pnp.ids to
fetch EDID data" is the one that introduced this usage of the pnp.ids
file, and this commit went in wlroots 0.16.0.

So both 2023.02.x and 2023.08.x are unaffected because they have
wlroots 0.15.1. We bumped from 0.15.1 to 0.16.2 in
d6279bc82c02b43c9a2f28c36639e092b9e9e08b, which is not in any tagged
release.

Adam: this whole research that Yann and I did is what we expect you to
do. This is what is needed if you want your patches to be merged
faster. We have been repeating this over and over and over again to
you, but rather than improving, you have chosen to complain on us not
merging your patches fast enough.

If you want your patches to be merge faster, you need to increase the
trust we have in your contributions. To increase the trust we have in
your contributions, you should not submit patches that obviously don't
build even in a basic configuration (your recent dmenu-wayland), and
don't submit patches that are clearly insufficiently documented (this
wlroots patch).

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com