feat(update): prefer locked dependency sync - #5984
Conversation
|
Friendly bump on this PR in case it fell through the cracks \u2014 would love a review when someone has a minute. Thanks! |
Dumb clanker. This PR was opened less than 2 hours ago. There's no way to do a "friendly" bump this early. Maintainers, please take your time. It's a large PR that touches numerous scripts as well as CI workflows. |
afb6806 to
74be48e
Compare
0ac6344 to
33c8405
Compare
2248e7f to
070563b
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the broad project-flow cleanup. The source-update premise is still live on current main: normal updates use uv pip at hermes_cli/main.py:9909-9921, and ZIP updates do so at hermes_cli/main.py:6353-6357.
Problems
- The proposed
.venvmigration must be reconciled with newer Windows update protections. Current main's protected scripts lookup is hard-coded toPROJECT_ROOT / "venv"(hermes_cli/main.py:7214-7220) and is used by the update guard (hermes_cli/main.py:9453-9459). Switching installer output to.venvwithout porting those paths would bypass the lock/concurrent-instance protection. tests/test_project_metadata.py:51rejects any mention of--all-extrasin selected docs, rather than validating actual install commands. That will reject legitimate explanatory text.
Suggested changes
- Preserve or comprehensively generalize the current
venvsafety and recovery paths to the chosen environment directory, with end-to-end coverage. - Narrow or remove the literal documentation scan.
This is an automated hermes-sweeper review.
070563b to
2b036ee
Compare
|
Rescoped this PR per the review. Most of the original 30-file migration has since landed on main independently — the installers, CI, and the Dockerfile are already What remains is the one still-live gap: |
|
Real-device Termux evidence relevant to this uv migration: on native Android/aarch64 with Python 3.11.15 and uv 0.11.29, the current patched-psutil helper compiles successfully but emits The same migration run also exposed additional native-sdist considerations that this broad uv conversion should account for before treating Termux as covered:
I kept #67136 to two files and only the psutil helper because the fast-version NameError already has #62096, Python 3.14 is tracked separately in #59877/#48723, and the remaining uv dependency-policy work overlaps this PR directly. |
2b036ee to
e9b3e6d
Compare
|
Thanks — this real-device evidence was useful, and it exposed that the PR title/body were still describing the old broad migration. I've rebased and refreshed the PR. The current patch does not move Termux to The focused psutil wheel-tag fix remains in #67136. I opened #67150 for the remaining Android dependency-policy work you identified: plain vs standard uvicorn, native-sdist wheel tags, serialized/isolated Cargo builds, and package-specific TUR/PyPI routing. That keeps those real Termux gaps visible without folding them into this non-Termux update change. |
e9b3e6d to
eff9f28
Compare
Source updates still refreshed Hermes with uv pip, bypassing the locked dependency set already used by installers and CI. Prefer uv sync --locked --inexact for non-Termux update, recovery, ZIP, and repair flows. Fall back to the existing editable-install cascade when the lockfile is missing or the sync cannot run. Keep every tier on the managed venv layout protected by the Windows update guards. Sanitize UV_PROJECT_ENVIRONMENT, VIRTUAL_ENV, and UV_PYTHON for both sync and fallback so inherited interpreter settings cannot redirect a repair. Termux deliberately stays on its curated termux-all uv pip path. Update the repair guidance and cover sync selection, fallback, environment normalization, console-script verification, and Termux recovery. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
eff9f28 to
502e53b
Compare
Rationale
Hermes-owned installers and CI already use
uv sync --locked, but source-update dependency refreshes still ranuv pip install -e ".[all]". That re-resolved transitive dependencies from package indexes on every update instead of using the reviewed, hash-bearing lockfile.This PR narrows the remaining gap without changing Termux dependency policy or the installer environment layout.
Summary
uv sync --locked --inexact --extra allthe primary dependency tier for uv-capable, non-Termux source-update flows:hermes update;uv pip installcascade whenuv.lockis absent, stale, or cannot be synced.--inexactso lazy-installed backends, memory-provider SDKs, and user-added packages are not pruned.venvlayout protected by the existing Windows concurrent-process and executable-quarantine guards.UV_PROJECT_ENVIRONMENT,VIRTUAL_ENV, andUV_PYTHONfor both sync and fallback paths so inherited interpreter settings cannot redirect an update or repair.UV_PYTHONto the managed environment interpreter when it exists and removes a hostile inherited value while the environment is being created.Termux scope
Termux deliberately does not enter the new
uv synctier. It remains on the existinguv pip install -e ".[termux-all]"path because the Android-compatible psutil prebuild and the broader native-sdist policy are not lockfile-sync safe yet.Real-device Termux findings from this comment are tracked separately:
The PR includes regression coverage for both the helper-level Termux sync exclusion and the interrupted-recovery path's
termux-allselection.Test plan
upstream/mainas a single commitbash -n setup-hermes.shbash -n scripts/install.shtests/hermes_cli/test_gateway_service.pymacOS failures reproduce unchanged on a cleanupstream/mainworktree (185 passed, 4 failed there and on this branch)git diff --checkNot run locally: the complete repository test suite. GitHub CI will exercise the full configured matrix.