Merge pull request #1147 from keyboardio/f/more-test-perf

A variety of performance and readability improvements to the setup, build and test processes

.github/workflows/build.yml

@@ -4,7 +4,6 @@ on: [push, pull_request]
 
 env:
   LC_ALL: C
-  KALEIDOSCOPE_CCACHE: 1
   ARDUINO_DIRECTORIES_USER: ${{ github.workspace }}/.arduino/user
   CLANG_FORMAT_CMD: clang-format-12
 jobs:
@@ -12,20 +11,27 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v2
-
+    - name: Cache arduino dep downloads
-## We delete the Bundle's version of Kaleidoscope, and symlink ourselves in.
+      uses: actions/cache@v2
-## This makes sure we're using the current version of the library.
+      with:
-    - run: sudo apt-get install ccache
+        path: ${{ github.workspace}}/.arduino/downloads
+        key: ${{ runner.os }}-arduino-downloads
     - run: make setup
-    - run: make -j $(nproc) smoke-sketches
+    - run: KALEIDOSCOPE_TEMP_PATH=${{ github.workspace}}/.kaleidoscope-temp make -j $(nproc) smoke-sketches
   run-google-tests:
     runs-on: ubuntu-20.04
     steps:
     - uses: actions/checkout@v2
-#    - uses: mxschmitt/action-tmate@v3
+    - name: Cache arduino dep downloads
-    - run: sudo apt-get install ccache
+      uses: actions/cache@v2
+      with:
+        path: ${{ github.workspace}}/.arduino/downloads
+        key: ${{ runner.os }}-arduino-downloads
+    - run: sudo apt update 
+    - name: ccache
+      uses: hendrikmuhs/ccache-action@v1.2
     - run: make setup
-    - run: make -j $(nproc) simulator-tests
+    - run: KALEIDOSCOPE_CCACHE=1 make -j $(nproc) --output-sync=recurse simulator-tests
   check-formatting:
     runs-on: ubuntu-latest
     steps:

Makefile

@@ -18,6 +18,13 @@ MAKEFLAGS += --no-builtin-rules
 
 .SUFFIXES:
 
+# GNU Make earlier than 4.0 don't have the output-sync option, but we need it
+# to make parallel simulator test output readable. (otherwise it gets interleaved)
+ifeq ($(shell test $(firstword $(subst ., ,$(MAKE_VERSION))) -ge 4; echo $$?),0)
+MAKEFLAGS+=--output-sync=target
+else
+_using_old_make=1
+endif
 
 include $(dir $(abspath $(lastword $(MAKEFILE_LIST))))/etc/makefiles/arduino-cli.mk
 
