Opened 8 years ago

Closed 7 years ago

#1870 closed defect (fixed)

install complete header files

Reported by: jinmei Owned by: jinmei
Priority: medium Milestone: Sprint-20121009
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: 5 Add Hours to Ticket: 0
Total Hours: 7.44 Internal?: no

Description

Currently 'make install' only installs an insufficient subset of
public header files, so, for example, libdns++ cannot be used as a
standalone library for a third party application (unless the app
directly refers to the BIND 10 source tree).

We should make sure necessary header files are installed, at least
for libdns++.

I also suggest moving the 'host' program under src/bin to a separate
repository so we can use it as a test case whether the installed
header files are sufficient to built it.

Subtickets

Change History (17)

comment:1 Changed 8 years ago by jreed

Also need to consider a subdirectory name ... maybe include/bind10/dns/ or include/libdns++/ for example.

See #1930.

comment:2 Changed 8 years ago by jinmei

  • Milestone set to Next-Sprint-Proposed

comment:3 Changed 7 years ago by jinmei

  • Milestone set to Next-Sprint-Proposed

comment:4 Changed 7 years ago by jelte

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

comment:5 Changed 7 years ago by jinmei

  • Owner changed from UnAssigned to jinmei
  • Status changed from new to accepted

comment:6 Changed 7 years ago by jinmei

(releasing this for now)

comment:7 Changed 7 years ago by jinmei

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

comment:8 Changed 7 years ago by jinmei

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

comment:9 Changed 7 years ago by jinmei

trac1870 is ready for review.

I've created a new "examples" directory, moved src/bin/host there,
and provided a separate configure.ac, Makefile.am, etc as a template
for external projects. The host program itself wasn't modified and
doesn't have to reviewed. The only non trivial things to review
is the top level configure.ac and m4 macros.

Note that with this change we won't install b10-host any more. But I
think it's okay - it's always been pretty experimental, not really
clean, with missing tests, so it actually hasn't had the required
quality to be include in releases in the first place.

One more note: I intentionally excluded some of the header files under
util and dns. These are generally expected to be internal or soon to
be deprecated. We can add them later if and when we really see the
need for including them.

This is proposed changelog:

