From 11d8684d2dadb10c3c1173bb0611896c314a9632 Mon Sep 17 00:00:00 2001 From: ChinYikMing Date: Sat, 8 Nov 2025 16:51:48 +0800 Subject: [PATCH 1/2] Add make format target and version_{eq,lt,lte,gt,gte} utils Simplifies the formatting before creating PR, just 'make format'. Version checking is needed for tools, e.g., emcc, browser, and black(python formatting tool). Thus, introduce and refactor with version_{eq,lt,lte,gt,gte} utils for better readability and maintainability (no more 'Pyramid of Doom' checking for MAJOR, MINOR and PATCH). --- Makefile | 48 +++++++++++++++++++++++++++++++++++++++++++++++- mk/common.mk | 14 ++++++++++++++ mk/toolchain.mk | 30 ++++++++---------------------- mk/wasm.mk | 11 ++++++----- 4 files changed, 75 insertions(+), 28 deletions(-) diff --git a/Makefile b/Makefile index 4481ed3ae..88fa36cca 100644 --- a/Makefile +++ b/Makefile @@ -488,6 +488,52 @@ quake: artifact $(quake_deps) endif endif +CLANG_FORMAT := $(shell which clang-format-18 2>/dev/null) + +SHFMT := $(shell which shfmt 2>/dev/null) + +BLACK := $(shell which black 2>/dev/null) +BLACK_VERSION := $(if $(strip $(BLACK)),$(shell $(BLACK) --version | head -n 1 | awk '{print $$2}'),) +BLACK_MAJOR := $(shell echo $(BLACK_VERSION) | cut -f1 -d.) +BLACK_MINOR := $(shell echo $(BLACK_VERSION) | cut -f2 -d.) +BLACK_PATCH := $(shell echo $(BLACK_VERSION) | cut -f3 -d.) +BLACK_ATLEAST_MAJOR := 25 +BLACK_ATLEAST_MINOR := 1 +BLACK_ATLEAST_PATCH := 0 +BLACK_FORMAT_WARNING := Python format check might fail at CI as you use version $(BLACK_VERSION). \ + You may switch black to version $(BLACK_ATLEAST_MAJOR).$(BLACK_ATLEAST_MINOR).$(BLACK_ATLEAST_PATCH) + +SUBMODULES := $(shell git config --file .gitmodules --get-regexp path | awk '{ print $$2 }') +SUBMODULES_PRUNE_PATHS := $(shell for subm in $(SUBMODULES); do echo -n "-path \"./$$subm\" -o "; done | sed 's/ -o $$//') + +format: +ifeq ($(CLANG_FORMAT),) + $(error clang-format-18 not found. Install clang-format version 18 and try again) +else + # Skip formatting submodules and everything in $(OUT), apply the same rule for shfmt and black + $(Q)$(CLANG_FORMAT) -i $(shell find . \( $(SUBMODULES_PRUNE_PATHS) -o -path \"./$(OUT)\" \) \ + -prune -o -name "*.[ch]" -print) +endif +ifeq ($(SHFMT),) + $(error shfmt not found. Install shfmt and try again) +else + $(Q)$(SHFMT) -w $(shell find . \( $(SUBMODULES_PRUNE_PATHS) -o -path \"./$(OUT)\" \) \ + -prune -o -name "*.sh" -print) +endif +ifeq ($(BLACK),) + $(error black not found. Install black version 25.1.0 or above and try again) +else + ifeq ($(call version_lt,\ + $(BLACK_MAJOR),$(BLACK_MINOR),$(BLACK_PATCH),\ + $(BLACK_ATLEAST_MAJOR),$(BLACK_ATLEAST_MINOR),$(BLACK_ATLEAST_PATCH)), 1) + $(warning $(BLACK_FORMAT_WARNING)) + else + $(Q)$(BLACK) --quiet $(shell find . \( $(SUBMODULES_PRUNE_PATHS) -o -path \"./$(OUT)\" \) \ + -prune -o \( -name "*.py" -o -name "*.pyi" \) -print) + endif +endif + $(Q)$(call notice,All files are properly formatted.) + clean: $(VECHO) "Cleaning... " $(Q)$(RM) $(BIN) $(OBJS) $(DEV_OBJS) $(BUILD_DTB) $(BUILD_DTB2C) $(HIST_BIN) $(HIST_OBJS) $(deps) $(WEB_FILES) $(CACHE_OUT) @@ -505,6 +551,6 @@ distclean: clean $(Q)$(call notice, [OK]) .PHONY: all config tool check check-hello misalign misalign-in-blk-emu mmu-test -.PHONY: gdbstub-test doom quake clean distclean artifact +.PHONY: gdbstub-test doom quake format clean distclean artifact -include $(deps) diff --git a/mk/common.mk b/mk/common.mk index 6935f1c95..3acd1539c 100644 --- a/mk/common.mk +++ b/mk/common.mk @@ -53,6 +53,20 @@ warn = $(PRINTF) "$(YELLOW)$(strip $1)$(NC)\n" # Used inside $(warning) or $(error) warnx = $(shell echo "$(YELLOW)$(strip $1)$(NC)\n") +# Version checking +version_num = $(shell printf "%d%03d%03d" $(1) $(2) $(3)) +# Compare two versions with bc +# Returns 1 if first == second else 0 +version_eq = $(shell echo "$$(($(call version_num,$(1),$(2),$(3)) == $(call version_num,$(4),$(5),$(6))))" | bc) +# Returns 1 if first < second else 0 +version_lt = $(shell echo "$$(($(call version_num,$(1),$(2),$(3)) < $(call version_num,$(4),$(5),$(6))))" | bc) +# Returns 1 if first <= second else 0 +version_lte = $(shell echo "$$(($(call version_num,$(1),$(2),$(3)) <= $(call version_num,$(4),$(5),$(6))))" | bc) +# Returns 1 if first > second else 0 +version_gt = $(shell echo "$$(($(call version_num,$(1),$(2),$(3)) > $(call version_num,$(4),$(5),$(6))))" | bc) +# Returns 1 if first >= second else 0 +version_gte = $(shell echo "$$(($(call version_num,$(1),$(2),$(3)) >= $(call version_num,$(4),$(5),$(6))))" | bc) + # File utilities SHA1SUM = sha1sum SHA1SUM := $(shell which $(SHA1SUM)) diff --git a/mk/toolchain.mk b/mk/toolchain.mk index eef08de30..f5c767eda 100644 --- a/mk/toolchain.mk +++ b/mk/toolchain.mk @@ -14,18 +14,10 @@ ifneq ($(shell $(CC) --version | head -n 1 | grep emcc),) SDL_MUSIC_PLAY_AT_EMCC_MINOR := 1 SDL_MUSIC_PLAY_AT_EMCC_PATCH := 51 SDL_MUSIC_CANNOT_PLAY_WARNING := Video games music might not be played. You may switch emcc to version $(SDL_MUSIC_PLAY_AT_EMCC_MAJOR).$(SDL_MUSIC_PLAY_AT_EMCC_MINOR).$(SDL_MUSIC_PLAY_AT_EMCC_PATCH) - ifeq ($(shell echo $(EMCC_MAJOR)\==$(SDL_MUSIC_PLAY_AT_EMCC_MAJOR) | bc), 1) - ifeq ($(shell echo $(EMCC_MINOR)\==$(SDL_MUSIC_PLAY_AT_EMCC_MINOR) | bc), 1) - ifeq ($(shell echo $(EMCC_PATCH)\==$(SDL_MUSIC_PLAY_AT_EMCC_PATCH) | bc), 1) - # do nothing - else - $(warning $(SDL_MUSIC_CANNOT_PLAY_WARNING)) - endif - else - $(warning $(SDL_MUSIC_CANNOT_PLAY_WARNING)) - endif - else - $(warning $(SDL_MUSIC_CANNOT_PLAY_WARNING)) + ifeq ($(call version_eq,\ + $(EMCC_MAJOR),$(EMCC_MINOR),$(EMCC_PATCH),\ + $(SDL_MUSIC_PLAY_AT_EMCC_MAJOR),$(SDL_MUSIC_PLAY_AT_EMCC_MINOR),$(SDL_MUSIC_PLAY_AT_EMCC_PATCH)), 0) + $(warning $(SDL_MUSIC_CANNOT_PLAY_WARNING)) endif # see commit 165c1a3 of emscripten @@ -33,16 +25,10 @@ ifneq ($(shell $(CC) --version | head -n 1 | grep emcc),) MIMALLOC_SUPPORT_SINCE_MINOR := 1 MIMALLOC_SUPPORT_SINCE_PATCH := 50 MIMALLOC_UNSUPPORTED_WARNING := mimalloc is supported after version $(MIMALLOC_SUPPORT_SINCE_MAJOR).$(MIMALLOC_SUPPORT_SINCE_MINOR).$(MIMALLOC_SUPPORT_SINCE_PATCH) - ifeq ($(shell echo $(EMCC_MAJOR)\>=$(MIMALLOC_SUPPORT_SINCE_MAJOR) | bc), 1) - ifeq ($(shell echo $(EMCC_MINOR)\>=$(MIMALLOC_SUPPORT_SINCE_MINOR) | bc), 1) - ifeq ($(shell echo $(EMCC_PATCH)\>=$(MIMALLOC_SUPPORT_SINCE_PATCH) | bc), 1) - CFLAGS_emcc += -sMALLOC=mimalloc - else - $(warning $(MIMALLOC_UNSUPPORTED_WARNING)) - endif - else - $(warning $(MIMALLOC_UNSUPPORTED_WARNING)) - endif + ifeq ($(call version_gte,\ + $(EMCC_MAJOR),$(EMCC_MINOR),$(EMCC_PATCH),\ + $(MIMALLOC_SUPPORT_SINCE_MAJOR),$(MIMALLOC_SUPPORT_SINCE_MINOR),$(MIMALLOC_SUPPORT_SINCE_PATCH)), 1) + CFLAGS_emcc += -sMALLOC=mimalloc else $(warning $(MIMALLOC_UNSUPPORTED_WARNING)) endif diff --git a/mk/wasm.mk b/mk/wasm.mk index 78b818cd3..f7d48128c 100644 --- a/mk/wasm.mk +++ b/mk/wasm.mk @@ -83,9 +83,10 @@ ifeq ($(UNAME_S),Darwin) SAFARI_MAJOR := SAFARI_MINOR := SAFARI_VERSION_CHECK_CMD := -SAFARI_SUPPORT_TCO_AT_MAJOR_MINOR := 18.2 +SAFARI_SUPPORT_TCO_AT_MAJOR := 18 +SAFARI_SUPPORT_TCO_AT_MINOR := 2 SAFARI_SUPPORT_TCO_INFO := Safari supports TCO, you can use Safari to request the wasm -SAFARI_NO_SUPPORT_TCO_WARNING := Safari not found or Safari must have at least version $(SAFARI_SUPPORT_TCO_AT_MAJOR_MINOR) to support TCO in wasm +SAFARI_NO_SUPPORT_TCO_WARNING := Safari not found or Safari must have at least version $(SAFARI_SUPPORT_TCO_AT_MAJOR).$(SAFARI_SUPPORT_TCO_AT_MINOR) to support TCO in wasm endif # FIXME: for Windows @@ -101,14 +102,14 @@ CHROME_MAJOR := $(shell $(CHROME_MAJOR_VERSION_CHECK_CMD)) FIREFOX_MAJOR := $(shell $(FIREFOX_MAJOR_VERSION_CHECK_CMD)) # Chrome -ifeq ($(shell echo $(CHROME_MAJOR)\>=$(CHROME_SUPPORT_TCO_AT_MAJOR) | bc), 1) +ifeq ($(call version_gte,$(CHROME_MAJOR),,,$(CHROME_SUPPORT_TCO_AT_MAJOR),,), 1) $(info $(call noticex, $(CHROME_SUPPORT_TCO_INFO))) else $(warning $(call warnx, $(CHROME_NO_SUPPORT_TCO_WARNING))) endif # Firefox -ifeq ($(shell echo $(FIREFOX_MAJOR)\>=$(FIREFOX_SUPPORT_TCO_AT_MAJOR) | bc), 1) +ifeq ($(call version_gte,$(FIREFOX_MAJOR),,,$(FIREFOX_SUPPORT_TCO_AT_MAJOR),,), 1) $(info $(call noticex, $(FIREFOX_SUPPORT_TCO_INFO))) else $(warning $(call warnx, $(FIREFOX_NO_SUPPORT_TCO_WARNING))) @@ -120,7 +121,7 @@ ifeq ($(UNAME_S),Darwin) SAFARI_VERSION := $(shell $(SAFARI_VERSION_CHECK_CMD)) SAFARI_MAJOR := $(shell echo $(SAFARI_VERSION) | cut -f1 -d.) SAFARI_MINOR := $(shell echo $(SAFARI_VERSION) | cut -f2 -d.) -ifeq ($(shell echo "$(SAFARI_MAJOR).$(SAFARI_MINOR)>=$(SAFARI_SUPPORT_TCO_AT_MAJOR_MINOR)" | bc), 1) +ifeq ($(call version_gte,$(SAFARI_MAJOR),$(SAFARI_MINOR),,$(SAFARI_SUPPORT_TCO_AT_MAJOR),$(SAFARI_SUPPORT_TCO_AT_MINOR),), 1) $(info $(call noticex, $(SAFARI_SUPPORT_TCO_INFO))) else $(warning $(call warnx, $(SAFARI_NO_SUPPORT_TCO_WARNING))) From b1ba59b0cb3e8950c18320c61f5887c9efb7f53a Mon Sep 17 00:00:00 2001 From: ChinYikMing Date: Sat, 8 Nov 2025 19:09:16 +0800 Subject: [PATCH 2/2] Update CONTRIBUTING.md - To reflect the 'make format' new target. - Remove 'or later' to use clang-format-18 for consistency. --- CONTRIBUTING.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b657654de..c4c462fa3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -47,7 +47,7 @@ However, participation requires adherence to fundamental ground rules: For instance, opt for "initialize" over "initialise" and "color" rather than "colour". Software requirement: -* [clang-format](https://clang.llvm.org/docs/ClangFormat.html) version 18 or later. +* [clang-format](https://clang.llvm.org/docs/ClangFormat.html) version 18. * [shfmt](https://github.com/mvdan/sh). * [black](https://github.com/psf/black) version 25.1.0. @@ -55,6 +55,7 @@ To maintain a uniform style across languages, run: * `clang-format -i *.{c,h}` to apply the project’s C/C++ formatting rules from the up-to-date .clang-format file. * `shfmt -w $(find . -type f -name "*.sh")` to clean and standardize all shell scripts. * `black .` to enforce a consistent, idiomatic layout for Python code. +* `make format` to automatically run all three formatters. ## Coding Style for Shell Script