fix(desktop): strip Windows extended-length prefix before git clone (#3707) - #4279
Open
iroiro147 wants to merge 1 commit into
Open
fix(desktop): strip Windows extended-length prefix before git clone (#3707)#4279iroiro147 wants to merge 1 commit into
iroiro147 wants to merge 1 commit into
Conversation
…lock#3707) On Windows, `Path::canonicalize()` returns an extended-length path (`\\?\C:\...`). When `canonicalize_repos_root`'s output flows into `clone_project_repository_blocking` as the `repo_dir` destination passed to the `git clone` CLI, Git for Windows rejects the verbatim prefix with "Invalid argument" — even with `core.longpaths` set. The canonical form is a deliberate internal security check (containment under the configured repos root); it just isn't a valid external CLI argument. Add a `path_for_external_command()` helper that converts a verbatim-prefixed absolute path to its ordinary fully-qualified form (`C:\Users\...`) on Windows and is a pass-through elsewhere. Use it once, at the boundary where the destination is turned into a CLI argument — every other internal use of `repo_dir` (containment checks, `align_unborn_head_branch`, the `ProjectRepoCloneResult.path` returned to the frontend) keeps the canonical form unchanged. Test coverage: - `clone_args_strip_verbatim_prefix_from_destination` (the Windows regression the reporter asked for; on Windows CI it validates the strip, elsewhere it validates the pass-through) - `path_for_external_command_leaves_ordinary_absolute_paths_alone` Verification performed: - `cargo check --lib` on macOS (cfg(not(windows)) branch): clean - `rustc --edition 2021 --crate-type lib --target x86_64-pc-windows-msvc` on an extracted copy of the helper (cfg(windows) branch): clean (caught and fixed a `prefix_component.kind` field-vs-method bug during review) - `cargo test --lib project_git_workflow::tests`: 14/14 pass - `cargo clippy --lib`: zero warnings Closes block#3707. Signed-off-by: Sarthak Singh <sarthak.singh@juspay.in>
|
@iroiro147 I cloned and built your branch and i was able to clone! |
Author
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.
Fixes #3707.
Root cause
When the Windows desktop app's repos root is canonicalized,
Path::canonicalize()returns a verbatim extended-length path (\\?\C:\...). Inclone_project_repository_blocking, that canonicalrepos_rootflows throughrepos_root.join(repo_name)and intogit clone <url> <destination>— and Git for Windows rejects the verbatim prefix withfatal: could not create work tree dir '\\?\C:\...': Invalid argument, even withcore.longpaths=true.The canonical form is the correct internal representation (the containment check against
repos_rootlegitimate on it); it simply isn't a valid argument for the external Git CLI.Fix
Add
path_for_external_command(), which strips the\\?\<DRIVE>:\prefix on Windows (cfg(windows)branch) and passes through unchanged elsewhere. Used exactly once, at the CLI-argument boundary. Everything else — containment checks,align_unborn_head_branch,.gitinspection, thepathfield returned to the frontend — continues to see the canonical form, so no security or behavior invariants shift.Test coverage
clone_args_strip_verbatim_prefix_from_destination— the regression test the reporter asked for. On Windows CI it validates the strip; on other hosts it validates the pass-through while still exercising cfg guard logic. (The clone-destination assembly itself iscfg-agnostic and covered by existing tests; the only Windows-specific branch is the prefix strip.)path_for_external_command_leaves_ordinary_absolute_paths_aloneVerification performed
cargo check --libon macOS (cfg(not(windows)) branch): cleanrustc --edition 2021 --crate-type lib --target x86_64-pc-windows-msvcon an extracted copy of the helper (cfg(windows) branch): clean. This caught a real bug during review — the initial draft usedprefix_component.kindas if it were a field; it's a method (prefix_component.kind()). The full cross-targetcargo checkcan't complete on macOS because some transitive deps need a Windows C toolchain (ring); the isolated-rustc route exercises the exact production code under the Windows cfg without the C-dep surface.cargo test --lib project_git_workflow::tests: 14/14 pass (12 prior + 2 new)cargo clippy --lib: zero warnings