[LinuxCNC/linuxcnc PR#515] add relative path to orignal folder to include dirs for easier header…

#1 – andypugh 于 2018-10-25

I will try to test this tonight. (Might need to change the path in mesa_uart.comp)

#2 – jepler 于 2018-10-25

If I understand correctly, the problem you’re trying to address is that a header “next to” a .comp file isn’t found during compilation, due to the way the generated .c file is in a temporary location.

I believe the added directive should be a “-iquote”, and the .comp file should use the “”-include syntax, not the <>-include syntax, if the goal is to include a header that is relative to the input, rather than a system header. Please see the documentation snipped below from the manual of gnu cpp:

But you also mention an in-tree component by name. That’s weird; those should all build without problems during the usual build process. Perhaps there’s something else going on, and more background would be helpful in understanding the problem.

“
2.1 Include Syntax
==================

Both user and system header files are included using the preprocessing
directive '#include'. It has two variants:

'#include '
This variant is used for system header files. It searches for a
file named FILE in a standard list of system directories. You can
prepend directories to this list with the '-I' option (*note
Invocation::).

'#include "FILE"'
This variant is used for header files of your own program. It
searches for a file named FILE first in the directory containing
the current file, then in the quote directories and then the same
directories used for ''. You can prepend directories to the
list of quote directories with the '-iquote' option.
“

#3 – andypugh 于 2018-10-25

The mesa_uart.comp is a sample file that includes hostmot2.h but isn’t in the hostmot2 folder.
So it includes a relative path, relative to the temp-file location.
https://github.com/LinuxCNC/linuxcnc/blob/master/src/hal/drivers/mesa_uart.comp#L39

That currently compile OK. I wanted to make sure that it still compiles OK with this change.

#4 – andypugh 于 2018-10-25

Looking at the Travis log, it seems that mesauart does not currently get built, but mesa7i65 (in the same folder) does.
https://github.com/LinuxCNC/linuxcnc/blob/0f91c553a238c3f5e8a52285044761c2dcfd7de5/src/hal/components/Submakefile#L13

Seems to be a uspace thing.

#5 – oro06 于 2018-10-25

yes the “-iquote” did the trick. this is what i tried yesterday.
but unless i did it wrongly this result in the .comp source being polluted with users path
which then may no work on others systems simply depending on where one have the source tree located.

as an example, working on wj200_vfd.comp i needed to add:
/home/linuxcnc/dev/linuxcnc-oro06.git/src/hal/usercomps/wj200vfd/wj200_vfd.h

what is proposed here is to add automatically this directory wherever it is.

#6 – andypugh 于 2018-10-26

OK, in theory I like this, but there is a problem. If I change the (messy) reference in mesauart.comp and mesa7i65.comp to “hostmot2/hostmot2.h” then halcompile works nicely when compiling with halcompile –install.
But: If I then try to build all of LinuxCNC with “Make” then LinuxCNC doesn’t build.

objects/hal/drivers/mesa_uart.c:13:36: fatal error: mesa-hostmot2/hostmot2.h: No such file or directory
#include “mesa-hostmot2/hostmot2.h”

With this change I haven’t found an “include” for comp that works both standalone halcompile and when running “Make”

This might not actually be a problem. Is there a requirement to compile built-in comps outside “Make”?It did work nicely when I made a test comp in my home folder with an “adjacent” .h file.

#7 – jepler 于 2018-10-26

Can we step back and state what the problem is? Maybe it is:

Using just the headers in linuxcnc-dev, or just the headers that are <>-includable in an RIP install, there is currently no way to build an out-of-tree driver that uses the functions such as hm2uartread which are actually intended to be part of the stable public interface of LinuxCNC’s hostmot2 driver

It seems to me that this is different from what you’re saying, which is that we should commit to having any single source file (including .comp files) be able to be built separately from LinuxCNC. There could be legitimate reasons for this, such as that they are using APIs that are NOT intended to be part of a stable public interface.

#8 – oro06 于 2018-10-26

as reference a took mesa7i65 and mesauart to try to reproduce the issue

a)i cloned a fresh linuxcnc tree from github
b)done full build (in my case rtai based system with ub12.04)
c)no error and mesa7i65.c , mesa7i65.o, mesauart.c, mesauart.o files where created
in src/objects/hal/drivers also mesa7i65.ko and mesauart.ko where created in src/ directory
d)for testing purpose i deleted these files mesa7i65.c , mesa7i65.o, mesauart.c, mesauart.o
from src/objects/hal/drivers
e)make went fine
Processing mesa_7i65.comp
Processing mesa_uart.comp
and .c, .o, .ko where created again
f)i applied the change -subject of this thread- in halcompile.g
deleted mesa7i65 and mesauart .c .o .ko
and run “make” again
files where created again

