Merge pull request #1144 from keyboardio/f/no-make-implicit-rules

A variety of build system perf improvements

.github/workflows/build.yml

@@ -30,25 +30,21 @@ jobs:
     steps:
     - run: sudo apt-get install astyle
     - uses: actions/checkout@v2
-    - run: make setup
     - run: make check-astyle
   check-shellcheck:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v2
-    - run: make setup
     - run: make shellcheck
   check-cpplint:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v2
-    - run: make setup
     - run: make cpplint
   find-filename-conflicts:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v2
-    - run: make setup
     - run: make find-filename-conflicts 
   publish-arduino-package-on-every-commit:
     runs-on: ubuntu-latest

Makefile

@@ -1,3 +1,24 @@
+# Reset a bunch of historical GNU make implicit rules that we never
+# use, but which have a disastrous impact on performance
+#
+# --no-builtin-rules in MAKEFLAGS apparently came in with GNU Make 4,
+# which is newer than what Apple ships
+MAKEFLAGS += --no-builtin-rules
+
+# These lines reset the implicit rules we really care about
+%:: %,v
+
+%:: RCS/%,v
+
+%:: RCS/%
+
+%:: s.%
+
+%:: SCCS/s.%
+
+.SUFFIXES:
+
+
 include $(dir $(abspath $(lastword $(MAKEFILE_LIST))))/etc/makefiles/arduino-cli.mk
 
 # Set up an argument for passing to the simulator tests in docker
@@ -65,9 +86,7 @@ adjust-git-timestamps:
 	bin/set-timestamps-from-git
 
 find-filename-conflicts:
-	@if [ -d "bin" ]; then \
-		bin/find-filename-conflicts; \
-	fi
+	bin/find-filename-conflicts
 
 .PHONY: astyle test cpplint cpplint-noisy shellcheck smoke-examples find-filename-conflicts prepare-virtual checkout-platform adjust-git-timestamps docker-bash docker-simulator-tests run-tests simulator-tests setup
 
@@ -89,7 +108,7 @@ cpplint:
 	bin/cpplint.py  --quiet --filter=-whitespace,-legal/copyright,-build/include,-readability/namespace,-runtime/references  --recursive --extensions=cpp,h,ino src examples
 
 
-SHELL_FILES = $(shell if [ -d bin ]; then egrep -n -r -l "(env (ba)?sh)|(/bin/(ba)?sh)" bin; fi)
+SHELL_FILES := $(shell if [ -d bin ]; then egrep -n -r -l "(env (ba)?sh)|(/bin/(ba)?sh)" bin; fi)
 
 shellcheck:
 	@if [ -d "bin" ]; then \
@@ -97,7 +116,7 @@ shellcheck:
 	fi
 
 
-SMOKE_SKETCHES=$(sort $(shell if [ -d ./examples ]; then find ./examples -type f -name \*ino | xargs -n 1 dirname; fi))
+SMOKE_SKETCHES := $(sort $(shell if [ -d ./examples ]; then find ./examples -type f -name \*ino | xargs -n 1 dirname; fi))
 
 smoke-sketches: $(SMOKE_SKETCHES)
 	@echo "Smoke-tested all the sketches"

etc/makefiles/arduino-cli.mk

@@ -55,33 +55,14 @@ endif
 # Otherwise, use the arduino-cli bundle
 export ARDUINO_DIRECTORIES_USER ?= $(ARDUINO_CONTENT)/user
 
-# If we're not calling setup, we should freak out if the hardware
-# definitions don't exist
-
-ifneq ($(MAKECMDGOALS),setup)
 
