Opened 10 years ago

Closed 9 years ago

#148 closed defect (fixed)

review: bind10_dns and bind10_xfr find needed libboost_python and libpython

Reported by: jreed Owned by: jinmei
Priority: medium Milestone: 05. 3rd Incremental Release: Serious Secondary
Component: build system Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity:
Sub-Project: Feature Depending on Ticket:
Estimated Difficulty: Add Hours to Ticket:
Total Hours: Internal?:

Description (last modified by jreed)

Build bind10_dns and bind10_xfr with RPATH details so can find libboost_python and libpython so don't have to use LD_LIBRARY_PATH for example. (I understand this is system specific.)

The libpython part is related to ticket #149

Subtickets

Attachments (3)

boosttest.cc (151 bytes) - added by jinmei 9 years ago.
configure.ac (19.8 KB) - added by jinmei 9 years ago.
configure.2.ac (19.8 KB) - added by jinmei 9 years ago.

Download all attachments as: .zip

Change History (32)

comment:1 Changed 10 years ago by jreed

  • Description modified (diff)

comment:2 in reply to: ↑ description Changed 10 years ago by jinmei

  • Owner set to jreed
  • Status changed from new to assigned

Replying to jreed:

Build bind10_dns and bind10_xfr with RPATH details so can find libboost_python and libpython so don't have to use LD_LIBRARY_PATH for example. (I understand this is system specific.)

Please review branch/trac148. The only diff is in configure.ac which can be retrieved by:

% svn diff -r 1771:1789 svn+ssh://bind10.isc.org/svn/bind10/branches/trac148

I tried in this patch to make the configure script even uglier so that it will automagically set up everything:

  1. it first tries to run a test program without any special options. in some cases that's sufficient.
  2. otherwise, if -R linker option is available it tries to embed the path to the boost libraries in executable and run the test program. If it works, choose this option.
  3. otherwise, if static version of library is available (e.g libboost_python.a), try to use it. This is a last resort option because the absolute path to the library needs to be guessed and the resulting binary may be bigger.

Alternatively, we could let the user specify the availability and use of the -R option, e.g.,

% ./configure --with-boost-rpath=/opt/local/lib

and/or explicitly specify the availability and use of static version of library:

% ./configure --with-boost-system-staticlib=/opt/local/lib/libbost_system.a

This is a tradeoff issue between amount of manual configuration (especially when something goes wrong) and complexity of the configure script. At this stage I think we should prioritize lower the bar for introduction and chose the latter option.

comment:3 Changed 10 years ago by jinmei

  • Summary changed from bind10_dns and bind10_xfr find needed libboost_python and libpython to review: bind10_dns and bind10_xfr find needed libboost_python and libpython

comment:4 follow-up: Changed 10 years ago by jreed

  • Type changed from defect to review

Just let you know, I haven't reviewed this yet since we may be moving away from using boost-python (and boost-system). But if that is not done soon I will review this so this is included in the next release.

comment:5 Changed 10 years ago by jreed

  • Status changed from assigned to reviewing
  • Type changed from review to defect

comment:6 in reply to: ↑ 4 Changed 10 years ago by jinmei

Replying to jreed:

Just let you know, I haven't reviewed this yet since we may be moving away from using boost-python (and boost-system). But if that is not done soon I will review this so this is included in the next release.

Note: we'll need (possibly a simpler version of) this patch anyway, for the path to libpython.

comment:7 Changed 9 years ago by jreed

The rpath is not needed for boost_system (since that is no longer in trunk). But it looks like the rpath setting for boost_python doesn't work for me. Will test some more.

comment:8 Changed 9 years ago by jreed

I tested more. The -R rpath setting for boost_python doesn't work for me. It finds the boost python library but doesn't include -R in the BOOST_LDFLAGS. (That is only working for me for boost-system checks but that will need to be removed for trunk.)

comment:9 follow-up: Changed 9 years ago by jreed

It matches on the "normal" mode (for boost-python checking) since some other LDFLAGS offer the -R rpath, but that isn't set in BOOST_LDFLAGS.

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

Replying to jreed:

It matches on the "normal" mode (for boost-python checking) since some other LDFLAGS offer the -R rpath, but that isn't set in BOOST_LDFLAGS.

Sorry, I don't understand the reports. Can you copy how ./configure went, with any modification to the branch you possible made?

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

Replying to jinmei:

Replying to jreed:

It matches on the "normal" mode (for boost-python checking) since some other LDFLAGS offer the -R rpath, but that isn't set in BOOST_LDFLAGS.

Sorry, I don't understand the reports. Can you copy how ./configure went, with any modification to the branch you possible made?

Still don't understand after reading jabber message.

What did you ./configure say in the following line (it should be helpful).

...
  checking for boost::python library... yes