so i’m unable to reproduce this issue
would you please share howto to reproduce it

#9 – andypugh 于 2018-10-26

I think I failed to do a proper A B A comparision. (It was past midnight when I finally found the time)
I think I am conflating two different things.
I think that, completely unrelated to your patch, there is a (very minor) problem that uspace doesn’t build mesauart.comp and the slightly more major problem that halcompile won’t build mesauart or mesa7i65. This is probably unrelated to your patch. Mesauart not building is a minor problem because it is only meant to be a demo source file, but it isn’t a useful one if it doesn’t halcompile.

I did build a .comp with associated .h out-of-tree successfully with the patch, but didn’t try to build it without the patch, so can’t actually say if the patch helped.

#10 – andypugh 于 2018-10-26

I have now tried what I thought was the problem, with your patch, with base master and with base 2.7.
And they all worked. So I think I am unclear what this fixes.
Can you give an example .comp that does not compile in master and does compile with the patch to show what it does?

I am pretty sure I have seen the problem you have seen, but the issue is finding an example that proves the fix.

#516 includes two useful thing that have been prompted by looking in this area, but Both Jeff and I are need a bit of help to see what your patch fixes.

I apologise that part of this confusion is my fault. I made an assumption about what was fixed, and then tested it badly.

#11 – oro06 于 2018-10-27

part of this message came from this post
https://forum.linuxcnc.org/24-hal-components/35418-halcompile-missing-additionnal-include-file#119503

As a concrete example, of out-of-tree compiling component, with the current master
, speaking of the wj200_vfd.comp located (in my case)
~/linuxcnc/dev/linuxcnc/src/hal/usercomps/wj200vfd
cd ~/linuxcnc/dev/linuxcnc/src/hal/usercomps/wj200vfd
halcompile --compile wj200_vfd.comp
wj200_vfd.comp:43:19: fatal error: modbus.h: No such file or directory
compilation terminated.
make: * [wj200_vfd] Error 1
then, a possibility is to add in the comp file, in the declaration section (before the “;;”)
option extracompileargs "-I/usr/include/modbus -std=c99";
option extralinkargs "-lmodbus";
went fine

now if one want to use an additionnal include file wj200_vfd.h, located in the same directory as the comp updating the corresponding .comp file like this
...
option userspace;
option userinit yes;
include ;
option extracompileargs "-I/usr/include/modbus -std=c99";
option extralinkargs "-lmodbus";
license "GPLv2 or greater";
;;
then compiling…
halcompile --compile wj200_vfd.comp
/tmp/tmp2r2Nhr/wj200vfd.c:13:23: fatal error: wj200vfd.h: No such file or directory
replacing
include ;
by
include "~/linuxcnc/dev/linuxcnc/src/hal/usercomps/wj200vfd/wj200_vfd.h";
will compile fine on my system,
but this will not compile properly on another system where sourcetree is located elsewhere in the fs.

so keeping include vfd.h>; in the wj200vfd.comp
i modified the halcompile.g as per this PR,
rm ~/linuxcnc/dev/linuxcnc/src/objects/hal/utils/halcompile.py
cd ~/linuxcnc/dev/linuxcnc/src
make
went fine and “~/linuxcnc/dev/linuxcnc/src/objects/hal/utils/halcompile.py” came back with expected
changes.
then compiling
cd ~/linuxcnc/dev/linuxcnc/src/hal/usercomps/wj200vfd
source ../../../../scripts/rip-environment
halcompile --compile wj200_vfd.comp
went fine

