I checked in board.c and the issue is only for native_motor, so I think it should be moved outside.

The main question for native, is what should be using the system headers instead of the RIOT ones. Restricting this to the lowest layer modules as possible would be good.
Another solution is to make our headers use the reference posix ones if available.

@cladmi So you say that the real solution is to move ´native_motor´ outside?

I checked in board.c and the issue is only for native_motor, so I think it should be moved outside.

I don't understand the relation with the problem that this PR is trying to solve. Moving native_motor "outside" doesn't solve the build issue you get with USEMODULE=log_printfnoformat make -C examples/hello-world.

I checked info-build target to compare the list of directories included (in INCLUDES), using USEMODULE=log_printfnoformat make -C examples/hello-world info-build:

• this PR:

INCLUDES: 
	-I/home/aabadie/softs/src/riot/RIOT/core/include  
	-I/home/aabadie/softs/src/riot/RIOT/drivers/include  
	-I/home/aabadie/softs/src/riot/RIOT/sys/include  
	-I/home/aabadie/softs/src/riot/RIOT/boards/native/include  
	-DNATIVE_INCLUDES  
	-I/home/aabadie/softs/src/riot/RIOT/boards/native/include/  
	-I/home/aabadie/softs/src/riot/RIOT/core/include/  
	-I/home/aabadie/softs/src/riot/RIOT/drivers/include/  
	-I/home/aabadie/softs/src/riot/RIOT/cpu/native/include  
	-I/home/aabadie/softs/src/riot/RIOT/sys/include  
	-I/home/aabadie/softs/src/riot/RIOT/cpu/native/include  
	-I/home/aabadie/softs/src/riot/RIOT/sys/log/log_printfnoformat

• master:

INCLUDES: 
	-I/home/aabadie/softs/src/riot/RIOT/core/include  
	-I/home/aabadie/softs/src/riot/RIOT/drivers/include  
	-I/home/aabadie/softs/src/riot/RIOT/sys/include  
	-I/home/aabadie/softs/src/riot/RIOT/boards/native/include  
	-DNATIVE_INCLUDES  
	-I/home/aabadie/softs/src/riot/RIOT/boards/native/include/  
	-I/home/aabadie/softs/src/riot/RIOT/core/include/  
	-I/home/aabadie/softs/src/riot/RIOT/drivers/include/  
	-I/home/aabadie/softs/src/riot/RIOT/cpu/native/include  
	-I/home/aabadie/softs/src/riot/RIOT/sys/include  
	-I/home/aabadie/softs/src/riot/RIOT/cpu/native/include  
	-I/home/aabadie/softs/src/riot/RIOT/sys/log/log_printfnoformat

They are exactly the same but the latter doesn't work. So for me the use of INCLUDES = $(NATIVEINCLUDES) in the board Makefile is somehow breaking the list of includes.

Now if I use the verbose build, one can see that the log_printfnoformat is not in the list of -I arguments of gcc, which is in contradiction with what info-build returns:

• on master (and it fails right after of course):

gcc \
	-DRIOT_FILE_RELATIVE=\"boards/native/board_init.c\" \
	-DRIOT_FILE_NOPATH=\"board_init.c\" \
	-Werror -Wall -Wextra -pedantic -std=gnu99 -m32 -fstack-protector-all -ffunction-sections -fdata-sections -fno-delete-null-pointer-checks -fdiagnostics-color -Wstrict-prototypes -Wold-style-definition -fno-common -Wall -Wextra -Wformat=2 -Wformat-overflow -Wformat-truncation -Wmissing-include-dirs -include '/home/aabadie/softs/src/riot/RIOT/examples/hello-world/bin/native/riotbuild/riotbuild.h'  -DNATIVE_INCLUDES -I/home/aabadie/softs/src/riot/RIOT/boards/native/include/ -I/home/aabadie/softs/src/riot/RIOT/core/include/ -I/home/aabadie/softs/src/riot/RIOT/drivers/include/ -I/home/aabadie/softs/src/riot/RIOT/cpu/native/include -I/home/aabadie/softs/src/riot/RIOT/sys/include -MD -MP -c -o /home/aabadie/softs/src/riot/RIOT/examples/hello-world/bin/native/board/board_init.o /home/aabadie/softs/src/riot/RIOT/boards/native/board_init.c
gcc \
	-DRIOT_FILE_RELATIVE=\"boards/native/board.c\" \
	-DRIOT_FILE_NOPATH=\"board.c\" \
	-Werror -Wall -Wextra -pedantic -std=gnu99 -m32 -fstack-protector-all -ffunction-sections -fdata-sections -fno-delete-null-pointer-checks -fdiagnostics-color -Wstrict-prototypes -Wold-style-definition -fno-common -Wall -Wextra -Wformat=2 -Wformat-overflow -Wformat-truncation -Wmissing-include-dirs -include '/home/aabadie/softs/src/riot/RIOT/examples/hello-world/bin/native/riotbuild/riotbuild.h'  -DNATIVE_INCLUDES -I/home/aabadie/softs/src/riot/RIOT/boards/native/include/ -I/home/aabadie/softs/src/riot/RIOT/core/include/ -I/home/aabadie/softs/src/riot/RIOT/drivers/include/ -I/home/aabadie/softs/src/riot/RIOT/cpu/native/include -I/home/aabadie/softs/src/riot/RIOT/sys/include -MD -MP -c -o /home/aabadie/softs/src/riot/RIOT/examples/hello-world/bin/native/board/board.o /home/aabadie/softs/src/riot/RIOT/boards/native/board.c
In file included from /home/aabadie/softs/src/riot/RIOT/boards/native/board.c:21:
/home/aabadie/softs/src/riot/RIOT/core/include/log.h:98:10: fatal error: log_module.h: No such file or directory
 #include "log_module.h"

• with this PR:

gcc \
	-DRIOT_FILE_RELATIVE=\"examples/hello-world/main.c\" \
	-DRIOT_FILE_NOPATH=\"main.c\" \
	-Werror -Wall -Wextra -pedantic -std=gnu99 -m32 -fstack-protector-all -ffunction-sections -fdata-sections -fno-delete-null-pointer-checks -fdiagnostics-color -Wstrict-prototypes -Wold-style-definition -fno-common -Wall -Wextra -Wformat=2 -Wformat-overflow -Wformat-truncation -Wmissing-include-dirs -include '/home/aabadie/softs/src/riot/RIOT/examples/hello-world/bin/native/riotbuild/riotbuild.h'  -I/home/aabadie/softs/src/riot/RIOT/core/include -I/home/aabadie/softs/src/riot/RIOT/drivers/include -I/home/aabadie/softs/src/riot/RIOT/sys/include -I/home/aabadie/softs/src/riot/RIOT/boards/native/include -DNATIVE_INCLUDES -I/home/aabadie/softs/src/riot/RIOT/boards/native/include/ -I/home/aabadie/softs/src/riot/RIOT/core/include/ -I/home/aabadie/softs/src/riot/RIOT/drivers/include/ -I/home/aabadie/softs/src/riot/RIOT/cpu/native/include -I/home/aabadie/softs/src/riot/RIOT/sys/include -I/home/aabadie/softs/src/riot/RIOT/cpu/native/include -I/home/aabadie/softs/src/riot/RIOT/sys/log/log_printfnoformat -MD -MP -c -o /home/aabadie/softs/src/riot/RIOT/examples/hello-world/bin/native/application_hello-world/main.o /home/aabadie/softs/src/riot/RIOT/examples/hello-world/main.c
