diff --git a/.claude/skills/tupfile-patterns/SKILL.md b/.claude/skills/tupfile-patterns/SKILL.md index b6e34aa4..47b3c02d 100644 --- a/.claude/skills/tupfile-patterns/SKILL.md +++ b/.claude/skills/tupfile-patterns/SKILL.md @@ -160,6 +160,8 @@ CFLAGS = -I$(B)/$(GMP_DIR) -I$(S)/$(GMP_DIR) Prefixed DIR vars (`GMP_DIR`, `MPFR_DIR`) are needed because `include_rules` merges ALL `Tuprules.tup` from root to leaf — a single `DIR` variable would collide. +Verify conventions with `putup parse --strict` — flags `=` instead of `?=` for anchor/toolchain variables in component Tuprules.tup files. + ## Scoped tup.config Each subdirectory can have its own `tup.config` for `@(VAR)` config variables: diff --git a/CLAUDE.md b/CLAUDE.md index 3b89f2ad..b6f60bd7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -60,6 +60,7 @@ make test # Run all tests ./build/test/unit/putup_test '[target]' # Target parsing tests ./build/test/unit/putup_test '[shell]' # Shell fixture tests ./build/test/unit/putup_test '[configure]' # Two-pass config generation tests +./build/test/unit/putup_test '[strict]' # Convention checker (--strict) tests ``` ### Writing E2E Tests diff --git a/DESIGN.md b/DESIGN.md index c6be2256..c501d9cf 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -43,7 +43,7 @@ With libstdc++ eliminated, putup provides its own runtime primitives: | `Function` | `std::function` | Move-only type-erased callable (32-byte SBO) | | `SteadyClock` / `SystemClock` | `std::chrono` clocks | Platform-implemented clocks | | `CPath` | manual `.c_str()` | Null-termination wrapper for string_view at C API boundary | -| `throw_stubs.cpp` | libstdc++ runtime | operator new/delete, `__cxa_guard_*`, `__throw_*` stubs | +| `throw_stubs.cpp` | libstdc++ runtime | operator new/delete, `__cxa_guard_*`, `__throw_*` stubs, macOS `__libcpp_verbose_abort` | All threading (`std::thread`, `std::mutex`, `std::atomic`, `std::future`) has been eliminated. The scheduler uses a single-threaded `poll()`-based event loop. Multi-variant parallelism uses `fork()`+`waitpid()` via `platform::run_parallel_tasks()`. @@ -1883,7 +1883,7 @@ Putup links with `-nostdlib++` and provides its own runtime primitives: - **Portability** - No dependency on a specific C++ standard library version; only requires a C runtime (libc) - **Control** - Custom `StringPool`, `Vec`, `Function`, and clock types are purpose-built for the build system's needs, with no overhead from generality -The `throw_stubs.cpp` file provides the minimal runtime surface: `operator new`/`delete` (delegating to `malloc`/`free`), `__cxa_guard_*` (for thread-safe static initialization), and `__throw_*` stubs that abort on use. Multi-variant parallelism uses `fork()`+`waitpid()` via `platform::run_parallel_tasks()` instead of `std::async`. +The `throw_stubs.cpp` file provides the minimal runtime surface: `operator new`/`delete` (delegating to `malloc`/`free`), `__cxa_guard_*` (for thread-safe static initialization), and `__throw_*` stubs that abort on use. On macOS, libc++ also requires `std::__1::__libcpp_verbose_abort` (used by `variant`/`optional` failure paths); the stub matches libc++'s `__1` inline ABI namespace directly. Multi-variant parallelism uses `fork()`+`waitpid()` via `platform::run_parallel_tasks()` instead of `std::async`. --- diff --git a/SKILL.md b/SKILL.md index 8ea7fcfc..5606a7cb 100644 --- a/SKILL.md +++ b/SKILL.md @@ -49,6 +49,7 @@ make test # All tests | `[scoped-config]` | Scoped configure commands | | `[shell]` | Shell fixture tests (`test.sh`) | | `[show]` | Show command (script, compdb, graph) | +| `[strict]` | Convention checker (`--strict` flag) | | `[target]` | Target parsing tests | | `[variant]` | Out-of-tree/variant builds, ghost nodes | diff --git a/Tupfile b/Tupfile index 0dcdc605..ccd18d96 100644 --- a/Tupfile +++ b/Tupfile @@ -60,6 +60,7 @@ srcs-y += src/cli/context.cpp srcs-y += src/cli/multi_variant.cpp srcs-y += src/cli/options.cpp srcs-y += src/cli/output.cpp +srcs-y += src/cli/strict_checks.cpp srcs-y += src/cli/target.cpp # Platform diff --git a/bootstrap-linux.sh b/bootstrap-linux.sh index 06809a12..9c0c5444 100755 --- a/bootstrap-linux.sh +++ b/bootstrap-linux.sh @@ -19,6 +19,7 @@ mkdir -p "build/test/unit" (cd "." && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/context.cpp -o build/context.o) (cd "." && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/config_commands.cpp -o build/config_commands.o) (cd "." && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/cmd_parse.cpp -o build/cmd_parse.o) +(cd "." && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/strict_checks.cpp -o build/strict_checks.o) (cd "." && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/cmd_show.cpp -o build/cmd_show.o) (cd "." && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/cmd_configure.cpp -o build/cmd_configure.o) (cd "." && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/cmd_clean.cpp -o build/cmd_clean.o) @@ -61,8 +62,8 @@ mkdir -p "build/test/unit" (cd "." && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/core/hash.cpp -o build/hash.o) (cd "." && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/core/arena.cpp -o build/arena.o) (cd "." && gcc -std=c11 -Wall -Wextra -Werror -fPIC -I./third_party -O2 -ffunction-sections -fdata-sections -Wno-error -c third_party/sha256/sha256.c -o build/sha256.o) -(cd "." && g++ build/sha256.o build/arena.o build/hash.o build/id_array.o build/id_bitset.o build/layout.o build/metrics.o build/path.o build/path_utils.o build/sorted_id_vec.o build/string_pool.o build/global_pool.o build/buf.o build/heap_buf.o build/string_utils.o build/terminal.o build/ast.o build/config.o build/depfile.o build/eval.o build/glob.o build/ignore.o build/lexer.o build/parser.o build/var_tracking.o build/builder.o build/dag.o build/dep_scanner.o build/rule_pattern.o build/gcc.o build/topo.o build/entry.o build/reader.o build/writer.o build/progress_display.o build/runner.o build/scheduler.o build/cmd_build.o build/cmd_clean.o build/cmd_configure.o build/cmd_show.o build/cmd_parse.o build/config_commands.o build/context.o build/multi_variant.o build/options.o build/output.o build/target.o build/process-posix.o build/file_io-posix.o build/env-posix.o build/path-posix.o build/main.o build/throw_stubs.o -o build/putup -nostdlib++ -Wl,--gc-sections ) -(cd "." && ar rcs build/libputup.a build/sha256.o build/arena.o build/hash.o build/id_array.o build/id_bitset.o build/layout.o build/metrics.o build/path.o build/path_utils.o build/sorted_id_vec.o build/string_pool.o build/global_pool.o build/buf.o build/heap_buf.o build/string_utils.o build/terminal.o build/ast.o build/config.o build/depfile.o build/eval.o build/glob.o build/ignore.o build/lexer.o build/parser.o build/var_tracking.o build/builder.o build/dag.o build/dep_scanner.o build/rule_pattern.o build/gcc.o build/topo.o build/entry.o build/reader.o build/writer.o build/progress_display.o build/runner.o build/scheduler.o build/cmd_build.o build/cmd_clean.o build/cmd_configure.o build/cmd_show.o build/cmd_parse.o build/config_commands.o build/context.o build/multi_variant.o build/options.o build/output.o build/target.o build/process-posix.o build/file_io-posix.o build/env-posix.o build/path-posix.o) +(cd "." && g++ build/sha256.o build/arena.o build/hash.o build/id_array.o build/id_bitset.o build/layout.o build/metrics.o build/path.o build/path_utils.o build/sorted_id_vec.o build/string_pool.o build/global_pool.o build/buf.o build/heap_buf.o build/string_utils.o build/terminal.o build/ast.o build/config.o build/depfile.o build/eval.o build/glob.o build/ignore.o build/lexer.o build/parser.o build/var_tracking.o build/builder.o build/dag.o build/dep_scanner.o build/rule_pattern.o build/gcc.o build/topo.o build/entry.o build/reader.o build/writer.o build/progress_display.o build/runner.o build/scheduler.o build/cmd_build.o build/cmd_clean.o build/cmd_configure.o build/cmd_show.o build/cmd_parse.o build/strict_checks.o build/config_commands.o build/context.o build/multi_variant.o build/options.o build/output.o build/target.o build/process-posix.o build/file_io-posix.o build/env-posix.o build/path-posix.o build/main.o build/throw_stubs.o -o build/putup -nostdlib++ -Wl,--gc-sections ) +(cd "." && ar rcs build/libputup.a build/sha256.o build/arena.o build/hash.o build/id_array.o build/id_bitset.o build/layout.o build/metrics.o build/path.o build/path_utils.o build/sorted_id_vec.o build/string_pool.o build/global_pool.o build/buf.o build/heap_buf.o build/string_utils.o build/terminal.o build/ast.o build/config.o build/depfile.o build/eval.o build/glob.o build/ignore.o build/lexer.o build/parser.o build/var_tracking.o build/builder.o build/dag.o build/dep_scanner.o build/rule_pattern.o build/gcc.o build/topo.o build/entry.o build/reader.o build/writer.o build/progress_display.o build/runner.o build/scheduler.o build/cmd_build.o build/cmd_clean.o build/cmd_configure.o build/cmd_show.o build/cmd_parse.o build/strict_checks.o build/config_commands.o build/context.o build/multi_variant.o build/options.o build/output.o build/target.o build/process-posix.o build/file_io-posix.o build/env-posix.o build/path-posix.o) (cd "test/unit" && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I../../include -I../../third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c test_var_tracking.cpp -o ../../build/test/unit/test_var_tracking.o) (cd "test/unit" && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I../../include -I../../third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c test_types.cpp -o ../../build/test/unit/test_types.o) (cd "test/unit" && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I../../include -I../../third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c test_target.cpp -o ../../build/test/unit/test_target.o) diff --git a/bootstrap-macos.sh b/bootstrap-macos.sh index e1cc28d7..efb6a143 100755 --- a/bootstrap-macos.sh +++ b/bootstrap-macos.sh @@ -19,6 +19,7 @@ mkdir -p "build/test/unit" (cd "." && clang++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/context.cpp -o build/context.o) (cd "." && clang++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/config_commands.cpp -o build/config_commands.o) (cd "." && clang++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/cmd_parse.cpp -o build/cmd_parse.o) +(cd "." && clang++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/strict_checks.cpp -o build/strict_checks.o) (cd "." && clang++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/cmd_show.cpp -o build/cmd_show.o) (cd "." && clang++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/cmd_configure.cpp -o build/cmd_configure.o) (cd "." && clang++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/cmd_clean.cpp -o build/cmd_clean.o) @@ -61,8 +62,8 @@ mkdir -p "build/test/unit" (cd "." && clang++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/core/hash.cpp -o build/hash.o) (cd "." && clang++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/core/arena.cpp -o build/arena.o) (cd "." && clang -std=c11 -Wall -Wextra -Werror -fPIC -I./third_party -O2 -ffunction-sections -fdata-sections -Wno-error -c third_party/sha256/sha256.c -o build/sha256.o) -(cd "." && clang++ build/sha256.o build/arena.o build/hash.o build/id_array.o build/id_bitset.o build/layout.o build/metrics.o build/path.o build/path_utils.o build/sorted_id_vec.o build/string_pool.o build/global_pool.o build/buf.o build/heap_buf.o build/string_utils.o build/terminal.o build/ast.o build/config.o build/depfile.o build/eval.o build/glob.o build/ignore.o build/lexer.o build/parser.o build/var_tracking.o build/builder.o build/dag.o build/dep_scanner.o build/rule_pattern.o build/gcc.o build/topo.o build/entry.o build/reader.o build/writer.o build/progress_display.o build/runner.o build/scheduler.o build/cmd_build.o build/cmd_clean.o build/cmd_configure.o build/cmd_show.o build/cmd_parse.o build/config_commands.o build/context.o build/multi_variant.o build/options.o build/output.o build/target.o build/process-posix.o build/file_io-posix.o build/env-posix.o build/path-posix.o build/main.o build/throw_stubs.o -o build/putup -nostdlib++ -Wl,-dead_strip ) -(cd "." && ar rcs build/libputup.a build/sha256.o build/arena.o build/hash.o build/id_array.o build/id_bitset.o build/layout.o build/metrics.o build/path.o build/path_utils.o build/sorted_id_vec.o build/string_pool.o build/global_pool.o build/buf.o build/heap_buf.o build/string_utils.o build/terminal.o build/ast.o build/config.o build/depfile.o build/eval.o build/glob.o build/ignore.o build/lexer.o build/parser.o build/var_tracking.o build/builder.o build/dag.o build/dep_scanner.o build/rule_pattern.o build/gcc.o build/topo.o build/entry.o build/reader.o build/writer.o build/progress_display.o build/runner.o build/scheduler.o build/cmd_build.o build/cmd_clean.o build/cmd_configure.o build/cmd_show.o build/cmd_parse.o build/config_commands.o build/context.o build/multi_variant.o build/options.o build/output.o build/target.o build/process-posix.o build/file_io-posix.o build/env-posix.o build/path-posix.o) +(cd "." && clang++ build/sha256.o build/arena.o build/hash.o build/id_array.o build/id_bitset.o build/layout.o build/metrics.o build/path.o build/path_utils.o build/sorted_id_vec.o build/string_pool.o build/global_pool.o build/buf.o build/heap_buf.o build/string_utils.o build/terminal.o build/ast.o build/config.o build/depfile.o build/eval.o build/glob.o build/ignore.o build/lexer.o build/parser.o build/var_tracking.o build/builder.o build/dag.o build/dep_scanner.o build/rule_pattern.o build/gcc.o build/topo.o build/entry.o build/reader.o build/writer.o build/progress_display.o build/runner.o build/scheduler.o build/cmd_build.o build/cmd_clean.o build/cmd_configure.o build/cmd_show.o build/cmd_parse.o build/strict_checks.o build/config_commands.o build/context.o build/multi_variant.o build/options.o build/output.o build/target.o build/process-posix.o build/file_io-posix.o build/env-posix.o build/path-posix.o build/main.o build/throw_stubs.o -o build/putup -nostdlib++ -Wl,-dead_strip ) +(cd "." && ar rcs build/libputup.a build/sha256.o build/arena.o build/hash.o build/id_array.o build/id_bitset.o build/layout.o build/metrics.o build/path.o build/path_utils.o build/sorted_id_vec.o build/string_pool.o build/global_pool.o build/buf.o build/heap_buf.o build/string_utils.o build/terminal.o build/ast.o build/config.o build/depfile.o build/eval.o build/glob.o build/ignore.o build/lexer.o build/parser.o build/var_tracking.o build/builder.o build/dag.o build/dep_scanner.o build/rule_pattern.o build/gcc.o build/topo.o build/entry.o build/reader.o build/writer.o build/progress_display.o build/runner.o build/scheduler.o build/cmd_build.o build/cmd_clean.o build/cmd_configure.o build/cmd_show.o build/cmd_parse.o build/strict_checks.o build/config_commands.o build/context.o build/multi_variant.o build/options.o build/output.o build/target.o build/process-posix.o build/file_io-posix.o build/env-posix.o build/path-posix.o) (cd "test/unit" && clang++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I../../include -I../../third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c test_var_tracking.cpp -o ../../build/test/unit/test_var_tracking.o) (cd "test/unit" && clang++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I../../include -I../../third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c test_types.cpp -o ../../build/test/unit/test_types.o) (cd "test/unit" && clang++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -I../../include -I../../third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c test_target.cpp -o ../../build/test/unit/test_target.o) diff --git a/bootstrap-mingw.sh b/bootstrap-mingw.sh index 0205eecb..0d0ca5a7 100755 --- a/bootstrap-mingw.sh +++ b/bootstrap-mingw.sh @@ -19,6 +19,7 @@ mkdir -p "build/test/unit" (cd "." && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -Wno-error=free-nonheap-object -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/context.cpp -o build/context.o) (cd "." && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -Wno-error=free-nonheap-object -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/config_commands.cpp -o build/config_commands.o) (cd "." && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -Wno-error=free-nonheap-object -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/cmd_parse.cpp -o build/cmd_parse.o) +(cd "." && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -Wno-error=free-nonheap-object -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/strict_checks.cpp -o build/strict_checks.o) (cd "." && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -Wno-error=free-nonheap-object -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/cmd_show.cpp -o build/cmd_show.o) (cd "." && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -Wno-error=free-nonheap-object -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/cmd_configure.cpp -o build/cmd_configure.o) (cd "." && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -Wno-error=free-nonheap-object -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/cli/cmd_clean.cpp -o build/cmd_clean.o) @@ -61,8 +62,8 @@ mkdir -p "build/test/unit" (cd "." && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -Wno-error=free-nonheap-object -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/core/hash.cpp -o build/hash.o) (cd "." && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -Wno-error=free-nonheap-object -I./include -I./third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c src/core/arena.cpp -o build/arena.o) (cd "." && gcc -std=c11 -Wall -Wextra -Werror -fPIC -I./third_party -O2 -ffunction-sections -fdata-sections -Wno-error -c third_party/sha256/sha256.c -o build/sha256.o) -(cd "." && g++ build/sha256.o build/arena.o build/hash.o build/id_array.o build/id_bitset.o build/layout.o build/metrics.o build/path.o build/path_utils.o build/sorted_id_vec.o build/string_pool.o build/global_pool.o build/buf.o build/heap_buf.o build/string_utils.o build/terminal.o build/ast.o build/config.o build/depfile.o build/eval.o build/glob.o build/ignore.o build/lexer.o build/parser.o build/var_tracking.o build/builder.o build/dag.o build/dep_scanner.o build/rule_pattern.o build/gcc.o build/topo.o build/entry.o build/reader.o build/writer.o build/progress_display.o build/runner.o build/scheduler.o build/cmd_build.o build/cmd_clean.o build/cmd_configure.o build/cmd_show.o build/cmd_parse.o build/config_commands.o build/context.o build/multi_variant.o build/options.o build/output.o build/target.o build/process-win32.o build/file_io-win32.o build/env-win32.o build/path-win32.o build/main.o build/throw_stubs.o -o build/putup.exe -nostdlib++ -Wl,--gc-sections -static ) -(cd "." && ar rcs build/libputup.a build/sha256.o build/arena.o build/hash.o build/id_array.o build/id_bitset.o build/layout.o build/metrics.o build/path.o build/path_utils.o build/sorted_id_vec.o build/string_pool.o build/global_pool.o build/buf.o build/heap_buf.o build/string_utils.o build/terminal.o build/ast.o build/config.o build/depfile.o build/eval.o build/glob.o build/ignore.o build/lexer.o build/parser.o build/var_tracking.o build/builder.o build/dag.o build/dep_scanner.o build/rule_pattern.o build/gcc.o build/topo.o build/entry.o build/reader.o build/writer.o build/progress_display.o build/runner.o build/scheduler.o build/cmd_build.o build/cmd_clean.o build/cmd_configure.o build/cmd_show.o build/cmd_parse.o build/config_commands.o build/context.o build/multi_variant.o build/options.o build/output.o build/target.o build/process-win32.o build/file_io-win32.o build/env-win32.o build/path-win32.o) +(cd "." && g++ build/sha256.o build/arena.o build/hash.o build/id_array.o build/id_bitset.o build/layout.o build/metrics.o build/path.o build/path_utils.o build/sorted_id_vec.o build/string_pool.o build/global_pool.o build/buf.o build/heap_buf.o build/string_utils.o build/terminal.o build/ast.o build/config.o build/depfile.o build/eval.o build/glob.o build/ignore.o build/lexer.o build/parser.o build/var_tracking.o build/builder.o build/dag.o build/dep_scanner.o build/rule_pattern.o build/gcc.o build/topo.o build/entry.o build/reader.o build/writer.o build/progress_display.o build/runner.o build/scheduler.o build/cmd_build.o build/cmd_clean.o build/cmd_configure.o build/cmd_show.o build/cmd_parse.o build/strict_checks.o build/config_commands.o build/context.o build/multi_variant.o build/options.o build/output.o build/target.o build/process-win32.o build/file_io-win32.o build/env-win32.o build/path-win32.o build/main.o build/throw_stubs.o -o build/putup.exe -nostdlib++ -Wl,--gc-sections -static ) +(cd "." && ar rcs build/libputup.a build/sha256.o build/arena.o build/hash.o build/id_array.o build/id_bitset.o build/layout.o build/metrics.o build/path.o build/path_utils.o build/sorted_id_vec.o build/string_pool.o build/global_pool.o build/buf.o build/heap_buf.o build/string_utils.o build/terminal.o build/ast.o build/config.o build/depfile.o build/eval.o build/glob.o build/ignore.o build/lexer.o build/parser.o build/var_tracking.o build/builder.o build/dag.o build/dep_scanner.o build/rule_pattern.o build/gcc.o build/topo.o build/entry.o build/reader.o build/writer.o build/progress_display.o build/runner.o build/scheduler.o build/cmd_build.o build/cmd_clean.o build/cmd_configure.o build/cmd_show.o build/cmd_parse.o build/strict_checks.o build/config_commands.o build/context.o build/multi_variant.o build/options.o build/output.o build/target.o build/process-win32.o build/file_io-win32.o build/env-win32.o build/path-win32.o) (cd "test/unit" && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -Wno-error=free-nonheap-object -I../../include -I../../third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c test_var_tracking.cpp -o ../../build/test/unit/test_var_tracking.o) (cd "test/unit" && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -Wno-error=free-nonheap-object -I../../include -I../../third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c test_types.cpp -o ../../build/test/unit/test_types.o) (cd "test/unit" && g++ -std=c++20 -Wall -Wextra -Werror -Wpedantic -fPIC -fno-exceptions -fno-rtti -Wno-error=free-nonheap-object -I../../include -I../../third_party -O2 -DNDEBUG -ffunction-sections -fdata-sections -c test_target.cpp -o ../../build/test/unit/test_target.o) diff --git a/docs/reference.md b/docs/reference.md index 12c07d1e..d41538a1 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -179,6 +179,22 @@ Supports path-based variant and scope selection. - Path-based targets: `putup parse build-debug`, `putup parse build-*` - Legacy `-B` flag still works for explicit selection +**Convention checking (`--strict`)** + +Verify Tupfiles follow dual-mode composability conventions: + +```bash +putup parse --strict +``` + +Checks that component libraries (directories with their own `Tuprules.tup`) follow conventions that allow building both standalone and as part of a larger project: + +- **Error:** Anchor variables `S` and `B` must use `?=` (not `=`) in component `Tuprules.tup` +- **Warning:** Toolchain variables (`CC`, `CXX`, `AR`, etc.) should use `?=` +- **Warning:** Component directories should contain `Tupfile.ini` for standalone builds + +Exit code is non-zero if any errors are found. Warnings print but don't fail. + **Examples:** ```bash putup parse # Validate all Tupfiles (auto-detects variants) @@ -186,6 +202,7 @@ putup parse -v # Show parsing progress putup parse build-debug # Parse single variant (path-based) putup parse build-* # Parse all matching variants putup parse build-debug/lib # Parse scoped to lib/ directory +putup parse --strict # Check convention compliance ``` ### 3.3 putup clean @@ -1648,6 +1665,8 @@ S = .. GMP_DIR = ../gmp → $(S)/$(GMP_DIR) = ../../gmp ✓ **Scoped `tup.config` defaults:** Components ship default config values in a `defaults.config` file with a Tupfile copy rule (`cp %f %o`) that installs it as `tup.config` during configure. Parent configs override child configs on collision (see §6.1 *Scoped Config Merging*). +Use `putup parse --strict` to verify that component `Tuprules.tup` files follow these conventions (see §3.2 *Convention checking*). + See `examples/bsp/gcc/` for a complete working example with three interdependent libraries. ## 8. Implicit Dependencies diff --git a/docs/superpowers/plans/2026-03-29-strict-checker.md b/docs/superpowers/plans/2026-03-29-strict-checker.md new file mode 100644 index 00000000..f0057aa8 --- /dev/null +++ b/docs/superpowers/plans/2026-03-29-strict-checker.md @@ -0,0 +1,713 @@ +# Strict Convention Checker Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add `putup parse --strict` that verifies Tupfiles follow dual-mode composability conventions (2 error checks + 2 warning checks). + +**Architecture:** Pure stateless check functions inspect AST nodes during evaluation via a new `on_statement` callback on `EvalContext`. Check results are `Vec`. The `cmd_parse` wiring collects diagnostics, prints them, and fails on errors. + +**Tech Stack:** Existing AST types (`Assignment`, `Expression`, `VarRef`), `EvalContext` callback pattern, `Buf` for message formatting. + +--- + +### Task 1: Add Diagnostic type and check_assignment function + +**Files:** +- Create: `include/pup/cli/strict_checks.hpp` +- Create: `src/cli/strict_checks.cpp` + +- [ ] **Step 1: Create the header with Diagnostic and check_assignment declaration** + +```cpp +// include/pup/cli/strict_checks.hpp +// SPDX-License-Identifier: MIT +// Copyright (c) 2024 Putup authors + +#pragma once + +#include "pup/core/string_id.hpp" +#include "pup/core/vec.hpp" +#include "pup/parser/ast.hpp" + +#include + +namespace pup::cli { + +struct Diagnostic { + StringId file = StringId::Empty; + std::size_t line = 0; + enum Severity { Warning, Error } severity = Warning; + StringId message = StringId::Empty; +}; + +/// Check an assignment statement for convention violations. +/// Only produces diagnostics for component Tuprules.tup files (not root). +[[nodiscard]] +auto check_assignment( + parser::Assignment const& stmt, + std::string_view file, + bool is_component +) -> Vec; + +/// Check component directories for filesystem-level conventions. +[[nodiscard]] +auto check_component_dirs( + Vec const& component_dirs +) -> Vec; + +} // namespace pup::cli +``` + +- [ ] **Step 2: Create the implementation with check_assignment** + +```cpp +// src/cli/strict_checks.cpp +// SPDX-License-Identifier: MIT +// Copyright (c) 2024 Putup authors + +#include "pup/cli/strict_checks.hpp" +#include "pup/core/buf.hpp" +#include "pup/core/global_pool.hpp" +#include "pup/core/string_pool.hpp" +#include "pup/platform/file_io.hpp" + +#include + +namespace pup::cli { + +namespace { + +auto is_tuprules(std::string_view file) -> bool +{ + auto name = pup::path::filename(file); + return name == "Tuprules.tup"; +} + +auto make_diag( + std::string_view file, + std::size_t line, + Diagnostic::Severity severity, + std::string_view msg +) -> Diagnostic +{ + auto& pool = global_pool(); + return Diagnostic { + .file = pool.intern(file), + .line = line, + .severity = severity, + .message = pool.intern(msg), + }; +} + +// Known toolchain variable names +auto is_toolchain_var(std::string_view name) -> bool +{ + return name == "CC" || name == "CXX" || name == "AR" || name == "LD" + || name == "AS" || name == "HOSTCC" || name == "HOSTCXX" + || name == "RANLIB" || name == "STRIP" || name == "OBJCOPY" || name == "NM"; +} + +/// Check if an Expression is exactly [Variable{name}] +auto is_single_var(parser::Expression const& expr, std::string_view var_name) -> bool +{ + if (expr.parts.size() != 1) { + return false; + } + auto const* var = std::get_if(&expr.parts[0]); + if (!var) { + return false; + } + return var->ref.kind == parser::VarRef::Kind::Regular + && global_pool().get(var->ref.name) == var_name; +} + +/// Check if an Expression contains a Variable with the given name +auto contains_var(parser::Expression const& expr, std::string_view var_name) -> bool +{ + for (auto const& part : expr.parts) { + auto const* var = std::get_if(&part); + if (var && var->ref.kind == parser::VarRef::Kind::Regular + && global_pool().get(var->ref.name) == var_name) { + return true; + } + } + return false; +} + +} // namespace + +auto check_assignment( + parser::Assignment const& stmt, + std::string_view file, + bool is_component +) -> Vec +{ + auto result = Vec {}; + + // Only check component Tuprules.tup files + if (!is_component || !is_tuprules(file)) { + return result; + } + + auto& pool = global_pool(); + auto name_sv = stmt.name.as_literal(); + if (name_sv.empty()) { + return result; // Dynamic variable name — skip + } + + // E1: S must use ?= in component Tuprules.tup + if (name_sv == "S") { + if (stmt.op != parser::Assignment::Op::SoftSet) { + auto buf = Buf {}; + buf.fmt("'S' must use '?=' in component Tuprules.tup (found '='). " + "Using '=' overwrites the parent project's root anchor."); + result.push_back(make_diag(file, stmt.location.line, Diagnostic::Error, buf.view())); + } else if (!is_single_var(stmt.value, "TUP_CWD")) { + auto buf = Buf {}; + buf.fmt("'S' should be set to '$(TUP_CWD)' in component Tuprules.tup."); + result.push_back(make_diag(file, stmt.location.line, Diagnostic::Warning, buf.view())); + } + return result; + } + + // E2: B must use ?= in component Tuprules.tup + if (name_sv == "B") { + if (stmt.op != parser::Assignment::Op::SoftSet) { + auto buf = Buf {}; + buf.fmt("'B' must use '?=' in component Tuprules.tup (found '='). " + "Using '=' overwrites the parent project's build anchor."); + result.push_back(make_diag(file, stmt.location.line, Diagnostic::Error, buf.view())); + } else if (!contains_var(stmt.value, "S")) { + auto buf = Buf {}; + buf.fmt("'B' should reference '$(S)' in its value."); + result.push_back(make_diag(file, stmt.location.line, Diagnostic::Warning, buf.view())); + } + return result; + } + + // W1: Toolchain variables should use ?= + if (is_toolchain_var(name_sv) && stmt.op == parser::Assignment::Op::Set) { + auto buf = Buf {}; + buf.fmt("'{}' should use '?=' in component Tuprules.tup. " + "Using '=' overrides the parent project's toolchain choice.", name_sv); + result.push_back(make_diag(file, stmt.location.line, Diagnostic::Warning, buf.view())); + } + + return result; +} + +auto check_component_dirs( + Vec const& component_dirs +) -> Vec +{ + auto result = Vec {}; + + for (auto dir : component_dirs) { + auto ini_path = global_pool().get(pup::path::join(dir, "Tupfile.ini")); + if (!pup::platform::exists(ini_path)) { + auto buf = Buf {}; + buf.fmt("Component directory '{}' has no Tupfile.ini — cannot be built standalone.", dir); + result.push_back(make_diag(dir, 0, Diagnostic::Warning, buf.view())); + } + } + + return result; +} + +} // namespace pup::cli +``` + +- [ ] **Step 3: Add to Tupfile** + +Add `srcs-y += src/cli/strict_checks.cpp` to `Tupfile` after the other cli sources. + +- [ ] **Step 4: Verify it compiles** + +```bash +make 2>&1 | tail -5 +``` + +Expected: Build succeeds. New code is not yet called. + +- [ ] **Step 5: Commit** + +```bash +git add include/pup/cli/strict_checks.hpp src/cli/strict_checks.cpp Tupfile +git commit -m "Add Diagnostic type and check_assignment/check_component_dirs functions" +``` + +--- + +### Task 2: Write unit tests for check functions + +**Files:** +- Create: `test/unit/test_strict_checks.cpp` +- Modify: `test/unit/Tupfile` + +- [ ] **Step 1: Write tests** + +```cpp +// test/unit/test_strict_checks.cpp +// SPDX-License-Identifier: MIT +// Copyright (c) 2024 Putup authors + +#include "catch_amalgamated.hpp" +#include "pup/cli/strict_checks.hpp" +#include "pup/core/global_pool.hpp" +#include "pup/core/string_pool.hpp" + +using namespace pup::cli; +using pup::StringId; +using pup::global_pool; +using pup::parser::Assignment; +using pup::parser::Expression; +using pup::parser::VarRef; + +namespace { + +auto intern(std::string_view s) -> StringId { return global_pool().intern(s); } +auto sv(StringId id) -> std::string_view { return global_pool().get(id); } + +auto make_literal_expr(std::string_view text) -> Expression +{ + auto expr = Expression {}; + expr.parts.push_back(Expression::Literal { intern(text) }); + return expr; +} + +auto make_var_expr(std::string_view var_name) -> Expression +{ + auto expr = Expression {}; + expr.parts.push_back(Expression::Variable { VarRef { VarRef::Kind::Regular, intern(var_name), {} } }); + return expr; +} + +auto make_assignment(std::string_view name, Assignment::Op op, Expression value) -> Assignment +{ + auto stmt = Assignment {}; + stmt.name = make_literal_expr(name); + stmt.op = op; + stmt.value = std::move(value); + stmt.location.line = 1; + return stmt; +} + +} // namespace + +TEST_CASE("check_assignment: S anchor variable", "[strict]") +{ + SECTION("S ?= $(TUP_CWD) in component — no diagnostic") + { + auto stmt = make_assignment("S", Assignment::Op::SoftSet, make_var_expr("TUP_CWD")); + auto diags = check_assignment(stmt, "libfoo/Tuprules.tup", true); + REQUIRE(diags.empty()); + } + + SECTION("S = $(TUP_CWD) in component — error") + { + auto stmt = make_assignment("S", Assignment::Op::Set, make_var_expr("TUP_CWD")); + auto diags = check_assignment(stmt, "libfoo/Tuprules.tup", true); + REQUIRE(diags.size() == 1); + REQUIRE(diags[0].severity == Diagnostic::Error); + REQUIRE(sv(diags[0].message).find("must use '?='") != std::string_view::npos); + } + + SECTION("S = $(TUP_CWD) in root — no diagnostic (exempt)") + { + auto stmt = make_assignment("S", Assignment::Op::Set, make_var_expr("TUP_CWD")); + auto diags = check_assignment(stmt, "Tuprules.tup", false); + REQUIRE(diags.empty()); + } + + SECTION("S ?= literal in component — warning") + { + auto stmt = make_assignment("S", Assignment::Op::SoftSet, make_literal_expr("..")); + auto diags = check_assignment(stmt, "libfoo/Tuprules.tup", true); + REQUIRE(diags.size() == 1); + REQUIRE(diags[0].severity == Diagnostic::Warning); + } +} + +TEST_CASE("check_assignment: B anchor variable", "[strict]") +{ + SECTION("B ?= with $(S) in component — no diagnostic") + { + auto expr = Expression {}; + expr.parts.push_back(Expression::Variable { VarRef { VarRef::Kind::Regular, intern("TUP_VARIANT_OUTPUTDIR"), {} } }); + expr.parts.push_back(Expression::Literal { intern("/") }); + expr.parts.push_back(Expression::Variable { VarRef { VarRef::Kind::Regular, intern("S"), {} } }); + + auto stmt = make_assignment("B", Assignment::Op::SoftSet, std::move(expr)); + auto diags = check_assignment(stmt, "libfoo/Tuprules.tup", true); + REQUIRE(diags.empty()); + } + + SECTION("B = ... in component — error") + { + auto stmt = make_assignment("B", Assignment::Op::Set, make_literal_expr("build")); + auto diags = check_assignment(stmt, "libfoo/Tuprules.tup", true); + REQUIRE(diags.size() == 1); + REQUIRE(diags[0].severity == Diagnostic::Error); + } +} + +TEST_CASE("check_assignment: toolchain variables", "[strict]") +{ + SECTION("CC ?= gcc in component — no diagnostic") + { + auto stmt = make_assignment("CC", Assignment::Op::SoftSet, make_literal_expr("gcc")); + auto diags = check_assignment(stmt, "libfoo/Tuprules.tup", true); + REQUIRE(diags.empty()); + } + + SECTION("CC = gcc in component — warning") + { + auto stmt = make_assignment("CC", Assignment::Op::Set, make_literal_expr("gcc")); + auto diags = check_assignment(stmt, "libfoo/Tuprules.tup", true); + REQUIRE(diags.size() == 1); + REQUIRE(diags[0].severity == Diagnostic::Warning); + } + + SECTION("CC = gcc in root — no diagnostic") + { + auto stmt = make_assignment("CC", Assignment::Op::Set, make_literal_expr("gcc")); + auto diags = check_assignment(stmt, "Tuprules.tup", false); + REQUIRE(diags.empty()); + } + + SECTION("CFLAGS = -O2 in component — no diagnostic (not toolchain)") + { + auto stmt = make_assignment("CFLAGS", Assignment::Op::Set, make_literal_expr("-O2")); + auto diags = check_assignment(stmt, "libfoo/Tuprules.tup", true); + REQUIRE(diags.empty()); + } +} + +TEST_CASE("check_assignment: non-Tuprules file — no diagnostic", "[strict]") +{ + auto stmt = make_assignment("S", Assignment::Op::Set, make_var_expr("TUP_CWD")); + auto diags = check_assignment(stmt, "libfoo/Tupfile", true); + REQUIRE(diags.empty()); +} + +TEST_CASE("check_component_dirs", "[strict]") +{ + SECTION("existing directory with Tupfile.ini — no diagnostic") + { + // Use the project root which has Tupfile.ini + auto dirs = pup::Vec { "." }; + auto diags = check_component_dirs(dirs); + REQUIRE(diags.empty()); + } + + SECTION("directory without Tupfile.ini — warning") + { + auto dirs = pup::Vec { "/tmp/nonexistent_dir_for_test" }; + auto diags = check_component_dirs(dirs); + REQUIRE(diags.size() == 1); + REQUIRE(diags[0].severity == Diagnostic::Warning); + } +} +``` + +- [ ] **Step 2: Add to test Tupfile** + +Add `test-srcs-y += test_strict_checks.cpp` after the existing test sources in `test/unit/Tupfile`. + +- [ ] **Step 3: Build and run tests** + +```bash +make 2>&1 | tail -5 +./build/test/unit/putup_test '[strict]' -s 2>&1 | tail -10 +``` + +Expected: All strict tests pass. + +- [ ] **Step 4: Commit** + +```bash +git add test/unit/test_strict_checks.cpp test/unit/Tupfile +git commit -m "Add unit tests for strict convention checks" +``` + +--- + +### Task 3: Add on_statement callback to EvalContext + +**Files:** +- Modify: `include/pup/parser/eval.hpp` + +- [ ] **Step 1: Add the callback** + +Add after the existing `on_env_var_used` callback (around line 156): + +```cpp + /// Callback for strict convention checking. + /// Called with each statement and the directory it belongs to. + Function on_statement = {}; +``` + +- [ ] **Step 2: Fire the callback in the evaluator** + +In `src/graph/builder.cpp`, find where statements are processed. Look for the loop that iterates over `tupfile.statements` (in the function that evaluates a parsed Tupfile). Add at the top of the loop body: + +```cpp +if (ctx.eval->on_statement) { + ctx.eval->on_statement(stmt, current_dir); +} +``` + +The exact location depends on the builder's evaluation loop. Read `builder.cpp` to find where each `Statement` is dispatched to `process_assignment`, `process_rule`, etc. Fire the callback before the dispatch. + +- [ ] **Step 3: Verify it compiles and tests pass** + +```bash +make 2>&1 | tail -5 +./build/test/unit/putup_test '~[e2e]' 2>&1 | tail -3 +``` + +Expected: Build succeeds, all tests pass (callback is never set yet). + +- [ ] **Step 4: Commit** + +```bash +git add include/pup/parser/eval.hpp src/graph/builder.cpp +git commit -m "Add on_statement callback to EvalContext for strict checking" +``` + +--- + +### Task 4: Wire --strict flag into cmd_parse + +**Files:** +- Modify: `include/pup/cli/options.hpp` +- Modify: `src/cli/cmd_parse.cpp` +- Modify: `src/cli/options.cpp` (if arg parsing is there) + +- [ ] **Step 1: Add strict flag to Options** + +Add to the `Options` struct in `options.hpp`: + +```cpp + bool strict = false; +``` + +- [ ] **Step 2: Parse --strict from command line** + +Find where `parse` command arguments are parsed (likely `src/cli/options.cpp`). Add `--strict` handling that sets `opts.strict = true`. + +- [ ] **Step 3: Wire the checker into parse_single_variant** + +In `src/cli/cmd_parse.cpp`, modify `parse_single_variant`: + +```cpp +#include "pup/cli/strict_checks.hpp" + +// ... inside parse_single_variant, before build_context(): + +auto diagnostics = Vec {}; +auto component_dirs = Vec {}; + +if (opts.strict) { + auto source_root_sv = pool.get(layout->source_root); + + ctx_opts.on_statement = [&](parser::Statement const& stmt, std::string_view dir) { + if (!stmt.is()) { + return; + } + auto const& assign = stmt.get(); + + // Determine if this is a component (has own Tuprules.tup, not root) + auto is_component = !dir.empty() && dir != "." && dir != source_root_sv; + + auto file_sv = /* reconstruct Tuprules.tup path from dir */; + auto diags = check_assignment(assign, file_sv, is_component); + for (auto& d : diags) { + diagnostics.push_back(std::move(d)); + } + }; +} + +// ... after build_context(), before printing graph: + +if (opts.strict) { + // Collect component dirs (dirs with Tuprules.tup that aren't root) + // ... detect from parsed_dirs or builder state + + auto fs_diags = check_component_dirs(component_dirs); + for (auto& d : fs_diags) { + diagnostics.push_back(std::move(d)); + } + + // Print diagnostics + auto has_errors = false; + for (auto const& d : diagnostics) { + auto severity_str = d.severity == Diagnostic::Error ? "error" : "warning"; + fprintf(stderr, "%s:%zu: %s: %s\n", + pool.get(d.file).data(), + d.line, + severity_str, + pool.get(d.message).data()); + if (d.severity == Diagnostic::Error) { + has_errors = true; + } + } + + if (has_errors) { + return EXIT_FAILURE; + } +} +``` + +Note: The exact wiring depends on how `build_context` exposes parsed directory info and how the `on_statement` callback receives the current file path. Read `builder.cpp` and `context.cpp` to determine the precise integration points. + +- [ ] **Step 4: Build and test** + +```bash +make 2>&1 | tail -5 +./build/test/unit/putup_test '~[e2e]' 2>&1 | tail -3 +``` + +- [ ] **Step 5: Commit** + +```bash +git add include/pup/cli/options.hpp src/cli/options.cpp src/cli/cmd_parse.cpp +git commit -m "Wire --strict flag into putup parse command" +``` + +--- + +### Task 5: Add E2E test + +**Files:** +- Create: `test/e2e/fixtures/strict_check/` +- Modify: `test/unit/test_e2e.cpp` + +- [ ] **Step 1: Create a fixture that violates conventions** + +```bash +mkdir -p test/e2e/fixtures/strict_check/libfoo +``` + +`test/e2e/fixtures/strict_check/Tupfile.fixture`: +``` +: |> echo ok |> +``` + +`test/e2e/fixtures/strict_check/Tuprules.tup.fixture`: +``` +S = $(TUP_CWD) +CC = gcc +``` + +`test/e2e/fixtures/strict_check/libfoo/Tuprules.tup.fixture`: +``` +S = $(TUP_CWD) +CC = gcc +``` + +`test/e2e/fixtures/strict_check/libfoo/Tupfile.fixture`: +``` +: |> echo libfoo |> +``` + +Note: libfoo uses `=` instead of `?=` for S and CC, and has no Tupfile.ini. + +- [ ] **Step 2: Write E2E test** + +```cpp +SCENARIO("Strict checker catches convention violations", "[e2e][strict]") +{ + GIVEN("a project with a component violating conventions") + { + auto f = E2EFixture { "strict_check" }; + REQUIRE(f.init().success()); + + WHEN("parse --strict is run") + { + auto result = f.pup({ "parse", "--strict" }); + + THEN("it fails with error diagnostics") + { + REQUIRE_FALSE(result.success()); + REQUIRE(result.stderr_output.find("must use '?='") != std::string::npos); + } + } + + WHEN("parse without --strict is run") + { + auto result = f.pup({ "parse" }); + + THEN("it succeeds (no strict checking)") + { + REQUIRE(result.success()); + } + } + } +} +``` + +- [ ] **Step 3: Build and run** + +```bash +make 2>&1 | tail -5 +./build/test/unit/putup_test '[strict]' -s 2>&1 | tail -10 +``` + +Expected: Both unit and E2E strict tests pass. + +- [ ] **Step 4: Commit** + +```bash +git add test/e2e/fixtures/strict_check/ test/unit/test_e2e.cpp +git commit -m "Add E2E test for --strict convention checker" +``` + +--- + +### Task 6: Update bootstrap scripts and documentation + +**Files:** +- Modify: `bootstrap-linux.sh`, `bootstrap-macos.sh`, `bootstrap-mingw.sh` +- Modify: `docs/reference.md` + +- [ ] **Step 1: Add strict_checks.cpp to bootstrap scripts** + +Add compile + link entries for `src/cli/strict_checks.cpp` → `build/strict_checks.o` in all three bootstrap scripts. Add `build/strict_checks.o` to the link and ar lines. + +- [ ] **Step 2: Document --strict in reference.md** + +Add to the `parse` subcommand section: + +```markdown +**Convention checking (`--strict`)** + +Verify Tupfiles follow dual-mode composability conventions: + +```bash +putup parse --strict +``` + +Checks that component libraries (directories with their own `Tuprules.tup`) follow conventions that allow building both standalone and as part of a larger project: + +- **Error:** Anchor variables `S` and `B` must use `?=` (not `=`) in component `Tuprules.tup` +- **Warning:** Toolchain variables (`CC`, `CXX`, `AR`, etc.) should use `?=` +- **Warning:** Component directories should contain `Tupfile.ini` for standalone builds + +Exit code is non-zero if any errors are found. Warnings print but don't fail. +``` + +- [ ] **Step 3: Build, test, format** + +```bash +make 2>&1 | tail -5 +./build/test/unit/putup_test 2>&1 | tail -3 +make format 2>&1 | tail -3 +``` + +- [ ] **Step 4: Commit** + +```bash +git add bootstrap-linux.sh bootstrap-macos.sh bootstrap-mingw.sh docs/reference.md +git commit -m "Update bootstrap scripts and docs for --strict flag" +``` diff --git a/docs/superpowers/specs/2026-03-29-strict-checker-design.md b/docs/superpowers/specs/2026-03-29-strict-checker-design.md new file mode 100644 index 00000000..54f1b0d4 --- /dev/null +++ b/docs/superpowers/specs/2026-03-29-strict-checker-design.md @@ -0,0 +1,139 @@ +# Strict Convention Checker for Dual-Mode Composability + +## Goal + +Add `putup parse --strict` that verifies Tupfiles follow conventions guaranteeing a project can be built both standalone and as a component of a larger project. + +## Motivation + +A library project with proper conventions can be built independently (`cd libfoo && putup -B build`) or as part of a larger project (`cd big-project && putup -B build`) with zero Tupfile changes. The conventions rely on `?=` defaults, anchor variables, and prefixed directory variables. Violations are silent -- the project builds fine standalone but breaks when composed. `--strict` catches these violations early. + +## Conventions Checked + +### Errors (non-zero exit) + +**E1: Anchor variable `S` must use `?=` in component Tuprules.tup.** + +`S ?= $(TUP_CWD)` sets the project-root anchor. With `=`, the component overwrites the parent's value during `include_rules` (root-to-leaf), breaking all `$(S)/...` path references in composed mode. + +Detection: Assignment AST where name is `"S"`, in a component Tuprules.tup (not root). Check that `op == SoftSet` and value Expression is `[Variable{TUP_CWD}]`. + +**E2: Anchor variable `B` must use `?=` in component Tuprules.tup.** + +`B ?= $(TUP_VARIANT_OUTPUTDIR)/$(S)` sets the build-root anchor. Same rationale as E1. + +Detection: Assignment AST where name is `"B"`, in a component Tuprules.tup. Check that `op == SoftSet` and value Expression contains `Variable{S}`. + +### Warnings (print but don't fail) + +**W1: Toolchain variables should use `?=` in component Tuprules.tup.** + +Variables `CC`, `CXX`, `AR`, `LD`, `AS`, `HOSTCC`, `HOSTCXX`, `RANLIB`, `STRIP`, `OBJCOPY`, `NM` with `=` in a component override the parent's toolchain choice. + +Detection: Assignment AST for known toolchain names, in a component Tuprules.tup. Check `op == SoftSet`. + +**W2: Component directory should contain `Tupfile.ini`.** + +Without `Tupfile.ini`, the component cannot be built standalone. + +Detection: Filesystem check after evaluation. Each component directory (has own Tuprules.tup, is not root) should contain `Tupfile.ini`. + +### Future phases (not in initial implementation) + +- `*_DIR` variables should use `?=` with default `.` in components +- `defaults.config` should exist in component directories +- Cross-directory group references should use anchored paths (`$(S)/$(DIR_VAR)/`) +- Rule input/output paths should not contain hardcoded `..` in Literal AST parts + +## Architecture + +### Data types + +```cpp +struct Diagnostic { + StringId file; + std::size_t line; + enum Severity { Warning, Error } severity; + StringId message; +}; +``` + +### Check functions (pure, stateless) + +```cpp +// Called per Assignment statement during evaluation +auto check_assignment( + parser::Assignment const& stmt, + std::string_view file, + std::string_view dir, + bool is_component +) -> Vec; + +// Called after evaluation for filesystem-level checks +auto check_component_dirs( + ProjectLayout const& layout, + Vec const& component_dirs +) -> Vec; +``` + +Each function takes input, returns diagnostics. No hidden state, no class. + +### Callback integration + +Add one callback slot to `EvalContext`: + +```cpp +Function on_statement = {}; +``` + +This fires for every statement during Tupfile evaluation. The `cmd_parse` wiring: + +1. Before `build_context()`: wire `on_statement` to call `check_assignment` for Assignment statements, append results to a `Vec` +2. Track which directories have their own `Tuprules.tup` (component detection) +3. After `build_context()`: call `check_component_dirs` for filesystem checks +4. Print diagnostics to stderr, return non-zero if any Error-severity diagnostics exist + +### Component detection + +A directory is a **component** if: +- It has its own `Tuprules.tup` +- It is not the project root (source_root) + +The root project is exempt from all checks -- it IS the authority for anchor variables and toolchain settings. + +### Severity escalation + +`--strict` uses natural severity (errors fail, warnings print). A future `--strict=error` could promote all warnings to errors for strict CI. + +## Files + +| File | Change | +|------|--------| +| `include/pup/cli/strict_checks.hpp` | New: Diagnostic struct, check functions | +| `src/cli/strict_checks.cpp` | New: check function implementations | +| `include/pup/parser/eval.hpp` | Add `on_statement` callback to EvalContext | +| `src/cli/cmd_parse.cpp` | Wire --strict flag, callbacks, print diagnostics | +| `include/pup/cli/options.hpp` | Add `bool strict` to parse options | +| `test/unit/test_strict_checks.cpp` | New: unit tests for check functions | + +## Testing + +Unit tests (pure function tests, no E2E): +- Assignment `S = $(TUP_CWD)` in component → Error +- Assignment `S ?= $(TUP_CWD)` in component → no diagnostic +- Assignment `S = $(TUP_CWD)` in root → no diagnostic (exempt) +- Assignment `CC = gcc` in component → Warning +- Assignment `CC ?= gcc` in component → no diagnostic +- Component dir without Tupfile.ini → Warning +- Component dir with Tupfile.ini → no diagnostic + +E2E test: create a fixture that violates conventions, run `putup parse --strict`, verify exit code and stderr output. + +Positive E2E test: `examples/bsp/gcc/` should pass `--strict` (after adding missing `Tupfile.ini` to sub-library dirs if needed). + +## What this does NOT do + +- Does not modify parsing, evaluation, or graph building +- Does not block builds -- only `parse --strict` runs checks +- Does not check rule command expressions (Phase 3, future) +- Does not auto-fix violations diff --git a/include/pup/cli/context.hpp b/include/pup/cli/context.hpp index 12bc1353..f83832b6 100644 --- a/include/pup/cli/context.hpp +++ b/include/pup/cli/context.hpp @@ -31,6 +31,9 @@ class Index; namespace pup::cli { +/// Callback type for statement inspection (strict checking) +using StatementCallback = Function; + /// Callback type for tracking variable assignments using VarAssignedCallback = Function targets = {}; Vec output_targets = {}; bool show_json = false; + bool strict = false; StringId config_file = StringId::Empty; }; diff --git a/include/pup/cli/strict_checks.hpp b/include/pup/cli/strict_checks.hpp new file mode 100644 index 00000000..3e3c90f1 --- /dev/null +++ b/include/pup/cli/strict_checks.hpp @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2024 Putup authors + +#pragma once + +#include "pup/core/string_id.hpp" +#include "pup/core/vec.hpp" +#include "pup/parser/ast.hpp" + +#include + +namespace pup::cli { + +struct Diagnostic { + StringId file = StringId::Empty; + std::size_t line = 0; + enum Severity { Warning, + Error } severity + = Warning; + StringId message = StringId::Empty; +}; + +/// Check an assignment statement for convention violations. +/// Only produces diagnostics for component Tuprules.tup files (not root). +[[nodiscard]] +auto check_assignment( + parser::Assignment const& stmt, + std::string_view file, + bool is_component +) -> Vec; + +/// Check component directories for filesystem-level conventions. +[[nodiscard]] +auto check_component_dirs( + Vec const& component_dirs +) -> Vec; + +} // namespace pup::cli diff --git a/include/pup/parser/eval.hpp b/include/pup/parser/eval.hpp index d7cfe595..42bb599e 100644 --- a/include/pup/parser/eval.hpp +++ b/include/pup/parser/eval.hpp @@ -155,6 +155,10 @@ struct EvalContext { /// Called with the variable name when an imported env var is accessed via $(VAR). Function on_env_var_used = {}; + /// Callback for strict convention checking. + /// Called with each statement and the directory it belongs to. + Function on_statement = {}; + /// String pool for resolving StringIds during variable lookup StringPool const* string_pool = nullptr; diff --git a/src/cli/cmd_parse.cpp b/src/cli/cmd_parse.cpp index 79bb5bf5..f72ca38b 100644 --- a/src/cli/cmd_parse.cpp +++ b/src/cli/cmd_parse.cpp @@ -4,12 +4,14 @@ #include "pup/cli/commands.hpp" #include "pup/cli/context.hpp" #include "pup/cli/multi_variant.hpp" +#include "pup/cli/strict_checks.hpp" #include "pup/core/global_pool.hpp" #include "pup/core/layout.hpp" #include "pup/core/path.hpp" #include "pup/core/string_pool.hpp" #include "pup/core/types.hpp" #include "pup/graph/dag.hpp" +#include "pup/platform/file_io.hpp" #include #include @@ -27,11 +29,35 @@ auto parse_single_variant(Options const& opts, std::string_view variant_name) -> return EXIT_FAILURE; } + auto diagnostics = Vec {}; + auto ctx_opts = BuildContextOptions { .verbose = opts.verbose, .parse_scopes = compute_build_scopes(opts, *layout), }; + if (opts.strict) { + auto source_root_sv = pool.get(layout->source_root); + + ctx_opts.on_statement = [&diagnostics, source_root_sv]( + parser::Statement const& stmt, + std::string_view /*dir*/ + ) { + auto const* assign = stmt.as(); + if (!assign) { + return; + } + + auto file_sv = stmt.location.filename; + auto file_dir = pup::path::parent(file_sv); + auto is_component = !file_dir.empty() && file_dir != "." && file_dir != source_root_sv; + auto diags = check_assignment(*assign, file_sv, is_component); + for (auto& d : diags) { + diagnostics.push_back(std::move(d)); + } + }; + } + auto result = pup::Result { build_context(opts, ctx_opts) }; if (!result) { fprintf(stderr, "[%.*s] Error: %s\n", static_cast(variant_name.size()), variant_name.data(), result.error().msg().data()); @@ -69,6 +95,43 @@ auto parse_single_variant(Options const& opts, std::string_view variant_name) -> printf("[%.*s] Parsed %zu Tupfile(s), %zu commands\n", static_cast(variant_name.size()), variant_name.data(), ctx.parsed_dirs().size(), commands.size()); + if (opts.strict) { + auto component_dirs = Vec {}; + for (auto dir_id : ctx.parsed_dirs()) { + auto dir_sv = pool.get(dir_id); + if (dir_sv.empty() || dir_sv == ".") { + continue; + } + auto tuprules_path = pool.get(pup::path::join( + pool.get(pup::path::join(pool.get(ctx.layout().source_root), dir_sv)), + "Tuprules.tup" + )); + if (pup::platform::exists(tuprules_path)) { + component_dirs.push_back( + pool.get(pup::path::join(pool.get(ctx.layout().source_root), dir_sv)) + ); + } + } + + auto fs_diags = check_component_dirs(component_dirs); + for (auto& d : fs_diags) { + diagnostics.push_back(std::move(d)); + } + + auto has_errors = false; + for (auto const& d : diagnostics) { + auto severity_str = d.severity == Diagnostic::Error ? "error" : "warning"; + fprintf(stderr, "%.*s:%zu: %s: %.*s\n", static_cast(pool.get(d.file).size()), pool.get(d.file).data(), d.line, severity_str, static_cast(pool.get(d.message).size()), pool.get(d.message).data()); + if (d.severity == Diagnostic::Error) { + has_errors = true; + } + } + + if (has_errors) { + return EXIT_FAILURE; + } + } + return EXIT_SUCCESS; } diff --git a/src/cli/context.cpp b/src/cli/context.cpp index 2ac807e8..934feebb 100644 --- a/src/cli/context.cpp +++ b/src/cli/context.cpp @@ -391,6 +391,7 @@ struct ParseContext { bool verbose; bool root_config_only; VarAssignedCallback const* on_var_assigned; + StatementCallback const* on_statement; }; auto parse_directory(std::string_view rel_dir, ParseContext& ctx) -> pup::Result @@ -502,6 +503,13 @@ auto parse_directory(std::string_view rel_dir, ParseContext& ctx) -> pup::Result }; } + if (ctx.on_statement && *ctx.on_statement) { + auto const* cb = ctx.on_statement; + eval_ctx.on_statement = [cb](pup::parser::Statement const& stmt, std::string_view dir) { + (*cb)(stmt, dir); + }; + } + auto result = pup::Result { ctx.builder.add_tupfile(ctx.graph, parse_result.tupfile, eval_ctx) }; sorted_erase(ctx.state.parsing, normalized_dir); @@ -815,6 +823,7 @@ auto build_context( .verbose = ctx_opts.verbose, .root_config_only = ctx_opts.root_config_only, .on_var_assigned = &ctx_opts.on_var_assigned, + .on_statement = &ctx_opts.on_statement, }; for (auto dir_id : sort_dirs_by_depth(ctx.impl_->state.available)) { diff --git a/src/cli/options.cpp b/src/cli/options.cpp index bbf3f1f9..b0f82058 100644 --- a/src/cli/options.cpp +++ b/src/cli/options.cpp @@ -121,6 +121,8 @@ auto parse_args(int argc, char** argv) -> Options } } else if (arg == "--json") { opts.show_json = true; + } else if (arg == "--strict") { + opts.strict = true; } else if (!arg.starts_with("-")) { if (is_empty(opts.command) && is_command(arg)) { opts.command = pool.intern(arg); @@ -147,7 +149,7 @@ auto print_usage() -> void " configure Generate tup.config files (two-stage build)\n" " clean Remove generated files\n" " distclean Full reset: remove .pup and variant directory\n" - " parse Parse and validate Tupfiles\n" + " parse [--strict] Parse and validate Tupfiles\n" " show Show build info:\n" " script - Shell script\n" " compdb - compile_commands.json\n" diff --git a/src/cli/strict_checks.cpp b/src/cli/strict_checks.cpp new file mode 100644 index 00000000..49e121ce --- /dev/null +++ b/src/cli/strict_checks.cpp @@ -0,0 +1,147 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2024 Putup authors + +#include "pup/cli/strict_checks.hpp" +#include "pup/core/buf.hpp" +#include "pup/core/global_pool.hpp" +#include "pup/core/path.hpp" +#include "pup/core/string_pool.hpp" +#include "pup/platform/file_io.hpp" + +#include + +namespace pup::cli { + +namespace { + +auto is_tuprules(std::string_view file) -> bool +{ + return pup::path::filename(file) == "Tuprules.tup"; +} + +auto make_diag( + std::string_view file, + std::size_t line, + Diagnostic::Severity severity, + std::string_view msg +) -> Diagnostic +{ + auto& pool = global_pool(); + return Diagnostic { + .file = pool.intern(file), + .line = line, + .severity = severity, + .message = pool.intern(msg), + }; +} + +auto is_toolchain_var(std::string_view name) -> bool +{ + return name == "CC" || name == "CXX" || name == "AR" || name == "LD" + || name == "AS" || name == "HOSTCC" || name == "HOSTCXX" + || name == "RANLIB" || name == "STRIP" || name == "OBJCOPY" || name == "NM"; +} + +auto is_single_var(parser::Expression const& expr, std::string_view var_name) -> bool +{ + if (expr.parts.size() != 1) { + return false; + } + auto const* var = std::get_if(&expr.parts[0]); + if (!var) { + return false; + } + return var->ref.kind == parser::VarRef::Kind::Regular + && global_pool().get(var->ref.name) == var_name; +} + +auto contains_var(parser::Expression const& expr, std::string_view var_name) -> bool +{ + for (auto const& part : expr.parts) { + auto const* var = std::get_if(&part); + if (var && var->ref.kind == parser::VarRef::Kind::Regular + && global_pool().get(var->ref.name) == var_name) { + return true; + } + } + return false; +} + +} // namespace + +auto check_assignment( + parser::Assignment const& stmt, + std::string_view file, + bool is_component +) -> Vec +{ + auto result = Vec {}; + + if (!is_component || !is_tuprules(file)) { + return result; + } + + auto name_sv = stmt.name.as_literal(); + if (name_sv.empty()) { + return result; + } + + if (name_sv == "S") { + if (stmt.op != parser::Assignment::Op::SoftSet) { + auto buf = Buf {}; + buf.fmt("'S' must use '?=' in component Tuprules.tup (found '='). " + "Using '=' overwrites the parent project's root anchor."); + result.push_back(make_diag(file, stmt.location.line, Diagnostic::Error, buf.view())); + } else if (!is_single_var(stmt.value, "TUP_CWD")) { + auto buf = Buf {}; + buf.fmt("'S' should be set to '$(TUP_CWD)' in component Tuprules.tup."); + result.push_back(make_diag(file, stmt.location.line, Diagnostic::Warning, buf.view())); + } + return result; + } + + if (name_sv == "B") { + if (stmt.op != parser::Assignment::Op::SoftSet) { + auto buf = Buf {}; + buf.fmt("'B' must use '?=' in component Tuprules.tup (found '='). " + "Using '=' overwrites the parent project's build anchor."); + result.push_back(make_diag(file, stmt.location.line, Diagnostic::Error, buf.view())); + } else if (!contains_var(stmt.value, "S")) { + auto buf = Buf {}; + buf.fmt("'B' should reference '$(S)' in its value."); + result.push_back(make_diag(file, stmt.location.line, Diagnostic::Warning, buf.view())); + } + return result; + } + + if (is_toolchain_var(name_sv) && stmt.op == parser::Assignment::Op::Set) { + auto buf = Buf {}; + buf.fmt("'{}' should use '?=' in component Tuprules.tup. " + "Using '=' overrides the parent project's toolchain choice.", + name_sv); + result.push_back(make_diag(file, stmt.location.line, Diagnostic::Warning, buf.view())); + } + + return result; +} + +auto check_component_dirs( + Vec const& component_dirs +) -> Vec +{ + auto result = Vec {}; + auto& pool = global_pool(); + + for (auto dir : component_dirs) { + auto ini_path = pool.get(pup::path::join(dir, "Tupfile.ini")); + if (!pup::platform::exists(ini_path)) { + auto buf = Buf {}; + buf.fmt("Component directory '{}' has no Tupfile.ini — cannot be built standalone.", dir); + result.push_back(make_diag(dir, 0, Diagnostic::Warning, buf.view())); + } + } + + return result; +} + +} // namespace pup::cli diff --git a/src/core/throw_stubs.cpp b/src/core/throw_stubs.cpp index eb098be4..823263e8 100644 --- a/src/core/throw_stubs.cpp +++ b/src/core/throw_stubs.cpp @@ -86,3 +86,18 @@ void __throw_out_of_range_fmt(char const* /* fmt */, ...) } } // namespace std + +// libc++ (macOS) uses std::__1::__libcpp_verbose_abort for variant/optional +// errors. The __1 is libc++'s inline ABI namespace. We match it directly. +#ifdef __APPLE__ +namespace std { // NOLINT(cert-dcl58-cpp) +inline namespace __1 { +[[noreturn]] +void __libcpp_verbose_abort(char const* /* fmt */, ...) +{ + fputs("fatal: libc++ abort\n", stderr); + abort(); +} +} // namespace __1 +} // namespace std +#endif diff --git a/src/graph/builder.cpp b/src/graph/builder.cpp index c2e3b8e2..af2a55b5 100644 --- a/src/graph/builder.cpp +++ b/src/graph/builder.cpp @@ -833,6 +833,11 @@ auto process_statement( parser::Statement const& stmt ) -> Result { + if (ctx.eval && ctx.eval->on_statement) { + auto dir = is_empty(ctx.current_dir) ? std::string_view { "." } : str(ctx.current_dir); + ctx.eval->on_statement(stmt, dir); + } + if (auto const* rule = stmt.as()) { return process_rule(ctx, state, *rule); } diff --git a/test/e2e/fixtures/strict_check/Tupfile.fixture b/test/e2e/fixtures/strict_check/Tupfile.fixture new file mode 100644 index 00000000..2237a37b --- /dev/null +++ b/test/e2e/fixtures/strict_check/Tupfile.fixture @@ -0,0 +1,2 @@ +include_rules +: |> echo ok |> diff --git a/test/e2e/fixtures/strict_check/Tupfile.ini b/test/e2e/fixtures/strict_check/Tupfile.ini new file mode 100644 index 00000000..e69de29b diff --git a/test/e2e/fixtures/strict_check/Tuprules.tup b/test/e2e/fixtures/strict_check/Tuprules.tup new file mode 100644 index 00000000..d7379c7f --- /dev/null +++ b/test/e2e/fixtures/strict_check/Tuprules.tup @@ -0,0 +1,2 @@ +S = $(TUP_CWD) +CC = gcc diff --git a/test/e2e/fixtures/strict_check/libfoo/Tupfile.fixture b/test/e2e/fixtures/strict_check/libfoo/Tupfile.fixture new file mode 100644 index 00000000..b83e7967 --- /dev/null +++ b/test/e2e/fixtures/strict_check/libfoo/Tupfile.fixture @@ -0,0 +1,2 @@ +include_rules +: |> echo libfoo |> diff --git a/test/e2e/fixtures/strict_check/libfoo/Tuprules.tup b/test/e2e/fixtures/strict_check/libfoo/Tuprules.tup new file mode 100644 index 00000000..d7379c7f --- /dev/null +++ b/test/e2e/fixtures/strict_check/libfoo/Tuprules.tup @@ -0,0 +1,2 @@ +S = $(TUP_CWD) +CC = gcc diff --git a/test/unit/Tupfile b/test/unit/Tupfile index ab670329..60c3f62f 100644 --- a/test/unit/Tupfile +++ b/test/unit/Tupfile @@ -40,6 +40,7 @@ test-srcs-y += test_platform_file_io.cpp test-srcs-y += test_platform_process.cpp test-srcs-y += test_rule_pattern.cpp test-srcs-y += test_sorted_id_vec.cpp +test-srcs-y += test_strict_checks.cpp test-srcs-y += test_string_pool.cpp test-srcs-y += test_string_utils.cpp test-srcs-y += test_target.cpp diff --git a/test/unit/test_e2e.cpp b/test/unit/test_e2e.cpp index d707d16f..97b665ed 100644 --- a/test/unit/test_e2e.cpp +++ b/test/unit/test_e2e.cpp @@ -5312,3 +5312,41 @@ SCENARIO("Sibling directory inputs work with incremental variant builds", "[e2e] } } } + +// ============================================================================= +// Strict Convention Checker Tests +// ============================================================================= + +SCENARIO("Strict checker catches convention violations", "[e2e][strict]") +{ + GIVEN("a project with a component violating conventions") + { + auto f = E2EFixture { "strict_check" }; + REQUIRE(f.init().success()); + + WHEN("parse --strict is run") + { + auto result = f.pup({ "parse", "--strict" }); + + THEN("it fails with error diagnostics") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE_FALSE(result.success()); + REQUIRE(result.stderr_output.find("must use") != std::string::npos); + } + } + + WHEN("parse without --strict is run") + { + auto result = f.pup({ "parse" }); + + THEN("it succeeds (no strict checking)") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE(result.success()); + } + } + } +} diff --git a/test/unit/test_strict_checks.cpp b/test/unit/test_strict_checks.cpp new file mode 100644 index 00000000..227f8562 --- /dev/null +++ b/test/unit/test_strict_checks.cpp @@ -0,0 +1,157 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2024 Putup authors + +#include "catch_amalgamated.hpp" +#include "pup/cli/strict_checks.hpp" +#include "pup/core/global_pool.hpp" +#include "pup/core/string_pool.hpp" + +using namespace pup::cli; +using pup::StringId; +using pup::global_pool; +using pup::parser::Assignment; +using pup::parser::Expression; +using pup::parser::VarRef; + +namespace { + +auto intern(std::string_view s) -> StringId { return global_pool().intern(s); } +auto sv(StringId id) -> std::string_view { return global_pool().get(id); } + +auto make_literal_expr(std::string_view text) -> Expression +{ + auto expr = Expression {}; + expr.parts.push_back(Expression::Literal { intern(text) }); + return expr; +} + +auto make_var_expr(std::string_view var_name) -> Expression +{ + auto expr = Expression {}; + expr.parts.push_back(Expression::Variable { VarRef { VarRef::Kind::Regular, intern(var_name), {} } }); + return expr; +} + +auto make_assignment(std::string_view name, Assignment::Op op, Expression value) -> Assignment +{ + auto stmt = Assignment {}; + stmt.name = make_literal_expr(name); + stmt.op = op; + stmt.value = std::move(value); + stmt.location.line = 1; + return stmt; +} + +} // namespace + +TEST_CASE("check_assignment: S anchor variable", "[strict]") +{ + SECTION("S ?= $(TUP_CWD) in component — no diagnostic") + { + auto stmt = make_assignment("S", Assignment::Op::SoftSet, make_var_expr("TUP_CWD")); + auto diags = check_assignment(stmt, "libfoo/Tuprules.tup", true); + REQUIRE(diags.empty()); + } + + SECTION("S = $(TUP_CWD) in component — error") + { + auto stmt = make_assignment("S", Assignment::Op::Set, make_var_expr("TUP_CWD")); + auto diags = check_assignment(stmt, "libfoo/Tuprules.tup", true); + REQUIRE(diags.size() == 1); + REQUIRE(diags[0].severity == Diagnostic::Error); + REQUIRE(sv(diags[0].message).find("must use '?='") != std::string_view::npos); + } + + SECTION("S = $(TUP_CWD) in root — no diagnostic (exempt)") + { + auto stmt = make_assignment("S", Assignment::Op::Set, make_var_expr("TUP_CWD")); + auto diags = check_assignment(stmt, "Tuprules.tup", false); + REQUIRE(diags.empty()); + } + + SECTION("S ?= literal in component — warning") + { + auto stmt = make_assignment("S", Assignment::Op::SoftSet, make_literal_expr("..")); + auto diags = check_assignment(stmt, "libfoo/Tuprules.tup", true); + REQUIRE(diags.size() == 1); + REQUIRE(diags[0].severity == Diagnostic::Warning); + } +} + +TEST_CASE("check_assignment: B anchor variable", "[strict]") +{ + SECTION("B ?= with $(S) in component — no diagnostic") + { + auto expr = Expression {}; + expr.parts.push_back(Expression::Variable { VarRef { VarRef::Kind::Regular, intern("TUP_VARIANT_OUTPUTDIR"), {} } }); + expr.parts.push_back(Expression::Literal { intern("/") }); + expr.parts.push_back(Expression::Variable { VarRef { VarRef::Kind::Regular, intern("S"), {} } }); + + auto stmt = make_assignment("B", Assignment::Op::SoftSet, std::move(expr)); + auto diags = check_assignment(stmt, "libfoo/Tuprules.tup", true); + REQUIRE(diags.empty()); + } + + SECTION("B = ... in component — error") + { + auto stmt = make_assignment("B", Assignment::Op::Set, make_literal_expr("build")); + auto diags = check_assignment(stmt, "libfoo/Tuprules.tup", true); + REQUIRE(diags.size() == 1); + REQUIRE(diags[0].severity == Diagnostic::Error); + } +} + +TEST_CASE("check_assignment: toolchain variables", "[strict]") +{ + SECTION("CC ?= gcc in component — no diagnostic") + { + auto stmt = make_assignment("CC", Assignment::Op::SoftSet, make_literal_expr("gcc")); + auto diags = check_assignment(stmt, "libfoo/Tuprules.tup", true); + REQUIRE(diags.empty()); + } + + SECTION("CC = gcc in component — warning") + { + auto stmt = make_assignment("CC", Assignment::Op::Set, make_literal_expr("gcc")); + auto diags = check_assignment(stmt, "libfoo/Tuprules.tup", true); + REQUIRE(diags.size() == 1); + REQUIRE(diags[0].severity == Diagnostic::Warning); + } + + SECTION("CC = gcc in root — no diagnostic") + { + auto stmt = make_assignment("CC", Assignment::Op::Set, make_literal_expr("gcc")); + auto diags = check_assignment(stmt, "Tuprules.tup", false); + REQUIRE(diags.empty()); + } + + SECTION("CFLAGS = -O2 in component — no diagnostic (not toolchain)") + { + auto stmt = make_assignment("CFLAGS", Assignment::Op::Set, make_literal_expr("-O2")); + auto diags = check_assignment(stmt, "libfoo/Tuprules.tup", true); + REQUIRE(diags.empty()); + } +} + +TEST_CASE("check_assignment: non-Tuprules file — no diagnostic", "[strict]") +{ + auto stmt = make_assignment("S", Assignment::Op::Set, make_var_expr("TUP_CWD")); + auto diags = check_assignment(stmt, "libfoo/Tupfile", true); + REQUIRE(diags.empty()); +} + +TEST_CASE("check_component_dirs: missing Tupfile.ini — warning", "[strict]") +{ + auto dirs = pup::Vec { "/tmp/nonexistent_dir_for_test" }; + auto diags = check_component_dirs(dirs); + REQUIRE(diags.size() == 1); + REQUIRE(diags[0].severity == Diagnostic::Warning); + REQUIRE(sv(diags[0].message).find("no Tupfile.ini") != std::string_view::npos); +} + +TEST_CASE("check_component_dirs: empty list — no diagnostic", "[strict]") +{ + auto dirs = pup::Vec {}; + auto diags = check_component_dirs(dirs); + REQUIRE(diags.empty()); +}