now, reverting the change in halcompile.g and doing this last step again
resulted in missing wj200_vfd.h error again.
i noticed also that building the full tree (eg ../src/make) without the patch
but with include ; in the comp declaration, failed also with the same error.

#12 – andypugh 于 2018-10-27

I think that the second problem is avoided (without your patch) by not using
include ;

but instead using
include “wj200_vfd.h”;

(As mentioned by jepler earlier:
“I believe the added directive should be a “-iquote”, and the .comp file should use the “”-include syntax, not the <>-include syntax”)

#13 – oro06 于 2018-10-27

i thought “-iquote” needed a full relative or absolute filepathname
understand now,
sorry for my narrow-mindedness
i’ll check this asap
thanks

#14 – jepler 于 2018-10-27

I think I finally understand! Thanks for your patience, @oro06 and your help @andypugh

I would still prefer to use “-iquote”. but aside from that I am in favor of the proposed change.

I have added another pull request which shows a test that fails now but works after this change (or the -iquote version of the change), #517. We can merge that one in after this one.

@SebKuzminsky do you want any of these fixes (#515, #516) in 2.7?

#15 – oro06 于 2018-10-30

imho, if this patch is not required, better not to add it, particularly if it’s a matter of
replacing < and > by “. so i reverted #515 locally

but it seems, there is still an issue
out-of-tree fail while in-tree succeed

a) <>-include syntax
keeping in the declaration section
include ;
as expected
halcompile --compile wj200_vfd.comp
Compiling hal/usercomps/wj200vfd/wj200_vfd.c
/tmp/tmptSMmk2/wj200vfd.c:13:23: fatal error: wj200vfd.h: No such file or directory
and in-tree
.../src/make
hal/usercomps/wj200vfd/wj200vfd.c:13:23: fatal error: wj200vfd.h: No such file or directory
fail also

b) “”-include syntax single filename
in the header section replacing the include with
include "wj200_vfd.h";
out-of-tree
halcompile --compile wj200_vfd.comp
/tmp/tmpb50RaV/wj200vfd.c:13:23: fatal error: wj200vfd.h: No such file or directory
fail and should not

and in-tree
.../src/make
succeed

c) “”-include syntax absolute fullfilepathname
then in the header section replacing the include with
include "/home/oro06/linuxcnc/dev/linuxcnc/src/hal/usercomps/wj200vfd/wj200_vfd.h";
out-of-tree
halcompile --compile wj200_vfd.comp
succeed

and in-tree
.../src/make
succeed

note: using instead
include "~/linuxcnc/dev/linuxcnc/src/hal/usercomps/wj200vfd/wj200_vfd.h"
fail for both in-tree and out-of-tree

#16 – oro06 于 2018-10-30

so unless another path is followed (like copying next-to .h in the temp directory together with the .comp file) , it’s imho still usefull

#17 – jepler 于 2018-10-31

Yes, I agree that without a change include <...> and include "..." both fail for a header that is in the directory of the .comp file.

That’s why I would like to -iquote the directory where the .comp file is. Once that is done your option “b)” above should work.

This is almost the same as the original pull request, but using -iquote instead of -I because of how the GNU manual suggests <> and "" includes should be used — the first for system headers, and the second for “local” headers.

#18 – andypugh 于 2018-10-31

I am a little confused now. In my testing include “…” for a header in the same directory as the .comp file passed with base master and with 2,7. However this was with the header and .comp in my root directory, I didn’t test any alternative locations.

#19 – jepler 于 2018-10-31

@andypugh did you test with a non-realtime (so-called “userspace”) component? That’s the case that fails in the test I want to add (#517), and which an added -iquote fixes.

#20 – andypugh 于 2018-10-31

No, my test was without “option userspace”

#21 – oro06 于 2018-10-31

@jepler looking at your pending PRs, i did saw the testcase but not the fix.
i then updated the current one accordingly. (after checking )
plz, let me know if it was elsewhere and if finally this PR need to be canceled
thanks you both @andypugh & @jepler