Skip to content

fix: support relative forward-slash paths on Windows - #909

Open
tianrking wants to merge 2 commits into
sharkdp:masterfrom
tianrking:agent/windows-forward-slash-paths
Open

fix: support relative forward-slash paths on Windows#909
tianrking wants to merge 2 commits into
sharkdp:masterfrom
tianrking:agent/windows-forward-slash-paths

Conversation

@tianrking

Copy link
Copy Markdown

Summary

  • run relative executable paths through standard Windows command-line quoting instead of passing them verbatim to cmd.exe /C
  • normalize forward slashes only in that executable path, leaving slash-containing command arguments unchanged
  • cover the regression and preserve the existing quoted-command behavior on Windows

Fixes #818

Root cause

The default Windows executor appended the full command line with raw_arg(). For a command beginning with ./ or ../, cmd.exe interpreted the leading . as a command instead of a relative path.

Validation

  • cargo fmt -- --check
  • cargo clippy --locked --all-targets
  • cargo test --locked normalizes_only_the_relative_executable_path
  • cargo test --locked windows_runs_executable_paths_with_forward_slashes
  • cargo test --locked windows_quote_args

cargo test --locked exercises the new regression and 34 of 36 integration tests on Windows. The remaining two integration tests invoke Unix shell syntax (echo '...' and sleep) without a Windows guard; they are unrelated to this change.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3c3990fa05

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/benchmark/executor.rs Outdated
// `cmd.exe /C` treats the leading `.` as a command when passed
// verbatim. Normalize only the executable path and let `Command`
// quote the complete invocation.
command_builder.arg(normalize_relative_command_path_for_cmd(&command_line));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve raw cmd quoting for relative commands

On Windows, this branch switches cmd.exe /C from raw_arg to arg whenever the benchmark starts with ./ or ../. For a command such as ./tool.exe "input file.txt", Rust applies C-runtime escaping to the inner quotes (for example \"...\"), but cmd.exe /c has different escaping rules—the Rust raw_arg docs call out this exact case: https://doc.rust-lang.org/std/os/windows/process/trait.CommandExt.html#tymethod.raw_arg. The result is that quoted arguments after a relative executable can be split or delivered with literal quotes/backslashes; normalize the executable path but keep appending the resulting command string with raw_arg so existing quoted-command behavior is preserved.

Useful? React with 👍 / 👎.

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.

Default execution on Windows fails when using forward slashes as directory separator

1 participant