hygiene round: typed dial error, narrowed catches, layout and ownership - #130
Merged
Merged
Conversation
…bug_requested share one tokenizer
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.
A hygiene round over the Go, Rust and Python sources: one bug fix with a
regression test, review commits that only lower comment and allocation counts,
and two cuts.
Commits
fix(python): typed error for a refused connection —
socket.create_connectionsat outside the try that wraps the rest of
dial_agent, so a refused,unreachable, or DNS-failed dial leaked a raw
OSErrorwhile every other failurecame back typed. Wrapped as
ProtocolErrorcarrying the cause; the test dials aclosed port and fails on the parent commit with
ConnectionRefusedError.review(python): narrow the background-thread catches; one-line comments —
the proxy dial, the
pump_outsuppress and_feed_stdin's suppress caught bareException, hiding programming errors in daemon threads; narrowed to(SandboxError, OSError), which is exactly what the code below them raises.review(go): layout, one func type, WalkDir, logger name, test comments —
retryArchiveDeletelogged under its sibling's name;func(NodeState) boolwasspelled twice in mesh and
claimFollowcarried two inline func types in a250-character signature, all now named types;
tarIntomoves tofilepath.WalkDirso a symlink or device entry costs no stat; payload types moveahead of the types they serve.
review(rust): ownership on the silkd request path and review(rust): fewer
allocations on the git and session paths — see the table below.
review(rust): layout, shared cmdline tokenizer, one-line docs —
sysutil.rsfree functions sorted public above private as a pure move, each functional
group's internal order preserved and every
// SAFETY:line travelling with itsblock;
cfg::parseandcfg::debug_requestedshared one cmdline tokenizer.cut(init): switch_root reuses move_mount —
switch_rootspelledmount(\".\", \"/\", None, libc::MS_MOVE, None)inline, which is exactlymove_mount(\".\", \"/\"). Same syscall, same error string, since both go throughthe one
mounthelper that formats it. A dedup, not a line cut.cut(init): drop the debug-flag divergence test —
parseanddebug_requestedno longer hand-roll separate cmdline walks, so the test thatcross-checked them is redundant. Its one unique case, that the last
sandbox.debugoccurrence wins forparse, moves intoparse_debug_forms.review: one-line docs and WHY comments (STE 101) — every multi-line comment
across
silkd/srcandboot/init/srccondensed to one line or deleted where itrestated the code. 630 comment lines to 304. Five multi-line blocks survive, each
because a contract genuinely does not fit one line: the wire-protocol module doc
in
proto.rs, the kernelip=positional field order incfg.rs, and themodule docs of
lsp.rs,tree.rsand the test harness. Every// SAFETY:blockis untouched.
Allocations removed on the silkd request path
parse_file_linepathVec, a joinStringand a secondStringString, truncated in placeGitFileStatusXY codesapply_configResponse::Info.versionProcInfo.stateIo::conversetree::pulltree::drainfs::write_atomicdisplay()&Paththreaded throughfs::scan_dirlsp::manifest_dirThe wire is unchanged.
charandCow<'static, str>serialize exactly as thestrings they replace: the Rust and Go fixture suites each pin exactly 61 frames
over
protocol/wire/fixtures/v1and both pass, andgit_e2easserts on the JSONa real silkd emits.
write_atomictaking&Pathalso drops a latent bug, sincedisplay()would have mangled a non-UTF-8 path.Gates
macOS:
cargo fmt --check,cargo clippy --all-targets -- -D warningsandcargo testclean insilkd(13 suites, 107 tests) andboot/init(15);GOWORK=off make go-lintprints ten0 issues.lines withfmt --diffsilent;aslandGOOS=linux aslfind nothing in all five Go modules;go test -race -count=1 ./...passes in each;ruff checkclean and 155 pytesttests pass.
Linux (
rust:1.98.0,--platform linux/arm64, repo root mounted so the fixturesresolve): silkd fmt, clippy and all 107 tests green;
boot/initthe same with 16tests, one of them Linux-gated.
ruff format --checkreports the same 8 files asmain. Reformatting them isunrelated churn.