From 528638f5ebea8c8d61a940d3a5a23ce62de8dc4a Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 24 Aug 2016 12:16:26 +0800 Subject: [PATCH 1/6] Add build system test (via bash script) Currently fails due to bootloader not automatically rebuilding anything, and ELF/BIN files generating when nothing changed. --- .gitlab-ci.yml | 6 ++ make/build_system_tests.sh | 163 +++++++++++++++++++++++++++++++++++++ 2 files changed, 169 insertions(+) create mode 100755 make/build_system_tests.sh diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 329f44f8d9..833640ddeb 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -25,6 +25,12 @@ test_nvs_on_host: - cd components/nvs_flash/test - make test +test_build_system: + stage: test + image: espressif/esp32-ci-env + script: + - ./make/build_system_tests.sh + push_master_to_github: stage: deploy only: diff --git a/make/build_system_tests.sh b/make/build_system_tests.sh new file mode 100755 index 0000000000..6006f05a90 --- /dev/null +++ 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 From 1fd22c574872b000515c88f3120272fd487221ac Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 24 Aug 2016 12:26:13 +0800 Subject: [PATCH 2/6] make bootloader: Always recurse into bootloader directory to check source dependencies --- components/bootloader/Makefile.projbuild | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/bootloader/Makefile.projbuild b/components/bootloader/Makefile.projbuild index c80878d423..2bf8ac433f 100644 --- a/components/bootloader/Makefile.projbuild +++ b/components/bootloader/Makefile.projbuild @@ -13,9 +13,9 @@ 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 +$(BOOTLOADER_BIN): $(Q) PROJECT_PATH= \ LDFLAGS= \ CFLAGS= \ From 938d13c5a3e0447801c0891539603b0fa674ecb6 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 24 Aug 2016 13:02:24 +0800 Subject: [PATCH 3/6] Makefile: Rename $(vecho) to $(summary), add new $(details) for echoing details when V=1 --- README.buildenv | 2 +- make/common.mk | 9 +++++++-- make/component.mk | 10 +++++----- make/project.mk | 2 +- make/project_config.mk | 8 ++++---- 5 files changed, 18 insertions(+), 13 deletions(-) diff --git a/README.buildenv b/README.buildenv index c7ea22dfac..fd68300055 100644 --- 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 diff --git a/make/common.mk b/make/common.mk index 064408dcfc..7f372cc305 100644 --- 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 diff --git a/make/component.mk b/make/component.mk index 9c0e5d602a..ae897b0099 100644 --- 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 diff --git a/make/project.mk b/make/project.mk index 99fa1c1a15..7b3a867339 100644 --- a/make/project.mk +++ b/make/project.mk @@ -203,7 +203,7 @@ $(foreach component,$(COMPONENT_PATHS_BUILDABLE),$(eval $(call GenerateComponent $(foreach component,$(COMPONENT_PATHS_BUILDABLE),$(eval $(call GenerateComponentTarget,$(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 diff --git a/make/project_config.mk b/make/project_config.mk index c813ac5849..e39fdac3bb 100644 --- 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 From 116e7301329942d02be6539b0babdf91116f27fe Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 24 Aug 2016 13:02:51 +0800 Subject: [PATCH 4/6] Makefile: Don't re-build libraries or re-link ELF files if nothing has changed in the component make/build_system_tests.sh should now pass. --- make/project.mk | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/make/project.mk b/make/project.mk index 7b3a867339..ac2b3e59d3 100644 --- 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,25 +188,31 @@ 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))) $(summary) RM $(APP_ELF) From a278c51d3eb90973a184f51968d36249e0eadc89 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 24 Aug 2016 13:23:00 +0800 Subject: [PATCH 5/6] Make: Building the bootloader depends on syncing its sdkconfig from the top-level project --- components/bootloader/Makefile.projbuild | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/bootloader/Makefile.projbuild b/components/bootloader/Makefile.projbuild index 2bf8ac433f..48c09d4816 100644 --- a/components/bootloader/Makefile.projbuild +++ b/components/bootloader/Makefile.projbuild @@ -15,7 +15,7 @@ BOOTLOADER_BIN=$(BOOTLOADER_BUILD_DIR)/bootloader.bin .PHONY: bootloader-clean bootloader-flash bootloader $(BOOTLOADER_BIN) -$(BOOTLOADER_BIN): +$(BOOTLOADER_BIN): $(COMPONENT_PATH)/src/sdkconfig $(Q) PROJECT_PATH= \ LDFLAGS= \ CFLAGS= \ From a233619429ec609884df93d69c83bcdb77613283 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 24 Aug 2016 13:31:33 +0800 Subject: [PATCH 6/6] CI: Pass IDF_PATH to build_system_tests.sh --- .gitlab-ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 833640ddeb..0b5b3ac651 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -28,6 +28,8 @@ test_nvs_on_host: test_build_system: stage: test image: espressif/esp32-ci-env + variables: + IDF_PATH: "$CI_PROJECT_DIR" script: - ./make/build_system_tests.sh