Repository navigation
octo: preserve UTF-8 text and LF bytes on Windows - #195
Open
jscjscjscjscjsc wants to merge 1 commit into
Open
jscjscjscjscjsc wants to merge 1 commit into
jscjscjscjscjsc wants to merge 1 commit into
Conversation
BH3GEI
approved these changes
Oct 14, 2026
BH3GEI
left a comment
Contributor
There was a problem hiding this comment.
Thanks for putting this together — a clean, tightly scoped fix for a real Windows authoring gap.
Verified on macOS against d0344682 with Python 3.12.13 (CI's version):
- Regression honesty: base + the new test file → FAILED (failures=1, errors=3), exactly the before-state described; PR head → 4/4 pass. The CP936/CRLF simulation is faithful:
pathlib'sread_text/openresolveio.open/io.text_encodingas module attributes at call time, so both patches intercept, and the wrapper only injects defaults when the caller omits encoding/newline. Deleting justnewline="\n"fromwrite_utf8makes the LF assertion fail, so that argument is load-bearing, not decorative. - Real CLI differential (no mocks,
PYTHONUTF8=0 PYTHONCOERCECLOCALE=0 LC_ALL=C→ US-ASCII default):octo publish-githubon a Unicode manifest dies with "'ascii' codec can't decode byte 0xe6" on base (exit 1) and installs the workflow with LF-only bytes on head (exit 0); a second run prints "already current" and leaves the mtime untouched.octo runon a Unicode signed manifest now reaches "card-host cannot run a sealed release" instead of a codec error, before any binary lookup. - Full tools suite: base 50/50 OK (6 skips), head 54/54 OK (same 6 skips) — no regressions. Every format-text read/write in
tools/octonow carries explicit UTF-8/LF; the one remaining bare text open (octo:467, card-host log) is subprocess stdout only and is read back with explicit UTF-8 +errors="replace". - The
test_octo.pyassertion change is correct:sdk_dependencywrites the path JSON-escaped, so decoding it withjson.loadsis the right comparison (and the only workable one for Windows backslash paths).
Two non-blocking observations for possible follow-ups (neither blocks this PR):
subprocess.run(..., text=True)sites (octo:557,571,1041;wasm_component.py:776) still decode hub/cargo output with the locale encoding, so on CP936 Windows a non-ASCII crate path can still mojibake or raise inocto check/wasm build. Pre-existing and outside this diff's declared scope.cmd_checkreads the manifest/listing (octo:552,584) outside try/except; a bundle authored in the machine locale that isn't valid UTF-8 now ends in a raw traceback. Such a bundle could never passhub checkanyway (the Rust side parses JSON as UTF-8), so failing early is defensible — wrapping those reads in the usualdie()would just be friendlier.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
octorelies on the OS default encoding for several UTF-8 templates, manifests, listings and source files. On Windows with CP936, an app containing晨报📰…can fail to decode, get rewritten incorrectly, or produce CRLF instead of LF. Publishing a Unicode manifest also fails, and re-reading a generated UTF-8 workflow can incorrectly defeat the idempotence check. Signed manifests should reach the existing sealed-release rejection rather than a decoding error.Specify UTF-8 for format-defined text reads and add
write_utf8()usingPath.open(encoding="utf-8", newline="\n"), compatible with Python 3.9. Preserve signed bundle bytes during checking, and preserve the refusal to run sealed releases. Decode the JSON-compatible SDK path string in one existing TOML test assertion so a Unicode workspace is compared correctly.Validation on Windows / Python 3.12.10 against
d0344682:new, UTF-8/LF workflow generation and repeat-publish idempotence, checking without restamping signed manifests, and sealed-release refusal before starting a host.OK (skipped=3)(Windows symlink privileges). The two independent PRs touch adjacent lines incmd_publish_github; the tested resolution keepsvalidate_publisher_root(app)followed by the UTF-8 manifest read. One PR needs that small rebase after the other merges.git diff --checkpasses. No native host build, GUI validation, real Actions workflow execution, tag, release, source-pin or signing changes are claimed.Team / 项目归属
Submitted by the 共创队(晨报卡 / Morning Brief;报名 ID:小鞠老师), via GitHub account
jscjscjscjscjsc. Project repository: jscjscjscjscjsc/morning-brief. This contribution comes from issues encountered and fixes developed collaboratively while building the Morning Brief hackathon project.本 PR 为晨报卡共创团队的上游贡献,关联项目仓库及已有修复记录;提交账号是团队贡献的提交入口。团队归属不代表主办方已经确认加分,上游合并状态以本 PR 为准。