Merge pull request #16104 from maribu/external_modules

build system: Rework EXTERNAL_MODULE_DIRS

Makefile.dep

@@ -10,7 +10,7 @@
 
 # include external modules dependencies
 # processed before RIOT ones to be evaluated before the 'default' rules.
--include $(EXTERNAL_MODULE_DIRS:%=%/Makefile.dep)
+-include $(EXTERNAL_MODULE_PATHS:%=%/Makefile.dep)
 
 # pull dependencies from sys and drivers
 include $(RIOTBASE)/sys/Makefile.dep

Makefile.include

@@ -41,22 +41,23 @@ include $(RIOT_MAKEFILES_GLOBAL_PRE)
 -include Makefile.local
 
 # set undefined variables
-RIOTBASE            ?= $(_riotbase)
-RIOTCPU             ?= $(RIOTBASE)/cpu
+RIOTBASE                        ?= $(_riotbase)
+RIOTCPU                         ?= $(RIOTBASE)/cpu
 # Deprecated to set RIOTBOARD, use EXTERNAL_BOARD_DIRS
-RIOTBOARD           ?= $(RIOTBASE)/boards
-EXTERNAL_BOARD_DIRS ?=
-RIOTMAKE            ?= $(RIOTBASE)/makefiles
-RIOTPKG             ?= $(RIOTBASE)/pkg
-RIOTTOOLS           ?= $(RIOTBASE)/dist/tools
-RIOTPROJECT         ?= $(shell git rev-parse --show-toplevel 2>/dev/null || pwd)
-BUILD_DIR           ?= $(RIOTBASE)/build
-APPDIR              ?= $(CURDIR)
-BINDIRBASE          ?= $(APPDIR)/bin
-BINDIR              ?= $(BINDIRBASE)/$(BOARD)
-PKGDIRBASE          ?= $(RIOTBASE)/build/pkg
-DLCACHE             ?= $(RIOTTOOLS)/dlcache/dlcache.sh
-DLCACHE_DIR         ?= $(RIOTBASE)/.dlcache
+RIOTBOARD                       ?= $(RIOTBASE)/boards
+EXTERNAL_BOARD_DIRS             ?=
+RIOTMAKE                        ?= $(RIOTBASE)/makefiles
+RIOTPKG                         ?= $(RIOTBASE)/pkg
+RIOTTOOLS                       ?= $(RIOTBASE)/dist/tools
+RIOTPROJECT                     ?= $(shell git rev-parse --show-toplevel 2>/dev/null || pwd)
+BUILD_DIR                       ?= $(RIOTBASE)/build
+APPDIR                          ?= $(CURDIR)
+BINDIRBASE                      ?= $(APPDIR)/bin
+BINDIR                          ?= $(BINDIRBASE)/$(BOARD)
+PKGDIRBASE                      ?= $(RIOTBASE)/build/pkg
+DLCACHE                         ?= $(RIOTTOOLS)/dlcache/dlcache.sh
+DLCACHE_DIR                     ?= $(RIOTBASE)/.dlcache
+WARNING_EXTERNAL_MODULE_DIRS    ?= 1
 
 # include CI info such as BOARD_INSUFFICIENT_MEMORY, if existing
 -include Makefile.ci
@@ -82,17 +83,20 @@ __OVERRIDE_DIRECTORY_VARIABLES := $(__DIRECTORY_VARIABLES)
 # Use absolute paths in recursive "make" even if overridden on command line.
 MAKEOVERRIDES += $(foreach v,$(__OVERRIDE_DIRECTORY_VARIABLES),$(v)=$($(v)))
 
-# Setting EXTERNAL_BOARD_DIRS as command line argument is too messy to handle:
-# Even when every path in EXTERNAL_BOARD_DIRS is turned into an absolute path
-# using override, sub-makes will still get the original value. Using
-# MAKEOVERRIDES has issues with spaces in the values, which are used as
-# separator in EXTERNAL_BOARD_DIRS. So we just enforce setting the value
+# Setting EXTERNAL_BOARD_DIRS and EXTERNAL_MODULE_DIRS as command line argument
+# is too messy to handle: Even when every path in EXTERNAL_BOARD_DIRS is turned
+# into an absolute path using override, sub-makes will still get the original
+# value. Using MAKEOVERRIDES has issues with spaces in the values, which are
+# used as separator in EXTERNAL_BOARD_DIRS. So we just enforce setting the value
 # either in a Makefile, or as environment variable.
