From 7df598a0626456afa71c629bba434ef8892c2728 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Wed, 13 Feb 2019 11:40:48 +0800 Subject: [PATCH] make: fix issues related to EXTRA_COMPONENT_DIRS 1. When one of the COMPONENT_DIRS points to a component directory (i.e. a directory containing component.mk, not a directory of multiple components), and there is a subdirectory in it which also contains a component, the subdirectory was mistakenly added to the list of components and compiled. For example: main/ component.mk main.c test/ component.mk test_main.c Would compile test_main.c and link libtest.a. 2. When one of the COMPONENT_DIRS points to a component directory, and the parent directory contained a directory with the same name as another component, that directory would be mistakenly added to the COMPONENT_PATHS. For example: esp/ esp-idf/ esp32/ (random stuff) mycomponent/ component.mk mycomponent.c myproject/ main/ Makefile and Makefile sets EXTRA_COMPONENT_DIRS=$(realpath ../mycomponent), then "esp32" directory which is at the same level as mycomponent was added to COMPONENT_PATHS. 3. If EXTRA_COMPONENT_DIRS pointed to a directory with a list of components, and one of the subdirectories was not a component, but had the same name as another component, than that directory would be mistakenly added to COMPONENT_PATHS instead of the real esp32 component directory. For example: my_components/ my_component/ component.mk my_component.c esp32/ (some random stuff) and EXTRA_COMPONENT_DIRS would point to my_components/, then "esp32" directory would be added to COMPONENT_PATHS instead of the real esp32 component directory. --- make/project.mk | 45 +++++++++++++++++++++++++++-------- tools/ci/test_build_system.sh | 31 +++++++++++++++++++++++- 2 files changed, 65 insertions(+), 11 deletions(-) diff --git a/make/project.mk b/make/project.mk index cbd93b346c..6b2e9a4d38 100644 --- a/make/project.mk +++ b/make/project.mk @@ -143,6 +143,9 @@ ifndef COMPONENT_DIRS EXTRA_COMPONENT_DIRS ?= COMPONENT_DIRS := $(PROJECT_PATH)/components $(EXTRA_COMPONENT_DIRS) $(IDF_PATH)/components $(PROJECT_PATH)/main endif +# Make sure that every directory in the list is an absolute path without trailing slash. +# This is necessary to split COMPONENT_DIRS into SINGLE_COMPONENT_DIRS and MULTI_COMPONENT_DIRS below. +COMPONENT_DIRS := $(foreach cd,$(COMPONENT_DIRS),$(abspath $(cd))) export COMPONENT_DIRS ifdef SRCDIRS @@ -150,33 +153,55 @@ $(warning SRCDIRS variable is deprecated. These paths can be added to EXTRA_COMP COMPONENT_DIRS += $(abspath $(SRCDIRS)) endif -# The project Makefile can define a list of components, but if it does not do this we just take all available components -# in the component dirs. A component is COMPONENT_DIRS directory, or immediate subdirectory, +# List of component directories, i.e. directories which contain a component.mk file +SINGLE_COMPONENT_DIRS := $(abspath $(dir $(dir $(foreach cd,$(COMPONENT_DIRS),\ + $(wildcard $(cd)/component.mk))))) + +# List of components directories, i.e. directories which may contain components +MULTI_COMPONENT_DIRS := $(filter-out $(SINGLE_COMPONENT_DIRS),$(COMPONENT_DIRS)) + +# The project Makefile can define a list of components, but if it does not do this +# we just take all available components in the component dirs. +# A component is COMPONENT_DIRS directory, or immediate subdirectory, # which contains a component.mk file. # # Use the "make list-components" target to debug this step. ifndef COMPONENTS # Find all component names. The component names are the same as the # directories they're in, so /bla/components/mycomponent/component.mk -> mycomponent. -COMPONENTS := $(dir $(foreach cd,$(COMPONENT_DIRS), \ - $(wildcard $(cd)/*/component.mk) $(wildcard $(cd)/component.mk) \ - )) +# We need to do this for MULTI_COMPONENT_DIRS only, since SINGLE_COMPONENT_DIRS +# are already known to contain component.mk. +COMPONENTS := $(dir $(foreach cd,$(MULTI_COMPONENT_DIRS),$(wildcard $(cd)/*/component.mk))) \ + $(SINGLE_COMPONENT_DIRS) COMPONENTS := $(sort $(foreach comp,$(COMPONENTS),$(lastword $(subst /, ,$(comp))))) endif -# After a full manifest of component names is determined, subtract the ones explicitly omitted by the project Makefile. +# After a full manifest of component names is determined, subtract the ones explicitly +# omitted by the project Makefile. +EXCLUDE_COMPONENTS ?= ifdef EXCLUDE_COMPONENTS -COMPONENTS := $(filter-out $(subst ",,$(EXCLUDE_COMPONENTS)), $(COMPONENTS)) +COMPONENTS := $(filter-out $(subst ",,$(EXCLUDE_COMPONENTS)), $(COMPONENTS)) # to keep syntax highlighters happy: ")) endif export COMPONENTS # Resolve all of COMPONENTS into absolute paths in COMPONENT_PATHS. +# For each entry in COMPONENT_DIRS: +# - either this is directory with multiple components, in which case check that +# a subdirectory with component name exists, and it contains a component.mk file. +# - or, this is a directory of a single component, in which case the name of this +# directory has to match the component name # # If a component name exists in multiple COMPONENT_DIRS, we take the first match. # # NOTE: These paths must be generated WITHOUT a trailing / so we # can use $(notdir x) to get the component name. -COMPONENT_PATHS := $(foreach comp,$(COMPONENTS),$(firstword $(foreach cd,$(COMPONENT_DIRS),$(wildcard $(dir $(cd))$(comp) $(cd)/$(comp))))) +COMPONENT_PATHS := $(foreach comp,$(COMPONENTS),\ + $(firstword $(foreach cd,$(COMPONENT_DIRS),\ + $(if $(findstring $(cd),$(MULTI_COMPONENT_DIRS)),\ + $(abspath $(dir $(wildcard $(cd)/$(comp)/component.mk))),)\ + $(if $(findstring $(cd),$(SINGLE_COMPONENT_DIRS)),\ + $(if $(filter $(comp),$(notdir $(cd))),$(cd),),)\ + ))) export COMPONENT_PATHS TEST_COMPONENTS ?= @@ -274,7 +299,7 @@ LDFLAGS ?= -nostdlib \ # before including project.mk. Default flags will be added before the ones provided in application Makefile. # CPPFLAGS used by C preprocessor -# If any flags are defined in application Makefile, add them at the end. +# If any flags are defined in application Makefile, add them at the end. CPPFLAGS ?= EXTRA_CPPFLAGS ?= CPPFLAGS := -DESP_PLATFORM -D IDF_VER=\"$(IDF_VER)\" -MMD -MP $(CPPFLAGS) $(EXTRA_CPPFLAGS) @@ -597,7 +622,7 @@ list-components: $(info $(TEST_COMPONENTS_LIST)) $(info $(call dequote,$(SEPARATOR))) $(info TEST_EXCLUDE_COMPONENTS (list of test excluded names)) - $(info $(if $(EXCLUDE_COMPONENTS) || $(TEST_EXCLUDE_COMPONENTS),$(EXCLUDE_COMPONENTS) $(TEST_EXCLUDE_COMPONENTS),(none provided))) + $(info $(if $(EXCLUDE_COMPONENTS) || $(TEST_EXCLUDE_COMPONENTS),$(EXCLUDE_COMPONENTS) $(TEST_EXCLUDE_COMPONENTS),(none provided))) $(info $(call dequote,$(SEPARATOR))) $(info COMPONENT_PATHS (paths to all components):) $(foreach cp,$(COMPONENT_PATHS),$(info $(cp))) diff --git a/tools/ci/test_build_system.sh b/tools/ci/test_build_system.sh index 99da318d61..91c7e85df5 100755 --- a/tools/ci/test_build_system.sh +++ b/tools/ci/test_build_system.sh @@ -271,7 +271,7 @@ function run_tests() ( make 2>&1 | grep "does not fit in configured flash size 1MB" ) || failure "Build didn't fail with expected flash size failure message" mv sdkconfig.bak sdkconfig - print_status "sdkconfig should have contents both files: sdkconfig and sdkconfig.defaults" + print_status "sdkconfig should have contents of both files: sdkconfig and sdkconfig.defaults" make clean > /dev/null; rm -f sdkconfig.defaults; rm -f sdkconfig; @@ -280,6 +280,35 @@ function run_tests() make defconfig > /dev/null; grep "CONFIG_PARTITION_TABLE_OFFSET=0x10000" sdkconfig || failure "The define from sdkconfig.defaults should be into sdkconfig" grep "CONFIG_PARTITION_TABLE_TWO_OTA=y" sdkconfig || failure "The define from sdkconfig should be into sdkconfig" + rm sdkconfig sdkconfig.defaults + make defconfig + + print_status "Empty directory not treated as a component" + mkdir -p components/esp32 + make || failure "Failed to build with empty esp32 directory in components" + rm -rf components + + print_status "If a component directory is added to COMPONENT_DIRS, its subdirectories are not added" + mkdir -p main/test + touch main/test/component.mk + echo "#error This should not be built" > main/test/test.c + make || failure "COMPONENT_DIRS has added component subdirectory to the build" + rm -rf main/test + + print_status "If a component directory is added to COMPONENT_DIRS, its sibling directories are not added" + mkdir -p mycomponents/mycomponent + touch mycomponents/mycomponent/component.mk + # first test by adding single component directory to EXTRA_COMPONENT_DIRS + mkdir -p mycomponents/esp32 + touch mycomponents/esp32/component.mk + make EXTRA_COMPONENT_DIRS=$PWD/mycomponents/mycomponent || failure "EXTRA_COMPONENT_DIRS has added a sibling directory" + rm -rf mycomponents/esp32 + # now the same thing, but add a components directory + mkdir -p esp32 + touch esp32/component.mk + make EXTRA_COMPONENT_DIRS=$PWD/mycomponents || failure "EXTRA_COMPONENT_DIRS has added a sibling directory" + rm -rf esp32 + rm -rf mycomponents print_status "All tests completed" if [ -n "${FAILURES}" ]; then