484.?	[bug]*		jinmei
	Make sure all public header files for libb10-dns++ are installed.
	Template configure.ac and utility AC macros for external projects
	using the library is provided under the "examples" directory.
	The src/bin/host was moved as part of the examples (and not
	installed with other BIND 10 programs any more).
	(Trac #1870, git TBD)

comment:10 Changed 7 years ago by jinmei

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

comment:11 Changed 7 years ago by vorner

  • Owner changed from UnAssigned to vorner

comment:12 follow-up: Changed 7 years ago by vorner

  • Owner changed from vorner to jinmei

Hello

Some things I noticed:

  • if test "${bind10_inc_path}" ; then ‒ should there be some kind of test -z or something?
  • When I run the configure in examples, it checks for boost twice:
    checking boost/shared_ptr.hpp usability... yes
    checking boost/shared_ptr.hpp presence... yes
    checking for boost/shared_ptr.hpp... yes
    checking boost/bind.hpp usability... yes
    checking boost/bind.hpp presence... yes
    checking for boost/bind.hpp... yes
    checking boost/function.hpp usability... yes
    checking boost/function.hpp presence... yes
    checking for boost/function.hpp... yes
    checking whether Boost tries to use threads... yes
    checking for boost/shared_ptr.hpp... (cached) yes
    checking for boost/bind.hpp... (cached) yes
    checking for boost/function.hpp... (cached) yes
    checking whether Boost tries to use threads... yes
    

Also, the configure doesn't seem to work for me :-(.

./configure --with-bind10-include=/home/vorner/testing/bind10/include/bind10-devel/ --with-bind10-include=/home/vorner/testing/bind10/lib/

checking util/buffer.h usability... no
checking util/buffer.h presence... no
checking for util/buffer.h... no
configure: error: Missing a commonly used BIND 10 header files
    1 vorner@hydra ~/work/bind10/examples (review/headers 18) 4%
$ ls /home/vorner/testing/bind10/include/bind10-devel/ 
dns  exceptions  util
12:25 vorner@hydra ~/work/bind10/examples (review/headers 18) 4%
$ ls /home/vorner/testing/bind10/include/bind10-devel/util/
buffer.h
12:25 vorner@hydra ~/work/bind10/examples (review/headers 18) 4%

Is it something I do wrong, or is the script broken? (please, don't get confused by my unusual prompt there)

Also, I noticed most of our changelogs are either in past tense or third form. Your looks like in imperative. Not that it would be a problem, but maybe just for consistency, it could be unified.

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

Thanks for the review.

Replying to vorner:

  • if test "${bind10_inc_path}" ; then ‒ should there be some kind of test -z or something?

Good catch, I believe I fixed it.

  • When I run the configure in examples, it checks for boost twice:

Ah, thanks for the careful check. I actually didn't have to call
AX_BOOST_INCLUDE explicitly because AC_REQUIRE does it internally.
Removed the extra call, and it should solve this.

Also, the configure doesn't seem to work for me :-(.

./configure --with-bind10-include=/home/vorner/testing/bind10/include/bind10-devel/ --with-bind10-include=/home/vorner/testing/bind10/lib/

checking util/buffer.h usability... no
checking util/buffer.h presence... no
checking for util/buffer.h... no
configure: error: Missing a commonly used BIND 10 header files
    1 vorner@hydra ~/work/bind10/examples (review/headers 18) 4%
$ ls /home/vorner/testing/bind10/include/bind10-devel/ 
dns  exceptions  util
12:25 vorner@hydra ~/work/bind10/examples (review/headers 18) 4%
$ ls /home/vorner/testing/bind10/include/bind10-devel/util/
buffer.h
12:25 vorner@hydra ~/work/bind10/examples (review/headers 18) 4%

Is it something I do wrong, or is the script broken? (please, don't get confused by my unusual prompt there)

Hmm, strange. Did you install this branch under
/home/vorner/testing/bind10/? util/buffer.h is one of the missing
header files that are not installed in the current master, so you need
to first build and install this branch. Explicit manual check may
also help: is there buffer.h in
/home/vorner/testing/bind10/include/bind10-devel/util?

If the answer to both is yes, I have no idea why it failed. Can you
show your config.log?

Also, I noticed most of our changelogs are either in past tense or third form. Your looks like in imperative. Not that it would be a problem, but maybe just for consistency, it could be unified.

You mean "Make sure..." sounds like imperative (and dramatically
you're right)?

484.?	[bug]*		jinmei
	Make sure all public header files for libb10-dns++ are installed.
...

If that's the concern, how about this?

484.?	[bug]*		jinmei
	All public header files for libb10-dns++ are now installed.
	Template configure.ac and utility AC macros for external projects
	using the library is provided under the "examples" directory.
	The src/bin/host was moved as part of the examples (and not
	installed with other BIND 10 programs any more).
	(Trac #1870, git TBD)

comment:14 Changed 7 years ago by jinmei

  • Owner changed from jinmei to vorner

comment:15 in reply to: ↑ 13 ; follow-up: Changed 7 years ago by vorner

  • Owner changed from vorner to jinmei
  • Total Hours changed from 0 to 1.94

Hello

Replying to jinmei:

Hmm, strange. Did you install this branch under
/home/vorner/testing/bind10/? util/buffer.h is one of the missing
header files that are not installed in the current master, so you need
to first build and install this branch. Explicit manual check may
also help: is there buffer.h in
/home/vorner/testing/bind10/include/bind10-devel/util?

If the answer to both is yes, I have no idea why it failed. Can you
show your config.log?

The answer was yes, but it now works for some unknown reason. So maybe it was the change with the test or something.

If that's the concern, how about this?

484.?	[bug]*		jinmei
	All public header files for libb10-dns++ are now installed.
	Template configure.ac and utility AC macros for external projects
	using the library is provided under the "examples" directory.
	The src/bin/host was moved as part of the examples (and not
	installed with other BIND 10 programs any more).
	(Trac #1870, git TBD)

Yes, that looks better.

Please merge

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

Replying to vorner:

Please merge

Thanks, merge done, closing.

comment:17 Changed 7 years ago by jinmei

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