-ifeq ($(origin EXTERNAL_BOARD_DIRS),command line)
+ifeq ($(INSIDE_DOCKER),0)
   # In Docker absolute paths are always given, so only fail when not in docker
-  ifeq ($(INSIDE_DOCKER),0)
+  ifeq ($(origin EXTERNAL_BOARD_DIRS),command line)
     $(error EXTERNAL_BOARD_DIRS must be passed as environment variable, and not as command line argument)
   endif
+  ifeq ($(origin EXTERNAL_MODULE_DIRS),command line)
+    $(error EXTERNAL_MODULE_DIRS must be passed as environment variable, and not as command line argument)
+  endif
 endif
 
 # Deprecation of configuring 'RIOTBOARD'
@@ -102,15 +106,27 @@ ifneq ($(abspath $(RIOTBASE)/boards),$(abspath $(RIOTBOARD)))
   __DIRECTORY_VARIABLES += RIOTBOARD
 endif
 
-ifneq (,$(BOARDSDIR))
-  # Only warn users, not the CI.
-  ifneq ($(RIOT_CI_BUILD),1)
-    # Do not warn when set from sub-make
-    ifeq ($(MAKELEVEL),0)
+# Only warn users, not the CI.
+ifneq ($(RIOT_CI_BUILD),1)
+  # Do not warn when set from sub-make
+  ifeq ($(MAKELEVEL),0)
+    ifneq (,$(BOARDSDIR))
       $(warning Using BOARDSDIR is deprecated use EXTERNAL_BOARD_DIRS instead)
       $(info EXTERNAL_BOARD_DIRS can contain multiple folders separated by space)
     endif
