include: move the exported headers into a directory of their own - #4461
include: move the exported headers into a directory of their own#4461grandixximo wants to merge 3 commits into
Conversation
|
Not sure why we need to copy the headers. Why not have them where they are supposed to be? Then we can also remove SRCHEADERS. The only downside is that |
5287210 to
bcacf3d
Compare
|
Reworked to this. The headers now live in On the parenthetical, I took the sources-outside- One thing I would rather ask than assume: dropping Existing trees pull cleanly, since |
| #include <hostmot2-serial.h> | ||
|
|
There was a problem hiding this comment.
This header should not really be exposed, at all.
The reason for doing it are hal/components/mesa_pktgyro_test.comp and the hal/drivers/mesa-hostmot2/modbus/mesa_modbus.c.tmpl. The first is a test and should probably be retired. The second is the old modbus driver via hostmot2, which has been deprecated.
There was a problem hiding this comment.
Agreed on the principle, and it is unchanged here. One correction to the plan though: those two are not the whole list.
src/hal/drivers/mesa_7i65.comp:49 include <hostmot2-serial.h>;
src/hal/drivers/mesa_uart.comp:42 include <hostmot2-serial.h>;
src/hal/components/mesa_pktgyro_test.comp:50 include <hostmot2-serial.h>;
src/hal/drivers/mesa-hostmot2/modbus/mesa_modbus.c.tmpl:26 #include <hostmot2-serial.h>
pktuart.c and hm2_modbus.c are beside the header, so they cost nothing. mesa_7i65.comp and mesa_uart.comp sit one directory up in hal/drivers/ and take it angled, so they reach it only through the export. Retiring the test and the old modbus driver leaves those two still needing it, unless they take a relative include or move down beside the driver. So it is a four-part job, not two...
| #include <rtapi_vsnprintf.h> | ||
|
|
There was a problem hiding this comment.
This should probably also be somewhere else. It includes code (like rtapi_common.h and uspace_common.h). The question is whether we can access/link the kernel's own version. Everything else in non-RT could use the libc version.
There was a problem hiding this comment.
Taken, as far as the exposure goes: it is now in rtapi/ and rtai_rtapi.c includes it from its own directory. rtapi_vsnprintf() itself stays declared in rtapi.h, so nothing callable moved.
On the kernel's own version, I do not think it can be reached for. The kernel deliberately has no floating point formatting, and this file is what supplies it: fnumber() over union ieee754_double, with rtapi.h:142 saying as much. 13 sources print doubles through rtapi_print, emc/motion/command.c and control.c among them, so dropping to the kernel version costs %f in realtime messages. Userspace is already the case you describe, since nothing outside rtai_rtapi.c includes this header and the uspace build has been on libc all along.
The 37 headers listed in SRCHEADERS are the API an out-of-tree component may use. They were spread over eight source directories, mixed in with the private headers of the libraries publishing them, and the build copied them into ../include by basename so that an angled include would find them. That copy is what makes the rule in the comment above SRCHEADERS necessary: a source beside an exported header must include it quoted, because the copy is not the file its own directory should compile against, while every other source must include it angled. Which form is correct therefore depends on where the includer sits, and getting it wrong is silent, because the wrong file is found rather than no file. Keep them in ../include instead, where every compile in and out of the tree already looks for them. There is then one copy rather than two, nothing sits beside an exported header, and every source in the tree takes them angled. SRCHEADERS, the rule that did the copying and headersclean all go; the directory is the list, and installing it is a wildcard. 25 include sites in 22 files change from quoted to angled. The local hal.h in hal/user_comps/xhc-whb04b-6 is a different header of the same name and keeps its "./hal.h" form. Inside include/ the headers keep quoting each other, which is now simply a same-directory include. No code changes, no -I flag changes, and $(EMC2_HOME)/include keeps meaning what it meant.
SRCHEADERS is gone, so read the directory instead. It is the better source anyway: the list cannot drift from what is installed when the directory is the list. The headers are scanned along with the sources so their own includes still count, they bucket as "include", and paths print relative to the tree root rather than to src, which the exported headers now sit beside rather than under. The "no in-tree user outside their own directory" section had its meaning taken away by the move, since no source shares a directory with an exported header any more. It now reports the headers with at most two in-tree users, which is the question that section was really asking.
rtapi_vsnprintf.h is 475 lines of implementation, not an interface, and the only code that includes it is rtai_rtapi.c beside it and the unit test. The documented entry point, rtapi_vsnprintf(), is declared in rtapi.h and stays exported, so nothing an out-of-tree component may legitimately call moves. The header goes to rtapi/ and rtai_rtapi.c takes it from its own directory. The three exported headers it pulls in go angled, since they are no longer beside it. tests/rtapi_printf.0 compiles the header, so it now needs the source tree and is skipped on installed packages, the same way tests/blendmath already is.
bcacf3d to
3a67b0b
Compare
2 participants
Reworked to your suggestion: no copy at all.
The headers in
SRCHEADERSwere spread over eight source directories and copied into../includeby basename. That copy is what forces the rule in the comment aboveSRCHEADERS: a source beside an exported header includes it quoted, everything else angled, so the correct form depends on where the includer sits and getting it wrong is silent.They now live in
include/, where every compile already looks for them.SRCHEADERS, the copy rule andheaderscleango with them; the directory is the list, and installing it is a wildcard. 25 include sites in 22 files go from quoted to angled. No-Iflag changes and$(EMC2_HOME)/includekeeps meaning what it meant, so out-of-tree builds are untouched, RIP and installed alike.On your parenthetical, sources outside
src/is the tradeoff I took because the alternatives cost more. A symlink needsheaderscleangone anyway and adds a tracked symlink. Retiring the top-levelinclude/breaks-I$(EMC2_HOME)/includeon installed systems, whereEMC2_HOMEis$prefixand there is nosrc/, so that one needs a new configure variable rather than a rename.Worth saying out loud rather than slipping in: dropping
SRCHEADERSalso drops the speed bump your comment above it describes. Adding a public header becomes adding a file toinclude/, just as visible in review, but it is your comment, so say if you would rather keep the list.Existing trees pull cleanly.
include/*.his ignored today, and git overwrites ignored files on checkout without complaint, so the stale copies are simply replaced. Checked with a real clone and pull.The second commit points
include-dep-report.pyat the directory instead ofSRCHEADERS; #4375's checker needs the same one-line follow-up. The report reads 72 directory-level edges against 84 on master, and the edges that go areemc/motion -> hal,emc/kinematics -> rtapiand the like, which only ever meant "uses the published API".Third commit answers your
rtapi_vsnprintf.hnote. It is 475 lines of implementation with two in-tree users,rtai_rtapi.cbeside it and the unit test, and the documented entry pointrtapi_vsnprintf()is declared inrtapi.hand stays exported, so nothing callable moves. The header goes tortapi/.tests/rtapi_printf.0compiles it, so it now needs the source tree and is skipped on installed packages the waytests/blendmathalready is. Whether RT can use the kernel's own vsnprintf is the larger question behind your comment and would delete the file rather than move it; I left that alone.On
hostmot2-serial.h: agreed, and it is unchanged here. Its in-tree users are four rather than two, sincemesa_7i65.compandmesa_uart.compalso reach it only through the export, so retiring the test component and the old modbus driver would not be enough on its own.Built warning free with
--with-realtime=uspace; halcompile, interp, motion-logger, linuxcncrsh, build and rtapi_printf tests pass.