Skip to content

Conversation

yaakovschectman
Copy link
Contributor

Add a warning to Flutter Doctor if Topaz OFD is found as a process on the system.
The protection module used by the Bank of Brazil has been identified as causing build failures when using VS with CMake for Windows (see #121366 (comment)). Disabling the software allows the build to succeed again.

If a running process is found by flutter doctor whose path contains Topaz OFD\Warsaw\core.exe, a warning message is generated to convey this.

Addresses #121366

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@yaakovschectman yaakovschectman added tool Affects the "flutter" command-line tool. See also t: labels. a: internationalization Supporting other languages or locales. (aka i18n) platform-windows Building on or for Windows specifically t: flutter doctor Problem related to the "flutter doctor" tool a: desktop Running on desktop a: build Building flutter applications with the tool labels Jan 8, 2024
@yaakovschectman yaakovschectman self-assigned this Jan 8, 2024
@github-actions github-actions bot removed the a: internationalization Supporting other languages or locales. (aka i18n) label Jan 8, 2024
Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

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

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

class ProcessLister {
Future<String> getProcessesWithPath(String? filter) async {
final String argument = filter == null ? 'Get-Process $filter | Format-List Path' : 'Get-Process | Format-List Path';
final ProcessResult taskResult = await Process.run('powershell', <String>['-command', argument]);
Copy link
Member

Choose a reason for hiding this comment

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

Could the subprocess fail with an error code? How should that be handled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What are your thoughts on returning partial with a message indicating that the check failed when the exit code is non-zero?

Copy link
Member

Choose a reason for hiding this comment

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

Looking at some of the other examples, I think missing might make more sense? Wondering if @christopherfujino has an opinion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think missing, because we'd want to investigate further and see if this approach is not robust for all windows setups.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So missing when the exit code is nonzero, but is partial still desired for when Topaz is found?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think missing in both cases. In my mind, partial is just if a validator has sub-validators, with mixed results.

@yaakovschectman yaakovschectman marked this pull request as ready for review January 9, 2024 15:06
class ProcessLister {
Future<String> getProcessesWithPath(String? filter) async {
final String argument = filter != null ? 'Get-Process $filter | Format-List Path' : 'Get-Process | Format-List Path';
final ProcessResult taskResult = await Process.run('powershell', <String>['-command', argument]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, please use the ProcessManager class, and not directly invoking the static Process.run method

import '../base/io.dart';
import '../doctor_validator.dart';

const String kCoreProcessPattern = r'Topaz OFD\\Warsaw\\core.exe';
Copy link
Contributor

Choose a reason for hiding this comment

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

if this is a regex, you need to escape the . too

Copy link
Contributor

@eliasyishak eliasyishak left a comment

Choose a reason for hiding this comment

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

LGTM after @loic-sharma's comments are finished! Thank you for refactoring

@yaakovschectman
Copy link
Contributor Author

@christopherfujino With Elias' approval submitted, do you have any remaining concerns or requests blocking this merge?

Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@loic-sharma loic-sharma left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@yaakovschectman yaakovschectman merged commit 3123d98 into flutter:master Jan 18, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 19, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 19, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 19, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 19, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 19, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 19, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 19, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jan 19, 2024
Manual roll Flutter from f77f824 to 684247a (39 revisions)

Manual roll requested by [email protected]

flutter/flutter@f77f824...684247a

2024-01-19 [email protected] Use Integer instead of int in map in flutter.groovy  (flutter/flutter#141895)
2024-01-19 [email protected] Roll Flutter Engine from c953c83112ba to f2b441a26416 (4 revisions) (flutter/flutter#141894)
2024-01-19 [email protected] Native assets: package in framework on iOS and MacOS (flutter/flutter#140907)
2024-01-19 [email protected] Revert "Make tests more resilient to Skia gold failures and refactor flutter_goldens for extensive technical debt removal (#140101)" (flutter/flutter#141814)
2024-01-19 [email protected] Roll Flutter Engine from 538975f2511b to c953c83112ba (3 revisions) (flutter/flutter#141886)
2024-01-19 [email protected] Add `showDragHandle` to `showBottomSheet` (flutter/flutter#141754)
2024-01-19 [email protected] Roll Flutter Engine from 9a6c64de8a46 to 538975f2511b (8 revisions) (flutter/flutter#141881)
2024-01-19 [email protected] Make pumpWidget's arguments named (flutter/flutter#141728)
2024-01-19 [email protected] Tiny fix inaccurate documentations about bindings (flutter/flutter#140282)
2024-01-19 [email protected] Roll engine to 9a6c64de8a4694cef59a338cd33ac1a9e7d23d9d (flutter/flutter#141870)
2024-01-19 [email protected] Roll Packages from 83c2c4d to 129e08c (13 revisions) (flutter/flutter#141865)
2024-01-19 [email protected] Add mac_x64_ios configuration. (flutter/flutter#141828)
2024-01-19 [email protected] Roll Flutter Engine from 90be25d8aac3 to d1afda52d254 (1 revision) (flutter/flutter#141825)
2024-01-19 [email protected] Move the requestKeyboard up to the widgets layer (flutter/flutter#141655)
2024-01-19 [email protected] Roll Flutter Engine from dde3ebf6551a to 90be25d8aac3 (1 revision) (flutter/flutter#141817)
2024-01-18 [email protected] enable more tests in web mode (flutter/flutter#141791)
2024-01-18 [email protected] Roll Flutter Engine from 9dded186bcff to dde3ebf6551a (2 revisions) (flutter/flutter#141811)
2024-01-18 [email protected] Update margin between label and icon in Tab to better reflect Material specs (flutter/flutter#140698)
2024-01-18 [email protected] Roll Flutter Engine from 3106e08e1219 to 9dded186bcff (2 revisions) (flutter/flutter#141807)
2024-01-18 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Enable native compilation for windows-arm64 " (flutter/flutter#141809)
2024-01-18 [email protected] Run framework_tests_misc in arm64 and x64. (flutter/flutter#141797)
2024-01-18 [email protected] Roll Flutter Engine from f4a4f046b173 to 3106e08e1219 (1 revision) (flutter/flutter#141802)
2024-01-18 49699333+dependabot[bot]@users.noreply.github.com Bump actions/upload-artifact from 4.1.0 to 4.2.0 (flutter/flutter#141803)
2024-01-18 [email protected] Update labeler.yml (flutter/flutter#141697)
2024-01-18 [email protected] Roll Flutter Engine from 75400c49fa0b to f4a4f046b173 (2 revisions) (flutter/flutter#141800)
2024-01-18 [email protected] Reland "Remove hack from PageView." (flutter/flutter#141533)
2024-01-18 [email protected] ScaleGestureRecognizer pointerCount=2 for trackpad gestures (flutter/flutter#140745)
2024-01-18 [email protected] Roll Flutter Engine from de68e7612948 to 75400c49fa0b (2 revisions) (flutter/flutter#141796)
2024-01-18 [email protected] Run `flutter_gallery_ios__start_up` test on Mac-14 in staging (flutter/flutter#141795)
2024-01-18 [email protected] Roll Flutter Engine from d80fe1cb5854 to de68e7612948 (1 revision) (flutter/flutter#141789)
2024-01-18 [email protected] Enable native compilation for windows-arm64  (flutter/flutter#137618)
2024-01-18 [email protected] [github actions] Fix token issue on actions/checkout package (flutter/flutter#141652)
2024-01-18 [email protected] Roll Flutter Engine from b75d6d80d813 to d80fe1cb5854 (2 revisions) (flutter/flutter#141785)
2024-01-18 [email protected] Revert "Native assets: roll deps" (flutter/flutter#141748)
2024-01-18 [email protected] Deprecate M2 curves (flutter/flutter#134417)
2024-01-18 [email protected] Fix: TextField can inherit `errorStyle` from `InputDecorationTheme`. (flutter/flutter#141227)
2024-01-18 [email protected] Add check for Bank of Brazil security module to Windows Flutter Doctor validators (flutter/flutter#141135)
2024-01-18 [email protected] Fix gradle lints No semantic change should be present. (flutter/flutter#141692)
2024-01-18 [email protected] Roll Packages from 1a2b780 to 83c2c4d (5 revisions) (flutter/flutter#141778)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: build Building flutter applications with the tool a: desktop Running on desktop platform-windows Building on or for Windows specifically t: flutter doctor Problem related to the "flutter doctor" tool tool Affects the "flutter" command-line tool. See also t: labels.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants