From 0ce3df0ba7f6d3cd8a9d4fe7f44630acb4c2dea1 Mon Sep 17 00:00:00 2001 From: cds-amal Date: Sun, 8 Mar 2026 01:20:20 -0500 Subject: [PATCH 1/2] feat(make): add self-documenting help target and clean up idioms Add a `make help` target that uses an awk script to parse structured comments in the Makefile itself. `##` comments describe individual targets; `###` comments act as section headers. The output is grouped into four sections: Build, Test, Code quality, and Graph generation. Internal helper targets (like `check-graphviz` and `rustup-clear-toolchain`) are intentionally left without `##` comments so they stay hidden from the help output. Idiomatic fixes applied across the board: - `.DEFAULT_GOAL := build` replaces the old `default: build` target. The old approach created a real target called `default` that showed up in tab-completion and could collide with other tooling; `.DEFAULT_GOAL` is the built-in make mechanism for this. - `$(MAKE)` replaces bare `make` in the `golden` target. When make is invoked with flags (e.g., `make -j4`), a bare `make` in a recipe starts a fresh make process that loses those flags and, critically, the jobserver file descriptors. `$(MAKE)` preserves both. - `.PHONY` is now declared immediately above every phony target. A phony target is one that doesn't produce a file of the same name (i.e., `make build` doesn't create a file called `build`). Without the declaration, if a file named `build` or `clean` happened to exist, make would see it as up-to-date and skip the target entirely. Previously several targets (`build`, `clean`, `golden`, `format`, `style-check`) were missing their declarations. - Variable references normalized from `${VAR}` to `$(VAR)`. Both work identically in GNU make, but `$(VAR)` is the conventional style. `${VAR}` is visually ambiguous with shell variable expansion inside recipes, so reserving curly braces for shell context (like `$${rust}`) and parens for make context makes it easier to tell which layer is doing the expansion. - `TOOLCHAIN_NAME=''` fixed to `TOOLCHAIN_NAME=`. Unlike in shell, make does not interpret quotes as delimiters: the old value was literally the two-character string `''`, not empty. This happened to be harmless (rustup would just fail to find a toolchain named `''`) but was misleading. - Removed the `ECHO_CMD=echo` variable, which was only used once and added indirection with no benefit. Replaced with a direct `@echo`. --- Makefile | 91 +++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 67 insertions(+), 24 deletions(-) diff --git a/Makefile b/Makefile index eace0fc9..a7d0422e 100644 --- a/Makefile +++ b/Makefile @@ -1,11 +1,31 @@ RELEASE_FLAG= -TOOLCHAIN_NAME='' - -default: build - +TOOLCHAIN_NAME= + +.DEFAULT_GOAL := build + +.PHONY: help +## Show this help message +help: + @echo "Available targets:" + @awk 'BEGIN {FS = ":.*"; printf "\nUsage:\n make \033[36m\033[0m\n"} \ + /^\s*###/ {printf "\n\033[1m%s\033[0m\n", substr($$0, 5); next} \ + /^\s*##/ {description=substr($$0, 4)} \ + /^[a-zA-Z0-9_-]+:/ { \ + if (description) { \ + printf " \033[36m%-18s\033[0m %s\n", $$1, description; \ + description = ""; \ + } \ + }' $(MAKEFILE_LIST) + +### Build + +.PHONY: build +## Build the project (use RELEASE_FLAG=--release for release) build: - cargo build ${RELEASE_FLAG} + cargo build $(RELEASE_FLAG) +.PHONY: clean +## Clean build artifacts, toolchain overrides, and graphs clean: rustup-clear-toolchain clean-graphs cargo clean @@ -13,7 +33,9 @@ clean: rustup-clear-toolchain clean-graphs rustup-clear-toolchain: rustup override unset rustup override unset --nonexistent - rustup toolchain uninstall "${TOOLCHAIN_NAME}" + rustup toolchain uninstall "$(TOOLCHAIN_NAME)" + +### Test TESTDIR=tests/integration/programs @@ -24,34 +46,29 @@ integration-test: SMIR ?= cargo run -- "-Zno-codegen" integration-test: NORMALIZE ?= jq -S -e -f $(TESTDIR)/../normalise-filter.jq # override this to re-make golden files integration-test: DIFF ?= | diff - +## Run integration tests against expected outputs integration-test: errors=""; \ report() { echo "$$1: $$2"; errors="$$errors\n$$1: $$2"; }; \ - for rust in ${TESTS}; do \ + for rust in $(TESTS); do \ target=$${rust%.rs}.smir.json; \ dir=$$(dirname $${rust}); \ echo "$$rust"; \ - ${SMIR} --out-dir $${dir} $${rust} || report "$$rust" "Conversion failed"; \ + $(SMIR) --out-dir $${dir} $${rust} || report "$$rust" "Conversion failed"; \ [ -f $${target} ] \ - && ${NORMALIZE} $${target} ${DIFF} $${target}.expected \ + && $(NORMALIZE) $${target} $(DIFF) $${target}.expected \ && rm $${target} \ || report "$$rust" "Unexpected json output"; \ done; \ [ -z "$$errors" ] || (echo "===============\nFAILING TESTS:$$errors"; exit 1) - +.PHONY: golden +## Regenerate expected test outputs (golden files) golden: - make integration-test DIFF=">" - -format: - cargo fmt - bash -O globstar -c 'nixfmt **/*.nix' - -style-check: format - cargo clippy -- -Dwarnings - -.PHONY: remake-ui-tests test-ui + $(MAKE) integration-test DIFF=">" +.PHONY: remake-ui-tests +## Regenerate UI test fixtures (requires RUST_DIR_ROOT) remake-ui-tests: # Check if RUST_DIR_ROOT is set if [ -z "$$RUST_DIR_ROOT" ]; then \ @@ -61,20 +78,33 @@ remake-ui-tests: # This will run without saving source files. Run the script manually to do this. bash tests/ui/remake_ui_tests.sh "$$RUST_DIR_ROOT" +.PHONY: test-ui +## Run UI tests (requires RUST_DIR_ROOT, VERBOSE=1 for details) test-ui: VERBOSE?=0 test-ui: bash tests/ui/run_ui_tests.sh $(if $(filter 1,$(VERBOSE)),--verbose) "$$RUST_DIR_ROOT" -.PHONY: dot svg png d2 clean-graphs check-graphviz +### Code quality + +.PHONY: format +## Format Rust and Nix source files +format: + cargo fmt + bash -O globstar -c 'nixfmt **/*.nix' + +.PHONY: style-check +## Run format + clippy lint checks +style-check: format + cargo clippy -- -Dwarnings + +### Graph generation OUTDIR_DOT=output-dot OUTDIR_SVG=output-svg OUTDIR_PNG=output-png OUTDIR_D2=output-d2 -clean-graphs: - @rm -rf $(OUTDIR_DOT) $(OUTDIR_SVG) $(OUTDIR_PNG) $(OUTDIR_D2) - +.PHONY: check-graphviz check-graphviz: @command -v dot >/dev/null 2>&1 || { \ echo "Error: Graphviz is not installed or 'dot' is not in PATH."; \ @@ -83,6 +113,8 @@ check-graphviz: exit 1; \ } +.PHONY: dot +## Generate DOT files from test programs dot: @mkdir -p $(OUTDIR_DOT) @for rs in $(TESTDIR)/*.rs; do \ @@ -92,6 +124,8 @@ dot: mv $$name.smir.dot $(OUTDIR_DOT)/ 2>/dev/null || true; \ done +.PHONY: svg +## Generate SVG files from DOT (requires graphviz) svg: check-graphviz dot @mkdir -p $(OUTDIR_SVG) @for dotfile in $(OUTDIR_DOT)/*.dot; do \ @@ -100,6 +134,8 @@ svg: check-graphviz dot dot -Tsvg $$dotfile -o $(OUTDIR_SVG)/$$name.svg; \ done +.PHONY: png +## Generate PNG files from DOT (requires graphviz) png: check-graphviz dot @mkdir -p $(OUTDIR_PNG) @for dotfile in $(OUTDIR_DOT)/*.dot; do \ @@ -108,6 +144,8 @@ png: check-graphviz dot dot -Tpng $$dotfile -o $(OUTDIR_PNG)/$$name.png; \ done +.PHONY: d2 +## Generate D2 diagram files from test programs d2: @mkdir -p $(OUTDIR_D2) @for rs in $(TESTDIR)/*.rs; do \ @@ -116,3 +154,8 @@ d2: cargo run --release -- --d2 -Zno-codegen $$rs 2>/dev/null; \ mv $$name.smir.d2 $(OUTDIR_D2)/ 2>/dev/null || true; \ done + +.PHONY: clean-graphs +## Remove generated graph output directories +clean-graphs: + @rm -rf $(OUTDIR_DOT) $(OUTDIR_SVG) $(OUTDIR_PNG) $(OUTDIR_D2) From 00185424f60027c026f55c232fcb13522bb0da99 Mon Sep 17 00:00:00 2001 From: cds-amal Date: Sun, 8 Mar 2026 12:18:39 -0400 Subject: [PATCH 2/2] fix(make): drop non-portable \s from awk patterns in help target `\s` is not POSIX awk; BSD awk (macOS default) treats it as a literal `s`. The patterns worked anyway because the comments are always at column 0, but this was a latent portability bug. Use bare `^##` / `^###` anchors instead. Addresses review feedback on PR #139. --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index a7d0422e..e6194f14 100644 --- a/Makefile +++ b/Makefile @@ -8,8 +8,8 @@ TOOLCHAIN_NAME= help: @echo "Available targets:" @awk 'BEGIN {FS = ":.*"; printf "\nUsage:\n make \033[36m\033[0m\n"} \ - /^\s*###/ {printf "\n\033[1m%s\033[0m\n", substr($$0, 5); next} \ - /^\s*##/ {description=substr($$0, 4)} \ + /^###/ {printf "\n\033[1m%s\033[0m\n", substr($$0, 5); next} \ + /^##/ {description=substr($$0, 4)} \ /^[a-zA-Z0-9_-]+:/ { \ if (description) { \ printf " \033[36m%-18s\033[0m %s\n", $$1, description; \