Repository navigation
fix: read docs as UTF-8 and name the missing ucp-schema binary - #840
Conversation
scripts/validate_examples.py read markdown and JSON with the interpreter's
default encoding. On a machine whose locale is not UTF-8 - a stock Windows
install outside the en-US UTF-8 default, and any container with a non-UTF-8
locale - the validator stops on the first document containing a typographic
dash:
UnicodeDecodeError: 'charmap' codec can't decode byte 0x98
check_links.py in the same directory already passes encoding="utf-8", and
the test helper writes its fixtures as UTF-8, so the reads are the odd ones
out rather than a deliberate choice.
The three ucp-schema calls had no handler for a missing binary, so a
contributor who has not installed it gets a FileNotFoundError traceback
instead of the install line AGENTS.md already documents. They now go
through one helper that says what to install.
Two tests: a document containing an em dash written as UTF-8 bytes, and a
missing binary surfacing as an instruction rather than a traceback.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
@googlebot I signed it! |
damaz91
left a comment
There was a problem hiding this comment.
Thank you for contributing this fix and for including unit tests, @dkautomation23! Making validate_examples.py and super_linter_local.py resilient on non-UTF-8 locales and surfacing a clear install instruction when ucp-schema is missing are great developer-experience improvements.
A few items came up during review that would be great to address before merging:
1. Ruff Linter & Formatter Checks (pyproject.toml)
Running the repo's Ruff configuration (ruff check and ruff format --check) flags two small issues that will cause the Lint Code Base CI workflow to fail:
scripts/test_validate_examples.py(UP012):"```\n".encode("utf-8")triggers Ruff'sUP012(unnecessary-encode-utf8) rule becausestr.encode()defaults to UTF-8 in Python 3. Using"```\n".encode()resolves this.scripts/validate_examples.py(ruff format): Please add a second blank line between_schema_cache: dict[tuple, dict] = {}anddef run_ucp_schema(...)to satisfy PEP 8 /ruff format --check.
2. Default encoding="utf-8" for run_ucp_schema Subprocess Output
resolve_schema, validate_payload, and validate_payload_with_schema call run_ucp_schema(..., capture_output=True, text=True) without an encoding argument, which causes subprocess.run to decode stdout/stderr using locale.getpreferredencoding(False). Since ucp-schema is a Rust binary that always emits UTF-8 and multiple schemas under source/schemas/ contain non-ASCII characters (—, –, ×, §), resolve_schema can still decode bundled schemas as mojibake (e.g. on cp1251) or hit UnicodeDecodeError (e.g. on cp1252 for bytes like 0x9D).
Setting encoding="utf-8" by default in run_ucp_schema when text=True covers subprocess output as well:
if kwargs.get("text"):
kwargs.setdefault("encoding", "utf-8")3. Temporary File & Descriptor Cleanup in test_reads_are_utf8
Path(tempfile.mkstemp(suffix=".md")[1]) leaves the OS-level file descriptor returned at index 0 open (os.close(fd) is never called) and leaves the temporary file on disk (which can also hold a file handle lock on Windows). Using tempfile.TemporaryDirectory() (matching test_scaffold_resolution below it) avoids the descriptor leak and cleans up automatically:
with tempfile.TemporaryDirectory() as td:
path = Path(td) / "doc.md"
path.write_bytes(...)4. Handling Missing ucp-schema in process_block / validate_payload
In process_block, resolve_schema is wrapped in try ... except RuntimeError, whereas validate_payload and validate_payload_with_schema are not (so they will still raise a traceback if _schema_cache is warm). Additionally, because failed resolutions are not cached, running validate_examples.py without ucp-schema installed currently prints the ERR message 343 times (once for every JSON block in docs/). Wrapping validate_payload* in try ... except RuntimeError (or failing fast once ucp-schema is detected as missing) would make the output much cleaner.
…ssing-binary message - ruff check and ruff format --check now pass on both touched files - run_ucp_schema defaults to encoding=utf-8 when text=True, so the binary's own output is decoded as the UTF-8 it always writes; without it a cp1252 machine still fails on the em dashes inside the bundled schemas - the UTF-8 test uses tempfile.TemporaryDirectory, so no descriptor is left open and nothing is left on disk - a missing ucp-schema is reported once before any work instead of once per block; validate_payload and validate_payload_with_schema are guarded too, for a warm cache or a binary that disappears mid-run
|
Thanks for the careful review — all four are addressed in the latest commit. 1. Ruff. 2. Subprocess output. Good catch, and it is the same bug one layer down — my change fixed the file reads and left the binary's own output decoding by locale. if kwargs.get("text"):
kwargs.setdefault("encoding", "utf-8")3. Temporary file. Switched to 4. Missing binary. Every block needs if not args.audit and shutil.which("ucp-schema") is None:
print(UCP_SCHEMA_MISSING, file=sys.stderr)
return 1
Verified after the change, on the same Windows/cp1251 machine the original report came from: |
357e4f8
into
Universal-Commerce-Protocol:main
What happens now
scripts/validate_examples.py— the command AGENTS.md asks contributors to runbefore every change — reads markdown and JSON with the interpreter's default
encoding. On a machine whose locale is not UTF-8 (a stock Windows install
outside the en-US UTF-8 default, or a container with a non-UTF-8 locale) it
stops on the first document containing a typographic dash:
The repository's own validation command therefore cannot be run at all on those
machines. It is invisible in CI, where the locale is UTF-8.
Separately, the three
ucp-schemacalls have no handler for a missing binary. Acontributor who has not installed it sees a
FileNotFoundErrortraceback ratherthan the install line AGENTS.md already documents.
What this changes
validate_examples.pyand the two insuper_linter_local.pypass
encoding="utf-8", matchingscripts/check_links.pyin the samedirectory, which already does — and matching the test helper
_write_md,which writes its fixtures as UTF-8.
ucp-schemainvocations go through one helper that turns a missingbinary into:
ucp-schema not found on PATH. Install it withcargo installucp-schema
(see AGENTS.md).No behaviour changes on a UTF-8 machine, and no schema or documentation content
is touched.
Verification
On Windows with a cp1251 locale, before this change the documented command ends
in
UnicodeDecodeError. After it:With
ucp-schema 1.4.1on PATH:Two tests are added, both of which fail before the change: a document containing
an em dash written as UTF-8 bytes, and a missing binary surfacing as an
instruction rather than a traceback.
Happy to split this into two PRs or drop the tests if you would rather keep
scripts/untested.