Review Request 126013: fix: kacess should not be built if xkb is not present
Sebastian Kügler
sebas at kde.org
Tue Nov 17 20:39:51 UTC 2015
> On Nov. 10, 2015, 1:26 a.m., David Edmundson wrote:
> > Ship It!
>
> Johan Ouwerkerk wrote:
> Forgot to mention that the patch can be pulled from the fix-cmake-xkb-dep branch at: git at github.com:cmacq2/plasma-desktop.git
>
> Johan Ouwerkerk wrote:
> Please note: I do not have commit access (just a KDE identity account) so please pull these changes because I cannot commit them!
>
> Sebastian Kügler wrote:
> I see a problem here. The features offered by kaccess are important and necessary for some users. Are we just disabling it and then be done? I think we need to put this stuff on a porting list and find a better solution than to just not build everything that #includes xcbsomething.
>
> Johan, I think you're getting pretty close to us asking you to get a KDE identity account with commit privileges, just so you know. https://identity.kde.org/index.php?r=developerApplication is currently broken, but I'd say as soon as it's fixed, let's start that process.
>
> Martin Gräßlin wrote:
> > Are we just disabling it and then be done?
>
> I think it's the distros task to built it properly.
>
> And yes, we might need to rethink which platform specific packages should be required in build. Just saying "hey let's make all X11 deps mandatory" is also not the way to go anymore with Wayland.
>
> Johan Ouwerkerk wrote:
> > Are we just disabling it and then be done?
>
> That is not what this change is about. The change rectifies an inconsistency between the top level cmake configure stuff which checks the build environment for dependencies, and the C++ code which uses these unconditionally (e.g. kaccess).
>
> The plasma-desktop build process deals with two classes of X11 deps: mandatory and optional. As it happens the XKB dep is classified optional in the top level cmake files, meaning configure will accept a build environment without XKB. This change makes sure that the 'optional' part is, indeed, consistently optional and does not result in build failure later (during make).
>
> People who need kaccess must necessarily install the XKB headers/development files anyway. If the XKB dep is found then kacess is still included in the build and nothing changes (effectively).
>
> (The alternative is to classify XKB as mandatory.)
I see, thanks for the clarification.
- Sebastian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126013/#review88208
-----------------------------------------------------------
On Nov. 10, 2015, 12:52 a.m., Johan Ouwerkerk wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126013/
> -----------------------------------------------------------
>
> (Updated Nov. 10, 2015, 12:52 a.m.)
>
>
> Review request for Plasma.
>
>
> Repository: plasma-desktop
>
>
> Description
> -------
>
> The XCB-XKB dependency of plasma-desktop is optional, however kacess depends directly and unconditionally on the xcb/xkb.h header file.
> This causes the build of plasma-desktop to fail at compile stage if the xkb header is not present.
>
> As per the convention in the kcm modules shipped by plasma-desktop, inclusion of the kacess subdirectory is made conditional on XKB being present and properly detected during the cmake configure stage.
>
>
> Diffs
> -----
>
> CMakeLists.txt 193238a8dcbb2c6e2bda01c390c57ff2ecfebeb0
>
> Diff: https://git.reviewboard.kde.org/r/126013/diff/
>
>
> Testing
> -------
>
> Built without xkb header, using kdesrc-build
>
>
> Thanks,
>
> Johan Ouwerkerk
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20151117/a3ae20cf/attachment.html>
More information about the Plasma-devel
mailing list