Skip to content

[native_toolchain_c] Decode vswhere UTF-8 output correctly on Windows - #3773

Merged
dcharkes merged 3 commits into
dart-lang:mainfrom
dzylikecode:fix/native-toolchain-c-vswhere-utf8
Nov 5, 2026
Merged

[native_toolchain_c] Decode vswhere UTF-8 output correctly on Windows#3773
dcharkes merged 3 commits into
dart-lang:mainfrom
dzylikecode:fix/native-toolchain-c-vswhere-utf8

Conversation

@dzylikecode

@dzylikecode dzylikecode commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Description

Related to #3593.

VisualStudioResolver requests vswhere.exe -format json -utf8, but
runProcess decoded its output using the Windows system encoding. On a Chinese
Windows host, this corrupts localized JSON text and can cause a
FormatException in parseVswhere or break non-ASCII installation paths.

Add optional stdout/stderr encoding parameters to the internal process helper,
keeping systemEncoding as the default for existing native tool invocations.
The vswhere -utf8 invocation explicitly selects UTF-8. Decode streams rather
than individual pipe chunks so multibyte characters split across events survive.

Add a resolver regression using localized JSON and a Chinese installation path,
with every byte emitted in a separate event. It fails against the original
implementation. Process tests cover independently selected stdout/stderr
encodings, captured output, streaming logs, nonzero exit codes, and preservation
of the default system encoding.

Validation on Windows x64 (Chinese system encoding), Dart 3.13.7:

  • Process utility, tool resolver and CBuilder unit tests: 32 passed.
  • Combined process/MSVC suite: 30 passed, 5 failed because the corresponding
    ARM64 Visual Studio components are not installed. The localized JSON
    regression and real Visual Studio x64 discovery pass.
  • Repository CI analysis, format, license and dependency checks pass.
  • The real hooks_runner native_add build integration test passes, including
    MSVC discovery and native compilation.
  • The real CBuilder flags test passes, compiling and executing a C program.

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.

@dzylikecode dzylikecode mentioned this pull request Oct 30, 2026
Merged
6 tasks done
dcharkes
dcharkes approved these changes Nov 3, 2026
View reviewed changes

@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.

LGTM!

Thanks @dzylikecode !

(Great to see that the fake process manager helps us test this! We had untestable fixes for other languages earlier before we had a process manager.)

@codecov

codecov Bot commented Nov 3, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.86%. Comparing base (264318e) to head (9aed011).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3773      +/-   ##
==========================================
+ Coverage   85.84%   85.86%   +0.01%     
==========================================
  Files         322      322              
  Lines       20877    20871       -6     
==========================================
- Hits        17922    17920       -2     
+ Misses       2955     2951       -4
Flag Coverage Δ
native_pkgs_macos 81.77% <100.00%> (+0.03%) ⬆️
native_pkgs_ubuntu 69.19% <100.00%> (+0.02%) ⬆️
native_pkgs_windows 72.13% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
code_assets 95.31% <ø> (ø)
data_assets 91.56% <ø> (ø)
ffi 100.00% <ø> (ø)
ffigen 91.45% <ø> (ø)
hooks 86.40% <ø> (ø)
hooks_runner 90.52% <ø> (ø)
jni 66.34% <ø> (ø)
jni_flutter ∅ <ø> (∅)
jni_util 25.00% <ø> (ø)
jnigen 82.85% <ø> (ø)
json_syntax_generator 42.01% <ø> (ø)
native_test_helpers 85.71% <ø> (ø)
native_toolchain_c 96.24% <100.00%> (+0.24%) ⬆️
objective_c 79.56% <ø> (ø)
pub_formats ∅ <ø> (∅)
record_use 81.48% <ø> (ø)
snippet_tool 66.23% <ø> (ø)
swift2objc 92.95% <ø> (ø)
swiftgen 80.41% <ø> (ø)
test_case_selector 97.28% <ø> (ø)
Files with missing lines Coverage Δ
...ive_toolchain_c/lib/src/native_toolchain/msvc.dart 95.62% <ø> (ø)
.../native_toolchain_c/lib/src/utils/run_process.dart 100.00% <100.00%> (+8.00%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dzylikecode dzylikecode changed the title [native_toolchain_c]: Decode vswhere UTF-8 output correctly on Windows [native_toolchain_c] Decode vswhere UTF-8 output correctly on Windows Nov 4, 2026
@dcharkes
dcharkes merged commit 0eada93 into dart-lang:main Nov 5, 2026
55 of 57 checks passed
copybara-service Bot pushed a commit to dart-lang/sdk that referenced this pull request Nov 12, 2026
@natebosch dart-scoped@luci-project-accounts.iam.gserviceaccount.com
[deps] rev dartdoc, http, native
5512abf 
Revisions updated by `dart tools/rev_sdk_deps.dart`.

dartdoc (https://github.com/dart-lang/dartdoc/compare/2f6847b..03ec6df):
  03ec6df6  Thu Nov 5 11:12:45 2026 +0200  Sergey G. Grekhov  Add tests for type parameters (dart-lang/dartdoc#4308)
  14a91302  Tue Nov 3 04:24:32 2026 +0100  Moritz  Migrate to ecosystem health publish check and setup-dart publish workflow (dart-lang/dartdoc#4310)
  cd8bf52c  Mon Nov 2 14:19:25 2026 +0100  Sigurd Meldgaard  Support standalone AOT compilation and embed resources in html_resources.g.dart (dart-lang/dartdoc#4298)
  cb1b1b2f  Mon Nov 2 14:39:28 2026 +0200  Sergey G. Grekhov  Add tests for parameters (dart-lang/dartdoc#4304)
  07506415  Mon Nov 2 12:14:22 2026 +0100  dependabot[bot]  Bump the github-actions group across 1 directory with 3 updates (dart-lang/dartdoc#4311)
  494aec7f  Mon Nov 2 12:14:05 2026 +0100  Sarah Zakarias  Add a "Documentation inherited from" note to inherited doc comments (dart-lang/dartdoc#4309)
  527502a9  Mon Nov 2 11:21:46 2026 +0100  Sarah Zakarias  Give the mock SDK the version of the SDK running the tests (dart-lang/dartdoc#4312)

http (https://github.com/dart-lang/http/compare/1f869d0..4646fe6):
  4646fe6  Thu Nov 5 02:41:04 2026 +0530  Chandra ObulaReddy  [http] Don't throw in RetryClient when `when` has read the response stream (dart-lang/http#2040)

native (https://github.com/dart-lang/native/compare/1838637..6287745):
  6287745ca  Thu Nov 5 20:37:11 2026 +0800  Dz  [hooks_runner] Fix UTF-8 decoding of hook output on Windows (dart-lang/native#3772)
  0eada93f9  Thu Nov 5 19:18:09 2026 +0800  Dz  [native_toolchain_c] Decode vswhere UTF-8 output correctly on Windows (dart-lang/native#3773)

Change-Id: If15b14dac054320ded19f0633a71b77463b9b718
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/574640
Auto-Submit: Nate Bosch <[email protected]>
Commit-Queue: Nicholas Shahan <[email protected]>
Reviewed-by: Nicholas Shahan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dcharkes dcharkes dcharkes approved these changes

Assignees

No one assigned

Labels

package:native_toolchain_c

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dzylikecode @dcharkes