Merge branch 'bugfix/build_system_linker_script_deps' into 'master'

build system: Linker scripts or binary libraries should trigger a re-link

Add binary libraries and linker scripts to the list of dependencies for the ELF file.

Fixes TW7816.


See merge request !307

components/bootloader/src/main/component.mk

@@ -5,5 +5,8 @@
 # we pull in bootloader-specific linker arguments.
 #
 
-COMPONENT_ADD_LDFLAGS := -L $(COMPONENT_PATH) -lmain -T esp32.bootloader.ld -T $(IDF_PATH)/components/esp32/ld/esp32.rom.ld
+LINKER_SCRIPTS := esp32.bootloader.ld $(IDF_PATH)/components/esp32/ld/esp32.rom.ld
 
+COMPONENT_ADD_LDFLAGS := -L $(COMPONENT_PATH) -lmain $(addprefix -T ,$(LINKER_SCRIPTS))
+
+COMPONENT_ADD_LINKER_DEPS := $(LINKER_SCRIPTS)

components/bt/component.mk

@@ -33,8 +33,10 @@ COMPONENT_ADD_INCLUDEDIRS :=	bluedroid/bta/include			\
 LIBS := btdm_app
 
 COMPONENT_ADD_LDFLAGS := -lbt -L $(COMPONENT_PATH)/lib \
-                           $(addprefix -l,$(LIBS)) \
-                          $(LINKER_SCRIPTS)
+                           $(addprefix -l,$(LIBS))
+
+# re-link program if BT binary libs change
+COMPONENT_ADD_LINKER_DEPS := $(patsubst %,$(COMPONENT_PATH)/lib/lib%.a,$(LIBS))
 
 COMPONENT_SRCDIRS := 	bluedroid/bta/dm			\
 			bluedroid/bta/gatt			\
@@ -68,7 +70,4 @@ COMPONENT_SRCDIRS := 	bluedroid/bta/dm			\
 			bluedroid				\
 			.
 
-ALL_LIB_FILES := $(patsubst %,$(COMPONENT_PATH)/lib/lib%.a,$(LIBS))
-$(COMPONENT_LIBRARY): $(ALL_LIB_FILES)
-
 COMPONENT_SUBMODULES += lib

components/esp32/component.mk

@@ -6,31 +6,27 @@ COMPONENT_SRCDIRS := . hwcrypto
 
 LIBS := core net80211 phy rtc pp wpa smartconfig coexist wps wpa2
 
-LINKER_SCRIPTS += -T esp32_out.ld -T esp32.common.ld -T esp32.rom.ld -T esp32.peripherals.ld
+LINKER_SCRIPTS += esp32.common.ld esp32.rom.ld esp32.peripherals.ld
 
 ifeq ("$(CONFIG_NEWLIB_NANO_FORMAT)","y")
-LINKER_SCRIPTS += -T esp32.rom.nanofmt.ld
+LINKER_SCRIPTS += esp32.rom.nanofmt.ld
 endif
 
 COMPONENT_ADD_LDFLAGS := -lesp32 \
-                           $(COMPONENT_PATH)/libhal.a \
-                           -L$(COMPONENT_PATH)/lib \
-                           $(addprefix -l,$(LIBS)) \
-                          -L $(COMPONENT_PATH)/ld \
-                          $(LINKER_SCRIPTS)
+                         $(COMPONENT_PATH)/libhal.a \
+                         -L$(COMPONENT_PATH)/lib \
+                         $(addprefix -l,$(LIBS)) \
+                         -L $(COMPONENT_PATH)/ld \
+                         -T esp32_out.ld \
+                         $(addprefix -T ,$(LINKER_SCRIPTS))
 
 ALL_LIB_FILES := $(patsubst %,$(COMPONENT_PATH)/lib/lib%.a,$(LIBS))
 
 COMPONENT_SUBMODULES += lib
 
-# this is a hack to make sure the app is re-linked if the binary
-# libraries change or are updated. If they change, the main esp32
-# library will be rebuild by AR andthis will trigger a re-linking of
-# the entire app.
-#
-# It would be better for components to be able to expose any of these
-# non-standard dependencies via get_variable, but this will do for now.
-$(COMPONENT_LIBRARY): $(ALL_LIB_FILES)
+# final linking of project ELF depends on all binary libraries, and
+# all linker scripts (except esp32_out.ld, as this is code generated here.)
+COMPONENT_ADD_LINKER_DEPS := $(ALL_LIB_FILES) $(addprefix ld/,$(LINKER_SCRIPTS))
 
 # Preprocess esp32.ld linker script into esp32_out.ld
 #

components/newlib/component.mk

@@ -5,6 +5,10 @@ else
 LIBC_PATH := $(COMPONENT_PATH)/lib/libc.a
 endif
 
-COMPONENT_ADD_LDFLAGS := $(LIBC_PATH) $(COMPONENT_PATH)/lib/libm.a -lnewlib
+LIBM_PATH := $(COMPONENT_PATH)/lib/libm.a
+
+COMPONENT_ADD_LDFLAGS := $(LIBC_PATH) $(LIBM_PATH) -lnewlib
+
+COMPONENT_ADD_LINKER_DEPS := $(LIBC_PATH) $(LIBM_PATH)
 
 COMPONENT_ADD_INCLUDEDIRS := include platform_include

docs/build_system.rst

@@ -188,6 +188,10 @@ The following variables can be set inside ``component.mk`` to control build sett
   are available at all times. It is necessary if one component
   generates an include file which you then want to include in another
   component. Most components do not need to set this variable.
