Merge branch 'bugfix/bootloader_incremental_build' into 'master'

Make: fix incremental builds, add build tests Four semi-related build changes in one: * Add basic tests for make system sanity (can be expanded as we find corner cases) * Fix incremental building of bootloader when source files change * Don't regenerate component libraries or re-link the ELF file if nothing changed * Rename `$(vecho)` to `$(summary)` and add a new `$(details)` macro to echo some helpful build details when V=1. See merge request !20
2016-08-24 20:24:33 +08:00 · 2016-08-24 20:24:33 +08:00 · 7ba0d90df2
parent accea4718b a233619429
commit 7ba0d90df2
8 changed files with 212 additions and 24 deletions
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@ -25,6 +25,14 @@ test_nvs_on_host:
    - cd components/nvs_flash/test
    - make test

+test_build_system:
+  stage: test
+  image: espressif/esp32-ci-env
+  variables:
+    IDF_PATH: "$CI_PROJECT_DIR"
+  script:
+    - ./make/build_system_tests.sh
+
 push_master_to_github:
  stage: deploy
  only:
--- a/README.buildenv
+++ b/README.buildenv
@ -239,7 +239,7 @@ COMPONENT_EXTRA_CLEAN := test_tjpgd_logo.h
 graphics_lib.o: logo.h

 logo.h: $(COMPONENT_PATH)/logo.bmp
-	$(vecho) BMP2H $@
+	$(summary) BMP2H $@
 	$(Q) bmp2h -i $^ -o $@

 include $(IDF_PATH)/make/component.mk
--- a/components/bootloader/Makefile.projbuild
+++ b/components/bootloader/Makefile.projbuild
@ -13,7 +13,7 @@ BOOTLOADER_COMPONENT_PATH := $(COMPONENT_PATH)
 BOOTLOADER_BUILD_DIR=$(BUILD_DIR_BASE)/bootloader
 BOOTLOADER_BIN=$(BOOTLOADER_BUILD_DIR)/bootloader.bin

-.PHONY: bootloader-clean bootloader-flash bootloader
+.PHONY: bootloader-clean bootloader-flash bootloader $(BOOTLOADER_BIN)

 $(BOOTLOADER_BIN): $(COMPONENT_PATH)/src/sdkconfig
 	$(Q) PROJECT_PATH= \
