Re: Review Request 126013: fix: kacess should not be built if xkb is not present
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126013/ --- (Updated Nov. 25, 2015, 2:04 p.m.) Status -- This change has been marked as submitted. Review request for Plasma. Changes --- Submitted with commit 5667cb503e513bc5197b2ad008d1a921d450ffb3 by Sebastian Kügler to branch Plasma/5.5. 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 ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 126013: fix: kacess should not be built if xkb is not present
On Monday, November 16, 2015 04:16:26 PM 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.) Ah, ok. Thanks for the clarification. -- sebas http://www.kde.org | http://vizZzion.org ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 126013: fix: kacess should not be built if xkb is not present
> 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: g...@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 > > ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 126013: fix: kacess should not be built if xkb is not present
> 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: g...@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. > 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.) - Johan --- 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 > > ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 126013: fix: kacess should not be built if xkb is not present
> 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: g...@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! 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. - 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 > > ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 126013: fix: kacess should not be built if xkb is not present
> On Nov. 10, 2015, 2: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: g...@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. > 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. - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126013/#review88208 --- On Nov. 10, 2015, 1: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, 1: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 > > ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 126013: fix: kacess should not be built if xkb is not present
> 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: g...@github.com:cmacq2/plasma-desktop.git Please note: I do not have commit access (just a KDE identity account) so please pull these changes because I cannot commit them! - Johan --- 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 > > ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 126013: fix: kacess should not be built if xkb is not present
> On Nov. 10, 2015, 1:26 a.m., David Edmundson wrote: > > Ship It! Forgot to mention that the patch can be pulled from the fix-cmake-xkb-dep branch at: g...@github.com:cmacq2/plasma-desktop.git - Johan --- 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 > > ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 126013: fix: kacess should not be built if xkb is not present
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126013/ --- 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 ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 126013: fix: kacess should not be built if xkb is not present
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126013/#review88208 --- Ship it! Ship It! - David Edmundson 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 > > ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
