Opened 2 years ago

Closed 7 months ago

#4242 closed defect (fixed)

configure is not able to detect boost version with gcc-5

Reported by: tomek Owned by: fdupont
Priority: medium Milestone: Kea1.2-final
Component: build system Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

Configure script is not able to detect boost version if gcc-5 is used. This seems benign now, as version is reported as unknown.

Output of the ./configure script on Ubuntu 15.10 x64:

...
checking Boost version... unknown
...
Boost:
  BOOST_VERSION:   unknown
  BOOST_INCLUDES:  
  BOOST_LIBS:      

This was rootcaused to a difference in how preprocessor operates in gcc-5. This is explained well here: https://gcc.gnu.org/gcc-5/porting_to.html, see Preprocessor issues.

This file generated in m4macros/ax_boost_for_kea.m4:

#include <boost/version.hpp>
AUTOCONF_BOOST_LIB_VERSION=BOOST_LIB_VERSION

is processed to this with gcc-4.9:

AUTOCONF_BOOST_LIB_VERSION="1_58"

but with gcc-5, it's converted to this:

# 1 "boost-conftest.cpp"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 1 "<command-line>" 2
# 1 "boost-conftest.cpp"
# 1 "/usr/include/boost/version.hpp" 1 3 4
# 2 "boost-conftest.cpp" 2
AUTOCONF_BOOST_LIB_VERSION=
# 2 "boost-conftest.cpp" 3 4
                          "1_58"

Note how AUTOCONF_BOOST_LIB_VERSION and 1_58 are in different lines. This confuses our grep processing.

Proposed solution: Add -P flag if gcc version detected starts with 5.

Subtickets

Attachments (1)

d (3.2 KB) - added by fdupont 10 months ago.
trac4242.diff

Download all attachments as: .zip

Change History (15)

comment:1 Changed 2 years ago by fdupont

Some comments:

  • I suggest to add -P unconditionally as it seems to be a standard C preprocessor option
  • the cpp | grep scheme can be replaced by a code using printf() but this kills cross-compiling so IMHO we should keep the current way to get package versions.
  • the previous comments apply too to other instances, e.g., the 2 CRYPTO_VERSIONs

BTW we can take the occasion to fix the gtest version when --with-gtest-source is used.

comment:2 Changed 2 years ago by fdupont

  • Owner set to fdupont
  • Status changed from new to accepted

comment:3 Changed 2 years ago by fdupont

  • Owner changed from fdupont to UnAssigned
  • Status changed from accepted to reviewing

Done (including gtest). Ready for review.

comment:4 Changed 22 months ago by tomek

  • Milestone changed from Kea-proposed to Outstanding Tasks

Moving all bugs to outstanding, we'll triage them with other defects.

comment:5 Changed 10 months ago by zeitounator

Hi. Came accross this ticket trying to compile under ubuntu. I see this has been worked on by fdupont 14 months ago but I could not find the related branch under git (trac4242). Since I hate to reinvent the wheel, is it possible to see the modified code for the solution ? Where can I get it from ?

Thanks.

comment:6 Changed 10 months ago by fdupont

  • Milestone changed from Outstanding Tasks to Kea-proposed

I am doing to things: first change the milestone so the ticket will return under triage (in ~8 hours from now), second I am attaching the diff for the trac4242 branch (which BTW does what description and comments suggest).
It should be very useful to know if to not get the gtest version is critical/blocking or just not convenient/user friendly.
And to finish Kea 1.2 will require C++ 11 and recent guest versions so in fact as far as I know --with-gtest-source on all current platforms: this configuration option does not support the gtest version lookup and anyway the last time I checked the package version in the gtest git sources was not updated (i.e., the lookup will return a wrong value)...

Changed 10 months ago by fdupont

  • Attachment d added

trac4242.diff

comment:7 Changed 9 months ago by zeitounator

Hi ! And thank you for the quick response.

As far as I'm concerned, the patch did the trick for boost. I applied on top of release 1.1.0 and master and could compile in both situations.

I also compiled with --with-gtest-source. The version is not reported by configure but this does not seem to harm the compilation and running the tests.

I started a pull request (against current version 1.1.0) regarding the script tools/test_in_valgrind.sh which I had to slightly modify to run all tests correctly: https://github.com/isc-projects/kea/pull/49

comment:8 Changed 9 months ago by hschempf

  • Milestone changed from Kea-proposed to Kea1.2-final

Per 9 Mar team meeting, accept this in 1.2-final

comment:9 Changed 8 months ago by tmark

  • Owner changed from UnAssigned to tmark

comment:10 Changed 8 months ago by stephen

  • Owner changed from tmark to stephen

comment:11 Changed 8 months ago by stephen

  • Owner changed from stephen to fdupont

Reviewed commit b71cbefe6bf8751c573d1d3f96d90e5909b29f4a

m4macros/ax_boost_for_kea.m4
At the point where the CPPP macro is used, it would be helpful to precede it with a comment explaining the CPPP is set in configure and includes -P depending on the version.

configure.ac
I would suggest that the initial definition of CPPP ('CPPP="$CPP"') immediately precede the test of CXX_DUMP_VERSION. The initial definition has no comment saying why it is being defined, whereas the test is preceded by a comment explaining why -P is required.

comment:12 Changed 7 months ago by fdupont

  • Owner changed from fdupont to UnAssigned

Addressed comments. I leave the final review (if needed) to the first free reviewer.

comment:13 Changed 7 months ago by tmark

  • Owner changed from UnAssigned to fdupont

Changes are fine please merge them.

comment:14 Changed 7 months ago by fdupont

  • Resolution set to fixed
  • Status changed from reviewing to closed

Merged. Closing.

Note: See TracTickets for help on using tickets.