[ Tool ] Don't use .NET APIs in update_engine_version.ps1
#172974
Conversation
Powershell instances running in `ConstrainedLanguage` mode can't always access .NET APIs due to system administrator security configurations, including gWindows VMs. This change removes the use of `System.*` APIs from update_engine_version.ps1 in favor of dedicated Powershell APIs. Fixes #172895
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
bkonyi
added
cp: beta
|
This fix was verified by @johnniwinther, who encountered the issue on his gWindows environment. I was unable to reproduce this locally. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly identifies a potential issue with using .NET APIs in constrained PowerShell environments and replaces them with idiomatic PowerShell cmdlets. This is a great improvement for the script's robustness.
My review focuses on a potential regression related to file encoding. The proposed changes might unintentionally add a Byte Order Mark (BOM) to the generated files, which could affect downstream tooling. I've suggested using -Encoding Ascii to prevent this and maintain the original behavior.
Overall, this is a valuable change, and with the suggested adjustments, it will be ready to merge.
|
@bkonyi 2 months from now we'll likely forget about PowerShell Constrained Mode, and someone might accidentally re-add .NET API usages. Is there an easy way to add a test that enables PowerShell Constrained Mode and runs a Flutter command to verify we didn't regress gWindows? Feel free to land this PR without adding such a test to unblock customers immediately. |
We could update our infra to set the Another option is that we could just update our scripts to force constrained language mode (i.e., adding |
|
However, that's not a guarantee that we would have caught this failure in the first place. For some reason, I was never able to reproduce this issue on my gWindows machine but Johnni was able to consistently. |
flutter-dashboard
bot
removed
the
autosubmit
I suspect you have your windows machine exempt? |
I do, but I've also confirmed that my Powershell is also running in constrained language mode as well. |
…172974) Powershell instances running in `ConstrainedLanguage` mode can't always access .NET APIs due to system administrator security configurations, including gWindows VMs. This change removes the use of `System.*` APIs from update_engine_version.ps1 in favor of dedicated Powershell APIs. Fixes flutter#172895
…172974) Powershell instances running in `ConstrainedLanguage` mode can't always access .NET APIs due to system administrator security configurations, including gWindows VMs. This change removes the use of `System.*` APIs from update_engine_version.ps1 in favor of dedicated Powershell APIs. Fixes flutter#172895
…172974) Powershell instances running in `ConstrainedLanguage` mode can't always access .NET APIs due to system administrator security configurations, including gWindows VMs. This change removes the use of `System.*` APIs from update_engine_version.ps1 in favor of dedicated Powershell APIs. Fixes flutter#172895
…172974) Powershell instances running in `ConstrainedLanguage` mode can't always access .NET APIs due to system administrator security configurations, including gWindows VMs. This change removes the use of `System.*` APIs from update_engine_version.ps1 in favor of dedicated Powershell APIs. Fixes flutter#172895
3 participants
Powershell instances running in
ConstrainedLanguagemode can't always access .NET APIs due to system administrator security configurations, including gWindows VMs.This change removes the use of
System.*APIs from update_engine_version.ps1 in favor of dedicated Powershell APIs.Fixes #172895