Re: Review Request 119025: Actually pass IBUS_DEFINITIONS when compiling ibus-panel

[Vadim Zhukov]

---
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

[David Edmundson]

---
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

[Raphael Kubo da Costa]

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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org

Re: Review Request 119025: Actually pass IBUS_DEFINITIONS when compiling ibus-panel

[Vadim Zhukov]

---
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

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119025: Actually pass IBUS_DEFINITIONS when compiling ibus-panel

[Vadim Zhukov]

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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119025: Actually pass IBUS_DEFINITIONS when compiling ibus-panel

[Raphael Kubo da Costa]

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



cmake/FindIBus.cmake


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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119025: Actually pass IBUS_DEFINITIONS when compiling ibus-panel

[Hui Ni]

---
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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119025: Actually pass IBUS_DEFINITIONS when compiling ibus-panel

[Vadim Zhukov]

---
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

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119025: Actually pass IBUS_DEFINITIONS when compiling ibus-panel

[Vadim Zhukov]

---
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

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 119025: Actually pass IBUS_DEFINITIONS when compiling ibus-panel

[Vadim Zhukov]

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

(Updated June 30, 2014, 1:19 p.m.)


Review request for 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)


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

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 119025: Actually pass IBUS_DEFINITIONS when compiling ibus-panel

[Vadim Zhukov]

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

Review request for Plasma.


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.


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

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel