[riot-notifications] [RIOT-OS/RIOT] build system: Rework EXTERNAL_MODULE_DIRS (#16104)

Francisco notifications at github.com
Tue May 4 10:31:16 CEST 2021


@fjmolinas commented on this pull request.

Some testing where I removed the `external_modules` directory in `tests/external_module_dirs` and put it outside of tree and exported the location:

<details> <summary> compile test <b>OK</b> </summary>

```
make -C tests/external_module_dirs/
Building application "tests_external_module_dirs" for "native" with MCU "native".

"make" -C /home/francisco/workspace/RIOT/boards/native
"make" -C /home/francisco/workspace/RIOT/boards/native/drivers
"make" -C /home/francisco/workspace/RIOT/core
"make" -C /home/francisco/workspace/RIOT/cpu/native
"make" -C /home/francisco/workspace/RIOT/cpu/native/periph
"make" -C /home/francisco/workspace/RIOT/cpu/native/stdio_native
"make" -C /home/francisco/workspace/RIOT/drivers
"make" -C /home/francisco/workspace/RIOT/drivers/periph_common
"make" -C /home/francisco/workspace/RIOT/sys
"make" -C /home/francisco/workspace/RIOT/sys/auto_init
"make" -C /home/francisco/workspace/RIOT/sys/luid
"make" -C /home/francisco/workspace/RIOT/sys/random
"make" -C /home/francisco/workspace/tmp/external_modules/external_module
   text	   data	    bss	    dec	    hex	filename
  29345	    628	  47820	  77793	  12fe1	/home/francisco/workspace/RIOT/tests/external_module_dirs/bin/native/tests_external_module_dirs.elf
```
</details>

<details> <summary> compile test in docker <b>OK</b> </summary>

```
Launching build container using image "riot/riotbuild:latest".
docker run --rm --tty --user $(id -u) -v '/usr/share/zoneinfo/Europe/Paris:/etc/localtime:ro' -v '/home/francisco/workspace/RIOT:/data/riotbuild/riotbase:delegated' -e 'RIOTBASE=/data/riotbuild/riotbase' -e 'CCACHE_BASEDIR=/data/riotbuild/riotbase' -e 'BUILD_DIR=/data/riotbuild/riotbase/build' -e 'RIOTPROJECT=/data/riotbuild/riotbase' -e 'RIOTCPU=/data/riotbuild/riotbase/cpu' -e 'RIOTBOARD=/data/riotbuild/riotbase/boards' -e 'RIOTMAKE=/data/riotbuild/riotbase/makefiles'   -v '/home/francisco/workspace/tmp/external_modules:/data/riotbuild/external/external_modules:delegated'      -w '/data/riotbuild/riotbase/tests/external_module_dirs/' 'riot/riotbuild:latest' make  'EXTERNAL_MODULE_DIRS=/data/riotbuild/external/external_modules /data/riotbuild/riotbase/tests/external_module_dirs/external_modules'    
Building application "tests_external_module_dirs" for "native" with MCU "native".

"make" -C /data/riotbuild/external/external_modules/external_module
"make" -C /data/riotbuild/riotbase/boards/native
"make" -C /data/riotbuild/riotbase/boards/native/drivers
"make" -C /data/riotbuild/riotbase/core
"make" -C /data/riotbuild/riotbase/cpu/native
"make" -C /data/riotbuild/riotbase/cpu/native/periph
"make" -C /data/riotbuild/riotbase/cpu/native/stdio_native
"make" -C /data/riotbuild/riotbase/drivers
"make" -C /data/riotbuild/riotbase/drivers/periph_common
"make" -C /data/riotbuild/riotbase/sys
"make" -C /data/riotbuild/riotbase/sys/auto_init
"make" -C /data/riotbuild/riotbase/sys/luid
"make" -C /data/riotbuild/riotbase/sys/random
   text	   data	    bss	    dec	    hex	filename
  28792	    628	  47756	  77176	  12d78	/data/riotbuild/riotbase/tests/external_module_dirs/bin/native/tests_external_module_dirs.elf
```
</details>

<details> <summary> compile test with setup matching previous handling <b>FAIL</b> </summary>

```
BUILD_IN_DOCKER=1 make -C tests/external_module_dirs/
Launching build container using image "riot/riotbuild:latest".
docker run --rm --tty --user $(id -u) -v '/usr/share/zoneinfo/Europe/Paris:/etc/localtime:ro' -v '/home/francisco/workspace/RIOT:/data/riotbuild/riotbase:delegated' -e 'RIOTBASE=/data/riotbuild/riotbase' -e 'CCACHE_BASEDIR=/data/riotbuild/riotbase' -e 'BUILD_DIR=/data/riotbuild/riotbase/build' -e 'RIOTPROJECT=/data/riotbuild/riotbase' -e 'RIOTCPU=/data/riotbuild/riotbase/cpu' -e 'RIOTBOARD=/data/riotbuild/riotbase/boards' -e 'RIOTMAKE=/data/riotbuild/riotbase/makefiles'   -v '/home/francisco/workspace/tmp/external_module:/data/riotbuild/external/external_module:delegated'      -w '/data/riotbuild/riotbase/tests/external_module_dirs/' 'riot/riotbuild:latest' make  'EXTERNAL_MODULE_DIRS=/data/riotbuild/external/external_module /data/riotbuild/riotbase/tests/external_module_dirs/external_modules'    
Building application "tests_external_module_dirs" for "native" with MCU "native".

/data/riotbuild/riotbase/tests/external_module_dirs/main.c:23:10: fatal error: external_module.h: No such file or directory
 #include "external_module.h"
          ^~~~~~~~~~~~~~~~~~~
compilation terminated.
/data/riotbuild/riotbase/Makefile.base:107: recipe for target '/data/riotbuild/riotbase/tests/external_module_dirs/bin/native/application_tests_external_module_dirs/main.o' failed
make[1]: *** [/data/riotbuild/riotbase/tests/external_module_dirs/bin/native/application_tests_external_module_dirs/main.o] Error 1
/data/riotbuild/riotbase/Makefile.include:641: recipe for target 'application_tests_external_module_dirs.module' failed
make: *** [application_tests_external_module_dirs.module] Error 2
make: *** [/home/francisco/workspace/RIOT/makefiles/docker.inc.mk:314: ..in-docker-container] Error 2
```
</details>

So the thing that remains IMO is how to notify external users of the change, since the variable name has not changed it's hard to add a warning. Too bad the current name is quite good already.... I would suggest adding a warning if `EXTERNAL_MODULE_DIRS` is set for a release or two, what do you think @maribu?



-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/16104#pullrequestreview-650998841
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210504/29697425/attachment-0001.htm>


More information about the notifications mailing list