--- a/make/build_system_tests.sh
+++ b/make/build_system_tests.sh
@ -0,0 +1,163 @@
+#!/bin/bash
+#
+# Build system tests
+#
+# Just a bash script that tests some likely make failure scenarios in a row
+# Creates its own test build directory under TMP and cleans it up when done.
+#
+# Environment variables:
+# IDF_PATH - must be set
+# TMP - can override /tmp location for build directory
+# ESP_IDF_TEMPLATE_GIT - Can override git clone source for template app. Otherwise github.
+# NOCLEANUP - Set to '1' if you want the script to leave its temporary directory when done, for post-mortem.
+#
+
+# Set up some variables
+#
+[ -z ${TMP} ] && TMP="/tmp"
+# override ESP_IDF_TEMPLATE_GIT to point to a local dir if you're testing and want fast iterations
+[ -z ${ESP_IDF_TEMPLATE_GIT} ] && ESP_IDF_TEMPLATE_GIT=https://github.com/espressif/esp-idf-template.git
+export V=1
+
+function run_tests()
+{
+	FAILURES=
+	STATUS="Starting"
+	print_status "Checking prerequisites"
+	[ -z ${IDF_PATH} ] && echo "IDF_PATH is not set. Need path to esp-idf installation." && exit 2
+
+	print_status "Cloning template..."
+	git clone ${ESP_IDF_TEMPLATE_GIT} template
+	cd template
+
+	BOOTLOADER_BINS="bootloader/bootloader.elf bootloader/bootloader.bin"
+	APP_BINS="app-template.elf app-template.bin"
+
+	print_status "Initial clean build"
+	# if make fails here, everything fails
+	make || exit $?
+	# check all the expected build artifacts from the clean build
+	assert_built ${APP_BINS} ${BOOTLOADER_BINS} partitions_singleapp.bin
+	[ -f ${BUILD}/partition*.bin ] || failure "A partition table should have been built"
+
+	print_status "Updating component source file rebuilds component"
+	# touch a file & do a build
+	take_build_snapshot
+	touch ${IDF_PATH}/components/esp32/syscalls.c
+	make || failure "Failed to partial build"
+	assert_rebuilt ${APP_BINS} esp32/libesp32.a esp32/syscalls.o
+	assert_not_rebuilt lwip/liblwip.a freertos/libfreertos.a ${BOOTLOADER_BINS} partitions_singleapp.bin
+
+	print_status "Bootloader source file rebuilds bootloader"
+	take_build_snapshot
+	touch ${IDF_PATH}/components/bootloader/src/main/bootloader_start.c
+	make bootloader || failure "Failed to partial build bootloader"
+	assert_rebuilt ${BOOTLOADER_BINS} bootloader/main/bootloader_start.o
+	assert_not_rebuilt ${APP_BINS} partitions_singleapp.bin
+
+	print_status "Partition CSV file rebuilds partitions"
+	take_build_snapshot
+	touch ${IDF_PATH}/components/partition_table/partitions_singleapp.csv
+	make partition_table
+	assert_rebuilt partitions_singleapp.bin
+	assert_not_rebuilt app-template.bin app-template.elf ${BOOTLOADER_BINS}
+
+	print_status "Partial build doesn't compile anything by default"
+	take_build_snapshot
+	# verify no build files are refreshed by a partial make
+	ALL_BUILD_FILES=$(find ${BUILD} -type f | sed "s@${BUILD}/@@")
+	make
+	assert_not_rebuilt ${ALL_BUILD_FILES}
+
+	print_status "Cleaning should remove all files from build"
+	make clean
+	ALL_BUILD_FILES=$(find ${BUILD} -type f)
+	if [ -n "${ALL_BUILD_FILES}" ]; then
+		 failure "Files weren't cleaned: ${ALL_BUILD_FILES}"
+	fi
+
+	print_status "All tests completed"
+	if [ -n "${FAILURES}" ]; then
+		echo "Some failures were detected:"
+		echo -e "${FAILURES}"
+		exit 1
+	else
+		echo "Build tests passed."
+	fi
+}
+
+function print_status()
+{
+	echo "******** $1"
+	STATUS="$1"
+}
+
+function failure()
+{
+	echo "!!!!!!!!!!!!!!!!!!!"
+	echo "FAILURE: $1"
+	echo "!!!!!!!!!!!!!!!!!!!"
+	FAILURES="${FAILURES}${STATUS} :: $1\n"
+}
+
+TESTDIR=${TMP}/build_system_tests_$$
+mkdir -p ${TESTDIR}
+# set NOCLEANUP=1 if you want to keep the test directory around
+# for post-mortem debugging
+[ -z ${NOCLEANUP} ] && trap "rm -rf ${TESTDIR}" EXIT KILL
+
+SNAPSHOT=${TESTDIR}/snapshot
+BUILD=${TESTDIR}/template/build
+
+
+# copy all the build output to a snapshot directory
+function take_build_snapshot()
+{
+	rm -rf ${SNAPSHOT}
+	cp -ap ${TESTDIR}/template/build ${SNAPSHOT}
+}
+
+# verify that all the arguments are present in the build output directory
+function assert_built()
+{
+	until [ -z "$1" ]; do
+		if [ ! -f "${BUILD}/$1" ]; then
+			failure "File $1 should be in the build output directory"
+		fi
+		shift
+	done
+}
+
+# verify all the arguments passed in were rebuilt relative to the snapshot
+function assert_rebuilt()
+{
+	until [ -z "$1" ]; do
+		assert_built "$1"
+		if [ ! -f "${SNAPSHOT}/$1" ]; then
+			failure "File $1 should have been original build snapshot"
+		fi
+		if [ ! "${SNAPSHOT}/$1" -ot "${BUILD}/$1" ]; then
+			failure "File $1 should have been rebuilt"
+		fi
+		shift
+	done
+}
+
+# verify all the arguments are in the build directory & snapshot,
+# but were not rebuilt
+function assert_not_rebuilt()
+{
+	until [ -z "$1" ]; do
+		assert_built "$1"
+		if [ ! -f "${SNAPSHOT}/$1" ]; then
+			failure "File $1 should be in snapshot build directory"
+		fi
+		if [ "${SNAPSHOT}/$1" -ot "${BUILD}/$1" ]; then
+			failure "File $1 should not have been rebuilt"
+		fi
+		shift
+	done
+}
+
+cd ${TESTDIR}
+run_tests
--- a/make/common.mk
+++ b/make/common.mk
@ -35,13 +35,18 @@ CXXFLAGS = -DESP_PLATFORM -Og -std=gnu++11 -g3 \
 endif

 #Handling of V=1/VERBOSE=1 flag
+#
+# if V=1, $(summary) does nothing and $(details) will echo extra details
+# if V is unset or not 1, $(summary) echoes a summary and $(details) does nothing
 V ?= $(VERBOSE)
 ifeq ("$(V)","1")
 Q :=
-vecho := @true
+summary := @true
+details := @echo
 else
 Q := @
-vecho := @echo
+summary := @echo
+details := @true
 endif

 # General make utilities