@@ -50,10 +57,15 @@ $(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio/virtual/boards.txt:
 $(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio/avr/boards.txt:
 	git clone -c core.symlinks=true \
 		--recurse-submodules \
+		--shallow-submodules \
+		--recurse-submodules=':(exclude)avr/bootloaders' \
+		--recurse-submodules=':(exclude)avr/libraries/Kaleidoscope' \
 		https://github.com/keyboardio/Kaleidoscope-Bundle-Keyboardio \
 		$(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio
 	git clone -c core.symlinks=true \
 		--recurse-submodules \
+		--shallow-submodules \
+		--recurse-submodules=':(exclude)libraries/Kaleidoscope' \
 		https://github.com/keyboardio/ArduinoCore-GD32-Keyboardio $(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio/gd32
 
 update:
@@ -63,6 +75,9 @@ update:
 		git submodule update --init --recursive
 
 simulator-tests:
+ifneq ($(_using_old_make),)
+	$(info You're using an older version of GNU Make that doesn't offer the --output-sync option. If you're running the test suite in parallel, output may be garbled. You might consider using GNU Make 4.0 or later instead)
+endif
 	$(MAKE) -C tests all
 
 docker-simulator-tests:

docs/UPGRADING.md

@@ -437,6 +437,16 @@ As a developer, one can continue using `millis()`, but migrating to `Kaleidoscop
 
 ## Breaking changes
 
+### Sketch preprocssing system
+
+We used to support the ability to amend all compiled sketches by
+adding code to
+`src/kaleidoscope_internal/sketch_preprocessing/sketch_header.h`
+and `src/kaleidoscope_internal/sketch_preprocessing/sketch_footer.h`.
+The functionality was never used by Kaleidoscope itself and frequently
+pulled the (empty) header files from the wrong copy of Kaleidoscope.
+If you need this functionality, please open a GitHub issue.
+
 ### Macros
 
 This is a guide to upgrading existing Macros code to use the new version of

etc/makefiles/sketch.mk

@@ -165,7 +165,7 @@ compile: kaleidoscope-hardware-configured
 
 	$(QUIET) install -d "${OUTPUT_PATH}"
 	$(QUIET) $(ARDUINO_CLI) compile --fqbn "${FQBN}" ${ARDUINO_VERBOSE} --warnings all ${ccache_wrapper_property} ${local_cflags_property} \
-	  ${_arduino_local_libraries_prop} \
+	  ${_arduino_local_libraries_prop} ${_ARDUINO_CLI_COMPILE_CUSTOM_FLAGS} \
 	  --library "${KALEIDOSCOPE_DIR}" \
 	  --libraries "${KALEIDOSCOPE_DIR}/plugins/" \
 	  --build-path "${BUILD_PATH}" \

src/kaleidoscope_internal/sketch_preprocessing/sketch_footer.h

@@ -1,2 +0,0 @@
-// Any code that appears here is added to the bottom of the preprocessed sketch file.
-// NOLINT(build/header_guard)

src/kaleidoscope_internal/sketch_preprocessing/sketch_header.h

@@ -1,3 +0,0 @@
-// Any code that appears here is added to the top of the preprocessed sketch file.
-// NOLINT(build/header_guard)
-#define KALEIDOSCOPE_SKETCH

testing/makefiles/testcase.mk

@@ -64,20 +64,24 @@ endif
 
 TEST_OBJS=$(patsubst $(SRC_DIR)/%.cpp,${OBJ_DIR}/%.o,$(TEST_FILES))
 
-build: ${BIN_DIR}/${BIN_FILE} compile-sketch
+build: $(if $(HAS_KTEST_FILE), generate-testcase) compile-sketch
 
 all: run
 
 run: ${BIN_DIR}/${BIN_FILE}
+	$(info )
+	$(info Running test $(testcase))
 	$(QUIET) "${BIN_DIR}/${BIN_FILE}" -t -q
 
-${BIN_DIR}/${BIN_FILE}: ${TEST_OBJS} 
+${BIN_DIR}/${BIN_FILE}: compile-sketch
 
 # We force sketch recompiliation because otherwise, make won't pick up changes to...anything on the arduino side
+.PHONY: compile-sketch
 compile-sketch: ${TEST_OBJS}
 	@install -d "${BIN_DIR}" "${LIB_DIR}"
 	$(QUIET) env LIBONLY=yes VERBOSE=${VERBOSE}  \
 		OUTPUT_PATH="${LIB_DIR}" \
+		_ARDUINO_CLI_COMPILE_CUSTOM_FLAGS='--build-property upload.maximum_size=""' \
 		$(MAKE) -f ${top_dir}/etc/makefiles/sketch.mk compile
 	$(QUIET) $(COMPILER_WRAPPER) $(call _arduino_prop,compiler.cpp.cmd) -o "${BIN_DIR}/${BIN_FILE}" \
 		-lpthread -g -w ${TEST_OBJS} \
@@ -88,13 +92,14 @@ compile-sketch: ${TEST_OBJS}
 
 
 # If we have a test.ktest file, it should be processed into a c++ testcase
-
+.PHONY: generate-testcase
 generate-testcase: $(if $(HAS_KTEST_FILE), ${SRC_DIR}/generated-testcase.cpp)
 
-
 ${SRC_DIR}/generated-testcase.cpp: test.ktest
 ifneq (,$(wildcard test.ktest))
-	$(info Compiling ${testcase} ktest script into ${SRC_DIR}/generated-testcase.cpp)
+ifdef VERBOSE
+	$(QUIET) $(info Compiling ${testcase} ktest script into ${SRC_DIR}/generated-testcase.cpp)
+endif
 	$(QUIET) install -d "${SRC_DIR}"
 	$(QUIET) perl ${top_dir}/testing/bin/ktest-to-cxx \
 		--ktest=test.ktest \

tests/Makefile

@@ -34,7 +34,9 @@ export FQBN	?= keyboardio:virtual:model01
 
 TESTS		:= $(shell cd $(tests_dir); find ${TEST_PATH} -name '*.ino' -exec dirname {} \;)
 
-KALEIDOSCOPE_CCACHE = 1
+# The clutter up the test output on Make 4.0 and newer
+MAKEFLAGS += --no-print-directory
+
 
 include $(top_dir)/etc/makefiles/arduino-cli.mk
 
@@ -44,48 +46,53 @@ include $(top_dir)/etc/makefiles/arduino-cli.mk
 KALEIDOSCOPE_ETC_DIR ?= $(top_dir)/etc
 
 
-.PHONY: clean cmake-clean all googletest generate-testcases
+.PHONY: clean cmake-clean all googletest
 
 
-generate-testcases:
+all: ${libcommon_a} googletest ${TESTS} 
-	@for test in ${TESTS}; do \
+	@:
-		${MAKE} -s -f ${top_dir}/testing/makefiles/testcase.mk -C $${test} testcase=$${test} generate-testcase; \
-	done
 
-all: ${TESTS} run-all
+cmake-clean:
+	rm -rf "${top_dir}"/testing/googletest/build/*
 
 run-all: ${TESTS}
 	@for test in ${TESTS}; do \
-		${MAKE} -s -f ${top_dir}/testing/makefiles/testcase.mk -C $${test} testcase=$${test} run || ERROR=$$?; \
+		${MAKE} -s -f ${top_dir}/testing/makefiles/testcase.mk \
+			-C $${test} \
+			testcase=$${test} run || ERROR=$$?; \
 	done; \
 	if [ -n $${ERROR} ]; then exit $${ERROR}; fi
 
 
 
-cmake-clean:
-	rm -rf "${top_dir}"/testing/googletest/build/*
-
 clean: cmake-clean
 	@for test in ${TESTS}; do \
-		${MAKE} -s -f ${top_dir}/testing/makefiles/testcase.mk -C $${test} testcase=$${test} clean; \
+		${MAKE} -s -f ${top_dir}/testing/makefiles/testcase.mk \
+			-C $${test} \
+			testcase=$${test} clean; \
 	done
 	rm -rf "${build_dir}"/*
 
 googletest: ${top_dir}/testing/googletest/build/Makefile
-	cd ${top_dir}/testing/googletest/build && $(MAKE)
+	${MAKE} -C ${top_dir}/testing/googletest/build
 
 ${top_dir}/testing/googletest/build/Makefile:
 	$(info googletest Makefile is being remade)
-	install -d ${top_dir}/testing/googletest/build && cd ${top_dir}/testing/googletest/build && cmake  -DCMAKE_C_COMPILER=$(call _arduino_prop,compiler.path)$(call _arduino_prop,compiler.c.cmd) -DCMAKE_CXX_COMPILER=$(call _arduino_prop,compiler.path)$(call _arduino_prop,compiler.cpp.cmd) ..
+	# This can fail if we're running in parallel, but that'd be harmless
+	-install -d ${top_dir}/testing/googletest/build
+	$(QUIET) cmake \
+	  -S ${top_dir}/testing/googletest \
+	  -B ${top_dir}/testing/googletest/build \
+	  -DCMAKE_C_COMPILER=$(call _arduino_prop,compiler.path)$(call _arduino_prop,compiler.c.cmd) \
+	  -DCMAKE_CXX_COMPILER=$(call _arduino_prop,compiler.path)$(call _arduino_prop,compiler.cpp.cmd) \
+	  -DCMAKE_C_COMPILER_LAUNCHER=ccache \
+	  -DCMAKE_CXX_COMPILER_LAUNCHER=ccache
 
 ${libcommon_a}:
-	${MAKE} -f ${top_dir}/testing/makefiles/libcommon.mk -C ${top_dir}/testing
+	$(QUIET) ${MAKE} -f ${top_dir}/testing/makefiles/libcommon.mk -C ${top_dir}/testing
 
 Makefile:
 	@:
 
 ${TESTS}: ${libcommon_a} googletest
-	${MAKE} -f ${top_dir}/testing/makefiles/testcase.mk -C $@ testcase=$@ build
+	$(QUIET) ${MAKE} -f ${top_dir}/testing/makefiles/testcase.mk -C $@ testcase=$@ build run
-
-%+run:
-	${MAKE} -s -f ${top_dir}/testing/makefiles/testcase.mk -C $* testcase=$* run