-ifeq ($(wildcard $(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio/avr/boards.txt),)
- 
-$(info Kaleidoscope hardware definitions not found in)
-$(info $(ARDUINO_DIRECTORIES_USER))
-$(info )
-$(info You may be able to resolve this issue by running the following command)
-$(info to initialize Kaleidoscope )
-$(info )
-$(info $(MAKE) -C $(KALEIDOSCOPE_DIR) setup )
-$(info )
-$(error )
-
-endif
-
-endif
 
 arduino_env = ARDUINO_DIRECTORIES_USER=$(ARDUINO_DIRECTORIES_USER) \
 	      ARDUINO_DIRECTORIES_DATA=$(ARDUINO_DIRECTORIES_DATA)
 
 ifeq ($(ARDUINO_CLI_PATH),) 
 
-system_arduino_cli ?= $(shell command -v arduino-cli || true)
+system_arduino_cli := $(shell command -v arduino-cli || true)
 
 ifeq ($(system_arduino_cli),) 
 export ARDUINO_CLI_PATH ?= $(KALEIDOSCOPE_BIN_DIR)/arduino-cli
@@ -123,22 +104,11 @@ _arduino_props := $(shell ${ARDUINO_CLI}  compile $(fqbn_arg) --show-properties
 
 _arduino_prop = $(subst $1=,,$(subst 🔥, ,$(filter $1=%,$(_arduino_props))))
 
-_arduino_version = $(shell ${ARDUINO_CLI} version | sed 's/.*Version: \([0-9][0-9\.]*\).*/\1/')
-
-export ARDUINO_CLI_VERSION ?= $(_arduino_version)
-
-_arduino_build_property_flag = $(shell echo -e "0.14\n${ARDUINO_CLI_VERSION}" | sort -C -t. -k1,1n -k2,2n && echo "YES")
-
-ARDUINO_BUILD_PROP_FLAG := --build-properties
-ifeq ($(_arduino_build_property_flag),YES)
-ARDUINO_BUILD_PROP_FLAG := --build-property
-endif
-
 # How to use_arduino_prop
 # $(call _arduino_prop,recipe.hooks.sketch.prebuild.2.pattern)
 
 ifneq ($(KALEIDOSCOPE_CCACHE),) 
-ccache_wrapper_property := $(ARDUINO_BUILD_PROP_FLAG) "compiler.wrapper.cmd=ccache"
+ccache_wrapper_property := --build-property compiler.wrapper.cmd=ccache
 endif
 
 .PHONY: configure-arduino-cli install-arduino-core-kaleidoscope install-arduino-core-avr
@@ -176,3 +146,20 @@ install-arduino-core-deps:
 	$(QUIET) $(ARDUINO_CLI) core install "keyboardio:gd32-tools-only"
 
 
+# If we're not calling setup, we should freak out if the hardware
+# definitions don't exist
+
+.PHONY: kaleidoscope-hardware-configured
+
+kaleidoscope-hardware-configured: $(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio/avr/platform.txt
+
+$(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio/avr/platform.txt:
+	$(info Kaleidoscope hardware definitions not found in)
+	$(info $(ARDUINO_DIRECTORIES_USER))
+	$(info )
+	$(info You may be able to resolve this issue by running the following command)
+	$(info to initialize Kaleidoscope )
+	$(info )
+	$(info $(MAKE) -C $(KALEIDOSCOPE_DIR) setup )
+	$(info )
+	$(error )

etc/makefiles/sketch.mk

@@ -48,7 +48,7 @@ ELF_FILE_PATH 			:= $(OUTPUT_PATH)/$(OUTPUT_FILE_PREFIX).elf
 LIB_FILE_PATH 			:= $(OUTPUT_PATH)/$(OUTPUT_FILE_PREFIX).a
 
 ifeq ($(FQBN),)
-possible_fqbns = $(shell $(ARDUINO_CLI) board list --format=json |grep FQBN| grep -v "keyboardio:virtual"|cut -d: -f 2-)
+possible_fqbns := $(shell $(ARDUINO_CLI) board list --format=json |grep FQBN| grep -v "keyboardio:virtual"|cut -d: -f 2-)
 
 possible_fqbn = $(firstword $(possible_fqbns))
 
@@ -99,20 +99,6 @@ endif
 
 
 
-# Flashing related config
-ifneq ($(FQBN),)
-ifeq ($(KALEIDOSCOPE_DEVICE_PORT),)
-KALEIDOSCOPE_DEVICE_PORT = $(shell $(ARDUINO_CLI) board list --format=text | grep $(FQBN) |cut -d' ' -f 1)
-endif
-endif
-
-flashing_instructions	:= $(call _arduino_prop,build.flashing_instructions)
-ifeq ($(flashing_instructions),)
-flashing_instructions	:= "If your keyboard needs you to do something to put it in flashing mode, do that now."
-endif
-
-unescaped_flashing_instructions = $(shell printf $(flashing_instructions) )
-
 DEFAULT_GOAL: compile
 
 
@@ -137,7 +123,6 @@ size-map: ${ELF_FILE_PATH}
 		$(call _arduino_prop,compiler.size-map.flags) \
 		"${ELF_FILE_PATH}"
 
-flash: ${HEX_FILE_PATH}
 
 ${ELF_FILE_PATH}: compile
 ${HEX_FILE_PATH}: compile
@@ -162,7 +147,7 @@ clean:
 
 
 ifneq ($(LOCAL_CFLAGS),)
-local_cflags_property = --build-properties "compiler.cpp.extra_flags=${LOCAL_CFLAGS}"
+local_cflags_property = --build-property "compiler.cpp.extra_flags=${LOCAL_CFLAGS}"
 else
 local_cflags_property =
 endif
@@ -175,7 +160,7 @@ ifneq ($(KALEIDOSCOPE_LOCAL_LIB_DIR),)
 _arduino_local_libraries_prop =  --libraries "${KALEIDOSCOPE_LOCAL_LIB_DIR}"
 endif
 
-compile: 
+compile: kaleidoscope-hardware-configured
 
 
 	$(QUIET) install -d "${OUTPUT_PATH}"
@@ -203,32 +188,19 @@ endif
 #TODO (arduino team) I'd love to do this with their json output
 #but it's short some of the data we kind of need
 
-.PHONY: ensure-device-port-defined
+flashing_instructions = $(call _arduino_prop,build.flashing_instructions)
 
-ensure-device-port-defined:  
-	@if [ -z $(KALEIDOSCOPE_DEVICE_PORT) ]; then \
-	echo "ERROR: Unable to detect keyboard serial port.";\
-	echo ;\
-	echo "Arduino should autodetect it, but you could also set";\
-	echo "KALEIDOSCOPE_DEVICE_PORT to your keyboard's serial port.";\
-	echo ;\
-	exit -1;fi
-
-ifneq ($(FQBN),)
-fqbn_arg = --fqbn $(FQBN)
-endif
-
-ifneq ($(KALEIDOSCOPE_DEVICE_PORT),)
-port_arg = --port $(KALEIDOSCOPE_DEVICE_PORT)
+flash: ${HEX_FILE_PATH}
+ifneq ($(flashing_instructions),)
+	$(info $(shell printf $(flashing_instructions)))
+else
+	$(info If your keyboard needs you to do something to put it in flashing mode, do that now.)
 endif
-
-
-flash: 
-	$(info $(unescaped_flashing_instructions))
 	$(info )
 	$(info When you're ready to proceed, press 'Enter'.)
 	$(info )
 	@$(shell read _)
-	$(QUIET) $(ARDUINO_CLI) upload $(fqbn_arg) \
+	$(QUIET) $(ARDUINO_CLI) upload --fqbn $(FQBN) \
+	$(shell $(ARDUINO_CLI) board list --format=text | grep $(FQBN) |cut -d' ' -f 1 | xargs -n 1 echo "--port" ) \
 	--input-dir "${OUTPUT_PATH}" \
-	  $(port_arg) $(ARDUINO_VERBOSE)
+	$(ARDUINO_VERBOSE)

testing/makefiles/delegate.mk

@@ -1,48 +0,0 @@
-# This makefile for a Kaleidoscope sketch pulls in all the targets
-# required to build the example
-
-
-
-
-ifneq ($(KALEIDOSCOPE_DIR),)
-search_path += $(KALEIDOSCOPE_DIR)
-endif
-
-ifneq ($(ARDUINO_DIRECTORIES_USER),)
-search_path += $(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio/avr/libraries/Kaleidoscope
-endif
-
-ifeq ($(shell uname -s),Darwin)
-search_path += $(HOME)/Documents/Arduino/hardware/keyboardio/avr/libraries/Kaleidoscope
-else
-search_path += $(HOME)/Arduino/hardware/keyboardio/avr/libraries/Kaleidoscope
-endif
-
-sketch_makefile := etc/makefiles/sketch.mk
-
-$(foreach candidate, $(search_path), $(if $(wildcard $(candidate)/$(sketch_makefile)), $(eval ks_dir ?= $(candidate))))
-
-ifneq ($(ks_dir),)
-
-$(info Using Kaleidoscope from $(ks_dir))
-
-export KALEIDOSCOPE_DIR := $(ks_dir)
-include $(ks_dir)/$(sketch_makefile)
-
-else
-
-$(info I can't find your Kaleidoscope installation.)
-$(info )
-$(info I tried looking in:)
-$(info )
-$(foreach candidate, $(search_path), $(info $(candidate)))
-$(info )
-$(info The easiest way to fix this is to set the 'KALEIDOSCOPE_DIR' environment)
-$(info variable to the location of your Kaleidoscope directory.)
-
-endif
-
-
-null-target:
-	$(info You should never see this message)
-	@:

testing/makefiles/testcase.mk

@@ -1,3 +1,24 @@
+# Reset a bunch of historical GNU make implicit rules that we never
+# use, but which have a disastrous impact on performance
+#
+# --no-builtin-rules in MAKEFLAGS apparently came in with GNU Make 4,
+# which is newer than what Apple ships
+MAKEFLAGS += --no-builtin-rules
+
+# These lines reset the implicit rules we really care about
+%:: %,v
+
+%:: RCS/%,v
+
+%:: RCS/%
+
+%:: s.%
+
+%:: SCCS/s.%
+
+.SUFFIXES:
+
+
 mkfile_dir 	:= $(dir $(lastword ${MAKEFILE_LIST}))
 top_dir         := $(abspath $(mkfile_dir)../..)
 
@@ -57,7 +78,7 @@ compile-sketch: ${TEST_OBJS}
 	@install -d "${BIN_DIR}" "${LIB_DIR}"
 	$(QUIET) env LIBONLY=yes VERBOSE=${VERBOSE}  \
 		OUTPUT_PATH="${LIB_DIR}" \
-		$(MAKE) -f ${top_dir}/testing/makefiles/delegate.mk compile
+		$(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} \
 		-L"${COMMON_LIB_DIR}" -lcommon \

tests/Makefile

@@ -1,3 +1,22 @@
+# Reset a bunch of historical GNU make implicit rules that we never
+# use, but which have a disastrous impact on performance
+#
+# --no-builtin-rules in MAKEFLAGS apparently came in with GNU Make 4,
+# which is newer than what Apple ships
+MAKEFLAGS += --no-builtin-rules
+
+# These lines reset the implicit rules we really care about
+%:: %,v
+
+%:: RCS/%,v
+
+%:: RCS/%
+
+%:: s.%
+
+%:: SCCS/s.%
+
+.SUFFIXES:
 
 tests_dir	:= $(abspath $(dir $(lastword ${MAKEFILE_LIST})))
 
@@ -13,7 +32,7 @@ TEST_PATH 	?= ./
 
 export FQBN	?= keyboardio:virtual:model01
 
-TESTS		?= $(shell cd $(tests_dir); find ${TEST_PATH} -name '*.ino' -exec dirname {} \;)
+TESTS		:= $(shell cd $(tests_dir); find ${TEST_PATH} -name '*.ino' -exec dirname {} \;)
 
 include $(top_dir)/etc/makefiles/arduino-cli.mk