Skip to content

[hooks_runner] Fix handling of executable paths and arguments containing spaces on Windows - #3466

Merged
dcharkes merged 4 commits into
dart-lang:mainfrom
jakobkordez:fix/hooks_runner_space
Jul 10, 2026
Merged

[hooks_runner] Fix handling of executable paths and arguments containing spaces on Windows#3466
dcharkes merged 4 commits into
dart-lang:mainfrom
jakobkordez:fix/hooks_runner_space

Conversation

@jakobkordez

Copy link
Copy Markdown
Contributor

Description

  • Adds tests for executable paths and arguments with spaces in runProcess
  • Fixes runProcess handling of executable paths that contain spaces

Related Issues

Fixes #2993
Relates to #3026

PR Checklist

  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
  • I've run dart tool/ci.dart --all locally and resolved all issues identified. This ensures the PR is formatted, has no lint errors, and ran all code generators. This applies to the packages part of the toplevel pubspec.yaml workspace.
  • All existing and new tests are passing. I added new tests to check the change I am making.
  • The PR is actually solving the issue. PRs that don't solve the issue will be closed. Please be respectful of the maintainers' time. If it's not clear what the issue is, feel free to ask questions on the GitHub issue before submitting a PR.
  • I have updated CHANGELOG.md for the relevant packages. (Not needed for small changes such as doc typos).
  • I have updated the pubspec package version if necessary.

@jakobkordez

Copy link
Copy Markdown
Contributor Author

@dcharkes do you have any ideas on what would be the best way to share the runProcess function between hooks_runner and native_toolchain_c?

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

PR Health

Breaking changes ✔️
Package Change Current Version New Version Needed Version Looking good?
hooks_runner Non-Breaking 1.5.0 1.6.0-wip 1.6.0-wip ✔️

This check can be disabled by tagging the PR with skip-breaking-check.

API leaks ✔️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package Leaked API symbol Leaking sources

This check can be disabled by tagging the PR with skip-leaking-check.

Changelog Entry ✔️
Package Changed Files

Changes to files need to be accounted for in their respective changelogs.

This check can be disabled by tagging the PR with skip-changelog-check.

@dcharkes

dcharkes commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

@dcharkes do you have any ideas on what would be the best way to share the runProcess function between hooks_runner and native_toolchain_c?

Not really. It's an internal implementation detail that should not go in a public API. That's why it's copy-pasta.

@dcharkes dcharkes left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should also add a test that verifies that the hooks_runner works with spaces now.

Comment thread pkgs/hooks_runner/CHANGELOG.md
Comment thread pkgs/hooks_runner/test/helpers.dart
@dcharkes
dcharkes merged commit 7dee061 into dart-lang:main Jul 10, 2026
28 checks passed
@jakobkordez
jakobkordez deleted the fix/hooks_runner_space branch July 10, 2026 11:09
@dcharkes

Copy link
Copy Markdown
Collaborator

This fails on the roll in Dart: https://dart-review.googlesource.com/c/sdk/+/523080 https://dart-ci.firebaseapp.com/cl/523080/1 new_test_failures__logs

Could you take a look at the failures? And reproduce the failures locally with a Dart SDK build with the DEP file updated?

(Otherwise, I'll have to revert this PR.)

@jakobkordez

Copy link
Copy Markdown
Contributor Author

I'm guessing this is why runInShell was used. Let me try to create a regression test and test out a few solutions

copybara-service Bot pushed a commit to dart-lang/sdk that referenced this pull request Jul 13, 2026
Roll in dart-lang/native#3466 before
releasing hooks_runner to pub and rolling it to Flutter.

Change-Id: I63af3959d3d1098351da8a2cc67819276076b546
Cq-Include-Trybots: luci.dart.try:pkg-linux-debug-try,pkg-linux-release-arm64-try,pkg-linux-release-try,pkg-mac-release-arm64-try,pkg-mac-release-try,pkg-win-release-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/523080
Auto-Submit: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Michael Goderbauer <goderbauer@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[hooks_runner] Can't run build_runner when profile folder has a space

2 participants