From 6ac8aba955c5c4fa88cc4b4b1b3d69a017e3f2f5 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 26 Feb 2021 18:41:51 +0100 Subject: [PATCH] build system: rework EXTERNAL_MODULE_DIRS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, external modules had to be individually added to both EXTERNAL_MODULE_DIRS and USEMODULE. If those where not in sync, this resulted in build errors. With this commit, search folders for external modules are added to EXTERNAL_MODULE_DIRS instead. So lets say the file system structure is like this ``` └── /path/to/external/modules    ├── mod_a    │   ├── Makefile    │   ├── Makefile.dep    │   ├── Makefile.include    │   ├── foo.c    │   └── include    │   └── external_module.h    └── mod_b   ├── Makefile    └── bar.c ``` One now adds `/path/to/external/modules` to EXTERNAL_MODULES and only with `USEMODULE += mod_a` the corresponding module, dependencies and include settings are actually used. Hence, it is possible to configure `EXTERNAL_MODULE_DIRS` from `~/.profile` or `~/.bashrc` once and never needs to worry about them again. --- Makefile.dep | 2 +- Makefile.include | 78 ++++++++++++++++---------- dist/tools/vera++/check.sh | 2 +- doc/doxygen/src/creating-modules.md | 10 +++- makefiles/dependency_resolution.inc.mk | 4 ++ makefiles/kconfig.mk | 4 +- 6 files changed, 65 insertions(+), 35 deletions(-) diff --git a/Makefile.dep b/Makefile.dep index ba85bed880..4c57278ff1 100644 --- a/Makefile.dep +++ b/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 diff --git a/Makefile.include b/Makefile.include index 9bb29c5ad7..284844f4c7 100644 --- a/Makefile.include +++ b/Makefile.include @@ -32,22 +32,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 @@ -73,17 +74,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' @@ -93,15 +97,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 @@ -121,6 +137,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)))))) @@ -501,7 +520,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 @@ -590,7 +609,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) diff --git a/dist/tools/vera++/check.sh b/dist/tools/vera++/check.sh index ce74912d48..61a5a7158b 100755 --- a/dist/tools/vera++/check.sh +++ b/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 diff --git a/doc/doxygen/src/creating-modules.md b/doc/doxygen/src/creating-modules.md index a9c90cc257..d217f791e3 100644 --- a/doc/doxygen/src/creating-modules.md +++ b/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 `/foo`. + An example can be found in [`tests/external_module_dirs`](https://github.com/RIOT-OS/RIOT/tree/master/tests/external_module_dirs) diff --git a/makefiles/dependency_resolution.inc.mk b/makefiles/dependency_resolution.inc.mk index 7b15ea2ef1..a3a76223c2 100644 --- a/makefiles/dependency_resolution.inc.mk +++ b/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) diff --git a/makefiles/kconfig.mk b/makefiles/kconfig.mk index 2d91fbe18f..dd9dd1bc3a 100644 --- a/makefiles/kconfig.mk +++ b/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