docs: correct os.replace claim and complete the hand-rolled site list
Phase 2 review findings: (1) _commit_staged_replacements' docstring cited os.replace while the code uses os.rename — the atomicity claim holds (same- filesystem rename is atomic on POSIX and NTFS) but named the wrong function. (2) venv_bin_dir's remaining hand-rolled site list missed agent/lsp/servers.py:270.
This commit is contained in:
parent
bbe93ab8a8
commit
b675fb2b3e
|
|
@ -665,9 +665,10 @@ def _commit_staged_replacements(staged) -> None:
|
|||
This covers plain files as well as directories: the repo root holds 20
|
||||
first-party modules (``run_agent.py``, ``cli.py``, ``hermes_constants.py``
|
||||
…), so a files-only failure reproduces exactly the bug class we are
|
||||
closing. ``os.replace`` is atomic on POSIX and maps to
|
||||
``MoveFileEx(REPLACE_EXISTING)`` on Windows, so a file swap can never
|
||||
leave a half-written module the way ``copy2`` onto a live path can.
|
||||
closing. Every swap is an ``os.rename`` onto a path that was just moved
|
||||
aside — a same-filesystem rename is atomic on POSIX and NTFS alike, so a
|
||||
file swap can never leave a half-written module the way ``copy2`` onto a
|
||||
live path can.
|
||||
|
||||
Splitting stage-all-then-swap-all shrinks the failure window from "the
|
||||
duration of a full tree copy" to "the duration of N renames", and makes
|
||||
|
|
|
|||
|
|
@ -1261,8 +1261,8 @@ def venv_bin_dir(venv_dir, *, windows: bool | None = None) -> Path:
|
|||
each new call site had to re-derive it, and #76091 shipped an eighth copy
|
||||
because the correct behaviour lived 2400 lines away in another function.
|
||||
A few sites outside ``hermes_cli`` (``tools/code_execution_tool.py``,
|
||||
``agent/lsp/install.py``) still hand-roll it — convert them as they are
|
||||
touched.
|
||||
``agent/lsp/install.py``, ``agent/lsp/servers.py``) still hand-roll it —
|
||||
convert them as they are touched.
|
||||
|
||||
*windows* lets a caller pass its own platform verdict. Several callers
|
||||
resolve this through predicates the test-suite patches to exercise
|
||||
|
|
|
|||
Loading…
Reference in New Issue