Opened 2 years ago

Closed 2 years ago

#5372 closed defect (fixed)

Revise header files to be installed with Kea 1.3.0

Reported by: marcin Owned by: marcin
Priority: medium Milestone: Kea1.3-final
Component: Unclassified Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

When we add new header files to Kea, we tend to forget to add them to the installed headers in the Makefile.am. The headers should be installed to allow for development of new hook libraries outside of the Kea repository. I'd even risk to say that all header files should be installed.

Subtickets

Change History (13)

comment:1 Changed 2 years ago by tomek

  • Milestone changed from Kea-proposed to Kea1.3-final

As discussed on 2017-10-04 call, moving to 1.3-final.

comment:2 Changed 2 years ago by marcin

  • Owner set to marcin
  • Status changed from new to accepted

comment:3 Changed 2 years ago by marcin

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

Per our discussions on one of the Kea calls I now install all header files during make install. The only exception is the util/io library which is not really used anywhere except socket creator.

Proposed ChangeLog entry:

13XX.	[build]		marcin
	Copy all header files from Kea libraries during "make install".
	(Trac #5372, git cafe)

comment:4 Changed 2 years ago by fdupont

  • Owner changed from UnAssigned to fdupont

comment:5 Changed 2 years ago by fdupont

  • Owner changed from fdupont to marcin

Some comments:

  • there were some missing headers which a priori are useful for users
  • when a header is considered as not useful a comment should explain this point
  • some headers are generated (and ignored/not saved by git) and useful. I found only some *_messages.h in this category
  • IMHO to check if nothing was forgotten the file name lists should be sorted (sort-lines on the region in emacs)

I tried to apply this, please pull and review. I have a make running to check for typos but I don't expect it will fail so the ticket is back to you. Note it is not clear we should do this (install all headers) only for libraries...

comment:6 Changed 2 years ago by fdupont

BTW I tried to run a "make distcheck". It fails first because a random race:

[ RUN      ] CtrlAgentCommandMgrTest.failForwardToServer
ca_unittests(76522,0x70000e173000) malloc: *** error for object 0x7fb2a9f013d0: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
/bin/sh: line 1: 76522 Abort trap: 6           /bin/sh ../../../../libtool --mode=execute ${dir}$tst
FAIL: ca_unittests

I tried to finish it but "make install && make uninstall && make distuninstallcheck" complained because some .conf files were left behind:

ERROR: files left after uninstall:
./etc/kea/keactrl.conf
./etc/kea/kea-dhcp-ddns.conf
./etc/kea/kea-dhcp6.conf
./etc/kea/kea-dhcp4.conf
./etc/kea/kea-ctrl-agent.conf
make: *** [distuninstallcheck] Error 1

Note this has nothing to do with #5372...

comment:7 follow-up: Changed 2 years ago by marcin

  • Owner changed from marcin to fdupont

Thanks for applying your changes. I tried to do "distcheck" and got the following error:

 (cd hooks && /Applications/Xcode.app/Contents/Developer/usr/bin/make  top_distdir=../../../kea-1.3.0-beta-git distdir=../../../kea-1.3.0-beta-git/src/lib/hooks \
     am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
../../../src/lib/log/compiler/kea-msg-compiler ../../../src/lib/hooks/hooks_messages.mes
make[4]: ../../../src/lib/log/compiler/kea-msg-compiler: No such file or directory

which seems to only show up when hooks_messages.h (generated file) is present. Removing this file allows for continuing but only until the next folder where message file header is installed. I have no idea how to fix it. Due to time constraints I'd suggest that we simply not install those header files until we figure out what is wrong. Would this be ok?

comment:8 in reply to: ↑ 7 Changed 2 years ago by fdupont

Replying to marcin:

Due to time constraints I'd suggest that we simply not install those header files until we figure out what is wrong. Would this be ok?

=> no it is not because it means hooks cannot emit logs. BTW there are at least 2 fixes:

  • add these files in the distrib (note it means to remove them from .gitignore files first).
  • say that before a "make distcheck" one must begin by a "make"

I use the second solution: plain make works with -j xxx when make distcheck does not so it adds a small delay.

comment:9 Changed 2 years ago by fdupont

Applying the first solution (commit generated headers).

comment:10 Changed 2 years ago by fdupont

  • Owner changed from fdupont to marcin

Added all generated xxx_messages.h files at the exception of log files which are already committed to avoid a dependency loop.
Note that as xxx_messages.cc were not added a make will regenerate both .h and .cc files (and careless cross-compiling will still fail).
Commit 3bdf9fec80..e5fd505f36

comment:11 Changed 2 years ago by fdupont

  • Owner changed from marcin to fdupont

Unfortunately it does not work as the current build process insists to regenerate xxx_messages.{h,cc} files.
Reverting last change and removing xxx_messages.h files from installation outside the log library.

comment:12 Changed 2 years ago by fdupont

  • Owner changed from fdupont to marcin

So currently we have two still open points:

  • generated message files: not easy (and arguable) to include in install using current build process
  • server headers (for servers which support hooks)

Nevertheless IMHO the branch is ready to be merged.

comment:13 Changed 2 years ago by marcin

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

Merged with commit cb38d39a942351ec04a655a396dd7396ea20548b

Note: See TracTickets for help on using tickets.