fix(doctor): detect environment-installed entry points (no false 'Venv entry point not found') - #77428
fix(doctor): detect environment-installed entry points (no false 'Venv entry point not found')#77428andrexibiza wants to merge 3 commits into
Conversation
Signed-off-by: andrexibiza <84248988+andrexibiza@users.noreply.github.com> # Conflicts: # tests/hermes_cli/test_doctor_command_install.py
Native Win11 review (monerostar) — gap on Windows entry-point namesHost: Windows 11 (build 26200). Real Hermes install at Intent is rightLooking past But the Windows console script is still missedOn this daily-driver install the entry point is Replaying this PR's detection literally against that env: So on a healthy Windows Hermes install the false positive remains. Source-checkout path has the same Windows holeEven when Suggested minimal fixWhen building candidates, include the platform script name: _names = ("hermes.exe", "hermes.cmd", "hermes.bat", "hermes") if os.name == "nt" else ("hermes",)
# for each scripts dir (PROJECT_ROOT/{venv,.venv}/Scripts or bin, plus sysconfig scripts):
# for name in _names: candidate = dir / nameAlso probe TestsCurrent tests create Happy to re-verify on this host once |
The environment-scripts-dir detection only checked bare 'hermes', which misses the actual console script on Windows (venv\Scripts\hermes.exe, verified on Win11). Scan all platform name variants so an environment-installed entry point is not reported missing, and add regression tests for the Windows layout and the still-warns-when-missing case. Addresses reviewer finding on NousResearch#77428.
|
Thank you for the native Win11 review — the gap is real and now fixed. The environment-scripts detection only checked bare The fix is pushed to the PR branch — |
…ole scripts reachable Blind verifier caught that the Windows console-script detection (hermes.exe/ .cmd/.bat) sat inside 'if sys.platform != win32', making it unreachable on the exact platform that produces hermes.exe. Restructure: entry-point detection (venv/ + sysconfig scripts dir + platform name variants) now runs on all platforms; only ~/.local/bin symlink management stays POSIX-guarded. Windows-layout tests now run natively on win32 with no platform fake, and assert the Command Installation section actually executes. Signed-off-by: andrexibiza <84248988+andrexibiza@users.noreply.github.com>
|
@monerostar — accepted as the native Windows residual. The candidate search must cover I have recorded the Windows-native regression test as part of the fix surface. This PR is not treated as complete while a healthy Windows install still reports the entry point missing. Source comment: #77428 (comment) |
monerostar
left a comment
There was a problem hiding this comment.
Native Win11 re-check (monerostar) — residual closed on tip e6c862d9f
Host: Windows 11 build 26200. Real install
C:\Users\Admin\AppData\Local\hermes\hermes-agent
venv Python 3.11.15, active sysconfig scripts dir = ...\venv\Scripts.
Live layout on this box
venv\Scripts\hermes.exe present
venv\Scripts\hermes missing
venv\bin\hermes missing (POSIX path)
Main vs this PR
- main: the whole Command Installation section is behind
if sys.platform != "win32", so Windows never runs entry-point detection and cannot emit the false "Venv entry point not found" from that path. - PR tip: detection runs on all platforms. On this install it walks
hermes→hermes.exe→ findsvenv\Scripts\hermes.exeand would report OK (Venv entry point exists (venv\Scripts\hermes.exe)). No missing-entry-point warn.
Tests (PR worktree, install venv pytest)
tests/hermes_cli/test_doctor_command_install.py
2 passed, 7 skipped
The two Win-only cases land here: native Scripts\hermes.exe layout, and missing-console-script still warns. Skips are the POSIX symlink cases.
Take
The Win residual from the earlier review is fixed on this tip. Healthy daily-driver install no longer false-fails entry-point detection under the PR logic. Looks good from this host.
|
Thanks for the native Win11 re-check. I independently ran the targeted doctor tests on the PR tip: |
Related #49529 #68505
What does this PR do?
Fixes
hermes doctor's false-positive "Venv entry point not found"warning on installs where the console entry point lives in the active
environment's scripts dir rather than
venv/bin/or.venv/bin/(#49529).
Root cause
run_doctor's Command Installation check only looked for the entry pointat
PROJECT_ROOT/venv/bin/hermesorPROJECT_ROOT/.venv/bin/hermes.For a wheel/venv-installed environment,
PROJECT_ROOTpoints insidesite-packages and the console script lives under the environment's
scripts directory (e.g.
/srv/hermes/venv/bin/hermeson POSIX,...\venv\Scripts\hermes.exeon Windows). The check warned "not found"even though
hermesworked, and suggested an editablepip install -ereinstall that was wrong for that layout.
The fix
sysconfig.get_path("scripts")on the active venv (detected viasys.prefix != base_prefix) and accept the entry point found there.environment — requiring a global
~/.local/binsymlink would turn ahealthy isolated venv into a false failure, so the check now reports
"Active environment entry point needs no global symlink" instead.
--fixpath no longer suggestspip install -e '.[all]'forwheel-installed environments (that would rewrite the install layout);
it suggests the correct
pip install --force-reinstall hermes-agent.hint remains for genuine source checkouts where the entry point really
is missing.
Credit
Cherry-picked from #68505 by @YuYigeng (authorship preserved in git
history). The PR was closed without merging; this ships the code with the
conflict against current main resolved.
How to test
The new tests cover: wheel venv needs no global symlink (both
--fixmodes), wheel venv with missing entry point avoids the editable-install
fix, and the existing source-checkout cases stay intact.
What platforms were tested?
dir resolves the wheel entry point); the full suite is Unix-only
(symlink semantics) and runs in CI's Linux slices.
git diff --checkclean; attribution audit clean.Why this matters to users
Before:
hermes doctoron any non-source install falsely reported theentry point missing and pushed users toward an editable reinstall —
scary, wrong, and potentially destructive to a working layout.
After: doctor recognizes environment-installed entry points, stops
demanding a global symlink for healthy venvs, and suggests the correct
reinstall command when something genuinely is missing.
Fixes #49529
Checklist
git diff --checkclean