Merge pull request #1155 from gedankenexperimenter/code-maintenance

Various minor build infrastructure improvements

.github/workflows/build.yml

@@ -32,11 +32,11 @@ jobs:
       uses: hendrikmuhs/ccache-action@v1.2
     - run: make setup
     - run: KALEIDOSCOPE_CCACHE=1 make -j $(nproc) --output-sync=recurse simulator-tests
-  check-formatting:
+  check-code-style:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v2
-    - run: make check-formatting
+    - run: make check-code-style
   check-shellcheck:
     runs-on: ubuntu-latest
     steps:

Makefile

@@ -36,24 +36,16 @@ ifneq ($(TEST_PATH),)
 TEST_PATH_ARG="TEST_PATH='$(TEST_PATH)'"
 endif
 
+.DEFAULT_GOAL := smoke-sketches
 
-DEFAULT_GOAL: smoke-sketches
+.PHONY: setup
-
+setup: $(ARDUINO_CLI_PATH) $(ARDUINO_DIRECTORIES_DATA)/arduino-cli.yaml install-arduino-core-avr install-arduino-core-kaleidoscope checkout-platform prepare-virtual
-
-setup: $(ARDUINO_CLI_PATH) $(ARDUINO_DIRECTORIES_DATA)/arduino-cli.yaml install-arduino-core-avr install-arduino-core-kaleidoscope $(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio/avr/boards.txt $(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio/virtual/boards.txt 
 	@:
 
-
+.PHONY: checkout-platform
 checkout-platform: $(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio/avr/boards.txt
 	@:
 
-prepare-virtual: $(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio/virtual/boards.txt
-	@:
-
-$(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio/virtual/boards.txt:
-	$(MAKE) -C $(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio prepare-virtual
-
-
 $(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio/avr/boards.txt:
 	git clone -c core.symlinks=true \
 		--recurse-submodules \
@@ -68,90 +60,97 @@ $(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio/avr/boards.txt:
 		--recurse-submodules=':(exclude)libraries/Kaleidoscope' \
 		https://github.com/keyboardio/ArduinoCore-GD32-Keyboardio $(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio/gd32
 
+.PHONY: prepare-virtual
+prepare-virtual: $(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio/virtual/boards.txt
+	@:
+
+$(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio/virtual/boards.txt:
+	$(MAKE) -C $(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio prepare-virtual
+
+.PHONY: update
 update:
 	cd $(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio; git pull; \
 		git submodule update --init --recursive
 	cd $(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio/gd32; git pull; \
 		git submodule update --init --recursive
 
+.PHONY: simulator-tests
 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
 
+.PHONY: docker-simulator-tests
 docker-simulator-tests:
 	ARDUINO_DIRECTORIES_USER="$(ARDUINO_DIRECTORIES_USER)" ./bin/run-docker "make simulator-tests $(TEST_PATH_ARG)"
 
+.PHONY: docker-clean
 docker-clean:
 	_NO_SYNC_KALEIDOSCOPE=1 ARDUINO_DIRECTORIES_USER="$(ARDUINO_DIRECTORIES_USER)" ./bin/run-docker "rm -rf -- testing/googletest/build/* _build/* /kaleidoscope-persist/temp/*"
 
+.PHONY: docker-bash
 docker-bash:
 	_NO_SYNC_KALEIDOSCOPE=1 DOCKER_LIVE_KALEIDOSCOPE_DIR=1 ARDUINO_DIRECTORIES_USER="$(ARDUINO_DIRECTORIES_USER)" ./bin/run-docker "bash"
 
-run-tests: $(ARDUINO_DIRECTORIES_USER)/hardware/keyboardio/virtual/boards.txt build-gtest-gmock
-	$(MAKE) -c tests
-	@: # blah
-
 build-gtest-gmock:
 	(cd testing/googletest && cmake -H. -Bbuild -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)  .)
 	$(MAKE) -C testing/googletest
 
-adjust-git-timestamps:
+.PHONY: find-filename-conflicts
-	bin/set-timestamps-from-git
-
 find-filename-conflicts:
-	bin/find-filename-conflicts
+	bin/find-filename-conflicts.py src plugins/*
-
-.PHONY: format check-formatting 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
 
+.PHONY: format
 format:
 	bin/format-code.py \
 		--exclude-dir 'testing/googletest' \
 		--exclude-file 'generated-testcase.cpp' \
 		src plugins examples testing
 
-check-formatting:
+.PHONY: check-code-style
+check-code-style:
 	bin/format-code.py \
 		--exclude-dir 'testing/googletest' \
 		--exclude-file 'generated-testcase.cpp' \
+		--force \
 		--check \
 		--verbose \
 		src plugins examples testing
 
+.PHONY: check-includes
+check-includes:
+	bin/iwyu.py -v src plugins
+	bin/format-code.py -f -v --check src plugins
+
+.PHONY: cpplint-noisy
 cpplint-noisy:
 	-bin/cpplint.py --config=.cpplint-noisy --recursive src plugins examples
 
 
+.PHONY: cpplint
 cpplint:
 	bin/cpplint.py --config=.cpplint --quiet --recursive src plugins examples
 
-
+.PHONY: shellcheck
-SHELL_FILES := $(shell if [ -d bin ]; then egrep -n -r -l "(env (ba)?sh)|(/bin/(ba)?sh)" bin; fi)
-
 shellcheck:
-	@if [ -d "bin" ]; then \
+	bin/check-shell-scripts.sh
-		shellcheck ${SHELL_FILES}; \
-	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"
 
-.PHONY: force all clean test
+.PHONY: force
+$(SMOKE_SKETCHES): force
+	$(MAKE) -C $@ -f $(KALEIDOSCOPE_ETC_DIR)/makefiles/sketch.mk compile
 
+.PHONY: clean
 clean: 
 	$(MAKE) -C tests clean
 	rm -rf -- "testing/googletest/build/*"
 	rm -rf -- "_build/*"
 
-
-$(SMOKE_SKETCHES): force
-	$(MAKE) -C $@ -f $(KALEIDOSCOPE_ETC_DIR)/makefiles/sketch.mk compile
-
-
 build-arduino-nightly-package:
 	perl bin/build-arduino-package \
 		--kaleidoscope-tag=master \

bin/check-shell-scripts.sh

@@ -0,0 +1,13 @@
+#!/usr/bin/env bash
+
+: "${KALEIDOSCOPE_DIR:=$(pwd)}"
+cd "${KALEIDOSCOPE_DIR}" || exit 1
+
+ERROR_COUNT=0
+
+while read -r SCRIPT; do
+    shellcheck "${SCRIPT}"
+    (( ERROR_COUNT += $? ))
+done < <(grep -E -n -r -l '(env (ba)?sh)|(/bin/(ba)?sh)' "${KALEIDOSCOPE_DIR}/bin")
+
+exit $ERROR_COUNT

bin/find-filename-conflicts

@@ -1,55 +0,0 @@
-#!/usr/bin/env bash
-# find-filename-conflicts - Finds cpp files with conflicting filenames
-# Copyright (C) 2020  Keyboard.io, Inc.
-#
-# This program is free software: you can redistribute it and/or modify it under
-# the terms of the GNU General Public License as published by the Free Software
-# Foundation, version 3.
-#
-# This program is distributed in the hope that it will be useful, but WITHOUT
-# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
-# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License along with
-# this program. If not, see <http://www.gnu.org/licenses/>.
-
-## When building Kaleidoscope, the compiled object files are linked together
-## into a static archive. This static archive has a very simple structure, and
-## only stores filenames, not paths, not even relative ones. As such, we can't
-## have files with the same name, because they will conflict, and one will
-## override the other.
-##
-## To avoid this situation, this script will find all cpp source files (we don't
-## need to care about header-only things, those do not result in an object
-## file), and will comb through them to find conflicting filenames.
-##
-## If a conflict is found, it will print all files that share the name, and will
-## exit with an error at the end. It does not exit at the first duplicate, but
-## will find and print all of them.
-##
-## If no conflict is found, the script just prints its status message and exits
-## with zero.
-
-set -e
-
-FILE_LIST="$(find src -name '*.cpp' | sed -e 's,\(\(.*\)/\([^/]*\)\),\3 \1,')"
-
-exit_code=0
-
-echo -n "Looking for conflicting filenames... "
-
-for f in $(echo "${FILE_LIST}" | cut -f1 -d" "); do
-    count=$(echo "${FILE_LIST}" | grep -c "^${f}")
-    if [ "$count" -gt 1 ]; then
-        echo >&2
-        echo " Conflict found for ${f}: " >&2
-        echo "${FILE_LIST}" | grep "${f}" | cut -d" " -f2 | sed -e 's,^,   ,' >&2
-        exit_code=1
-    fi
-done
-
-if [ "${exit_code}" -eq 0 ]; then
-    echo "done."
-fi
-
-exit ${exit_code}

bin/find-filename-conflicts.py

@@ -0,0 +1,96 @@
+#!/usr/bin/env python3
+# -*- coding: utf-8 -*-
+# ------------------------------------------------------------------------------
+# Copyright (c) 2022 Michael Richters <gedankenexperimenter@gmail.com>
+
+# This is free and unencumbered software released into the public domain.
+
+# Anyone is free to copy, modify, publish, use, compile, sell, or
+# distribute this software, either in source code form or as a compiled
+# binary, for any purpose, commercial or non-commercial, and by any
+# means.
+
+# In jurisdictions that recognize copyright laws, the author or authors
+# of this software dedicate any and all copyright interest in the
+# software to the public domain. We make this dedication for the benefit
+# of the public at large and to the detriment of our heirs and
+# successors. We intend this dedication to be an overt act of
+# relinquishment in perpetuity of all present and future rights to this
+# software under copyright law.
+
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+# IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+# OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+# ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+# OTHER DEALINGS IN THE SOFTWARE.
+
+# For more information, please refer to <http://unlicense.org/>
+# ------------------------------------------------------------------------------
+"""When building Kaleidoscope, the compiled object files are linked together
+into a static archive. This static archive has a very simple structure, and only
+stores filenames, not paths, not even relative ones. As such, we can't have
+files with the same name, because they will conflict, and one will override the
+other.
+
+To avoid this situation, this script will find all cpp source files (we don't
+need to care about header-only things, those do not result in an object file),
+and will comb through them to find conflicting filenames.
+
+If a conflict is found, it will print all files that share the name, and will
+exit with an error at the end. It does not exit at the first duplicate, but will
+find and print all of them.
+
+If no conflict is found, the script just prints its status message and exits
+with zero."""
+
+import os
+import re
+import sys
+
+cpp_regex = re.compile('.*\.cpp')
+
+
+def find_duplicates(root):
+    """Search for files with the same basename, but in different directories in
+    the tree under <root>.  Prints a message for each conflict found, and
+    returns a count of the number of non-unique basenames."""
+
+    # Search the specified tree for matching basenames:
+    basenames = {}
+    for dir_path, dirs, files in os.walk(root):
+        for file_name in files:
+            if cpp_regex.match(file_name):
+                if file_name not in basenames:
+                    basenames[file_name] = []
+                basenames[file_name].append(dir_path)
+
+    conflict_count = 0
+    for file_name, dirs in basenames.items():
+        # Prune unique basenames from the dict:
+        if len(dirs) <= 1:
+            continue
+
+        conflict_count += 1
+        # Print info about basenames with conflicts:
+        print(f"Conflict found for file name '{file_name}':")
+        for root in dirs:
+            path = os.path.join(root, file_name)
+            print(f'  -> {path}')
+
+    return conflict_count
+
+
+def main(args):
+    print('Searching for conflicting filenames...')
+    exit_code = 0
+    for path in args:
+        exit_code += find_duplicates(path)
+    if exit_code != 0:
+        sys.exit(exit_code)
+    print('No filename conflicts found.')
+
+
+if __name__ == "__main__":
+    main(sys.argv[1:])

bin/set-timestamps-from-git

@@ -1,15 +0,0 @@
-#!/bin/bash -e
-
-# This script sets all of the files inside src and example to have mtimes
-# that match the times of the last git commit that touched each file
-
-# This can be useful when build tools depend on file timestamps to
-# make caching decisions
-
-find src examples -type f -exec sh -c '
-  timestamp=$(git log --pretty=format:%ad --date=format:%Y%m%d%H%M.%S -n 1 HEAD "$1" 2> /dev/null)
-  if [ "x$timestamp" != "x" ]; then
-    touch -t "$timestamp" "$1"
-  fi
-' sh {} \;
-

etc/makefiles/arduino-cli.mk

@@ -114,9 +114,10 @@ endif
 .PHONY: configure-arduino-cli install-arduino-core-kaleidoscope install-arduino-core-avr
 .PHONY: stupid-workaround-for-make-inclusion-semantics
 
-stupid-workaround-for-make-inclusion-semantics: DEFAULT_GOAL
+stupid-workaround-for-make-inclusion-semantics:
 	@: # This is here so that the sketch makefile including this file doesn't
 	@: # default to arduino-cli installation as its priamry target
+	@echo "No default target specified; aborting. Please set the .DEFAULT_GOAL variable in your makefile."
 
 $(KALEIDOSCOPE_BIN_DIR)/arduino-cli:
 	$(QUIET) curl -fsSL https://raw.githubusercontent.com/arduino/arduino-cli/master/install.sh | BINDIR="$(KALEIDOSCOPE_BIN_DIR)" sh

etc/makefiles/sketch.mk

@@ -99,7 +99,7 @@ endif
 
 
 
-DEFAULT_GOAL: compile
+.DEFAULT_GOAL := compile
 
 
 #$(SKETCH_FILE_PATH):

tests/Makefile

@@ -37,21 +37,19 @@ TESTS		:= $(shell cd $(tests_dir); find ${TEST_PATH} -name '*.ino' -exec dirname
 # The clutter up the test output on Make 4.0 and newer
 MAKEFLAGS += --no-print-directory
 
-
 include $(top_dir)/etc/makefiles/arduino-cli.mk
 
+.DEFAULT_GOAL := all
+
 # If we start off in tests to run make all, the sketch makefiles guess the wrong location for
 # Kaliedoscope's makefiles
-
 KALEIDOSCOPE_ETC_DIR ?= $(top_dir)/etc
 
-
+.PHONY: all
-.PHONY: clean cmake-clean all googletest
-
-
 all: ${libcommon_a} googletest ${TESTS} 
 	@:
 
+.PHONY: cmake-clean
 cmake-clean:
 	rm -rf "${top_dir}"/testing/googletest/build/*
 
@@ -63,8 +61,7 @@ run-all: ${TESTS}
 	done; \
 	if [ -n $${ERROR} ]; then exit $${ERROR}; fi
 
-
+.PHONY: clean
-
 clean: cmake-clean
 	@for test in ${TESTS}; do \
 		${MAKE} -s -f ${top_dir}/testing/makefiles/testcase.mk \
@@ -73,6 +70,7 @@ clean: cmake-clean
 	done
 	rm -rf "${build_dir}"/*
 
+.PHONY: googletest
 googletest: ${top_dir}/testing/googletest/build/Makefile
 	${MAKE} -C ${top_dir}/testing/googletest/build