From eae650a4a71e5ecb40110c5a8ebafd11ea3dbf5f Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 08:50:44 +0200 Subject: [PATCH 1/2] fix(build): preserve developer script paths and failures --- .vscode/tasks.json | 130 ++++++++++++++++------ scripts/conan_install | 2 + scripts/conan_lock | 2 + scripts/gen-vscode-env.py | 22 ++-- scripts/git_hooks/git-hooks | 53 ++++----- scripts/profile.sh | 8 ++ scripts/setup | 2 + scripts/tests/test_development_scripts.py | 85 ++++++++++++++ 8 files changed, 227 insertions(+), 77 deletions(-) create mode 100755 scripts/profile.sh create mode 100644 scripts/tests/test_development_scripts.py diff --git a/.vscode/tasks.json b/.vscode/tasks.json index 576b74009..2d35c3248 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -25,67 +25,115 @@ "tasks": [ { "label": "build: odr (library)", - "type": "shell", - "command": "cmake --build cmake-build-relwithdebinfo --target odr", + "type": "process", + "command": "cmake", "group": "build", - "problemMatcher": ["$gcc"] + "problemMatcher": [ + "$gcc" + ], + "args": [ + "--build", + "cmake-build-relwithdebinfo", + "--target", + "odr" + ] }, { "label": "build: odr_test", - "type": "shell", - "command": "cmake --build cmake-build-relwithdebinfo --target odr_test", + "type": "process", + "command": "cmake", "group": { "kind": "build", "isDefault": true }, - "problemMatcher": ["$gcc"] + "problemMatcher": [ + "$gcc" + ], + "args": [ + "--build", + "cmake-build-relwithdebinfo", + "--target", + "odr_test" + ] }, { "label": "build: translate", - "type": "shell", - "command": "cmake --build cmake-build-relwithdebinfo --target translate", + "type": "process", + "command": "cmake", "group": "build", - "problemMatcher": ["$gcc"] + "problemMatcher": [ + "$gcc" + ], + "args": [ + "--build", + "cmake-build-relwithdebinfo", + "--target", + "translate" + ] }, { "label": "build: all CLI tools", - "type": "shell", - "command": "cmake --build cmake-build-relwithdebinfo --target translate back_translate meta server", + "type": "process", + "command": "cmake", "group": "build", - "problemMatcher": ["$gcc"] + "problemMatcher": [ + "$gcc" + ], + "args": [ + "--build", + "cmake-build-relwithdebinfo", + "--target", + "translate", + "back_translate", + "meta", + "server" + ] }, { "label": "test: run odr_test (filtered)", - "type": "shell", - "command": "${workspaceFolder}/scripts/run-with-env.sh ./test/odr_test --gtest_filter='${input:gtestFilter}'", + "type": "process", + "command": "${workspaceFolder}/scripts/run-with-env.sh", "options": { "cwd": "${workspaceFolder}/cmake-build-relwithdebinfo" }, - "dependsOn": ["build: odr_test"], + "dependsOn": [ + "build: odr_test" + ], "group": { "kind": "test", "isDefault": true }, - "problemMatcher": [] + "problemMatcher": [], + "args": [ + "./test/odr_test", + "--gtest_filter=${input:gtestFilter}" + ] }, { "label": "cli: translate ", - "type": "shell", - "command": "scripts/run-with-env.sh ./cmake-build-relwithdebinfo/cli/translate '${input:translateInput}' '${input:translateOutput}'", - "dependsOn": ["build: translate"], - "problemMatcher": [] + "type": "process", + "command": "${workspaceFolder}/scripts/run-with-env.sh", + "dependsOn": [ + "build: translate" + ], + "problemMatcher": [], + "args": [ + "./cmake-build-relwithdebinfo/cli/translate", + "${input:translateInput}", + "${input:translateOutput}" + ] }, { "label": "compare: reference output (headless)", "detail": "Runs compare-html in Docker (Firefox headless) against test/data/reference-output. Build + run odr_test first to populate output.", - "type": "shell", + "type": "process", "command": "test/scripts/compare_output.sh", "problemMatcher": [] }, { "label": "compare: reference output (server on :8000)", "detail": "Runs compare-html-server in Docker; open http://localhost:8000 to inspect diffs interactively. Ctrl-C in the terminal to stop.", - "type": "shell", + "type": "process", "command": "test/scripts/compare_output_server.sh", "isBackground": true, "problemMatcher": [] @@ -93,28 +141,44 @@ { "label": "env: regenerate .vscode/.env", "detail": "Re-run after every `conan install` to refresh Conan cache paths.", - "type": "shell", - "command": "python3 scripts/gen-vscode-env.py", - "problemMatcher": [] + "type": "process", + "command": "python3", + "problemMatcher": [], + "args": [ + "scripts/gen-vscode-env.py" + ] }, { "label": "profile: Instruments Time Profiler (odr_test, filtered)", "detail": "Records a .trace with Instruments; opens in Instruments.app when done.", - "type": "shell", - "command": "rm -rf profile.trace && ${workspaceFolder}/scripts/run-with-env.sh xctrace record --template 'Time Profiler' --output profile.trace --launch -- ./test/odr_test --gtest_filter='${input:gtestFilter}' && open profile.trace", + "type": "process", + "command": "${workspaceFolder}/scripts/profile.sh", "options": { "cwd": "${workspaceFolder}/cmake-build-relwithdebinfo" }, - "dependsOn": ["build: odr_test"], - "problemMatcher": [] + "dependsOn": [ + "build: odr_test" + ], + "problemMatcher": [], + "args": [ + "./test/odr_test", + "--gtest_filter=${input:gtestFilter}" + ] }, { "label": "profile: Instruments Time Profiler (translate)", "detail": "Records a .trace of the translate CLI on one file.", - "type": "shell", - "command": "rm -rf profile.trace && scripts/run-with-env.sh xctrace record --template 'Time Profiler' --output profile.trace --launch -- ./cmake-build-relwithdebinfo/cli/translate '${input:translateInput}' '${input:translateOutput}' && open profile.trace", - "dependsOn": ["build: translate"], - "problemMatcher": [] + "type": "process", + "command": "${workspaceFolder}/scripts/profile.sh", + "dependsOn": [ + "build: translate" + ], + "problemMatcher": [], + "args": [ + "./cmake-build-relwithdebinfo/cli/translate", + "${input:translateInput}", + "${input:translateOutput}" + ] } ] } diff --git a/scripts/conan_install b/scripts/conan_install index 6107326fc..d9025e564 100755 --- a/scripts/conan_install +++ b/scripts/conan_install @@ -1,4 +1,6 @@ #!/usr/bin/env bash +set -euo pipefail +cd "$(dirname "${BASH_SOURCE[0]}")/.." conan install . --output-folder=cmake-build-relwithdebinfo --build=missing -s build_type=RelWithDebInfo -s "&:build_type=RelWithDebInfo" conan install . --output-folder=cmake-build-debug --build=missing -s build_type=RelWithDebInfo -s "&:build_type=Debug" diff --git a/scripts/conan_lock b/scripts/conan_lock index e9631b7b3..c75bdef37 100755 --- a/scripts/conan_lock +++ b/scripts/conan_lock @@ -1,4 +1,6 @@ #!/usr/bin/env bash +set -euo pipefail +cd "$(dirname "${BASH_SOURCE[0]}")/.." conan lock create . diff --git a/scripts/gen-vscode-env.py b/scripts/gen-vscode-env.py index 3dd12b975..50334f185 100755 --- a/scripts/gen-vscode-env.py +++ b/scripts/gen-vscode-env.py @@ -1,18 +1,13 @@ #!/usr/bin/env python3 -"""Extract the runtime environment from the Conan-generated CMake test preset -into a plain KEY=VALUE file that VS Code launch configs (CodeLLDB `envFile`) and -tasks can consume. +"""Export the Conan test environment for VS Code tasks and CodeLLDB. -The paths point into the Conan cache and change whenever dependencies are -re-installed, so re-run this after every `conan install` (there is a VS Code task -"env: regenerate .vscode/.env" that does exactly this). - -Usage: scripts/gen-vscode-env.py [BUILD_DIR] - BUILD_DIR defaults to cmake-build-relwithdebinfo. +Usage: scripts/gen-vscode-env.py [BUILD_DIR]. Regenerate after conan install. +BUILD_DIR defaults to cmake-build-relwithdebinfo. """ import json import os import re +import shlex import sys from pathlib import Path @@ -29,14 +24,17 @@ def resolve(value: str) -> str: - # Resolve `$penv{VAR}` (parent/current environment) references to the value - # present when this script runs; CodeLLDB envFile has no variable expansion. + # Resolve CMake parent-environment references before quoting literal values. return re.sub(r"\$penv\{(\w+)\}", lambda m: os.environ.get(m.group(1), ""), value) out = repo / ".vscode" / ".env" out.parent.mkdir(exist_ok=True) -lines = [f"{k}={resolve(v)}" for k, v in env.items()] +for key in env: + if not re.fullmatch(r"[A-Za-z_][A-Za-z0-9_]*", key): + sys.exit(f"invalid environment variable name: {key!r}") +# Single-quote expansion is shared by Bash and CodeLLDB's dotenv reader. +lines = [f"{k}={shlex.quote(resolve(v))}" for k, v in env.items()] out.write_text("\n".join(lines) + "\n") print(f"wrote {len(lines)} vars to {out}") for line in lines: diff --git a/scripts/git_hooks/git-hooks b/scripts/git_hooks/git-hooks index c653b7679..7eba3e6ca 100755 --- a/scripts/git_hooks/git-hooks +++ b/scripts/git_hooks/git-hooks @@ -12,8 +12,8 @@ function hook_dirs git rev-parse --git-dir &> /dev/null if [ $? -eq 0 ]; then - if [ $(git rev-parse --is-bare-repository) = 'false' ]; then - cd $(git rev-parse --show-toplevel) + if [ "$(git rev-parse --is-bare-repository)" = 'false' ]; then + cd "$(git rev-parse --show-toplevel)" || return echo "${PWD}/scripts/git_hooks${hook}" fi fi @@ -23,17 +23,13 @@ function list_hooks_in_dir { path="${1}" level="${2}" - find --help 2>&1 | grep -- '-L' 2>/dev/null >/dev/null - if [ $? -eq 1 ] ; then - find "${path}/" -mindepth ${level} -maxdepth ${level} -perm -111 -type f 2>/dev/null | grep -v "^.$" | sort - else - find -L "${path}/" -mindepth ${level} -maxdepth ${level} -perm -111 -type f 2>/dev/null | grep -v "^.$" | sort - fi + find -L "${path}/" -mindepth "${level}" -maxdepth "${level}" \ + -perm -111 -type f -print0 2>/dev/null | sort -z } function list_hooks { - GITDIR=`git rev-parse --git-dir` + GITDIR=$(git rev-parse --git-common-dir) cat "${GITDIR}/hooks/pre-commit" 2> /dev/null | grep 'git-hooks' > /dev/null 2> /dev/null if [ $? = 0 ]; then echo "Git hooks ARE installed in this repository." @@ -45,14 +41,11 @@ function list_hooks echo 'Listing Project hooks:' echo '---' - for dir in `hook_dirs`; do - echo "${dir}:" - for hook in `list_hooks_in_dir "${dir}" 2`; do - echo -n *`basename \`dirname "${hook}"\`` - echo -e "/`basename "${hook}"` - `${hook} --about`" - done - echo "" - done + dir="$(hook_dirs)" + echo "${dir}:" + while IFS= read -r -d '' hook; do + echo "*$(basename "$(dirname "${hook}")")/$(basename "${hook}") - $("${hook}" --about)" + done < <(list_hooks_in_dir "${dir}" 2) } function run_hooks @@ -63,16 +56,14 @@ function run_hooks return 1 fi shift 1 - for hook in `list_hooks_in_dir "${dir}" 1` - do - export last_run_hook="${hook} $@" - if [ ! -z ${GIT_HOOKS_VERBOSE} ]; then + while IFS= read -r -d '' hook; do + export last_run_hook="${hook} $*" + if [ -n "${GIT_HOOKS_VERBOSE:-}" ]; then echo -n "@@ Running hook: " - echo -n `basename \`dirname "${hook}"\`` - echo "/`basename "${hook}"`" + echo "$(basename "$(dirname "${hook}")")/$(basename "${hook}")" fi - ${hook} "$@" - done + "${hook}" "$@" + done < <(list_hooks_in_dir "${dir}" 1) } function run_hook @@ -84,12 +75,10 @@ function run_hook return 1 fi shift 1 - for dir in `hook_dirs "${hook}"`; do - if [ ! -d "${dir}" ] ; then - continue - fi + dir="$(hook_dirs "${hook}")" + if [ -d "${dir}" ]; then run_hooks "${dir}" "$@" - done + fi set +e } @@ -113,12 +102,12 @@ function install_hooks_into function install_hooks { - GITDIR=`git rev-parse --git-dir` + GITDIR=$(git rev-parse --git-common-dir) if [ ! $? -eq 0 ]; then echo "${1} must be run inside a git repository" return 1 fi - cd "${GITDIR}" + cd "${GITDIR}" || return if [ "${1}" = "--install" ]; then if [ -d hooks.old ]; then echo "hooks.old already exists, perhaps you already installed?" diff --git a/scripts/profile.sh b/scripts/profile.sh new file mode 100755 index 000000000..e88e99be9 --- /dev/null +++ b/scripts/profile.sh @@ -0,0 +1,8 @@ +#!/usr/bin/env bash +# Record the command in Instruments without replacing an earlier trace. +set -euo pipefail +repo="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +trace="$(mktemp -d "${TMPDIR:-/tmp}/odr-profile.XXXXXX")/profile.trace" +"$repo/scripts/run-with-env.sh" xctrace record --template 'Time Profiler' \ + --output "$trace" --launch -- "$@" +open "$trace" diff --git a/scripts/setup b/scripts/setup index 73e930099..c4d3f3764 100755 --- a/scripts/setup +++ b/scripts/setup @@ -1,4 +1,6 @@ #!/usr/bin/env bash +set -euo pipefail +cd "$(dirname "${BASH_SOURCE[0]}")/.." # setup git hooks scripts/git_hooks/git-hooks --install diff --git a/scripts/tests/test_development_scripts.py b/scripts/tests/test_development_scripts.py new file mode 100644 index 000000000..887ec42a6 --- /dev/null +++ b/scripts/tests/test_development_scripts.py @@ -0,0 +1,85 @@ +import json +import os +import shutil +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] + + +class DevelopmentScriptsTest(unittest.TestCase): + def test_generated_environment_preserves_literal_values(self): + with tempfile.TemporaryDirectory(prefix="odr scripts '") as directory: + root = Path(directory) + (root / "scripts").mkdir() + for name in ("gen-vscode-env.py", "run-with-env.sh"): + shutil.copy2(ROOT / "scripts" / name, root / "scripts" / name) + build = root / "build" + build.mkdir() + value = " space's \\\"quote\\\" # $HOME $(touch marker) `touch marker` \\path " + preset = {"testPresets": [{"environment": { + "ODR_LITERAL": value, "ODR_EMPTY": "", "ODR_PARENT": "$penv{ODR_PARENT}" + }}]} + (build / "CMakePresets.json").write_text(json.dumps(preset)) + subprocess.run([sys.executable, root / "scripts/gen-vscode-env.py", build], + cwd=root, env={**os.environ, "ODR_PARENT": "parent value"}, + check=True, capture_output=True) + result = subprocess.run( + [root / "scripts/run-with-env.sh", sys.executable, "-c", + "import json,os; print(json.dumps([os.environ[k] for k in " + "['ODR_LITERAL','ODR_EMPTY','ODR_PARENT']]))"], + cwd=root, check=True, capture_output=True, text=True, + ) + self.assertEqual(json.loads(result.stdout), [value, "", "parent value"]) + self.assertFalse((root / "marker").exists()) + + def test_setup_and_conan_stop_on_failure(self): + with tempfile.TemporaryDirectory(prefix="odr scripts ") as directory: + root = Path(directory) + (root / "scripts/git_hooks").mkdir(parents=True) + fake = '#!/bin/sh\nprintf "called\\n" >> calls\nexit 7\n' + for name in ("conan", "scripts/git_hooks/git-hooks"): + path = root / name + path.write_text(fake) + path.chmod(0o755) + for name in ("conan_install", "conan_lock", "setup"): + shutil.copy2(ROOT / "scripts" / name, root / "scripts" / name) + for name in ("conan_install", "conan_lock", "setup"): + with self.subTest(name=name): + (root / "calls").write_text("") + result = subprocess.run( + [root / "scripts" / name], cwd=root, capture_output=True, + env={**os.environ, "PATH": str(root) + os.pathsep + os.environ["PATH"]}, + ) + self.assertEqual(result.returncode, 7, result.stderr) + self.assertEqual((root / "calls").read_text(), "called\n") + + def test_hook_installation_from_a_worktree_with_spaces(self): + with tempfile.TemporaryDirectory(prefix="odr hooks ") as directory: + root = Path(directory) / "repo with spaces" + root.mkdir() + def git(*args, cwd=root, check=True): + return subprocess.run(["git", *args], cwd=cwd, check=check, + capture_output=True, text=True) + git("init", "-q") + git("config", "user.name", "Review") + git("config", "user.email", "review@example.invalid") + hooks = root / "scripts/git_hooks" + (hooks / "pre-commit").mkdir(parents=True) + shutil.copy2(ROOT / "scripts/git_hooks/git-hooks", hooks / "git-hooks") + hook = hooks / "pre-commit/reject" + hook.write_text("#!/bin/sh\nexit 7\n") + hook.chmod(0o755) + git("add", ".") + git("commit", "-qm", "initial") + worktree = Path(directory) / "worktree with spaces" + git("worktree", "add", "-qb", "review", str(worktree)) + subprocess.run([worktree / "scripts/git_hooks/git-hooks", "--install"], + cwd=worktree, check=True, capture_output=True) + self.assertTrue((root / ".git/hooks/pre-commit").exists()) + result = git("commit", "--allow-empty", "-m", "must fail", cwd=worktree, + check=False) + self.assertNotEqual(result.returncode, 0, result.stdout) From 80c5a8f8a57055f9dbeccf7efc3e5490fbcee44a Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 6 Oct 2026 20:46:18 +0200 Subject: [PATCH 2/2] chore: keep short task arrays on one line The switch to process tasks also spread every single-item array over three lines. Arrays that fit on one line stand on one line again, as in the rest of the file, so the diff shows only the real change. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01MxyTMutqSUJRGfxA8CyzMc --- .vscode/tasks.json | 67 ++++++++++------------------------------------ 1 file changed, 14 insertions(+), 53 deletions(-) diff --git a/.vscode/tasks.json b/.vscode/tasks.json index 2d35c3248..2a3fff2ed 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -28,15 +28,8 @@ "type": "process", "command": "cmake", "group": "build", - "problemMatcher": [ - "$gcc" - ], - "args": [ - "--build", - "cmake-build-relwithdebinfo", - "--target", - "odr" - ] + "problemMatcher": ["$gcc"], + "args": ["--build", "cmake-build-relwithdebinfo", "--target", "odr"] }, { "label": "build: odr_test", @@ -46,39 +39,23 @@ "kind": "build", "isDefault": true }, - "problemMatcher": [ - "$gcc" - ], - "args": [ - "--build", - "cmake-build-relwithdebinfo", - "--target", - "odr_test" - ] + "problemMatcher": ["$gcc"], + "args": ["--build", "cmake-build-relwithdebinfo", "--target", "odr_test"] }, { "label": "build: translate", "type": "process", "command": "cmake", "group": "build", - "problemMatcher": [ - "$gcc" - ], - "args": [ - "--build", - "cmake-build-relwithdebinfo", - "--target", - "translate" - ] + "problemMatcher": ["$gcc"], + "args": ["--build", "cmake-build-relwithdebinfo", "--target", "translate"] }, { "label": "build: all CLI tools", "type": "process", "command": "cmake", "group": "build", - "problemMatcher": [ - "$gcc" - ], + "problemMatcher": ["$gcc"], "args": [ "--build", "cmake-build-relwithdebinfo", @@ -96,26 +73,19 @@ "options": { "cwd": "${workspaceFolder}/cmake-build-relwithdebinfo" }, - "dependsOn": [ - "build: odr_test" - ], + "dependsOn": ["build: odr_test"], "group": { "kind": "test", "isDefault": true }, "problemMatcher": [], - "args": [ - "./test/odr_test", - "--gtest_filter=${input:gtestFilter}" - ] + "args": ["./test/odr_test", "--gtest_filter=${input:gtestFilter}"] }, { "label": "cli: translate ", "type": "process", "command": "${workspaceFolder}/scripts/run-with-env.sh", - "dependsOn": [ - "build: translate" - ], + "dependsOn": ["build: translate"], "problemMatcher": [], "args": [ "./cmake-build-relwithdebinfo/cli/translate", @@ -144,9 +114,7 @@ "type": "process", "command": "python3", "problemMatcher": [], - "args": [ - "scripts/gen-vscode-env.py" - ] + "args": ["scripts/gen-vscode-env.py"] }, { "label": "profile: Instruments Time Profiler (odr_test, filtered)", @@ -156,23 +124,16 @@ "options": { "cwd": "${workspaceFolder}/cmake-build-relwithdebinfo" }, - "dependsOn": [ - "build: odr_test" - ], + "dependsOn": ["build: odr_test"], "problemMatcher": [], - "args": [ - "./test/odr_test", - "--gtest_filter=${input:gtestFilter}" - ] + "args": ["./test/odr_test", "--gtest_filter=${input:gtestFilter}"] }, { "label": "profile: Instruments Time Profiler (translate)", "detail": "Records a .trace of the translate CLI on one file.", "type": "process", "command": "${workspaceFolder}/scripts/profile.sh", - "dependsOn": [ - "build: translate" - ], + "dependsOn": ["build: translate"], "problemMatcher": [], "args": [ "./cmake-build-relwithdebinfo/cli/translate",