[native_toolchain_c] Fix/windows program files spaces by JoshuaR457 · Pull Request #3323 · dart-lang/native

JoshuaR457 · 2026-04-21T10:23:12Z

Description

Building a Flutter Windows app fails when Visual Studio Build Tools is installed in the default path (C:\Program Files (x86)\...) because runInShell: true causes cmd.exe to parse the executable path, splitting on the space in "Program Files" and trying to execute C:\Program as a command.

Fix

Remove runInShell: Platform.isWindows && workingDirectory != null. Modern Dart supports workingDirectory on Windows natively without needing a shell wrapper, so this workaround is no longer needed. Removing it means cl.exe is invoked directly, bypassing cmd.exe path parsing entirely.
Quote environment variable values containing spaces in the commandString log string so the logged command is accurate and reproducible.

Related Issues

Fixes #3321.
Fixes #2848.

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.

google-cla · 2026-04-21T10:23:16Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

dcharkes

Great! 🙏

dcharkes · 2026-04-21T10:29:56Z

+## 0.18.1
+- Remove `runInShell: Platform.isWindows && workingDirectory != null`. Modern
+  Dart supports `workingDirectory` on Windows natively without needing a shell
+  wrapper, so this workaround is no longer needed. Removing it means `cl.exe`


Do you have a reference to a PR that fixed that issue? (I don't remember why I added this, but I did add this because something failed on Windows.)

I don't have a specific PR reference for when workingDirectory support was added for Windows without runInShell. If you added it originally because something failed, it may be safer to keep runInShell as-is and instead focus the fix on quoting the executable path when runInShell is true.

Would you prefer I take that approach instead?

I think the first copy of that code I added in https://dart-review.googlesource.com/c/sdk/+/300203/19/pkg/native_assets_builder/lib/src/utils/run_process.dart but I can't find the reason why. I just remember things broke without it.

it may be safer to keep runInShell as-is and instead focus the fix on quoting the executable path when runInShell is true.

Agreed. Let's try if that works.

Done! I've reverted the runInShell change and instead quote the executable path when it contains spaces and runInShell is true on Windows. This prevents cmd.exe from splitting the path on spaces (e.g. C:\Program Files (x86)\...).

dcharkes · 2026-04-21T10:30:33Z

    workingDirectory: workingDirectory?.toFilePath(),
    environment: environment,
-    runInShell: Platform.isWindows && workingDirectory != null,
+    runInShell: false,


We need an integration test that exercises this. Can you add a test that fails without the fix?

I've updated the tests to use an executable that lives in a path containing spaces. Without the fix (runInShell: true), cmd.exe fails to find the executable. With the fix (runInShell: false), Process.start passes the path directly and it works correctly.

github-actions · 2026-04-21T10:32:04Z

Package publishing

Package	Version	Status	Publish tag (post-merge)
package:code_assets	1.0.0	already published at pub.dev
package:data_assets	0.19.6	already published at pub.dev
package:ffi	2.2.0	already published at pub.dev
package:hooks	1.0.3	already published at pub.dev
package:hooks_runner	1.2.1	already published at pub.dev
package:jni_flutter	1.0.1	already published at pub.dev
package:native_toolchain_c	0.18.1	ready to publish	`native_toolchain_c-v0.18.1`
package:record_use	0.6.1-wip	WIP (no publish necessary)
package:swift2objc	0.2.0-wip	WIP (no publish necessary)
package:swiftgen	0.1.2	already published at pub.dev

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

github-actions · 2026-04-21T10:32:53Z

PR Health

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.

Breaking changes ✔️

Package	Change	Current Version	New Version	Needed Version	Looking good?
code_assets	None	1.0.0	1.0.0	1.0.0	✔️
native_toolchain_c	None	0.18.0	0.18.1	0.18.0	✔️

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.

dcharkes · 2026-04-21T20:58:40Z

  final stderrBuffer = StringBuffer();
+  final executablePath = executable.toFilePath();
+  // When running in shell on Windows, cmd.exe splits unquoted paths on spaces.
+  // Quote the executable path if it contains spaces to prevent this.


maybe make a final runInShell =
and then final executable = runInShell && executablePath.contains(' ') ? ...

Refactored as recommended. Also looked into the CI failure, I think I got the issue but please don't qute me on that.

dcharkes · 2026-04-23T10:13:19Z

 import 'package:native_toolchain_c/src/utils/run_process.dart';
 import 'package:test/test.dart';

+export 'package:native_toolchain_c/src/utils/run_process.dart';


What does this export do here?

Hey, to be honest I'm not to sure. I tried to fix the CI but didn't really understand it, Claude told me this was the fix. Sorry! I'm happy to reverse it though if you want to look into it.
The fix for the original issue however should be correct.

You should query the AI to understand why. Good AI practices is that you understand the code that you're sending out for review. (I also write almost all of my code with AI, this wonderful new reality. 😄)

I did and i believe I got it now. Unfortunately one of the steps to get the workflow to not fail, was formatting the files. If this isn't what you want feel free to revert it.

Yeah formatting 👍 Edit: Make sure you're on the last version of the Dart/Flutter SDK. Because it looks like the formatting is undoing some formatting that I saw a couple of weeks ago.

dcharkes · 2026-04-23T16:03:09Z

The PR also needs a rebase due to merge conflict.

github-actions Bot added package:native_toolchain_c package:code_assets labels Apr 21, 2026

dcharkes reviewed Apr 21, 2026

View reviewed changes

JoshuaR457 changed the title ~~Fix/windows program files spaces~~ [native_toolchain_c] Fix/windows program files spaces Apr 21, 2026

dcharkes reviewed Apr 21, 2026

View reviewed changes

dcharkes reviewed Apr 23, 2026

View reviewed changes

github-actions Bot added package:hooks_runner package:jnigen package:jni package:native_doc_dartifier package:jni_flutter labels Apr 23, 2026

dcharkes mentioned this pull request Apr 24, 2026

[native_toolchain_c] Fix Windows env quoting in runProcess (#3321) #3329

Open

6 tasks

JoshuaR457 added 9 commits April 24, 2026 18:10

fix: quote env values with spaces and remove runInShell on Windows

e85ecb9

typo fix

2533484

ran dart tool/ci.dart --all

b4d942a

added changelog and version bump

2477392

test: add regression test for runProcess with spaces in env values

d94218a

test: add regression test for runProcess with spaces in executable path

b997ca9

fix: quote executable path with spaces when runInShell on Windows

fb85924

fix: refactor runInShell and executable quoting per review

7c4d3b6

fixed the CI Issue

203ec15

JoshuaR457 force-pushed the fix/windows-program-files-spaces branch from d8ec02c to 203ec15 Compare April 24, 2026 18:15

Conversation

JoshuaR457 commented Apr 21, 2026

Description

Fix

Related Issues

PR Checklist

Uh oh!

google-cla Bot commented Apr 21, 2026

Uh oh!

dcharkes left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Package publishing

Uh oh!

github-actions Bot commented Apr 21, 2026

PR Health

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dcharkes Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dcharkes commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Apr 21, 2026 •

edited

Loading

dcharkes Apr 23, 2026 •

edited

Loading