phase12: sf3_cc --help was broken — three of four workers tripped on it
Worker B, worker C and worker D all probed `tools/sf3_cc` with `--help` during the Phase 12 capability probe, and all three got a raw coreutils `basename` error instead of help: ./tools/sf3_cc: line 29: .run/sf3_cc/Usage: basename NAME [SUFFIX] ... : No such file ANY leading-`-` argument was taken as a source path. `basename --help .c` prints coreutils' own help text, and that entire text then became the output filename, so the failure surfaced as a confusing redirection/cpp error. Every worker lost time on it, on the first command they tried, and worker D spent probe effort diagnosing it as a "genuinely broken path". This is cookbook 179 from the other side. That finding says a rule every worker must follow belongs in a TRACKED tool rather than a file each worker copies. Here the tool WAS tracked -- and it was still unusable the way every single worker reaches for it first. Tracking a tool is necessary but not sufficient; it also has to survive first contact. Fixed: `--help`/`-h` print real help and exit 0, an unknown option is rejected BY NAME, and a missing source file says so rather than failing inside cpp. The compile path is unchanged. Also documented its LIMIT, which was previously unwritten and is easy to misread as a match: the maspsx stage always runs with DEFAULT options and no region option is passed, so this shows a SPELLING's shape and NOT a region's final bytes. A region needing `maspsx=epilogue`, `nopmarker`, `moves`, `regread`, `off` or `gp=`/`cc1=`/`as=` looks different here than under `sf3_match range`. Sweep spellings with this; decide matches with `sf3_match range` -- and decide an `epilogue` token from the CANDIDATE's tail, never the original's (finding 165/180). 9 new tests (tools/tests/test_sf3_cc.py): the argument-handling cases run without a toolchain, including one that pins the exact symptom (no coreutils help text in the output, and no file created named after it); the compile-path test skips cleanly when the ignored toolchain or any src/func_*.c is absent, and one test pins the documented LIMIT so it cannot be dropped. make test 290 tests, OK (from 281)
This commit is contained in:
@@ -12,6 +12,23 @@
|
||||
#
|
||||
# Pipeline: cpp -> cc1 (the pinned 2.7.2-psx build) -> maspsx (the project's vendored patched
|
||||
# version), which is exactly what sf3_match does internally.
|
||||
#
|
||||
# PHASE 12 ADDITION -- argument handling, and why it is here rather than in a charter.
|
||||
# Three of the four Phase 12 workers probed this tool with `--help` and every one got a raw
|
||||
# coreutils `basename` error, because ANY leading-`-` argument was taken as a source path.
|
||||
# That is cookbook 179 from the other side: the rule lives in a tracked tool, but the tool
|
||||
# was not usable the way every worker reaches for it first. A tool that fails confusingly on
|
||||
# its own help flag costs a worker time on the one path it will always try. So: `--help`
|
||||
# works, an unknown option is rejected BY NAME, and a missing source file says so instead of
|
||||
# surfacing a cpp error. The compile path is unchanged.
|
||||
#
|
||||
# LIMIT -- READ BEFORE TRUSTING THE OUTPUT. The maspsx stage always runs with DEFAULT options,
|
||||
# and no region option is passed. This tool shows you a SPELLING's shape, NOT a region's final
|
||||
# bytes: a region needing a maspsx token (`epilogue`, `nopmarker`, `moves`, `regread`, `off`)
|
||||
# or `gp=`/`cc1=`/`as=` will look DIFFERENT here than under `sf3_match range`. Sweep SPELLINGS
|
||||
# with this; decide MATCHES with `sf3_match range`. An `epilogue` row in particular shows the
|
||||
# UNFILLED tail here, and that token must be decided from the CANDIDATE's tail, never the
|
||||
# original's (cookbook 165/180).
|
||||
set -e
|
||||
REPO=$(cd "$(dirname "$0")/.." && pwd)
|
||||
cd "$REPO"
|
||||
@@ -21,7 +38,30 @@ if [ $# -lt 1 ]; then
|
||||
exit 2
|
||||
fi
|
||||
|
||||
case "$1" in
|
||||
-h|--help)
|
||||
echo "usage: sf3_cc <file.c> -> prints cc1 assembly (maspsx applied) for ONE file"
|
||||
echo
|
||||
echo 'The spelling-sweep tool: one spelling costs ~0.1 s, which is what turns'
|
||||
echo '"try a few variants" into "grid the whole space". Pair it with tools/sf3_diff,'
|
||||
echo "which names WHICH DIMENSION a residual lives in -- compile+diff together remove"
|
||||
echo "both the cost of a spelling and the guesswork about what is wrong."
|
||||
echo
|
||||
echo "Env: SF3_CC_OUT (default .run/sf3_cc) selects the scratch directory."
|
||||
exit 0
|
||||
;;
|
||||
-*)
|
||||
echo "sf3_cc: unknown option: $1" >&2
|
||||
echo "usage: sf3_cc <file.c>" >&2
|
||||
exit 2
|
||||
;;
|
||||
esac
|
||||
|
||||
SRC=$1
|
||||
if [ ! -f "$SRC" ]; then
|
||||
echo "sf3_cc: no such source file: $SRC" >&2
|
||||
exit 2
|
||||
fi
|
||||
BASE=$(basename "$SRC" .c)
|
||||
OUT=${SF3_CC_OUT:-.run/sf3_cc}
|
||||
mkdir -p "$OUT"
|
||||
|
||||
@@ -0,0 +1,133 @@
|
||||
"""Synthetic-only tests for the spelling-sweep compile tool.
|
||||
|
||||
`tools/sf3_cc` is a POSIX shell wrapper: cpp -> cc1 (the pinned 2.7.2-psx build) -> maspsx.
|
||||
Only its ARGUMENT HANDLING is tested here without a toolchain, because that is the part that
|
||||
broke: in Phase 12 **three of the four workers** probed it with `--help` and every one got a
|
||||
raw coreutils `basename` error, since any leading-`-` argument was taken as a source path.
|
||||
Every one of them lost time on it, on the first command they tried.
|
||||
|
||||
Cookbook 179 says a rule every worker must follow belongs in a TRACKED tool rather than in a
|
||||
file each worker copies. This is the same failure from the other side: the tool WAS tracked,
|
||||
but it was not usable the way every worker reaches for it first.
|
||||
|
||||
The compile-path test skips cleanly when the ignored local toolchain or any ``src/func_*.c``
|
||||
is absent, in the style of the existing local-toolchain skips. It never reads the disc or the
|
||||
game executable -- ``sf3_cc`` reads only the C source it is handed.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
REPO = Path(__file__).resolve().parents[2]
|
||||
TOOL = REPO / "tools" / "sf3_cc"
|
||||
CC1 = REPO / "tools" / "old-gcc" / "gcc-2.7.2-psx" / "cc1"
|
||||
CPP = REPO / "tools" / "old-gcc" / "gcc-2.7.2-psx" / "cpp"
|
||||
MASPSX = REPO / "tools" / "maspsx" / "maspsx.py"
|
||||
|
||||
|
||||
def _toolchain_available() -> bool:
|
||||
return TOOL.is_file() and CC1.is_file() and CPP.is_file() and MASPSX.is_file()
|
||||
|
||||
|
||||
def _any_source() -> Path | None:
|
||||
return next(iter(sorted((REPO / "src").glob("func_*.c"))), None)
|
||||
|
||||
|
||||
def _run(args: list[str], out_dir: Path | None = None) -> subprocess.CompletedProcess:
|
||||
env = dict(os.environ)
|
||||
if out_dir is not None:
|
||||
env["SF3_CC_OUT"] = str(out_dir)
|
||||
return subprocess.run(
|
||||
[str(TOOL), *args], cwd=REPO, capture_output=True, text=True, env=env
|
||||
)
|
||||
|
||||
|
||||
class ArgumentHandlingTests(unittest.TestCase):
|
||||
"""The defect: a `-`-leading argument was treated as a source path.
|
||||
|
||||
`basename --help .c` prints coreutils' own help, and that whole text then became the
|
||||
output filename, so the failure surfaced as a confusing redirection/cpp error rather
|
||||
than as "unknown option".
|
||||
"""
|
||||
|
||||
def test_help_exits_zero_and_names_the_usage(self) -> None:
|
||||
result = _run(["--help"])
|
||||
self.assertEqual(result.returncode, 0, result.stderr)
|
||||
self.assertIn("usage: sf3_cc", result.stdout)
|
||||
|
||||
def test_short_help_alias_also_works(self) -> None:
|
||||
result = _run(["-h"])
|
||||
self.assertEqual(result.returncode, 0, result.stderr)
|
||||
self.assertIn("usage: sf3_cc", result.stdout)
|
||||
|
||||
def test_help_output_does_not_leak_coreutils_basename_help(self) -> None:
|
||||
"""The exact confusing symptom three workers saw."""
|
||||
result = _run(["--help"])
|
||||
combined = result.stdout + result.stderr
|
||||
self.assertNotIn("Report bugs to", combined)
|
||||
self.assertNotIn("basename NAME", combined)
|
||||
self.assertNotIn("GNU coreutils", combined)
|
||||
|
||||
def test_no_arguments_exits_two_with_usage_on_stderr(self) -> None:
|
||||
result = _run([])
|
||||
self.assertEqual(result.returncode, 2)
|
||||
self.assertIn("usage: sf3_cc", result.stderr)
|
||||
|
||||
def test_unknown_option_exits_two_and_names_the_option(self) -> None:
|
||||
result = _run(["-x"])
|
||||
self.assertEqual(result.returncode, 2)
|
||||
self.assertIn("-x", result.stderr)
|
||||
self.assertIn("usage: sf3_cc", result.stderr)
|
||||
|
||||
def test_a_missing_source_file_says_so_rather_than_failing_in_cpp(self) -> None:
|
||||
result = _run(["definitely-not-here.c"])
|
||||
self.assertEqual(result.returncode, 2)
|
||||
self.assertIn("definitely-not-here.c", result.stderr)
|
||||
self.assertIn("no such source file", result.stderr)
|
||||
|
||||
def test_an_option_like_argument_is_not_accepted_as_a_path(self) -> None:
|
||||
"""`--help` must not create a file named after coreutils' help text."""
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
result = _run(["--help"], out_dir=Path(tmp))
|
||||
self.assertEqual(result.returncode, 0, result.stderr)
|
||||
self.assertEqual(list(Path(tmp).iterdir()), [])
|
||||
|
||||
|
||||
@unittest.skipUnless(_toolchain_available(), "ignored local toolchain is absent")
|
||||
class CompilePathTests(unittest.TestCase):
|
||||
"""The path workers actually use: one spelling costs ~0.1 s."""
|
||||
|
||||
def setUp(self) -> None:
|
||||
source = _any_source()
|
||||
if source is None:
|
||||
self.skipTest("no tracked src/func_*.c to compile")
|
||||
self.source = source
|
||||
self._tmp = tempfile.TemporaryDirectory()
|
||||
self.addCleanup(self._tmp.cleanup)
|
||||
self.out = Path(self._tmp.name)
|
||||
|
||||
def test_compiles_a_real_source_to_assembly(self) -> None:
|
||||
result = _run([str(self.source.relative_to(REPO))], out_dir=self.out)
|
||||
self.assertEqual(result.returncode, 0, result.stderr)
|
||||
self.assertIn(".end", result.stdout)
|
||||
self.assertTrue(any(self.out.iterdir()))
|
||||
|
||||
def test_the_documented_limit_is_stated_in_the_source(self) -> None:
|
||||
"""A tool's LIMITS belong in the tool: sf3_cc passes no region option.
|
||||
|
||||
A worker who trusts this output as a region's final bytes will mis-read every
|
||||
`epilogue`/`nopmarker`/`gp=` row, so the limit must be written down where it is read.
|
||||
"""
|
||||
text = TOOL.read_text()
|
||||
self.assertIn("LIMIT", text)
|
||||
self.assertIn("DEFAULT options", text)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user