Re: Review Request 119025: Actually pass IBUS_DEFINITIONS when compiling ibus-panel
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119025/ --- (Updated Sept. 22, 2016, 8:40 p.m.) Status -- This change has been discarded. Review request for kde-workspace, Plasma and Hui Ni. Repository: kimtoy Description --- The ibus-panel can't build on OpenBSD because some required definitions obtained from pkgconfig file are not used. This happens due to the following reasons: 1. IBUS_DEFINITIONS is not used in ibus-panel/CMakeLists.txt 2. IBUS_DEFINITIONS is not saved in cache, and thus isn't available at compile time This patch resolves both issues and makes ibus-panel compile on OpenBSD. (I found no suitable review group and therefore used "plasma" instead, as it was in "plasma-addons" previously; please, feel free to correct me if I'm wrong and sorry for any possible inconvenience) ((as there was no feedback for more than a week, I've added "kde-workspace" group to list of reviewers, too)) Diffs - ibus-panel/CMakeLists.txt 3a1ee49 Diff: https://git.reviewboard.kde.org/r/119025/diff/ Testing --- OpenBSD/i386-CURRENT, KDE 4.13 (it doesn't have kimtoy package, of course, but the code is same) Thanks, Vadim Zhukov
Re: Review Request 119025: Actually pass IBUS_DEFINITIONS when compiling ibus-panel
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119025/#review99471 --- Closing as this review request is more than 2 years old. If it still applies to current Plasma please reopen this review request. Thanks - David Edmundson On July 19, 2014, 6:43 p.m., Vadim Zhukov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119025/ > --- > > (Updated July 19, 2014, 6:43 p.m.) > > > Review request for kde-workspace, Plasma and Hui Ni. > > > Repository: kimtoy > > > Description > --- > > The ibus-panel can't build on OpenBSD because some required definitions > obtained from pkgconfig file are not used. This happens due to the following > reasons: > > 1. IBUS_DEFINITIONS is not used in ibus-panel/CMakeLists.txt > 2. IBUS_DEFINITIONS is not saved in cache, and thus isn't available at > compile time > > This patch resolves both issues and makes ibus-panel compile on OpenBSD. > > (I found no suitable review group and therefore used "plasma" instead, as it > was in "plasma-addons" previously; please, feel free to correct me if I'm > wrong and sorry for any possible inconvenience) > > ((as there was no feedback for more than a week, I've added "kde-workspace" > group to list of reviewers, too)) > > > Diffs > - > > ibus-panel/CMakeLists.txt 3a1ee49 > > Diff: https://git.reviewboard.kde.org/r/119025/diff/ > > > Testing > --- > > OpenBSD/i386-CURRENT, KDE 4.13 (it doesn't have kimtoy package, of course, > but the code is same) > > > Thanks, > > Vadim Zhukov > >
Re: Review Request 119025: Actually pass IBUS_DEFINITIONS when compiling ibus-panel
On July 19, 2014, 12:17 a.m., Vadim Zhukov wrote: (As a general note, for build system related stuff like this you can also try including the buildsystem group, which can be more responsive at times) The ibus-panel can't build on OpenBSD because some required definitions obtained from pkgconfig file are not used. 1. IBUS_DEFINITIONS is not used in ibus-panel/CMakeLists.txt Can you post the error you get here? I've tried building kimtoy here on FreeBSD expecting to hit the same issue(s), but it all went along just fine. 2. IBUS_DEFINITIONS is not saved in cache, and thus isn't available at compile time This doesn't make much sense; all values found at configuration time in CMake are then used to generate your make/ninja/whatever files, regardless of whether they are cached or not. Vadim Zhukov wrote: Raphael, thank you for your input! The issue was caused by the fact that X includes are placed in the /usr/X11R6/include directory on OpenBSD. This catalog is mentioned in PC_IBUS_DEFINITIONS variable but isn't propagated to the caller of find_package(IBus). Yes, I was wrong: the CACHE part may and should be omitted. This patch was added by me a long time ago (7 Feb 2012 according to git log; guess the KDE version used then), when I was much less expirienced in CMake... I've started a massive push of OpenBSD local patches upstream recently during OpenBSD hackathon, when I got time for such cleanup. At the present time, the /usr/X11R6/include gets to the include_directories() from another place, so the patch isn't required at all. But, still, the PC_IBUS_DEFINITIONS should be respected, IMHO. What do you think? Please note that FreeBSD and OpenBSD and quiet different. So you can't test on one OS instead of another. The issue was caused by the fact that X includes are placed in the /usr/X11R6/include directory on OpenBSD. This catalog is mentioned in PC_IBUS_DEFINITIONS variable but isn't propagated to the caller of find_package(IBus). [...] At the present time, the /usr/X11R6/include gets to the include_directories() from another place, so the patch isn't required at all. But, still, the PC_IBUS_DEFINITIONS should be respected, IMHO. What do you think? While that is not wrong, the approach we normally take with CMake is that pkg-config's presence is optional, and we don't depend on its output to be able to build a module. In practice, this means one should look for IBus and X11 separately, as well as add their compiler flags/link against them independently. If kimtoy already does that, I just wouldn't make any change. Please note that FreeBSD and OpenBSD and quiet different. So you can't test on one OS instead of another. You don't need to preach to the choir :-) I'm well aware of the differences between them, my point is that in many cases packaging problems in one also impact the other (missing includes because people assume all software is in `/usr`, reliance on non-POSIX features without checking for their availability etc). - Raphael --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119025/#review62669 --- On July 19, 2014, 9:43 p.m., Vadim Zhukov wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119025/ --- (Updated July 19, 2014, 9:43 p.m.) Review request for kde-workspace, Plasma and Hui Ni. Repository: kimtoy Description --- The ibus-panel can't build on OpenBSD because some required definitions obtained from pkgconfig file are not used. This happens due to the following reasons: 1. IBUS_DEFINITIONS is not used in ibus-panel/CMakeLists.txt 2. IBUS_DEFINITIONS is not saved in cache, and thus isn't available at compile time This patch resolves both issues and makes ibus-panel compile on OpenBSD. (I found no suitable review group and therefore used plasma instead, as it was in plasma-addons previously; please, feel free to correct me if I'm wrong and sorry for any possible inconvenience) ((as there was no feedback for more than a week, I've added kde-workspace group to list of reviewers, too)) Diffs - ibus-panel/CMakeLists.txt 3a1ee49 Diff: https://git.reviewboard.kde.org/r/119025/diff/ Testing --- OpenBSD/i386-CURRENT, KDE 4.13 (it doesn't have kimtoy package, of course, but the code is same) Thanks, Vadim Zhukov
Re: Review Request 119025: Actually pass IBUS_DEFINITIONS when compiling ibus-panel
On Июль 19, 2014, 1:17 д.п., Vadim Zhukov wrote: (As a general note, for build system related stuff like this you can also try including the buildsystem group, which can be more responsive at times) The ibus-panel can't build on OpenBSD because some required definitions obtained from pkgconfig file are not used. 1. IBUS_DEFINITIONS is not used in ibus-panel/CMakeLists.txt Can you post the error you get here? I've tried building kimtoy here on FreeBSD expecting to hit the same issue(s), but it all went along just fine. 2. IBUS_DEFINITIONS is not saved in cache, and thus isn't available at compile time This doesn't make much sense; all values found at configuration time in CMake are then used to generate your make/ninja/whatever files, regardless of whether they are cached or not. Raphael, thank you for your input! The issue was caused by the fact that X includes are placed in the /usr/X11R6/include directory on OpenBSD. This catalog is mentioned in PC_IBUS_DEFINITIONS variable but isn't propagated to the caller of find_package(IBus). Yes, I was wrong: the CACHE part may and should be omitted. This patch was added by me a long time ago (7 Feb 2012 according to git log; guess the KDE version used then), when I was much less expirienced in CMake... I've started a massive push of OpenBSD local patches upstream recently during OpenBSD hackathon, when I got time for such cleanup. At the present time, the /usr/X11R6/include gets to the include_directories() from another place, so the patch isn't required at all. But, still, the PC_IBUS_DEFINITIONS should be respected, IMHO. What do you think? Please note that FreeBSD and OpenBSD and quiet different. So you can't test on one OS instead of another. - Vadim --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119025/#review62669 --- On Июль 12, 2014, 5:12 п.п., Vadim Zhukov wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119025/ --- (Updated Июль 12, 2014, 5:12 п.п.) Review request for kde-workspace, Plasma and Hui Ni. Repository: kimtoy Description --- The ibus-panel can't build on OpenBSD because some required definitions obtained from pkgconfig file are not used. This happens due to the following reasons: 1. IBUS_DEFINITIONS is not used in ibus-panel/CMakeLists.txt 2. IBUS_DEFINITIONS is not saved in cache, and thus isn't available at compile time This patch resolves both issues and makes ibus-panel compile on OpenBSD. (I found no suitable review group and therefore used plasma instead, as it was in plasma-addons previously; please, feel free to correct me if I'm wrong and sorry for any possible inconvenience) ((as there was no feedback for more than a week, I've added kde-workspace group to list of reviewers, too)) Diffs - cmake/FindIBus.cmake 8250c49 ibus-panel/CMakeLists.txt 3a1ee49 Diff: https://git.reviewboard.kde.org/r/119025/diff/ Testing --- OpenBSD/i386-CURRENT, KDE 4.13 (it doesn't have kimtoy package, of course, but the code is same) Thanks, Vadim Zhukov
Re: Review Request 119025: Actually pass IBUS_DEFINITIONS when compiling ibus-panel
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119025/ --- (Updated Июль 19, 2014, 10:43 п.п.) Review request for kde-workspace, Plasma and Hui Ni. Changes --- Simplified patch, no CACHE needed. Repository: kimtoy Description --- The ibus-panel can't build on OpenBSD because some required definitions obtained from pkgconfig file are not used. This happens due to the following reasons: 1. IBUS_DEFINITIONS is not used in ibus-panel/CMakeLists.txt 2. IBUS_DEFINITIONS is not saved in cache, and thus isn't available at compile time This patch resolves both issues and makes ibus-panel compile on OpenBSD. (I found no suitable review group and therefore used plasma instead, as it was in plasma-addons previously; please, feel free to correct me if I'm wrong and sorry for any possible inconvenience) ((as there was no feedback for more than a week, I've added kde-workspace group to list of reviewers, too)) Diffs (updated) - ibus-panel/CMakeLists.txt 3a1ee49 Diff: https://git.reviewboard.kde.org/r/119025/diff/ Testing --- OpenBSD/i386-CURRENT, KDE 4.13 (it doesn't have kimtoy package, of course, but the code is same) Thanks, Vadim Zhukov
Re: Review Request 119025: Actually pass IBUS_DEFINITIONS when compiling ibus-panel
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119025/#review62669 --- cmake/FindIBus.cmake https://git.reviewboard.kde.org/r/119025/#comment43439 Typo: paramaters (As a general note, for build system related stuff like this you can also try including the buildsystem group, which can be more responsive at times) The ibus-panel can't build on OpenBSD because some required definitions obtained from pkgconfig file are not used. 1. IBUS_DEFINITIONS is not used in ibus-panel/CMakeLists.txt Can you post the error you get here? I've tried building kimtoy here on FreeBSD expecting to hit the same issue(s), but it all went along just fine. 2. IBUS_DEFINITIONS is not saved in cache, and thus isn't available at compile time This doesn't make much sense; all values found at configuration time in CMake are then used to generate your make/ninja/whatever files, regardless of whether they are cached or not. - Raphael Kubo da Costa On July 12, 2014, 4:12 p.m., Vadim Zhukov wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119025/ --- (Updated July 12, 2014, 4:12 p.m.) Review request for kde-workspace, Plasma and Hui Ni. Repository: kimtoy Description --- The ibus-panel can't build on OpenBSD because some required definitions obtained from pkgconfig file are not used. This happens due to the following reasons: 1. IBUS_DEFINITIONS is not used in ibus-panel/CMakeLists.txt 2. IBUS_DEFINITIONS is not saved in cache, and thus isn't available at compile time This patch resolves both issues and makes ibus-panel compile on OpenBSD. (I found no suitable review group and therefore used plasma instead, as it was in plasma-addons previously; please, feel free to correct me if I'm wrong and sorry for any possible inconvenience) ((as there was no feedback for more than a week, I've added kde-workspace group to list of reviewers, too)) Diffs - cmake/FindIBus.cmake 8250c49 ibus-panel/CMakeLists.txt 3a1ee49 Diff: https://git.reviewboard.kde.org/r/119025/diff/ Testing --- OpenBSD/i386-CURRENT, KDE 4.13 (it doesn't have kimtoy package, of course, but the code is same) Thanks, Vadim Zhukov
Re: Review Request 119025: Actually pass IBUS_DEFINITIONS when compiling ibus-panel
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119025/#review62595 --- Ship it! Ship It! - Hui Ni On 七月 12, 2014, 1:12 p.m., Vadim Zhukov wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119025/ --- (Updated 七月 12, 2014, 1:12 p.m.) Review request for kde-workspace, Plasma and Hui Ni. Repository: kimtoy Description --- The ibus-panel can't build on OpenBSD because some required definitions obtained from pkgconfig file are not used. This happens due to the following reasons: 1. IBUS_DEFINITIONS is not used in ibus-panel/CMakeLists.txt 2. IBUS_DEFINITIONS is not saved in cache, and thus isn't available at compile time This patch resolves both issues and makes ibus-panel compile on OpenBSD. (I found no suitable review group and therefore used plasma instead, as it was in plasma-addons previously; please, feel free to correct me if I'm wrong and sorry for any possible inconvenience) ((as there was no feedback for more than a week, I've added kde-workspace group to list of reviewers, too)) Diffs - cmake/FindIBus.cmake 8250c49 ibus-panel/CMakeLists.txt 3a1ee49 Diff: https://git.reviewboard.kde.org/r/119025/diff/ Testing --- OpenBSD/i386-CURRENT, KDE 4.13 (it doesn't have kimtoy package, of course, but the code is same) Thanks, Vadim Zhukov
Re: Review Request 119025: Actually pass IBUS_DEFINITIONS when compiling ibus-panel
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119025/ --- (Updated Июль 12, 2014, 5:12 п.п.) Review request for kde-workspace, Plasma and Hui Ni. Changes --- As a last resort for the patch to be reviewed, adding nihui@ to reviewers, who committed to the ibus-panel last times. Repository: kimtoy Description --- The ibus-panel can't build on OpenBSD because some required definitions obtained from pkgconfig file are not used. This happens due to the following reasons: 1. IBUS_DEFINITIONS is not used in ibus-panel/CMakeLists.txt 2. IBUS_DEFINITIONS is not saved in cache, and thus isn't available at compile time This patch resolves both issues and makes ibus-panel compile on OpenBSD. (I found no suitable review group and therefore used plasma instead, as it was in plasma-addons previously; please, feel free to correct me if I'm wrong and sorry for any possible inconvenience) ((as there was no feedback for more than a week, I've added kde-workspace group to list of reviewers, too)) Diffs - cmake/FindIBus.cmake 8250c49 ibus-panel/CMakeLists.txt 3a1ee49 Diff: https://git.reviewboard.kde.org/r/119025/diff/ Testing --- OpenBSD/i386-CURRENT, KDE 4.13 (it doesn't have kimtoy package, of course, but the code is same) Thanks, Vadim Zhukov
Re: Review Request 119025: Actually pass IBUS_DEFINITIONS when compiling ibus-panel
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119025/ --- (Updated Июль 9, 2014, 7:52 п.п.) Review request for kde-workspace and Plasma. Repository: kimtoy Description (updated) --- The ibus-panel can't build on OpenBSD because some required definitions obtained from pkgconfig file are not used. This happens due to the following reasons: 1. IBUS_DEFINITIONS is not used in ibus-panel/CMakeLists.txt 2. IBUS_DEFINITIONS is not saved in cache, and thus isn't available at compile time This patch resolves both issues and makes ibus-panel compile on OpenBSD. (I found no suitable review group and therefore used plasma instead, as it was in plasma-addons previously; please, feel free to correct me if I'm wrong and sorry for any possible inconvenience) ((as there was no feedback for more than a week, I've added kde-workspace group to list of reviewers, too)) Diffs - cmake/FindIBus.cmake 8250c49 ibus-panel/CMakeLists.txt 3a1ee49 Diff: https://git.reviewboard.kde.org/r/119025/diff/ Testing --- OpenBSD/i386-CURRENT, KDE 4.13 (it doesn't have kimtoy package, of course, but the code is same) Thanks, Vadim Zhukov
