> 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)
> 
> Alex Merry wrote:
>     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.
> 
> Martin Gräßlin wrote:
>     Ok, I'll discard the review. Unfortunately I don't know how to go forward 
> as my CMake knowledge is not sufficient enough to add such a warning.

I'll try to set aside some time to look at it.


- Alex


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


On Sept. 13, 2015, 9:41 a.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125139/
> -----------------------------------------------------------
> 
> (Updated Sept. 13, 2015, 9:41 a.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
> 
>

_______________________________________________
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Reply via email to