Bug 320807 - The build breaks without adding Boost headers path
Summary: The build breaks without adding Boost headers path
Status: RESOLVED FIXED
Alias: None
Product: kig
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: OpenBSD OpenBSD
: NOR normal
Target Milestone: ---
Assignee: David E. Narvaez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-06-06 09:12 UTC by Vadim Zhukov
Modified: 2013-08-06 16:25 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In: 4.12


Attachments
Patch to fix issue (474 bytes, patch)
2013-06-06 09:12 UTC, Vadim Zhukov
Details
Patch to fix issue in master branch (497 bytes, patch)
2013-06-06 20:59 UTC, Vadim Zhukov
Details
Kig 4.10.4 configure output (4.37 KB, text/plain)
2013-06-06 20:59 UTC, Vadim Zhukov
Details
CMakeCache.txt after successful configure of patched Kit 4.10.4 (70.55 KB, text/plain)
2013-06-06 21:00 UTC, Vadim Zhukov
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vadim Zhukov 2013-06-06 09:12:28 UTC
Kig's CMakeLists.txt contains the  find_package(BoostPython) but lacks of find_package(Boost): the latter is needed to get and use ${Boost_INCLUDE_DIRS}. Patch to resolve issue is attached.

Reproducible: Always
Comment 1 Vadim Zhukov 2013-06-06 09:12:58 UTC
Created attachment 80339 [details]
Patch to fix issue
Comment 2 David E. Narvaez 2013-06-06 11:27:03 UTC
Thanks for the bug report, I have a couple of questions:

- What's the Kig version you are trying to compile? The patch does not apply to master, which is the only version containing changes to CMake, so my guess is this is nothing particularily new.

- What does break mean in the context? Since Boost is not required, this should not halt your configuration, it should just skip the Boost integration. If it doesn't, then that's another bug.

If you could also provide the output of the configuration step, that would be useful.
Comment 3 Vadim Zhukov 2013-06-06 20:58:34 UTC
(In reply to comment #2)
> Thanks for the bug report, I have a couple of questions:
> 
> - What's the Kig version you are trying to compile? The patch does not apply
> to master, which is the only version containing changes to CMake, so my
> guess is this is nothing particularily new.

The version from KDE 4.10.4. I'll add a patch for master branch now, too.

> - What does break mean in the context? Since Boost is not required, this
> should not halt your configuration, it should just skip the Boost
> integration. If it doesn't, then that's another bug.

No, no. Python boost bindings presence is detected, and Kig tries to use them. But because Boost include directories are not added to compile command, actual build fails. And those directories are obtained through Boost package, not BoostPython. ${BOOST_PYTHON_INCLUDES} does not contains Boost include directories itself.

> If you could also provide the output of the configuration step, that would
> be useful.

Attaching output from patched version on KDE 4.10.4 and CMakeCache.txt from there, too. So you can see what's actually found and where. :)
Comment 4 Vadim Zhukov 2013-06-06 20:59:02 UTC
Created attachment 80355 [details]
Patch to fix issue in master branch
Comment 5 Vadim Zhukov 2013-06-06 20:59:51 UTC
Created attachment 80356 [details]
Kig 4.10.4 configure output
Comment 6 Vadim Zhukov 2013-06-06 21:00:45 UTC
Created attachment 80357 [details]
CMakeCache.txt after successful configure of patched Kit 4.10.4
Comment 7 Vadim Zhukov 2013-06-06 21:02:09 UTC
BTW, the problem persistent long enough, I was just busy/lazy to report. :)
Comment 8 David E. Narvaez 2013-06-07 06:06:06 UTC
Well, I see we need to patch something but I don't think your patch is the right patch (I'm of course not saying it doesn't work, I'm saying it hides another problem).

There are a couple of things I do not understand, maybe I'm misinterpreting your attachments, but here are a couple of questions/considerations:

