Opened 7 years ago

Closed 7 years ago

#2147 closed defect (fixed)

boost::offset_ptr causes build failures

Reported by: jinmei Owned by: jinmei
Priority: medium Milestone: Sprint-20120807
Component: build system Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 4 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description (last modified by jinmei)

See this:
http://git.bind10.isc.org/~tester/builder/BIND10/20120723120000-FreeBSD8-i386-Clang/logs/build.out
and
http://git.bind10.isc.org/~tester/builder/BIND10/20120724134032-Solaris11-i386-GCC/logs/build.out

Regarding the first one: From experiments offset_ptr in some older
versions of boost causes this error on clang++. Confirmed with boost
1.43 + clang 2.8 and boost 1.42 + clang 3.1, so it's more about the
boost version and doesn't depend on the clang version.

Regarding the second one: it turned out the failing system installed
boost via pkgsrc, which installs its own boost/config/user.hpp, which
caused this error.

Maybe these are minor cases, but it's probably helpful to detect this in
./configure and tell the user to upgrade boost. It should suffice to
check it with AC_TRY_COMPILE() using this test code:

#include <boost/interprocess/offset_ptr.hpp>

(Note: for the second one we need -Werror because technically it's not
an "error", but a warning then converted to an error due to -Werror)

Also: we should not just writing the check code in configure.ac, but
figure out how exactly these errors happen, especially for the second
one.

Subtickets

Change History (12)

comment:1 Changed 7 years ago by jinmei

  • Description modified (diff)
  • Summary changed from boost::offset_ptr doesn't compile with clang + old boost to boost::offset_ptr causes build failures

comment:2 Changed 7 years ago by jinmei

  • Description modified (diff)

comment:3 Changed 7 years ago by jinmei

  • Description modified (diff)

comment:4 Changed 7 years ago by jelte

  • Milestone changed from Next-Sprint-Proposed to Sprint-20120807

comment:5 follow-up: Changed 7 years ago by jinmei

Please review the proposed change to configure.ac in trac2147.

Regarding the second issue, on a closer look I guess the difference is
that the installed user.hpp defines BOOST_NO_CONFIG, which prevents
compiler-specific config from being included, which would otherwise
prevent this build failure. We could avoid this by not including
the user.hpp by specifying BOOST_NO_USER_CONFIG:

% ./configure <other options> CXXFLAGS=-DBOOST_NO_USER_CONFIG

This is the proposed ChangeLog entry:

456.?	[build]*	jinmei
	BIND 10 now relies on Boost offset_ptr, which caused some new
	portability issues.  Such issues are detected at ./configure time.
	If ./configure stops due to this, try the following workaround:
	- If it's about the use of mutable for a reference with clang++,
	  upgrade Boost version to 1.44 or higher, or try a different
	  compiler (e.g. g++ generally seems to be free from this issue)
	- If it's about the use of "variadic templates", specify
	  --without-werror so the warning won't be promoted to an error.
	  Specifying BOOST_NO_USER_CONFIG in CXXFLAGS may also work
	  (which would be the case if Boost is installed via pkgsrc)
	(Trac #2147, git TBD)

comment:6 Changed 7 years ago by jinmei

  • Status changed from new to reviewing

comment:7 in reply to: ↑ 5 ; follow-up: Changed 7 years ago by muks

Replying to jinmei:

Please review the proposed change to configure.ac in trac2147.

May I suggest that we add info on what should be done in configure.ac itself? i.e., instead of just saying "look at ticket XXX", the relevant instruction is printed directly.

Regarding the second issue, on a closer look I guess the difference is
that the installed user.hpp defines BOOST_NO_CONFIG, which prevents
compiler-specific config from being included, which would otherwise
prevent this build failure. We could avoid this by not including
the user.hpp by specifying BOOST_NO_USER_CONFIG:

% ./configure <other options> CXXFLAGS=-DBOOST_NO_USER_CONFIG

This is the proposed ChangeLog entry:

456.?	[build]*	jinmei
	BIND 10 now relies on Boost offset_ptr, which caused some new
	portability issues.  Such issues are detected at ./configure time.
	If ./configure stops due to this, try the following workaround:
	- If it's about the use of mutable for a reference with clang++,
	  upgrade Boost version to 1.44 or higher, or try a different
	  compiler (e.g. g++ generally seems to be free from this issue)
	- If it's about the use of "variadic templates", specify
	  --without-werror so the warning won't be promoted to an error.
	  Specifying BOOST_NO_USER_CONFIG in CXXFLAGS may also work
	  (which would be the case if Boost is installed via pkgsrc)
	(Trac #2147, git TBD)

comment:8 in reply to: ↑ 7 ; follow-up: Changed 7 years ago by jinmei

Replying to muks:

Thanks for the review (and thanks for picking up #2136:-)

May I suggest that we add info on what should be done in configure.ac itself? i.e., instead of just saying "look at ticket XXX", the relevant instruction is printed directly.

I considered that but didn't do it in the initial commit because the
condition is pretty complicated for such usually simple output from
AC_MSG_ERROR(). But as it's now asked I tried to provide a simplified
version of the instruction while still referring to ChangeLog. Is
that okay for you?

comment:9 Changed 7 years ago by jinmei

  • Owner changed from UnAssigned to muks

comment:10 in reply to: ↑ 8 ; follow-up: Changed 7 years ago by muks

  • Owner changed from muks to jinmei

Hi Jinmei

Replying to jinmei:

Replying to muks:

Thanks for the review (and thanks for picking up #2136:-)

May I suggest that we add info on what should be done in configure.ac itself? i.e., instead of just saying "look at ticket XXX", the relevant instruction is printed directly.

I considered that but didn't do it in the initial commit because the
condition is pretty complicated for such usually simple output from
AC_MSG_ERROR(). But as it's now asked I tried to provide a simplified
version of the instruction while still referring to ChangeLog. Is
that okay for you?

Yesterday, I didn't assign the review to myself as I was not sure I could reproduce the bug on my box. If the test for <boost/interprocess/offset_ptr.hpp> header will catch the issue, it looks good to me. It does seem that both cases include offset_ptr.hpp and fail, so a testcase that includes offset_ptr.hpp and passes should be enough to test that the issues don't exist.

The update you have made to the configure.ac output is good too. I think you can merge.

Last edited 7 years ago by muks (previous) (diff)

comment:11 in reply to: ↑ 10 Changed 7 years ago by jinmei

Replying to muks:

Yesterday, I didn't assign the review to myself as I was not sure I could reproduce the bug on my box. If the test for <boost/interprocess/offset_ptr.hpp> header will catch the issue, it looks good to me. It does seem that both cases include offset_ptr.hpp and fail, so a testcase that includes offset_ptr.hpp and passes should be enough to test that the issues don't exist.

Ah, sorry, I thought you took on the review. Anyway, based on the
discussion the branch now looks okay for merge, so I merged it to
master, and am now closing the ticket.
(btw: I reproduced the issues and confirmed the ./configure check
detects them as expected).

comment:12 Changed 7 years ago by jinmei

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.