-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Ignore replacement characters from vswhere.exe output #104284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ignore replacement characters from vswhere.exe output #104284
Conversation
packages/flutter_tools/test/general.shard/windows/visual_studio_test.dart
Outdated
Show resolved
Hide resolved
cbracken
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good! Thanks for fixing this. Just a couple minor nits!
packages/flutter_tools/test/general.shard/windows/visual_studio_test.dart
Outdated
Show resolved
Hide resolved
cbracken
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
christopherfujino
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooh nice! Thanks so much for fixing this
da9ccc1 to
4e560c2
Compare
Flutter uses `vswhere.exe` to find Visual Studio installations and determine if they satisfy Flutter's requirements. However, `vswhere.exe`'s JSON output is known to contain bad UTF-8. This change ignores bad UTF-8 as long as they affect JSON properties that are either unused, or, used only for display purposes by Flutter. Fixes: flutter#102451
…ere.exe (#105153) * [tool] Consistent FakeProcessManager.run/runSync (#103947) `FakeProcessManager` is a test-oriented implementation of `ProcessManager` that simulates launching processes and returning `ProcessResult` objects whose `exitCode`, `stdout`, `stderr` can be used to write platform-portable, hermetic tests that don't rely on actually launching processes from executables on disk. Its `run` and `runSync` methods provide asynchronous and synchronous variants of this functionality. Previously, the behaviour of `run` and `runSync` were inconsistent with regards to the treatment of the `stdoutEncoding` (similarly, `stderrEncoding`) parameters: `run`: * if the encoding was null, `ProcessResult.stdout` was returned as a String in UTF-8 encoding. This was incorrect. The behaviour as specified in `ProcessResult.stdout` is that in this case, a raw `List<int>` should be returned. * If the encoding was unspecified, `ProcessResult.stdout` was returned as a `String` in the `io.systemEncoding` encoding. This was correct. * If the encoding was non-null, `ProcessResult.stdout` was returned as a `String` in the specified encoding. This was correct. `runSync`: * if the encoding was null, `ProcessResult.stdout` was returned as a `List<int>` in UTF-8 encoding. This was incorrect. The behaviour as specified in `ProcessResult.stdout` is that in this case, a raw `List<int>` should be returned. * If the encoding was unspecified, `ProcessResult.stdout` was returned as `List<int>` in UTF-8 encoding. This was incorrect. The behaviour as specified in `ProcessResult.stdout` is that in this case, a String a `String` in the `io.systemEncoding` encoding should be returned. * if the encoding was non-null, `ProcessResult.stdout` was returned as a `String` in unknown (but probably UTF-8) encoding. This was incorrect. The behaviour as specified in `ProcessResult.stdout` is that in this case, a `String` in the specified encoding should be returned. `_FakeProcess`, from which we obtain the fake stdout and stderr values now holds these fields as raw `List<int>` of bytes rather than as `String`s. It is up to the user to supply values that can be decoded with the encoding passed to `run`/`runAsync`. `run` and `runAsync` have been updated to set stdout (likewise, stderr) as specified in the `ProcessResult` documentation. This is pre-factoring for #102451, in which the tool throws an exception when processing the JSON output from stdout of the `vswhere.exe` tool, whose output was found to include the `U+FFFD` Unicode replacement character during UTF-8 decoding, which triggers a `toolExit` exception when decoded using our [Utf8Decoder][decoder] configured with `reportErrors` = true. Because `FakeProcessManager.runAsync` did not previously invoke `utf8.decode` on its output (behaviour which differs from the non-fake implementation), it was impossible to write tests to verify the fix. Ref: https://api.flutter.dev/flutter/dart-io/ProcessResult/stdout.html Issue: #102451 [decoder]: https://github.com/flutter/flutter/blob/fd312f1ccff909fde28d2247a489bf210bbc6c48/packages/flutter_tools/lib/src/convert.dart#L51-L60 * Refactor vswhere.exe integration (#104133) `VisualStudio` calls `vswhere.exe` to find Visual Studio installations and determine if they satisfy Flutter's requirements. Previously, `VisualStudio` stored the JSON output from `vswhere.exe` as `Map`s, resulting in duplicated logic to read the JSON output (once to validate values, second to expose values). Also, `VisualStudio` stored two copies of the JSON output (the latest valid installation as well as the latest VS installation). This change simplifies `VisualStudio` by introducing a new `VswhereDetails`. This type contains the logic to read `vswhere.exe`'s JSON output, and, understand whether an installation is usable by Flutter. In the future, this `VswhereDetails` type will be used to make Flutter doctor resilient to bad UTF-8 output from `vswhere.exe`. Part of #102451. * Ignore replacement characters from vswhere.exe output (#104284) Flutter uses `vswhere.exe` to find Visual Studio installations and determine if they satisfy Flutter's requirements. However, `vswhere.exe`'s JSON output is known to contain bad UTF-8. This change ignores bad UTF-8 as long as they affect JSON properties that are either unused, or, used only for display purposes by Flutter. Fixes: #102451 Co-authored-by: Chris Bracken <[email protected]> Co-authored-by: Kevin Chisholm <[email protected]>
Flutter uses `vswhere.exe` to find Visual Studio installations and determine if they satisfy Flutter's requirements. However, `vswhere.exe`'s JSON output is known to contain bad UTF-8. This change ignores bad UTF-8 as long as they affect JSON properties that are either unused, or, used only for display purposes by Flutter. Fixes: flutter#102451

Flutter uses
vswhere.exeto find Visual Studio installations and determine if they satisfy Flutter's requirements. However,vswhere.exe's JSON output is known to contain bad UTF-8. This change ignores bad UTF-8 as long as they affect JSON properties that are unused by Flutter.Fixes: #102451
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.