+
+    # API change warning for EXTERNAL_MODULE_DIRS, remove by 2021.10
+    ifneq (,$(EXTERNAL_MODULE_DIRS))
+      ifeq (1,$(WARNING_EXTERNAL_MODULE_DIRS))
+        $(info Warning! EXTERNAL_MODULE_DIRS is a search folder since 2021.07-branch, see \
+               https://doc.riot-os.org/creating-modules.html#modules-outside-of-riotbase)
+      endif
+    endif
   endif
+endif
+
+# Needed for backward compatibility:
+ifneq (,$(BOARDSDIR))
   EXTERNAL_BOARD_DIRS += $(BOARDSDIR)
 endif
 
@@ -130,6 +146,9 @@ override DLCACHE_DIR            := $(abspath $(DLCACHE_DIR))
 EXTERNAL_BOARD_DIRS             := $(foreach dir,\
                                              $(EXTERNAL_BOARD_DIRS),\
                                              $(abspath $(dir)))
+EXTERNAL_MODULE_DIRS            := $(foreach dir,\
+                                             $(EXTERNAL_MODULE_DIRS),\
+                                             $(abspath $(dir)))
 
 # Ensure that all directories are set and don't contain spaces.
 ifneq (, $(filter-out 1, $(foreach v,$(__DIRECTORY_VARIABLES),$(words $($(v))))))
@@ -511,7 +530,7 @@ include $(RIOTBASE)/sys/Makefile.include
 -include $(USEPKG:%=$(RIOTPKG)/%/Makefile.include)
 
 # include external modules configuration
--include $(EXTERNAL_MODULE_DIRS:%=%/Makefile.include)
+-include $(EXTERNAL_MODULE_PATHS:%=%/Makefile.include)
 
 # Deduplicate includes without sorting them
 # see https://stackoverflow.com/questions/16144115/makefile-remove-duplicate-words-without-sorting
@@ -600,7 +619,8 @@ endif
 # We assume $(LINK) to be gcc-like. Use `LINKFLAGPREFIX :=` for ld-like linker options.
 LINKFLAGPREFIX ?= -Wl,
 
-DIRS += $(EXTERNAL_MODULE_DIRS)
+# Also build external modules
+DIRS += $(EXTERNAL_MODULE_PATHS)
 
 # Define dependencies required for building (headers, downloading source files,)
 BUILDDEPS += $(RIOTBUILD_CONFIG_HEADER_C)

dist/tools/vera++/check.sh

@@ -16,7 +16,7 @@ CURDIR=$(cd "$(dirname "$0")" && pwd)
 # tests/pkg_utensor/models/deep_mlp_weight.hpp is an auto-generated file
 # with lots of commas so T009 takes very long. Since it is auto-generated, just
 # exclude it.
-EXCLUDE='^(.+/vendor/|dist/tools/coccinelle/include|dist/tools/fixdep/fixdep.c|dist/tools/lpc2k_pgm/src|tests/pkg_utensor/models)'
+EXCLUDE='^(.+/vendor/|dist/tools/coccinelle/include|dist/tools/fixdep/fixdep.c|dist/tools/lpc2k_pgm/src|tests/pkg_utensor/external_modules/models)'
 FILES=$(changed_files)
 
 if [ -z "${FILES}" ]; then

doc/doxygen/src/creating-modules.md

@@ -68,13 +68,19 @@ their dependencies.
 Modules outside of RIOTBASE                      {#modules-outside-of-riotbase}
 ===========================
 Modules can be defined outside `RIOTBASE`. In addition to add it to `USEMODULE`
-the user needs to add the module path to `EXTERNAL_MODULE_DIRS`.
+the user needs to add the directory (or directories) containing external modules
+to `EXTERNAL_MODULE_DIRS`.
 
-The external module can optionally define the following files:
+External modules can optionally define the following files:
 * `Makefile.include` file to set global build configuration like `CFLAGS` or add
   API headers include paths to the `USEMODULE_INCLUDES` variable.
 * `Makefile.dep` file to set module dependencies
 
+***NOTE:*** The name of an external module must be unique (both in regard to other
+            external modules, as well to native RIOT modules). Additionally, the
+            directory containing the module must match the module name, e.g.
+            module `foo` must be located in `<PATH_IN_EXTERNAL_MODULE_DIRS>/foo`.
+
 An example can be found in
 [`tests/external_module_dirs`](https://github.com/RIOT-OS/RIOT/tree/master/tests/external_module_dirs)
 

makefiles/dependency_resolution.inc.mk

@@ -2,6 +2,10 @@
 # until no new modules, pkgs, or features are pull in order to catch all
 # transient dependencies
 
+# Locate used modules in $(EXTERNAL_MODULE_DIRS).
+EXTERNAL_MODULE_PATHS := $(sort $(foreach dir,$(EXTERNAL_MODULE_DIRS),\
+  $(foreach mod,$(USEMODULE),$(dir $(wildcard $(dir)/$(mod)/Makefile)))))
+
 # Back up current state to detect changes
 OLD_STATE := $(USEMODULE) $(USEPKG) $(FEATURES_USED)
 

makefiles/kconfig.mk

@@ -143,8 +143,8 @@ $(KCONFIG_GENERATED_DEPENDENCIES): FORCE | $(GENERATED_DIR)
 	      printf "config %s\n\tbool\n\tdefault y\n", toupper($$0)}' \
 	  | $(LAZYSPONGE) $(LAZYSPONGE_FLAGS) $@
 
-# All directories in EXTERNAL_MODULES_DIR which have a Kconfig file
-EXTERNAL_MODULE_KCONFIGS ?= $(sort $(foreach dir,$(EXTERNAL_MODULE_DIRS),\
+# All directories in EXTERNAL_MODULES_PATHS which have a Kconfig file
+EXTERNAL_MODULE_KCONFIGS ?= $(sort $(foreach dir,$(EXTERNAL_MODULE_PATHS),\
                               $(wildcard $(dir)/Kconfig)))
 # Build a Kconfig file that source all external modules configuration
 # files. Every EXTERNAL_MODULE_DIRS with a Kconfig file is written to

tests/cpp_exclude/Makefile

@@ -3,6 +3,6 @@ include ../Makefile.tests_common
 FEATURES_REQUIRED += cpp libstdcpp
 
 USEMODULE += module_exclude
-EXTERNAL_MODULE_DIRS += $(CURDIR)/module_exclude
+EXTERNAL_MODULE_DIRS += external_modules
 
 include $(RIOTBASE)/Makefile.include

tests/cpp_ext/Makefile

@@ -3,6 +3,6 @@ include ../Makefile.tests_common
 FEATURES_REQUIRED += cpp libstdcpp
 
 USEMODULE += module
-EXTERNAL_MODULE_DIRS += $(CURDIR)/module
+EXTERNAL_MODULE_DIRS += external_modules
 
 include $(RIOTBASE)/Makefile.include

tests/external_module_dirs/Makefile

@@ -2,6 +2,6 @@ include ../Makefile.tests_common
 USEMODULE += random
 
 USEMODULE += external_module
-EXTERNAL_MODULE_DIRS += $(CURDIR)/external_module
+EXTERNAL_MODULE_DIRS += external_modules
 
 include $(RIOTBASE)/Makefile.include

tests/external_module_dirs/external_modules/external_module_not_used/this_should_not_compile.c

@@ -0,0 +1,24 @@
+/*
+ * Copyright (C) 2020 Otto-von-Guericke-Universität Magdeburg
+ *
+ * This file is subject to the terms and conditions of the GNU Lesser
+ * General Public License v2.1. See the file LICENSE in the top level
+ * directory for more details.
+ */
+
+/**
+ * @ingroup     tests
+ * @{
+ *
+ * @file
+ * @brief       Test the EXTERNAL_MODULE_DIRS feature
+ * @note        Define a shared variable
+ *
+ * @author      Marian Buschsieweke <marian.buschsieweke@ovgu.de>
+ *
+ * @}
+ */
+
+#error "This is should not be compiled"
+
+typedef int dont_be_pedantic;

tests/kconfig/Makefile

@@ -2,7 +2,6 @@ include ../Makefile.tests_common
 
 USEMODULE += external_module_1
 USEMODULE += external_module_2
-EXTERNAL_MODULE_DIRS += $(CURDIR)/external_module_1
-EXTERNAL_MODULE_DIRS += $(CURDIR)/external_module_2
+EXTERNAL_MODULE_DIRS += external_modules
 
 include $(RIOTBASE)/Makefile.include

tests/pkg_openwsn_sock_udp/Makefile

@@ -38,7 +38,7 @@ USEMODULE += sock_udp
 
 # Mock OpenWSN scheduler to handle the udp_transmit task.
 USEMODULE += openwsn_scheduler_mock
-EXTERNAL_MODULE_DIRS += $(CURDIR)/scheduler
+EXTERNAL_MODULE_DIRS += external_modules
 
 # Explicitly include ztimer to enable timeout
 USEMODULE += ztimer_usec

tests/pkg_openwsn_sock_udp/external_modules/openwsn_scheduler_mock/Makefile

@@ -1,3 +1 @@
-MODULE := openwsn_scheduler_mock
-
 include $(RIOTBASE)/Makefile.base

tests/pkg_tensorflow-lite/Makefile

@@ -14,7 +14,7 @@ ifeq (mnist,$(EXAMPLE))
   # default for now
   DISABLE_MODULE += cortexm_fpu
   USEMODULE += $(EXAMPLE)
-  EXTERNAL_MODULE_DIRS += $(CURDIR)/$(EXAMPLE)
+  EXTERNAL_MODULE_DIRS += external_modules
 else
   # Use upstream example
   USEMODULE += tensorflow-lite-$(EXAMPLE)

tests/pkg_utensor/Makefile

@@ -7,7 +7,7 @@ BLOBS += digit
 
 # Include C++ model and training weights code as an external module
 USEMODULE += models
-EXTERNAL_MODULE_DIRS += $(CURDIR)/models
+EXTERNAL_MODULE_DIRS += external_modules
 
 include $(RIOTBASE)/Makefile.include
 

tests/pkg_utensor/external_modules/models/Makefile

@@ -0,0 +1 @@
+include $(RIOTBASE)/Makefile.base

tests/pkg_utensor/main.cpp

@@ -23,7 +23,7 @@
 
 #include "blob/digit.h"         //contains a sample taken from the MNIST test set
 
-#include "models/deep_mlp.hpp"  //generated model file
+#include "deep_mlp.hpp"         //generated model file
 #include "tensor.hpp"           //useful tensor classes
 
 int main()