Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#35 closed task (complete)

review needed: configure.ac, Makefile.am and related for main level

Reported by: jreed Owned by: jreed
Priority: low Milestone:
Component: build system Version:
Keywords: review Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 0.0 Add Hours to Ticket:
Total Hours: Internal?: no

Description

Review what is in trunk as of revision 738 following code review procedures. If this is your code, please add notes here as appropriate.

(I assigned this to myself.)

Subtickets

Change History (9)

comment:1 Changed 9 years ago by shane

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

comment:2 Changed 9 years ago by jelte

  • Owner changed from jelte to jreed

configure.ac:51: PYTHON_INCLUDES=${PYTHON}-config --includes

I actually added a check for the existence of ${PYTHON}-config in
experiments/python-bindings r1715 (where I also added the check for
Python.h which that branch needs); on most linux systems, most packages
are split between a runtime and a development package, and the latter
is usually not installed by default (named <package>-dev in most cases).
In this specific case, python3-dev provides the python3-config tool,
and currently this configure code results in a warning that the tool
could not be found which is easily missed. We should probably merge
back that specific change. I'll fix the conflict that we get if we
later merge back experiments/python-bindings :)

+if test ! -x ${PYTHON}-config; then
+ AC_MSG_ERROR([${PYTHON}-config not found])
+fi

Should we consider moving our specific configure functions into a
included m4 file? (at Labs we used a shared acx_nlnetlabs.m4 file, for
two reasons, one might not be relevant for us and that is that we
shared a lot of functions between different projects, but the other
was to keep the main configure.ac more readable).

Another change we discussed yesterday is to fail by default if the
correct boost-python library cannot be found (and add a flag to
specifically disable that, so that the person who builds it knows
that the xfrs won't work for them).

Another thing, and that might be me, and it is probably more of a
documentation thing (should at least go into both the guide and the
README, is what our different configure flags do exactly, mainly
the several --with-boost-XXX flags are not quite clear. I think one
of them (--with-boost-system) is also not described in the user
guide right now.

Makefiles look fine to me.

comment:3 Changed 9 years ago by shane

  • Milestone changed from 02. Running, functional authoritative-only server to 04. 2nd Incremental Release

Would be nice if we could get this ticket closed for the next release.

comment:4 Changed 9 years ago by shane

  • Component changed from Unclassified to build system

comment:5 Changed 9 years ago by jreed

  • Milestone changed from 04. 2nd Incremental Release: Early Adopters to feature backlog item

The review was done some time ago. This ticket is still open do to the suggestion still left: separate configure.ac into specific .m4 files. That may be a good idea, but not important for releases now. I am ignoring this for now and changed it from this milestone.

comment:6 Changed 9 years ago by jreed

  • Priority changed from blocker to minor

comment:7 Changed 8 years ago by stephen

  • Milestone feature backlog item deleted

Milestone feature backlog item deleted

comment:8 Changed 8 years ago by shane

  • Defect Severity set to N/A
  • Internal? unset
  • Milestone set to New Tasks
  • Resolution set to complete
  • Status changed from assigned to closed
  • Sub-Project set to DNS

Moving the configure.ac separation into a separate ticket, #968, and resolving this.

comment:9 Changed 8 years ago by shane

  • Milestone New Tasks deleted
Note: See TracTickets for help on using tickets.