--- a/make/component.mk
+++ b/make/component.mk
@ -71,14 +71,14 @@ build: $(COMPONENT_LIBRARY)
 #Build the archive. We remove the archive first, otherwise ar will get confused if we update
 #an archive when multiple filenames have the same name (src1/test.o and src2/test.o)
 $(COMPONENT_LIBRARY): $(COMPONENT_OBJS)
-	$(vecho) AR $@
+	$(summary) AR $@
 	$(Q) rm -f $@
 	$(Q) $(AR) cru $@ $(COMPONENT_OBJS)
 endif

 ifeq ("$(COMPONENT_OWNCLEANTARGET)", "")
 clean:
-	$(vecho) RM $(COMPONENT_LIBRARY) $(COMPONENT_OBJS) $(COMPONENT_OBJS:.o=.d) $(COMPONENT_EXTRA_CLEAN)
+	$(summary) RM $(COMPONENT_LIBRARY) $(COMPONENT_OBJS) $(COMPONENT_OBJS:.o=.d) $(COMPONENT_EXTRA_CLEAN)
 	$(Q) rm -f $(COMPONENT_LIBRARY) $(COMPONENT_OBJS) $(COMPONENT_OBJS:.o=.d) $(COMPONENT_EXTRA_CLEAN)
 endif

@ -92,15 +92,15 @@ CXXFLAGS+=-MMD
 define GenerateCompileTargets
 # $(1) - directory containing source files, relative to $(COMPONENT_PATH)
 $(1)/%.o: $$(COMPONENT_PATH)/$(1)/%.c | $(1)
-	$$(vecho) CC $$@
+	$$(summary) CC $$@
 	$$(Q) $$(CC) $$(CFLAGS) $$(addprefix -I ,$$(COMPONENT_INCLUDES)) $$(addprefix -I ,$$(COMPONENT_EXTRA_INCLUDES)) -I$(1) -c $$< -o $$@

 $(1)/%.o: $$(COMPONENT_PATH)/$(1)/%.cpp | $(1)
-	$$(vecho) CC $$@
+	$$(summary) CC $$@
 	$$(Q) $$(CXX) $$(CXXFLAGS) $$(addprefix -I,$$(COMPONENT_INCLUDES)) $$(addprefix -I,$$(COMPONENT_EXTRA_INCLUDES)) -I$(1) -c $$< -o $$@

 $(1)/%.o: $$(COMPONENT_PATH)/$(1)/%.S | $(1)
-	$$(vecho) CC $$@
+	$$(summary) CC $$@
 	$$(Q) $$(CC) $$(CFLAGS) $$(addprefix -I ,$$(COMPONENT_INCLUDES)) $$(addprefix -I ,$$(COMPONENT_EXTRA_INCLUDES)) -I$(1) -c $$< -o $$@

 # CWD is build dir, create the build subdirectory if it doesn't exist