...

comment:12 follow-up: Changed 9 years ago by jreed

checking for boost::python library... yes

which means it is doing the normal mode.

Changed 9 years ago by jinmei

comment:13 in reply to: ↑ 12 Changed 9 years ago by jinmei

Replying to jreed:

checking for boost::python library... yes

which means it is doing the normal mode.

Ah, that's what you meant by "normal", okay.

That's strange.

Can you compile and run the attached sample test code without specifying -Wl,-Rpath_to_shlib?

For example, on bind10.isc, without -Wl,-R I see the following:

% g++ `python3.1-config --include` -I${HOME}/opt/include -o boosttest boosttest.cc -L${HOME}/opt/lib -lboost_python -L /opt/pkg/lib -lpython3.1 
10-06-02 1:31 < bind10:~/tmp >

% ./boosttest 
./boosttest: error while loading shared libraries: libboost_python.so.1.42.0: cannot open shared object file: No such file or directory

only if I specify -Wl,-R, it works:

% g++ `python3.1-config --include` -I${HOME}/opt/include -o boosttest boosttest.cc -L${HOME}/opt/lib -Wl,-R${HOME}/opt/lib -lboost_python -L /opt/pkg/lib -Wl,-R/opt/pkg/lib -lpython3.1  

% ./boosttest
(no error)

The script essentially does the same check.

comment:14 follow-up: Changed 9 years ago by jreed

Yes, it needs -Wl,-R

Changed 9 years ago by jinmei

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

Replying to jreed:

Yes, it needs -Wl,-R

Try the attached configure.ac on trunk. Still the same result?

Changed 9 years ago by jinmei

comment:16 in reply to: ↑ 15 Changed 9 years ago by jinmei

Replying to jinmei:

Replying to jreed:

Yes, it needs -Wl,-R

Try the attached configure.ac on trunk. Still the same result?

Okay, I've come up with an explanation of what happened to you.

Try the latest configure.ac on trunk.

comment:17 Changed 9 years ago by jreed

  • Milestone set to 05. 3rd Incremental Release

This is needed. Now I assign it a milestone so it won't be forgotten again.

Note that the boost-system rpath is no longer needed.

comment:18 Changed 9 years ago by shane

  • Component changed from Unclassified to build system

comment:19 follow-up: Changed 9 years ago by jreed

The branches/trac148 does not work for me. But we can explore this after the boost-system dependency is removed.

comment:20 in reply to: ↑ 19 Changed 9 years ago by jinmei

Replying to jreed:

The branches/trac148 does not work for me. But we can explore this after the boost-system dependency is removed.

I believe I understood why it didn't work for you, and I could introduce yet another hack to work around it; however perhaps we can forget this patch once we also eliminate the need for boost.python.

I was probably wrong when I said "we'll need (possibly a simpler version of) this patch anyway, for the path to libpython." Since libpython is only needed in the python loadable modules, which are run under the python program, which should know the path to libpython, I guess we won't have to worry about the path ourselves.

So my suggestion is: forget this branch for now, and see if we still need it after migrating to non-boost python binding. If not we'll just close the ticket, dropping the patch.

(also, maybe we can remove ticket from the review queue for now).

comment:21 Changed 9 years ago by jreed

  • Status changed from reviewing to accepted

comment:22 Changed 9 years ago by jreed

libboost_python rpath is not needed now.

comment:23 follow-up: Changed 9 years ago by shane

So... um... do we need the work from branch or not?

comment:24 in reply to: ↑ 23 Changed 9 years ago by jinmei

Replying to shane:

So... um... do we need the work from branch or not?

We need a simpler version of patch, only for libpython. We'd probably rather start a new branch and merge the relevant part of the old patch by hand.

comment:25 Changed 9 years ago by jinmei

  • Owner changed from jreed to jinmei
  • Status changed from accepted to assigned

comment:26 follow-up: Changed 9 years ago by jreed

I tried trac148b r2421. That works for me. Thanks.

I committed a minor r2423 to make configure print out these details at end.

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

Replying to jreed:

I tried trac148b r2421. That works for me. Thanks.

I committed a minor r2423 to make configure print out these details at end.

I've updated the branch with some comments, simplifications and cleanups. Could you try and review it again? If it's okay I'll merge it to trunk.

comment:28 in reply to: ↑ 27 ; follow-up: Changed 9 years ago by jreed

Replying to jinmei:

I've updated the branch with some comments, simplifications and cleanups. Could you try and review it again? If it's okay I'll merge it to trunk.

Yes, that works for me. Thanks!

comment:29 in reply to: ↑ 28 Changed 9 years ago by jinmei

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

Replying to jreed:

Yes, that works for me. Thanks!

Committed to trunk, closing ticket.

Note: See TracTickets for help on using tickets.