Skip to content

fix(scripts): resolve editable file:// URLs with url2pathname - #486

Open
SanHsien wants to merge 1 commit into
NVIDIA:mainfrom
SanHsien:fix/windows-file-url-editable-dependency
Open

fix(scripts): resolve editable file:// URLs with url2pathname#486
SanHsien wants to merge 1 commit into
NVIDIA:mainfrom
SanHsien:fix/windows-file-url-editable-dependency

Conversation

@SanHsien

@SanHsien SanHsien commented Sep 6, 2026

Copy link
Copy Markdown

Fixes #485.

Problem

In _RUNTIME_IDENTITY_PROBE, an editable dependency's direct_url.json file:// URL is converted to a local path with Path(urllib.parse.unquote(parsed.path)).

A Windows file URL is file:///C:/Users/.../pkg, so urlsplit(...).path is /C:/Users/.../pkg. Path() reads that leading slash as a root, so the result is C:\C:\Users\...\pkg and resolve(strict=True) raises:

OSError: [WinError 123] The filename, directory name, or volume label syntax is incorrect:
'C:\C:\Users\...\editable-dependency'

On POSIX both forms give the same string, so CI never sees it.

Change

One line, plus the import:

-editable_root = Path(urllib.parse.unquote(parsed.path)).resolve(strict=True)
+editable_root = Path(urllib.request.url2pathname(parsed.path)).resolve(strict=True)

urllib.request.url2pathname is the stdlib conversion for URL path to local path. It strips the leading slash before a drive letter on Windows and is a no-op relative to the previous behavior on POSIX, where there is no drive letter to double. It also still percent-decodes, so no separate unquote is needed.

Verification

tests/unit/test_compare_scan_accuracy.py::test_runtime_probe_hashes_installed_and_editable_dependency_bytes already covers this path — it just never runs on a Windows host in CI. No test changes in this PR.

On Windows 11 (native, not WSL) / Python 3.13.14 / uv 0.12.8, from this branch:

  • without the change: 1 failed with the WinError 123 above
  • with the change: 1 passed
  • ruff check scripts/compare_scan_accuracy.py — clean
  • ruff format --check scripts/compare_scan_accuracy.py — already formatted

I did not add a regression test: on Linux url2pathname("/C:/x") and unquote("/C:/x") return the same string, so no cross-platform test can distinguish the two implementations. The existing test is the real guard, and it only bites on a Windows runner.

Notes

Found while getting the suite green on a native Windows host. This is the only product-code bug among the failures there — the other 22 were POSIX assumptions in the tests themselves.

🤖 Generated with Claude Code

A Windows file URL's path is "/C:/Users/...". Path() reads the leading
slash as a root, so unquote() produced "C:\C:\Users\..." and the
following resolve(strict=True) raised WinError 123. url2pathname is the
stdlib conversion for this and is identical to the old behavior on POSIX,
where the path has no drive letter to double.

test_runtime_probe_hashes_installed_and_editable_dependency_bytes already
covers this; it just never runs on a Windows host in CI. On Windows 11 /
Python 3.13 it fails before this change and passes after.

Fixes NVIDIA#485

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: SanHsien <34234698+SanHsien@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 6, 2026 02:19
SanHsien added a commit to SanHsien/SkillSpector that referenced this pull request Sep 6, 2026
The url2pathname change is now NVIDIA#486, tracking
issue NVIDIA#485. DIVERGENCE.md carries the follow-up rule: drop the row once
upstream merges, rather than carrying it as a permanent divergence.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is minimal, uses the appropriate stdlib API for cross-platform URL-path conversion, and directly addresses the documented Windows failure mode without altering broader logic.

Pull request overview

This PR fixes a Windows-specific path conversion bug in the runtime identity probe used by scripts/compare_scan_accuracy.py when hashing editable dependencies discovered via direct_url.json. It replaces a naive unquote + Path(...) conversion with the stdlib’s url2pathname, which correctly handles Windows drive-letter paths that appear with a leading slash in file:// URLs.

Changes:

  • Import urllib.request in the embedded _RUNTIME_IDENTITY_PROBE.
  • Convert file:// URL paths to local filesystem paths using urllib.request.url2pathname (fixing /C:/...C:\... handling on Windows).
  • Add inline rationale comments explaining the Windows drive-letter/leading-slash behavior.
File summaries
File Description
scripts/compare_scan_accuracy.py Fixes Windows editable file:// URL-to-path conversion inside _RUNTIME_IDENTITY_PROBE by using url2pathname.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

compare_scan_accuracy: editable-dependency file:// URL resolves to "C:\C:\..." on Windows

2 participants