[Xamarin.Android.Build.Tasks] Add LogCustomBuildEvent method to AsyncTask #1065
Conversation
jonathanpeppers
left a comment
jonathanpeppers
left a comment
There was a problem hiding this comment.
Should we also think about making the Log property non-private (protected) so there are build warnings if we accidentally use it: https://github.com/dellis1972/xamarin-android/blob/f7974f61258e48dd2c6a918eaa6ca7296148d8e7/src/Xamarin.Android.Build.Tasks/Tasks/AsyncTask.cs#L41
We could also think about making the AsyncTask.Log property throw if it is used as a way to force us to find them all. Maybe I can look into this down the road, probably don't want to do it in this PR.
|
@jonathanpeppers the Obsolete should take care of that. It might be worth seeing if we can throw an exception.. I'd do that in a separate PR though. |
|
@dellis1972 but if it's |
| Queue logMessageQueue =new Queue (); | ||
| Queue warningMessageQueue = new Queue (); | ||
| Queue errorMessageQueue = new Queue (); | ||
| Queue customMessageQueue = new Queue (); |
There was a problem hiding this comment.
Queue<T> does not include a SyncRoot property which we need for threading. Now I can add a bunch of extra objects to handle that. Remember this code was sourced from the msbuild ToolTask. At the time it was safer to follow their lead rather than introduce possible unknowns.
| #pragma warning restore 618 | ||
| } | ||
|
|
||
| lock (customMessageQueue.SyncRoot) { |
There was a problem hiding this comment.
This feels like an opportunity for cleanup/a new helper method, as LogCustomBuildEvent() looks very similar to LogMessage() looks very similar to LogError() looks...
Particularly around the locking and message queuing.
There was a problem hiding this comment.
I can rework this into a helper. Each queue still needs its own reset event though.
|
|
||
| private void LogCustomData () | ||
| { | ||
| lock (customMessageQueue.SyncRoot) { |
There was a problem hiding this comment.
Ditto this body: LogCustomData() looks like LogErrors() looks like LogWarnings()... Surely we can have a helper method for consistency.
|
Build failure is our old |
…Task There is a possilility that we might be locking up the UI thread when logging CustomBuildEvents.. This happens in the `InstallPackageAssemblies` task. Currently we only use BuildEngine.LogCustomEvent Which may well cause problems when called from a non UI thread. So in keeping with the other Log methods available we should add a new LogCustomBuildEvent. This will queue and marshall the data to the UI Thread just like the other methods do.
dellis1972
force-pushed
the
AddCustomBuildLogging
branch
from
d1b2a7f to
797306a
Compare
Changes: https://github.com/xamarin/monodroid/compare/0f6725edfa09559afad58233d43f385122e31e8d...b2841feb5a69f9ccd3c480a9341fcb1da62af552 Context: https://build.azdo.io/3395342 * xamarin/monodroid@b2841feb5: Bump to xamarin/android-sdk-installer/master@78c3d16 (dotnet#1065) * xamarin/monodroid@3c8601c55: [tools/msbuild] ported <GetPrimaryCpuAbi/> to AndroidAsyncTask (dotnet#1067) * xamarin/monodroid@ad8d0f348: Support for new typemaps (dotnet#1061)
* Bump to xamarin/monodroid@b2841feb Changes: xamarin/monodroid@0f6725e...b2841fe Context: https://build.azdo.io/3395342 * xamarin/monodroid@b2841feb5: Bump to xamarin/android-sdk-installer@78c3d16 (#1065) * xamarin/monodroid@3c8601c55: [tools/msbuild] ported <GetPrimaryCpuAbi/> to AndroidAsyncTask (#1067) * xamarin/monodroid@ad8d0f348: Support for new typemaps (#1061) * Bump to xamarin/monodroid@78d56588 Changes: xamarin/monodroid@b2841fe...78d5658 Bump to xamarin/androidtools@b01b4db [Mono.AndroidTools] use pm list packages -f --show-versioncode Co-authored-by: Jonathan Peppers <[email protected]>
4 participants
There is a possilility that we might be locking up the UI thread
when logging CustomBuildEvents.. This happens in the
InstallPackageAssembliestask. Currently we only useWhich may well cause problems when called from a non UI thread.
So in keeping with the other Log methods available we should add
a new LogCustomBuildEvent. This will queue and marshall the data
to the UI Thread just like the other methods do.