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.

diff --git a/make/project.mk b/make/project.mk
index cbd93b346c074da37451a902127d489d50b85874..6b2e9a4d38e683def779f31d7e84540ecce08743 100644 (file)
--- 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 99da318d61c6f91acb2ce3ef2aba3a4b16263bec..91c7e85df5a273af9b52e5707797f2e0cf35e690 100755 (executable)
--- 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