- If we divide all cmake/FindBoostPython.cmake from line 44 to line 144 in 3 sections, that would be the Section #1: Finding shared_ptr.hpp (44-45) Section #2: Find Boost and Python using CMake and PkgConfig (47-65) Section #3: Find Boost and Python using brute force (67-144)
- Sections #2 and #3 should not happen if Section #1 fails, so my question is how come Section #1 succeeded in the first place (before your patch), if the Boost include directory is not correctly set; and if it didn't succeed, how come you tell me it finds Boost+Python and later dies during compilation if Sections #2 and #3 didn't happen
-  If we assume Sections #2 and #3 did happen (because Section #1 succeeded somehow), then if Section #2 had succeeded, line 58 does set BOOST_PYTHON_INCLUDES correctly, so I'm assuming Section #2 failed and CMake went on to Section #3 where I, in fact, see we are missing to set BOOST_PYTHON_INCLUDES to the right directory

My preferred goal would be to help you make Section #2 work, although I think we need patches to fix Section #1 and Section #3 too, I will be working on that. Do you know enough PkgConfig and CMake to figure out what happens with your Python PkgConfig setup? I don't have OpenBSD to find out myself but could either find a fellow KDE developer using that or help you sort out your setup.
Comment 9 Vadim Zhukov 2013-06-07 08:11:23 UTC
I got your point. At first, I should say that missed the fact that BoostPython.cmake is local to Kig. Given that, it definitely should be patched instead. I'm more or less experienced in CMake, so I'll prepare a new version and put it on review, okay?
Comment 10 David E. Narvaez 2013-08-06 16:25:57 UTC
Git commit a563705e50882b24b6b973d8bb5247abbf983ba9 by David E. Narváez.
Committed on 05/08/2013 at 22:17.
Pushed by narvaez into branch 'master'.

Stylistic changes to FindBoostPython.cmake and framework

Polish FindBoostPython.cmake from CMake's point of view:

 * Use BoostPython_ prefix instead of BOOST_PYTHON_
   (BOOSTPYTHON_ is also acceptable but is unreadable);

 * Remove extra call to find_package(BoostPython);

 * Replace macro_optional_find_package() with simple find_package(),
   CMake has CMAKE_DISABLE_FIND_PACKAGE_Foo feature for a while.

This is a first patch of a bigger set, containing only minor fixes.
REVIEW: 110953

M  +21   -12   CMakeLists.txt
M  +0    -11   KigConfigureChecks.cmake
M  +19   -19   cmake/modules/FindBoostPython.cmake

http://commits.kde.org/kig/a563705e50882b24b6b973d8bb5247abbf983ba9
Comment 11 David E. Narvaez 2013-08-06 16:25:57 UTC
Git commit 724396ea5208acb176ebd67ebc6beb08e2eb7a08 by David E. Narváez.
Committed on 06/08/2013 at 16:21.
Pushed by narvaez into branch 'master'.

Rewrite FindBoostPython.cmake for correctness and stability

1. Return compile check (see BoostPython_TRY_COMPILE() macro), it's
needed for some cases. Say, default Python in system is Python 3.3, and
it gets picked up. But Boost 1.53 python library could not work with it
and requires Python 2.x. Without compile check you'll pick up
uncompatible Python version.

2. find_package(PkgConfig) moved under corresponding if(...) and
if(PKG_CONFIG_FOUND) is checked after. While it's spread enough, there
is no guarantee you'll have pkg-config on that system. Also,
PYTHON_VERSIONS moved closer to the scope where it's used to improve
readability.

3. Use standard CMake find_package_handle_standard_args() and
cmake_push_check_state()/cmake_pop_check_state() instead of rolling own
equivalent logic. The latter logic was ever wrong, making
HAVE_BOOST_SHARED_PTR_HPP check fail.

4. BoostPython_INCLUDE_DIRS and BoostPython_LIBRARIES are saved in
cache, as other CMake modules do. This unbreaks the situation when CMake
detects that it should be re-run (for example, some CMakeLists.txt file
was modified), and runs through modules again - in this case, if
BoostPython_INCLUDE_DIRS and BoostPython_LIBRARIES are not in cache, all
tests will be run again, and probably fail now.
FIXED-IN: 4.12
REVIEW: 110954

M  +82   -40   cmake/modules/FindBoostPython.cmake

http://commits.kde.org/kig/724396ea5208acb176ebd67ebc6beb08e2eb7a08