+- ``COMPONENT_ADD_LINKER_DEPS``: Optional list of component-relative paths
+  to files which should trigger a re-link of the ELF file if they change.
+  Typically used for linker script files and binary libraries. Most components do
+  not need to set this variable.
 
 The following variable only works for components that are part of esp-idf itself:
 

make/common.mk

@@ -52,5 +52,5 @@ endef
 #
 # example $(call resolvepath,$(CONFIG_PATH),$(CONFIG_DIR))
 define resolvepath
-$(if $(filter /%,$(1)),$(1),$(subst //,/,$(2)/$(1)))
+$(foreach dir,$(1),$(if $(filter /%,$(dir)),$(dir),$(subst //,/,$(2)/$(dir))))
 endef

make/component_wrapper.mk

@@ -95,7 +95,7 @@ COMPONENT_INCLUDES := $(OWN_INCLUDES) $(filter-out $(OWN_INCLUDES),$(COMPONENT_I
 #
 # This means if directories move (breaking absolute paths), don't need to 'make clean'
 define MakeVariablePath
-$(subst $(IDF_PATH),$$(IDF_PATH),$(subst $(PROJECT_PATH),$$(PROJECT_PATH),$(subst $(BUILD_DIR_BASE),\$$(BUILD_DIR_BASE),$(1))))
+$(subst $(IDF_PATH),$$(IDF_PATH),$(subst $(PROJECT_PATH),$$(PROJECT_PATH),$(subst $(BUILD_DIR_BASE),$$(BUILD_DIR_BASE),$(1))))
 endef
 
 # component_project_vars.mk target for the component. This is used to
@@ -116,6 +116,7 @@ component_project_vars.mk::
 	@echo '# Automatically generated build file. Do not edit.' > $@
 	@echo 'COMPONENT_INCLUDES += $(call MakeVariablePath,$(addprefix $(COMPONENT_PATH)/,$(COMPONENT_ADD_INCLUDEDIRS)))' >> $@
 	@echo 'COMPONENT_LDFLAGS += $(call MakeVariablePath,$(COMPONENT_ADD_LDFLAGS))' >> $@
+	@echo 'COMPONENT_LINKER_DEPS += $(call MakeVariablePath,$(call resolvepath,$(COMPONENT_ADD_LINKER_DEPS),$(COMPONENT_PATH)))' >> $@
 	@echo 'COMPONENT_SUBMODULES += $(call MakeVariablePath,$(addprefix $(COMPONENT_PATH)/,$(COMPONENT_SUBMODULES)))' >> $@
 	@echo '$(COMPONENT_NAME)-build: $(addsuffix -build,$(COMPONENT_DEPENDS))' >> $@
 

make/project.mk

@@ -151,8 +151,6 @@ endif
 # Set default LDFLAGS
 
 LDFLAGS ?= -nostdlib \
-	-L$(IDF_PATH)/lib \
-	-L$(IDF_PATH)/ld \
 	$(addprefix -L$(BUILD_DIR_BASE)/,$(COMPONENTS) $(TEST_COMPONENT_NAMES) $(SRCDIRS) ) \
 	-u call_user_start_cpu0	\
 	$(EXTRA_LDFLAGS) \
@@ -276,7 +274,10 @@ COMPONENT_LIBRARIES = $(filter $(notdir $(COMPONENT_PATHS_BUILDABLE)) $(TEST_COM
 
 # ELF depends on the library archive files for COMPONENT_LIBRARIES
 # the rules to build these are emitted as part of GenerateComponentTarget below
-$(APP_ELF): $(foreach libcomp,$(COMPONENT_LIBRARIES),$(BUILD_DIR_BASE)/$(libcomp)/lib$(libcomp).a)
+#
+# also depends on additional dependencies (linker scripts & binary libraries)
+# stored in COMPONENT_LINKER_DEPS, built via component.mk files' COMPONENT_ADD_LINKER_DEPS variable
+$(APP_ELF): $(foreach libcomp,$(COMPONENT_LIBRARIES),$(BUILD_DIR_BASE)/$(libcomp)/lib$(libcomp).a) $(COMPONENT_LINKER_DEPS)
 	$(summary) LD $(notdir $@)
 	$(CC) $(LDFLAGS) -o $@ -Wl,-Map=$(APP_MAP)
 
@@ -374,7 +375,7 @@ $(IDF_PATH)/$(1)/.git:
 # Parse 'git submodule status' output for out-of-date submodule.
 # Status output prefixes status line with '+' if the submodule commit doesn't match
 ifneq ("$(shell cd ${IDF_PATH} && git submodule status $(1) | grep '^+')","")
-$$(info WARNING: git submodule $(1) may be out of date. Run 'git submodule update' to update.)
+$$(info WARNING: esp-idf git submodule $(1) may be out of date. Run 'git submodule update' in IDF_PATH dir to update.)
 endif
 endef
 

make/test_build_system.sh

@@ -141,6 +141,19 @@ function run_tests()
     assert_built ${APP_BINS} ${BOOTLOADER_BINS} partitions_singleapp.bin
     [ -f ${BUILD}/partition*.bin ] || failure "A partition table should have been built in CRLF mode"
 
+    print_status "Touching rom ld file should re-link app and bootloader"
+    make
+    take_build_snapshot
+    touch ${IDF_PATH}/components/esp32/ld/esp32.rom.ld
+    make
+    assert_rebuilt ${APP_BINS} ${BOOTLOADER_BINS}
+
+    print_status "Touching peripherals ld file should only re-link app"
+    take_build_snapshot
+    touch ${IDF_PATH}/components/esp32/ld/esp32.peripherals.ld
+    make
+    assert_rebuilt ${APP_BINS}
+    assert_not_rebuilt ${BOOTLOADER_BINS}
 
     print_status "All tests completed"
     if [ -n "${FAILURES}" ]; then