Re: [PATCH v2] uhid: Remove local copy of uhid header

[Luiz Augusto von Dentz <luiz.dentz@gmail.com>]

Hi Marcel, Bastien,

On Tue, Nov 23, 2021 at 5:24 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Batien,
>
> >>>> uhid.h is part of kernel uapi nowadays so it can be included
> >>>> directly from linux/uhid.h so this removes the local copy to use
> >>>> it
> >>>> instead.
> >>>
> >>> This will cause the same problems that importing those headers into
> >>> the
> >>> repository was supposed to solve. If you remove those headers, then
> >>> older distributions will be stuck on old kernel headers, and bluez
> >>> compilations will regularly fail when it uses new structs, struct
> >>> members, functions, or constants that are in the upstream uapi
> >>> headers
> >>> but not yet propagated to the user's distribution.
> >>
> >> I'm not following you on this, the distros don't have uapi headers
> >> that match their kernel release? Afaik being a kernel uapi means
> >> these
> >> APIs are considered stable and I suspect they haven't been changed in
> >> a while, you are right that this introduces a kernel dependency if we
> >> were to use new members but I still think this is better than having
> >> copies that at some point goes out of sync, specially when nothing
> >> indicates that things like input_event was not accepted by the
> >> kernel.
> >
> > Let's say you want to use a KEY_* definition that was recently added to
> > the kernel, what would you do?
> >
> >>> Strong NAK on this one and the uinput header change too.
> >>
> >> I didn't introduce the usage of any new symbols, so the only new
> >> requirement is that linux/uinput.h and linux/uhid.h headers exist, I
> >> could however rollback if we have another way to check if those
> >> headers exists use then otherwise we can keep using copies, perhaps
> >> move then under linux/ directory, anyway it is not like we don't have
> >> kernel dependencies already and we actually have been debating on
> >> having the bluetooth socket definitions as part of the uapi instead
> >> of
> >> libbluetooth, so we will need to sort out how we can update the
> >> userspace parts with new kernel interface without breaking the build
> >> on systems that don't actually ship with e.g. linux/bluetooth.h.
> >
> > You're talking about the state of things now, I'm talking about the
> > compilation failures once you rely on a slightly newer header that
> > isn't shipped with a distribution.
> >
> > But if you're willing to react to the compilation failure in the
> > future, I can't really stand in your way. It just seems weird to do
> > this now just to undo it the moment you'll need a slightly newer kernel
> > header.
>
> if I can’t build the tarballs on 5.10.0-7-powerpc, then they don’t get released.

The way I see this is only really a problem for unstable uapi headers,
so in case we are early adopters it is probably a good practice to
have a copy to make sure the packages can be compiled on systems
without these headers, once these headers becomes stable and most
distros already ship with them, imo, there is no reason to keep the
copies as they are not subject to major redesign and normally just
receive minor updates for bugs, etc,  which is the kind of change we
would likely miss given we were no longer paying attention to changes
on those headers as they are considered stable.

Also we might as well include these headers to be check like we do in
the following check:

https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/configure.ac#n66

-- 
Luiz Augusto von Dentz