-
Notifications
You must be signed in to change notification settings - Fork 564
[Xamarin.Android.Build.Tasks] Catch Exception when setting Console.InputEncoding #4735
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
Conversation
|
:'( Looks like this isn't an option. |
|
I'm not sure how these changes break the windows build... |
89d35ad to
d24d261
Compare
Context: dotnet/android#4735 > https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=3771781&view=ms.vss-test-web.build-test-results-tab&runId=13535824&resultId=100072&paneView=attachments Context: dotnet/android#4567 Context: https://github.com/xamarin/androidtools/commit/a3965baeea566ba6a1f346c676d9e2d5ecba6167 The Windows Smoke Tests are failing on PR #4735, because the wrong Android SDK Build-tools version is being used: Task "ResolveAndroidTooling" (TaskId:9) Task Parameter:AndroidSdkBuildToolsVersion=29.0.2 (TaskId:9) … Trying build-tools path: C:\Users\dlab14\android-toolchain\sdk\build-tools\30.0.0-rc4 (TaskId:9) … ResolveAndroidTooling Outputs: (TaskId:9) AndroidSdkBuildToolsPath: C:\Users\dlab14\android-toolchain\sdk\build-tools\30.0.0-rc4 (TaskId:9) Here, Build-tools 30.0.0-rc4 is used. Why is that a problem? Because JDK 1.8 is still used! C:\Program Files\Android\jdk\microsoft_dist_openjdk_1.8.0.25\bin\java.exe -jar C:\Users\dlab14\android-toolchain\sdk\build-tools\30.0.0-rc4\lib\apksigner.jar sign --ks "C:\Users\dlab14\AppData\Local\Xamarin\Mono for Android\debug.keystore" --ks-pass pass:android --ks-key-alias androiddebugkey --key-pass pass:android --min-sdk-version 21 --max-sdk-version 29 … java.lang.UnsupportedClassVersionError: com/android/apksigner/ApkSignerTool has been compiled by a more recent version of the Java Runtime (class file version 53.0), this version of the Java Runtime only recognizes class file versions up to 52.0 JDK11 is required in order to use Build-tools r30+, but we don't currently support JDK11 use; see PR #4567. What's happening is that *because of* PR #4567, some of the build machines are being provisioned with Build-tools 30.0.0-rc4, i.e. e.g. `$HOME/android-toolchain/sdk/build-tools/30.0.0-rc4` *exists*. *Because* that directory exists -- and *isn't* parse-able by `System.Version.TryParse()` -- it's considered to be a "preview" version, and thus *has priority over non-preview versions*, and thus is returned *first*. *Because* it's returned *first*, `<ResolveAndroidTooling/>` prefers it, causing the `<AndroidApkSigner/>` task to subsequently fail, because it's unusable. The "workaround" is for `$(AndroidSdkBuildToolsVersion)` to be set to the desired version to use. In retrospect, however, this is all *backwards*: `AndroidSdkInfo.GetBuildToolsPaths()` shouldn't prefer preview versions, it should prefer *non-* preview versions. If you *really* want a preview version, *then* you should override `$(AndroidSdkBuildToolsPath)`. Previews should be opt-in, not opt-out. With that in mind, why were preview versions preferred in the first place? That was done in xamarin/androidtools@a3965bae to fix [Bug #30555][0], stating: > handle[] by searching for these "preview" directories first, so that > the latest tooling will be used IF the user decides to install it. > The assumption is that a stable release will NOT inlcude a suffix. While "a preview directory will only exist if someone explicitly installs it, and thus should be preferred" sounds reasonable, in retrospect it's a recipe for pain when using a shared CI environment. Just because it's there does *not* mean it should be used. [0]: https://bugzilla.xamarin.com/show_bug.cgi?id=30555
…putEncoding Context https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1130414 For some unknown reason the setting of `Console.InputEncoding` on a customers machine is throwing an IOException. This then causes Visual Studio to completely crash. ``` Description: The process was terminated due to an unhandled exception. Exception Info: System.IO.IOException at System.IO.__Error.WinIOError(Int32, System.String) at System.IO.__Error.WinIOError() at System.Console.set_InputEncoding(System.Text.Encoding) at Xamarin.Android.Tasks.Aapt2Daemon.Aapt2DaemonStart() at System.Threading.ThreadHelper.ThreadStart_Context(System.Object) at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean) at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean) at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object) at System.Threading.ThreadHelper.ThreadStart() ``` At this time we are still unsure as to what is causing the problem. But we should at least catch the exception to that Visual Studio does not crash. We need to figure out a way of reporting this issue somehow in the exception handler so we can get more data. Note: Not sure what is going on with the line ending changes :/ For reference the reason we have to use `Console.InputEncoding` is because of [this](https://referencesource.microsoft.com/#System/services/monitoring/system/diagnosticts/Process.cs,2154). The `System.Diagnostic.Process` uses the `Console.InputEncoding` when creating the `StreamWriter` for `StandardInput`.
Context: dotnet/android#4735 > https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=3771781&view=ms.vss-test-web.build-test-results-tab&runId=13535824&resultId=100072&paneView=attachments Context: dotnet/android#4567 Context: https://github.com/xamarin/androidtools/commit/a3965baeea566ba6a1f346c676d9e2d5ecba6167 The Windows Smoke Tests are failing on PR #4735, because the wrong Android SDK Build-tools version is being used: Task "ResolveAndroidTooling" (TaskId:9) Task Parameter:AndroidSdkBuildToolsVersion=29.0.2 (TaskId:9) … Trying build-tools path: C:\Users\dlab14\android-toolchain\sdk\build-tools\30.0.0-rc4 (TaskId:9) … ResolveAndroidTooling Outputs: (TaskId:9) AndroidSdkBuildToolsPath: C:\Users\dlab14\android-toolchain\sdk\build-tools\30.0.0-rc4 (TaskId:9) Here, Build-tools 30.0.0-rc4 is used. Why is that a problem? Because JDK 1.8 is still used! C:\Program Files\Android\jdk\microsoft_dist_openjdk_1.8.0.25\bin\java.exe -jar C:\Users\dlab14\android-toolchain\sdk\build-tools\30.0.0-rc4\lib\apksigner.jar sign --ks "C:\Users\dlab14\AppData\Local\Xamarin\Mono for Android\debug.keystore" --ks-pass pass:android --ks-key-alias androiddebugkey --key-pass pass:android --min-sdk-version 21 --max-sdk-version 29 … java.lang.UnsupportedClassVersionError: com/android/apksigner/ApkSignerTool has been compiled by a more recent version of the Java Runtime (class file version 53.0), this version of the Java Runtime only recognizes class file versions up to 52.0 JDK11 is required in order to use Build-tools r30+, but we don't currently support JDK11 use; see PR #4567. What's happening is that *because of* PR #4567, some of the build machines are being provisioned with Build-tools 30.0.0-rc4, i.e. e.g. `$HOME/android-toolchain/sdk/build-tools/30.0.0-rc4` *exists*. *Because* that directory exists -- and *isn't* parse-able by `System.Version.TryParse()` -- it's considered to be a "preview" version, and thus *has priority over non-preview versions*, and thus is returned *first*. *Because* it's returned *first*, `<ResolveAndroidTooling/>` prefers it, causing the `<AndroidApkSigner/>` task to subsequently fail, because it's unusable. The "workaround" is for `$(AndroidSdkBuildToolsVersion)` to be set to the desired version to use. In retrospect, however, this is all *backwards*: `AndroidSdkInfo.GetBuildToolsPaths()` shouldn't prefer preview versions, it should prefer *non-* preview versions. If you *really* want a preview version, *then* you should override `$(AndroidSdkBuildToolsPath)`. Previews should be opt-in, not opt-out. With that in mind, why were preview versions preferred in the first place? That was done in xamarin/androidtools@a3965bae to fix [Bug #30555][0], stating: > handle[] by searching for these "preview" directories first, so that > the latest tooling will be used IF the user decides to install it. > The assumption is that a stable release will NOT inlcude a suffix. While "a preview directory will only exist if someone explicitly installs it, and thus should be preferred" sounds reasonable, in retrospect it's a recipe for pain when using a shared CI environment. Just because it's there does *not* mean it should be used. [0]: https://bugzilla.xamarin.com/show_bug.cgi?id=30555
| foreach (var arg in job.Commands) | ||
| { | ||
| writer.WriteLine (arg); | ||
| using (StreamWriter writer = new StreamWriter (aapt2.StandardInput.BaseStream, MonoAndroidHelper.UTF8withoutBOM, bufferSize: 1024, leaveOpen: true)) { |
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.
Does this change fix the problem on its own? Or does the encoding have to also be set when the process is started?
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.
ok so on my machine it fixes it on its own, on CI it does not. So its not 100% effective, so I've left it in for the case where the Console.InputEncoding call fails and we need a back up.
|
Commit message: |
) Fixes: https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1130414 Context: 0d6fd83 For some unknown reason setting the [`Console.InputEncoding`][0] property on a customer's machine is throwing an `IOException`. This causes Visual Studio (Windows) to completely crash: Description: The process was terminated due to an unhandled exception. Exception Info: System.IO.IOException at System.IO.__Error.WinIOError(Int32, System.String) at System.IO.__Error.WinIOError() at System.Console.set_InputEncoding(System.Text.Encoding) at Xamarin.Android.Tasks.Aapt2Daemon.Aapt2DaemonStart() at System.Threading.ThreadHelper.ThreadStart_Context(System.Object) at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean) at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean) at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object) at System.Threading.ThreadHelper.ThreadStart() The cause of the crash is that commit 0d6fd83 sets `Console.InputEncoding` to "UTF-8 without BOM" for communication with `aapt2`, so that we can sanely use Unicode paths, and setting `Console.InputEncoding` may result in a `System.IO.IOException`. (Paths are written to `Process.StandardInput`, which can contain Unicode characters. `Console.InputEncoding` on the other hand, defaults to "ANSI", which may not support full Unicode.) Why did 0d6fd83 need to set `Console.InputEncoding` in the first place? Because [`ProcessStartInfo.StandardInputEncoding`][1] wasn't added until .NET Standard 2.1, and .NET Framework <= 4.8 doesn't support .NET Standard 2.1, only .NET Standard 2.0. Furthermore, when `ProcessStartInfo.RedirectStandardInput` is True, [`Process.StandardInput` is a `StreamWriter`][2] which uses `Console.InputEncoding` as the encoding for the underlying stream. Thus, setting `Console.InputEncoding` is the only way in .NET Standard 2.0/.NET Framework <= 4.8 to control the encoding used by `Process.StandardInput`'s `StreamWriter`. Further exploration since 0d6fd83 suggests that we could create a new `StreamWriter` around `Process.StandardInput.BaseStream` with the correct "UTF-8 without BOM" encoding, but that approach fails on CI: …\Xamarin.Android.Aapt2.targets(124,3): error APT0001: Unknown option `-o`. Please check `$(AndroidAapt2CompileExtraArgs)` and `$(AndroidAapt2LinkExtraArgs)` to see if they include any `aapt` command line arguments that are no longer valid for `aapt2` and ensure that all other arguments are valid for `aapt2`. …\Xamarin.Android.Aapt2.targets(124,3): error APT0001: Unknown option `-o`. Please check `$(AndroidAapt2CompileExtraArgs)` and `$(AndroidAapt2LinkExtraArgs)` to see if they include any `aapt` command line arguments that are no longer valid for `aapt2` and ensure that all other arguments are valid for `aapt2`. …\Xamarin.Android.Aapt2.targets(124,3): error APT0001: Unknown option `-o`. Please check `$(AndroidAapt2CompileExtraArgs)` and `$(AndroidAapt2LinkExtraArgs)` to see if they include any `aapt` command line arguments that are no longer valid for `aapt2` and ensure that all other arguments are valid for `aapt2`. …\Xamarin.Android.Aapt2.targets(124,3): error APT0001: Unknown option `-o`. Please check `$(AndroidAapt2CompileExtraArgs)` and `$(AndroidAapt2LinkExtraArgs)` to see if they include any `aapt` command line arguments that are no longer valid for `aapt2` and ensure that all other arguments are valid for `aapt2`. We do not understand why this fails on CI unless `Console.InputEncoding` is previously set. All that said, we don't want Visual Studio to crash. Fix the crash by: 1. Using System.Reflection to probe for `ProcessStartInfo.StandardInputEncoding`. If it exists, use it. 2. If (2) fails, "fall back" and set `Console.InputEncoding` as before, wrapping in a `try`/`catch` block so that `IOException`s don't escape and crash Visual Studio. TODO: We need to figure out a way of reporting this exception somehow in the exception handler so we can get more data. Regardless of whether (1) or (2) fails, as part of writing to `aapt2`s `Process.StandardInput`, we wrap `Process.StandardInput.BaseStream` into a new `StreamWriter` which uses "UTF-8 without BOM" as the encoding. This is "defense in depth," though could plausibly result in the previously mentioned APT0001 errors if setting `Console.InputEncoding` failed. We'll see what happens "in the wild." [0]: https://docs.microsoft.com/en-us/dotnet/api/system.console.inputencoding?view=netcore-3.1 [1]: https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.processstartinfo.standardinputencoding?view=netcore-3.1 [2]: https://referencesource.microsoft.com/#System/services/monitoring/system/diagnosticts/Process.cs,2154
Context: https://issuetracker.google.com/issues/150189789 Update the "legacy" `<ValidateJavaVersion/>` task to require JDK11 when using Build-tools r30, because `apksigner.jar` in Build-tools r30.0.0-rc4 requires JDK11 to execute. For example, see [this build][0] for PR #4735, which fails because JDK 1.8 is used with Build-tools r30's apksigner.jar: Task "AndroidApkSigner" Task Parameter:ApkSignerJar=C:\Users\dlab14\android-toolchain\sdk\build-tools\30.0.0-rc4\lib\apksigner.jar … Task Parameter:ToolPath=C:\Program Files\Android\jdk\microsoft_dist_openjdk_1.8.0.25\bin Task Parameter:ManifestFile=obj\Release\android\AndroidManifest.xml C:\Program Files\Android\jdk\microsoft_dist_openjdk_1.8.0.25\bin\java.exe -jar C:\Users\dlab14\android-toolchain\sdk\build-tools\30.0.0-rc4\lib\apksigner.jar sign --ks "C:\Users\dlab14\AppData\Local\Xamarin\Mono for Android\debug.keystore" --ks-pass pass:android --ks-key-alias androiddebugkey --key-pass pass:android --min-sdk-version 21 --max-sdk-version 29 "C:\A\vs2019xam00000Y-1\_work\1\s\bin\TestRelease\temp\BuildAotApplication AndÜmläüts_x86_64_True_True\bin\Release\UnnamedProject.UnnamedProject-Signed.apk" java.lang.UnsupportedClassVersionError: com/android/apksigner/ApkSignerTool has been compiled by a more recent version of the Java Runtime (class file version 53.0), this version of the Java Runtime only recognizes class file versions up to 52.0 at java.lang.ClassLoader.defineClass1(Native Method) … …\Xamarin.Android.Common.targets(2559,2): error MSB6006: "java.exe" exited with code 1. This was "implicitly" the case before commit a2b827e, because `$(AndroidSdkBuildToolsVersion)` couldn't be parsed, and thus `$(LatestSupportedJavaVersion)` was being checked for, which caused all the Designer and Windows unit tests to fail. Commit a2b827e fixed those failures, at the cost of allowing `apksigner.jar` use with JDK 1.8, which isn't correct. Fix the Build-tools + JDK version semantics within `<ValidateJavaVersion/>`. This will "re-break the world." To attempt to address *some* of that breakage, update the macOS Designer unit tests to export the `$(JavaSdkDirectory)` environment variable. This will hopefully cause the Designer build system to use the JDK11 install that xamarin-android provisions, thus avoiding an XA0032 build error. [0]: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=3771781&view=ms.vss-test-web.build-test-results-tab&runId=13535824&resultId=100072&paneView=attachments
Context: dotnet/android#4735 > https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=3771781&view=ms.vss-test-web.build-test-results-tab&runId=13535824&resultId=100072&paneView=attachments Context: dotnet/android#4567 Context: https://github.com/xamarin/androidtools/commit/a3965baeea566ba6a1f346c676d9e2d5ecba6167 The Windows Smoke Tests are failing on PR #4735, because the wrong Android SDK Build-tools version is being used: Task "ResolveAndroidTooling" (TaskId:9) Task Parameter:AndroidSdkBuildToolsVersion=29.0.2 (TaskId:9) … Trying build-tools path: C:\Users\dlab14\android-toolchain\sdk\build-tools\30.0.0-rc4 (TaskId:9) … ResolveAndroidTooling Outputs: (TaskId:9) AndroidSdkBuildToolsPath: C:\Users\dlab14\android-toolchain\sdk\build-tools\30.0.0-rc4 (TaskId:9) Here, Build-tools 30.0.0-rc4 is used. Why is that a problem? Because JDK 1.8 is still used! C:\Program Files\Android\jdk\microsoft_dist_openjdk_1.8.0.25\bin\java.exe -jar C:\Users\dlab14\android-toolchain\sdk\build-tools\30.0.0-rc4\lib\apksigner.jar sign --ks "C:\Users\dlab14\AppData\Local\Xamarin\Mono for Android\debug.keystore" --ks-pass pass:android --ks-key-alias androiddebugkey --key-pass pass:android --min-sdk-version 21 --max-sdk-version 29 … java.lang.UnsupportedClassVersionError: com/android/apksigner/ApkSignerTool has been compiled by a more recent version of the Java Runtime (class file version 53.0), this version of the Java Runtime only recognizes class file versions up to 52.0 JDK11 is required in order to use Build-tools r30+, but we don't currently support JDK11 use; see PR #4567. What's happening is that *because of* PR #4567, some of the build machines are being provisioned with Build-tools 30.0.0-rc4, i.e. e.g. `$HOME/android-toolchain/sdk/build-tools/30.0.0-rc4` *exists*. *Because* that directory exists -- and *isn't* parse-able by `System.Version.TryParse()` -- it's considered to be a "preview" version, and thus *has priority over non-preview versions*, and thus is returned *first*. *Because* it's returned *first*, `<ResolveAndroidTooling/>` prefers it, causing the `<AndroidApkSigner/>` task to subsequently fail, because it's unusable. The "workaround" is for `$(AndroidSdkBuildToolsVersion)` to be set to the desired version to use. In retrospect, however, this is all *backwards*: `AndroidSdkInfo.GetBuildToolsPaths()` shouldn't prefer preview versions, it should prefer *non-* preview versions. If you *really* want a preview version, *then* you should override `$(AndroidSdkBuildToolsPath)`. Previews should be opt-in, not opt-out. With that in mind, why were preview versions preferred in the first place? That was done in xamarin/androidtools@a3965bae to fix [Bug #30555][0], stating: > handle[] by searching for these "preview" directories first, so that > the latest tooling will be used IF the user decides to install it. > The assumption is that a stable release will NOT inlcude a suffix. While "a preview directory will only exist if someone explicitly installs it, and thus should be preferred" sounds reasonable, in retrospect it's a recipe for pain when using a shared CI environment. Just because it's there does *not* mean it should be used. [0]: https://bugzilla.xamarin.com/show_bug.cgi?id=30555
Context https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1130414
For some unknown reason the setting of
Console.InputEncodingon a customers machine is throwing an IOException. This then
causes Visual Studio to completely crash.
At this time we are still unsure as to what is causing the problem.
But we should at least catch the exception to that Visual Studio does
not crash.
Note: Not sure what is going on with the line ending changes :/
For reference the reason we have to use
Console.InputEncodingisbecause of this.
The
System.Diagnostic.Processuses theConsole.InputEncodingwhencreating the
StreamWriterforStandardInput.