"make" -C /home/aabadie/softs/src/riot/RIOT/boards/native
make[2]: Entering directory '/home/aabadie/softs/src/riot/RIOT/boards/native'
mkdir -p /home/aabadie/softs/src/riot/RIOT/examples/hello-world/bin/native/board/
gcc \
	-DRIOT_FILE_RELATIVE=\"boards/native/board_init.c\" \
	-DRIOT_FILE_NOPATH=\"board_init.c\" \
	-Werror -Wall -Wextra -pedantic -std=gnu99 -m32 -fstack-protector-all -ffunction-sections -fdata-sections -fno-delete-null-pointer-checks -fdiagnostics-color -Wstrict-prototypes -Wold-style-definition -fno-common -Wall -Wextra -Wformat=2 -Wformat-overflow -Wformat-truncation -Wmissing-include-dirs -include '/home/aabadie/softs/src/riot/RIOT/examples/hello-world/bin/native/riotbuild/riotbuild.h'  -I/home/aabadie/softs/src/riot/RIOT/core/include -I/home/aabadie/softs/src/riot/RIOT/drivers/include -I/home/aabadie/softs/src/riot/RIOT/sys/include -I/home/aabadie/softs/src/riot/RIOT/boards/native/include -DNATIVE_INCLUDES -I/home/aabadie/softs/src/riot/RIOT/boards/native/include/ -I/home/aabadie/softs/src/riot/RIOT/core/include/ -I/home/aabadie/softs/src/riot/RIOT/drivers/include/ -I/home/aabadie/softs/src/riot/RIOT/cpu/native/include -I/home/aabadie/softs/src/riot/RIOT/sys/include -I/home/aabadie/softs/src/riot/RIOT/cpu/native/include -I/home/aabadie/softs/src/riot/RIOT/sys/log/log_printfnoformat -DNATIVE_INCLUDES -I/home/aabadie/softs/src/riot/RIOT/boards/native/include/ -I/home/aabadie/softs/src/riot/RIOT/core/include/ -I/home/aabadie/softs/src/riot/RIOT/drivers/include/ -I/home/aabadie/softs/src/riot/RIOT/cpu/native/include -I/home/aabadie/softs/src/riot/RIOT/sys/include -MD -MP -c -o /home/aabadie/softs/src/riot/RIOT/examples/hello-world/bin/native/board/board_init.o /home/aabadie/softs/src/riot/RIOT/boards/native/board_init.c

NATIVEINCLUDES is not as simple as this. It is there for a good reason.

#8940

And indeed, the modules using NATIVEINCLUDES cannot use any riot modules includes. It is on purpose.

@jcarrano currently it is using APIs that are not available for boards/native/board.c as it uses restrictive includes, so the placement is wrong.
As native/boards is not using any system posix headers it should be possible to use the normal INCLUDES there. Doing a rationalization of the NATIVEINCLUDES handling in general could be a good thing.

However, in practice, why is there even a native_motor_driver_qdec_simulation in boards/native/board.c?
If there is a need for a fake test device, why not put it in its own module ?
It is always compiled even if motor_driver features is not used.

And indeed, the modules using NATIVEINCLUDES cannot use any riot modules includes. It is on purpose.

Can you explain this a bit more ?

And indeed, the modules using NATIVEINCLUDES cannot use any riot modules includes. It is on purpose.

Can you explain this a bit more ?

From #8940.

The goal of NATIVEINCLUDES, as I understood it, is to handle conflicts between RIOT headers and the system headers which may contain different values.
(I know about POSIX for example)
POSIX is only an API and not an ABI, so "names" are the same but not the values for the enums/defines and even differ between linux/mac.

For native our POSIX headers do not match with the ones from the system.
Using INCLUDES directories can make the system use RIOT posix values, which are not the one for the host system. So not working when communication with the system.

Every modules that need to communicate with the Linux/mac underlying system, must use the system headers, not the ones we provide if they can be different.

Currently who should use NATIVEINCLUDES is not that clear and maybe too general as here board for native does not interact directly with Host system.

@cladmi, see #11603 where there is an alternative to the change proposed here: extend NATIVEINCLUDES from the log submodule.
I have no idea what would be better. Do you have an even better suggestion ?

@aabadie I think we can change that NATIVEINCLUDES is not used at all boards/native/* as neither board nor native-drivers implement direct interaction with the system. If another periph using system interaction must be implemented it should be put in its own directory .
So the definition of NATIVEINCLUDES could be moved to cpu/native too.

@cladmi, I remove INCLUDES = $(NATIVEINCLUDES) from boards/native and cpu/native and it seems to work. I'm not sure if that was what you proposed.

I'll removed the previous commits if it's ok for you, just keeping the commit with the new test application.

I was thinking about boards/native and boards/native/drivers.

cpu/native should keep it, async_read.c uses posix headers, and we have a custom fcntl.h. It may be ok but would not want to try.

When removing it needs to be reviewed that the included header did not change unexpectedly.
This can be checked by comparing the .d files output after compilation for all affected files like bin/native/board/board.d. So this needs its specific pull request.

@cladmi, I updated this PR following your suggestion: cpu/native/Makefile change is reverted and only boards/native/Makefile/boards/native/drivers/Makefile are modified.

When removing it needs to be reviewed that the included header did not change unexpectedly.

After building examples/hello-world, I also checked the content of bin/native/board/board.d, it doesn't seem to have changed: same number of files included (using a combination of cat and grep with regexp).

So this needs its specific pull request.

I'm not sure to understand. Do you mean the included files verification ? e.g. a new static test or something ?

ping @cladmi

So this needs its specific pull request.

I'm not sure to understand. Do you mean the included files verification ? e.g. a new static test or something ?

I mean tests/log_printfnoformat: add test application & script should not stay in the pr removing NATIVE_INCLUDES from boards/native and boards/native/drivers.

I will test the update now.

Comparing with your reference PR, it indeed did not change the .d files for board and native-led according to diff RIOT_CI_BUILD=1 make -C examples/hello-world/

diff -u bin_master/native/native-drivers/native-led.d bin_pr/native/native-drivers/native-led.d
diff -u bin_master/native/board/board.d bin_pr/native/board/board.d

And to be even more precise, the whole binary directories are the same

diff -r -a bin_master/ bin_pr/

And both the log_printfnoformat and the USEMODULE=log_printfnoformat make -C examples/hello-world/ clean all test case work.

Can you please rebase and only keep the commit removing NATIVEINCLUDES from native, and update the PR description.

As a justification somehow if you want inspiration

Both `board` and `native-drivers` do not interact with the host operating system so does not need to use system headers. So using restricted `NATIVEINCLUDES` is not needed.

It also fixes the issue that they use RIOT headers for, for example logging, which were not available in `NATIVEINCLUDES`.

I mean tests/log_printfnoformat: add test application & script should not stay in the pr removing NATIVE_INCLUDES from boards/native and boards/native/drivers.

I'm sorry but I read 3 times your initial comment about "So this needs its specific pull request" and this cannot the reason. It seems at least that the test application had nothing to with "So this needs its specific pull request".
Can you explain this again better ?

Also I think it's fine to keep the test application in this PR event if it doesn't seem related: it gives a way to verify that log_printfnoformat can be build for native (which was the initial goal of this PR).

I cut out the test application from this PR and opened #11968

So where do NATIVEINCLUDES go now? IIRC native needs some different defines, otherwise their is a collision with our POSIX headers, and the host system's POSIX headers.

So where do NATIVEINCLUDES go now?

They are not needed in boards/native

IIRC native needs some different defines, otherwise their is a collision with our POSIX headers, and the host system's POSIX headers.

See #11572 (comment)

So where do NATIVEINCLUDES go now?

They are not needed in boards/native

IIRC native needs some different defines, otherwise their is a collision with our POSIX headers, and the host system's POSIX headers.

See #11572 (comment)

I see. I did not read the whole discussion.

Triggered rebuild, since last build was a few days back.

All green, let's merge. Thanks @miri64 !