Review Request 125139: Do not find XINPUT in FindXCB.cmake

[Alex Merry]

> On Sept. 10, 2015, 5:42 p.m., Alex Merry wrote:
> > I'm wary about this change as it technically breaks module compatibility. Specifically, upgrading e-c-m could cause packages that formerly built to suddenly stop building, and they wouldn't even have any way to fix that. extragear/base/wacomtablet is an example of such a project.
> > 
> > I understand the wish to discourage cavalier use of XINPUT, though. Printing a warning if XINPUT was specifically requested is an option, but there's no way to know if someone calling `find_package(XCB)` will make use of the `XCB_XINPUT_FOUND` variable.
> > 
> > As a compromise, I'd be willing to consider a solution where XINPUT wasn't in the default set of things to search for, but could be specified explicitly (and that could be accompanied by a warning if you like). Note that this would require extending `ecm_find_package_parse_components` to allow a separate list of default components.
> > 
> > That solution shouldn't break anything too much, because if a project is not explicitly specifying components, then they are optional by default, so such a project will already need to cope in some way with XINPUT not being found (unless they are just letting `feature_summary` enforce the requirement).
> 
> Martin Gräßlin wrote:
>     > they wouldn't even have any way to fix that. extragear/base/wacomtablet is an example of such a project.
>     
>     yes, that's my aim here. I do want to explicitly break wacomtablet, because currently it's just implicitly broken - which is worse IMHO. For the record: wacomtablet triggered this review request.
>     
>     I would also not say that this is a breakage, but a bug fix. It's a bug that ecm finds xinput in the first place.
>     
>     
>     My suggestion as a comporomise is to add an option to allow finding it: 
>     set(ECM_XCB_I_KNOW_XINPUT_IS_NOT_STABLE_AND_WONT_WORK_ON_DISTROS TRUE)

I disagree. If FindXCB.cmake were going in for the first time, then yes, this change would be fine. However, the problem with compatibility guarantees is that you have to live with your mistakes. I'm not willing to have an update of ECM break a use that was previously documented to work, no matter how misguided you believe that use was.

I don't even really like removing it from the default set of components to search for, TBH, but I'm willing to accept it in the circumstances. That and a big fat warning when you explicitly specify XINPUT will have to be sufficient.


- Alex


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125139/#review85125
-----------------------------------------------------------


On Sept. 10, 2015, 2:52 p.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125139/
> -----------------------------------------------------------
> 
> (Updated Sept. 10, 2015, 2:52 p.m.)
> 
> 
> Review request for Extra Cmake Modules, Alex Merry and Harald Sitter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> -------
> 
> The XCB bindings for XINPUT are not finished and explictly disabled
> in XCB, see for xcb 1.11 [1].
> 
> Because of that most distributions do not include xinput. Trying to find
> xinput only creates problems - the distro of the user might provide it
> and they might not be aware that it's not available normally.
> 
> Thus let's not pretend it exists. Make life of everybody easier.
> 
> [1] http://cgit.freedesktop.org/xcb/libxcb/tree/configure.ac?id=1.11#n234
> 
> 
> Diffs
> -----
> 
>   find-modules/FindXCB.cmake b2a800f73058382ee84f7d93132a96f8852b4aa7 
> 
> Diff: https://git.reviewboard.kde.org/r/125139/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20150912/3944dce4/attachment.html>