--- a/make/project.mk
+++ b/make/project.mk
@ -166,9 +166,15 @@ $(foreach componentpath,$(COMPONENT_PATHS),$(eval $(call includeProjBuildMakefil
 # once we know component paths, we can include the config
 include $(IDF_PATH)/make/project_config.mk

-# ELF depends on the -build target of every component
-$(APP_ELF): $(addsuffix -build,$(notdir $(COMPONENT_PATHS_BUILDABLE)))
-	$(vecho) LD $(notdir $@)
+# A "component" library is any library in the LDFLAGS where
+# the name of the library is also a name of the component
+APP_LIBRARIES = $(patsubst -l%,%,$(filter -l%,$(LDFLAGS)))
+COMPONENT_LIBRARIES = $(filter $(notdir $(COMPONENT_PATHS_BUILDABLE)),$(APP_LIBRARIES))
+
+# 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)
+	$(summary) LD $(notdir $@)
 	$(Q) $(CC) $(LDFLAGS) -o $@ -Wl,-Map=$(APP_MAP)

 # Generation of $(APP_BIN) from $(APP_ELF) is added by the esptool
@ -182,28 +188,34 @@ all_binaries: $(APP_BIN)
 $(BUILD_DIR_BASE):
 	mkdir -p $(BUILD_DIR_BASE)

-define GenerateComponentTarget
+define GenerateComponentPhonyTarget
 # $(1) - path to component dir
 # $(2) - target to generate (build, clean)
-# $(3) - optional dependencies to add
 .PHONY: $(notdir $(1))-$(2)
-$(notdir $(1))-$(2): $(3) | $(BUILD_DIR_BASE)/$(notdir $(1))
+$(notdir $(1))-$(2): | $(BUILD_DIR_BASE)/$(notdir $(1))
 	@+$(MAKE) -C $(BUILD_DIR_BASE)/$(notdir $(1)) -f $(1)/Makefile COMPONENT_BUILD_DIR=$(BUILD_DIR_BASE)/$(notdir $(1)) $(2)
 endef

-define GenerateComponentBuildDirTarget
+define GenerateComponentTargets
 # $(1) - path to component dir
 $(BUILD_DIR_BASE)/$(notdir $(1)):
 	@mkdir -p $(BUILD_DIR_BASE)/$(notdir $(1))
+
+# tell make it can build any component's library by invoking the recursive -build target
+# (this target exists for all components even ones which don't build libraries, but it's
+# only invoked for the targets whose libraries appear in COMPONENT_LIBRARIES and hence the
+# APP_ELF dependencies.)
+$(BUILD_DIR_BASE)/$(notdir $(1))/lib$(notdir $(1)).a: $(notdir $(1))-build
+	$(details) echo "$$^ responsible for $$@" # echo which build target built this file
 endef

-$(foreach component,$(COMPONENT_PATHS_BUILDABLE),$(eval $(call GenerateComponentBuildDirTarget,$(component))))
+$(foreach component,$(COMPONENT_PATHS_BUILDABLE),$(eval $(call GenerateComponentTargets,$(component))))

-$(foreach component,$(COMPONENT_PATHS_BUILDABLE),$(eval $(call GenerateComponentTarget,$(component),build,$(PROJECT_PATH)/build/include/sdkconfig.h)))
-$(foreach component,$(COMPONENT_PATHS_BUILDABLE),$(eval $(call GenerateComponentTarget,$(component),clean)))
+$(foreach component,$(COMPONENT_PATHS_BUILDABLE),$(eval $(call GenerateComponentPhonyTarget,$(component),build)))
+$(foreach component,$(COMPONENT_PATHS_BUILDABLE),$(eval $(call GenerateComponentPhonyTarget,$(component),clean)))

 app-clean: $(addsuffix -clean,$(notdir $(COMPONENT_PATHS_BUILDABLE)))
-	$(vecho) RM $(APP_ELF)
+	$(summary) RM $(APP_ELF)
 	$(Q) rm -f $(APP_ELF) $(APP_BIN) $(APP_MAP)

 clean: app-clean
--- a/make/project_config.mk
+++ b/make/project_config.mk
@ -16,7 +16,7 @@ $(KCONFIG_TOOL_DIR)/mconf $(KCONFIG_TOOL_DIR)/conf:
 	$(MAKE) -C $(KCONFIG_TOOL_DIR)

 menuconfig: $(KCONFIG_TOOL_DIR)/mconf $(IDF_PATH)/Kconfig $(BUILD_DIR_BASE)
-	$(vecho) MENUCONFIG
+	$(summary) MENUCONFIG
 	$(Q) KCONFIG_AUTOHEADER=$(PROJECT_PATH)/build/include/sdkconfig.h \
 	KCONFIG_CONFIG=$(PROJECT_PATH)/sdkconfig \
 	COMPONENT_KCONFIGS="$(COMPONENT_KCONFIGS)" \
@ -29,7 +29,7 @@ $(PROJECT_PATH)/sdkconfig: menuconfig
 endif

 defconfig: $(KCONFIG_TOOL_DIR)/mconf $(IDF_PATH)/Kconfig $(BUILD_DIR_BASE)
-	$(vecho) DEFCONFIG
+	$(summary) DEFCONFIG
 	$(Q) mkdir -p $(PROJECT_PATH)/build/include/config
 	$(Q) KCONFIG_AUTOHEADER=$(PROJECT_PATH)/build/include/sdkconfig.h \
 	KCONFIG_CONFIG=$(PROJECT_PATH)/sdkconfig \
@ -53,7 +53,7 @@ endif
 endif

 $(AUTO_CONF_REGEN_TARGET) $(PROJECT_PATH)/build/include/sdkconfig.h: $(PROJECT_PATH)/sdkconfig $(KCONFIG_TOOL_DIR)/conf $(COMPONENT_KCONFIGS) $(COMPONENT_KCONFIGS_PROJBUILD)
-	$(vecho) GENCONFIG
+	$(summary) GENCONFIG
 	$(Q) mkdir -p $(PROJECT_PATH)/build/include/config
 	$(Q) cd build; KCONFIG_AUTOHEADER="$(PROJECT_PATH)/build/include/sdkconfig.h" \
 	KCONFIG_CONFIG=$(PROJECT_PATH)/sdkconfig \
@ -68,6 +68,6 @@ $(AUTO_CONF_REGEN_TARGET) $(PROJECT_PATH)/build/include/sdkconfig.h: $(PROJECT_P
 clean: config-clean
 .PHONY: config-clean
 config-clean:
-	$(vecho RM CONFIG)
+	$(summary RM CONFIG)
 	$(MAKE) -C $(KCONFIG_TOOL_DIR) clean
 	$(Q) rm -rf $(PROJECT_PATH)/build/include/config $(PROJECT_PATH)/build/include/sdkconfig.h