-
Notifications
You must be signed in to change notification settings - Fork 537
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
[Build] Log better errors when compiling/linking with binutils #7566
Conversation
If an error happens when compiling or linking native assembler files generated by Xamarin.Android, we log an error which doesn't contain much information: Error XA3006 Could not compile native assembly file: compressed_assemblies.x86.ll The real cause of the issue is logged as well, however not as an error but rather a "plain" message, which is not immediately accessible to the user experiencing the issue. Keep logging the output messages of the assembler and linker programs as before, but whenever an error occurs, log their entire output (both stdout and stderr) as an MSBuild error message.
@@ -97,9 +103,22 @@ void RunLinker (Config config) | |||
|
|||
if (proc.ExitCode != 0) { | |||
LogCodedError ("XA3007", Properties.Resources.XA3007, Path.GetFileName (config.OutputSharedLibrary)); | |||
LogErrors ("stdout", stdoutLines); |
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.
Not sure what the best thing to do here is. These errors will not be "attached" to the XA3007. they will be reported as different errors per line.
Should we instead make the Properties.Resources.XA3007
take an additional argument which is the output of the tool?
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.
It, somewhat, mimics this behavior. If we added the new argument, as you suggest, would it retain the newlines from the outputs?
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.
@dellis1972 my hope was that the errors will be presented together, so that even though the coded error will be "separate", the following lines would be shown just below it, in the correct context.
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.
@jonpryor do we have an old comment/commit message that talks about multiline errors and how they show up in Visual Studio? I think we might be able to just append all these messages in a single error message?
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.
@jonpryor do we have an old comment/commit message that talks about multiline errors and how they show up in Visual Studio? I think we might be able to just append all these messages in a single error message?
If the appended content preserves newlines, then sure, I'll be fine with it. Otherwise I'd rather do what this PR currently does.
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.
If we do log them seperately it would be better if it had the same Error code (XA3007) So maybe use Log.LogCodedError
instead.
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.
do we have an old comment/commit message that talks about multiline errors and how they show up in Visual Studio?
We have this:
CreateErrorsItemGroupSingleLine was the best looking approach, which joins all lines together with \n
between them. This appears to be the approach currently taken in this PR.
* main: Bump to xamarin/xamarin-android-tools/main@fa3711b (dotnet#7501)
* main: Bump to mono/mono@6dd9def5 (dotnet#7574) Bump to dotnet/installer@296eeb3 8.0.100-alpha.1.22570.9 (dotnet#7571) [ci] Add API-33 to the nightly build (dotnet#7552) [docs] Fix typo in XA0134 (dotnet#7569) [docs] how to `dotnet trace` our build (dotnet#7573) [Xamarin.Android.Build.Tasks] Useful errors when using binutils (dotnet#7566) Bump SQLite to 3.40.0 (dotnet#7564)
* main: Bump to mono/mono@6dd9def5 (dotnet#7574) Bump to dotnet/installer@296eeb3 8.0.100-alpha.1.22570.9 (dotnet#7571) [ci] Add API-33 to the nightly build (dotnet#7552) [docs] Fix typo in XA0134 (dotnet#7569) [docs] how to `dotnet trace` our build (dotnet#7573) [Xamarin.Android.Build.Tasks] Useful errors when using binutils (dotnet#7566) Bump SQLite to 3.40.0 (dotnet#7564)
Context: b002dc3 Context: #2905 (comment) If an error occurs when compiling or linking native assembler files generated by .NET Android, we log an error which doesn't contain any useful information: error XA3006: Could not compile native assembly file: compressed_assemblies.x86.ll The real cause of the issue is logged as well, but not as an error; it's visible in the Diagnostic log as a "plain" message, which is not immediately accessible to the user experiencing the issue. Keep logging the output messages of the assembler and linker programs as before, but whenever an error occurs, log their entire output (stdout and stderr) as a single MSBuild message with newlines between each each line of stdout/stderr. This will render within Visual Studio as a multi-line error message ("CreateErrorsItemGroupSingleLine" in #2905).
If an error happens when compiling or linking native assembler files
generated by Xamarin.Android, we log an error which doesn't contain
much information:
The real cause of the issue is logged as well, however not as an error
but rather a "plain" message, which is not immediately accessible
to the user experiencing the issue.
Keep logging the output messages of the assembler and linker programs as
before, but whenever an error occurs, log their entire output (both
stdout and stderr) as an MSBuild error message.