Opened 9 years ago

Closed 9 years ago

#436 closed defect (fixed)

review: bind10.isc (debian) requires libboost_thread

Reported by: jinmei Owned by: jelte
Priority: high Milestone: y2 12 month milestone
Component: build system Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity:
Sub-Project: Feature Depending on Ticket:
Estimated Difficulty: 0.0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

This debian box installs an older version of boost (1.35), which
requires libboost_thread for (some of the things needed by)
boost::mutex used in our LruList? class.

The attached patch should fix the build problem. Please review it ASAP as this problem keeps the buildbot failing.

Subtickets

Attachments (1)

boost-thread.diff (2.4 KB) - added by jinmei 9 years ago.

Download all attachments as: .zip

Change History (8)

Changed 9 years ago by jinmei

comment:1 Changed 9 years ago by jinmei

  • Owner changed from jreed to UnAssigned
  • Status changed from new to reviewing

comment:2 Changed 9 years ago by jinmei

(oops, please ignore the garbage in hash_table.h. We don't have to change it although we don't need to include boost/thread.hpp from this file)

comment:3 follow-up: Changed 9 years ago by jelte

Have not tested it, but do have a few comments

I'd prefer if the indentation followed other guidelines (the without/with checks appear to be serial on first glance, while the with is only done if the without fails).

Also, i'd prefer if the final error is a bit more clear;

Perhaps you are using an older version of Boost that requires
libboost_thread for the mutex support. You may want to check
the availability of the library or to upgrade Boost.

to

Perhaps you are using an older version of Boost that requires
libboost_thread for the mutex support, which does not appear
to be available. You may want to check the availability of
the library or to upgrade Boost.

(Don't know how much better this is, but initially I though that we errored on requiring libboost_thread in the first place, not on needing it but not finding it)

Thirdly, I hope we are planning on getting rid of that indirect dependency (can we?), in which case it might be prudent to add a little comment in both additions to remove the other one if it is removed itself (and perhaps the other two comments can be ignored in that case) :)

comment:4 in reply to: ↑ 3 ; follow-up: Changed 9 years ago by jinmei

  • Owner changed from UnAssigned to jelte

Replying to jelte:

Have not tested it, but do have a few comments

I'd prefer if the indentation followed other guidelines (the without/with checks appear to be serial on first glance, while the with is only done if the without fails).

Do you mean something like this?

	[ AC_MSG_RESULT(yes (without libboost_thread)) ],
	[ LIBS=" $LIBS -lboost_thread"
	  AC_TRY_LINK([
#include <boost/thread.hpp>
],[
boost::mutex m;
],
		  [ AC_MSG_RESULT(yes (with libboost_thread))
		    need_libboost_thread=1 ],
		  [ AC_MSG_RESULT(no)
		    AC_MSG_ERROR([boost::mutex cannot be linked in this build environment.
...

Also, i'd prefer if the final error is a bit more clear;

I have no preference on this. I'll simply use your suggested text.

Thirdly, I hope we are planning on getting rid of that indirect dependency (can we?), in which case it might be prudent to add a little comment in both additions to remove the other one if it is removed itself (and perhaps the other two comments can be ignored in that case) :)

What do you mean by "indirect dependency"? Dependency on libboost_thread for boost::mutex (in older boost)? If so, you mean by requiring newer versions of boost? That can be an option, although I think we should discuss it separately (it's a tradeoff between simpler source and lowering the introduction bar). I have no problem in adding comments to this hack per se.

comment:5 in reply to: ↑ 4 ; follow-ups: Changed 9 years ago by jelte

Replying to jinmei:

Do you mean something like this?

	[ AC_MSG_RESULT(yes (without libboost_thread)) ],
	[ LIBS=" $LIBS -lboost_thread"
	  AC_TRY_LINK([
#include <boost/thread.hpp>
],[
boost::mutex m;
],
		  [ AC_MSG_RESULT(yes (with libboost_thread))
		    need_libboost_thread=1 ],
		  [ AC_MSG_RESULT(no)
		    AC_MSG_ERROR([boost::mutex cannot be linked in this build environment.
...

Yes, that makes it much more clear to me that the second try is because the first failed.

Thirdly, I hope we are planning on getting rid of that indirect dependency (can we?), in which case it might be prudent to add a little comment in both additions to remove the other one if it is removed itself (and perhaps the other two comments can be ignored in that case) :)

What do you mean by "indirect dependency"? Dependency on libboost_thread for boost::mutex (in older boost)? If so, you mean by requiring newer versions of boost? That can be an option, although I think we should discuss it separately (it's a tradeoff between simpler source and lowering the introduction bar). I have no problem in adding comments to this hack per se.

yeah, i meant depending on things we don't use ourselves because classes we do depend on them, that newer version don't seem to have that makes the choice a bit harder, since it looks like the problem will go away eventually, but indeed that would require suggesting to use a newer version. (btw i don't have much problems requiring a somewhat newer version at this point, but indeed that is a different discussion)

so ack, commit :)

comment:6 in reply to: ↑ 5 Changed 9 years ago by jinmei

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

Replying to jelte:

so ack, commit :)

Okay, I addressed other outstanding points in r3864 and r3865 as discussed, and merged the branch to trunk.

Hopefully we'll see much fewer (if not no) build failures now.

Closing ticket.

(I'm not intending to add a changelog entry for this fix)

comment:6 in reply to: ↑ 5 Changed 9 years ago by jinmei

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

Replying to jelte:

so ack, commit :)

Okay, I addressed other outstanding points in r3864 and r3865 as discussed, and merged the branch to trunk.

Hopefully we'll see much fewer (if not no) build failures now.

Closing ticket.

(I'm not intending to add a changelog entry for this fix)

Note: See TracTickets for help on using tickets.