[Xamarin.Android.Build.Tasks] fix for parallel builds #2051

jonathanpeppers · 2018-08-09T15:15:53Z

Context: #2043

When parallel builds occur, we can get obj/Debug/lp into a broken
state:

Resources/layout/abc_action_menu_layout.xml warning XA1001:
    AndroidResgen: Warning while updating Resource XML '/var/folders/nx/rzr6lcqj6j3_vp7vtp28v2cw0000gn/T/tmp6bafd8e4.tmp': Root element is missing.

Then later, <Aapt /> fails on the empty file:

obj/Debug/lp/11/jl/res/layout/abc_action_menu_layout.xml(2,0): error APT0000: Error parsing XML: no element found

It appears that ConvertResourcesCases still copies the file, even if
AndroidResource.UpdateXmlResource fails. Subsequent builds will
continue to fail, because an empty file has been copied with a newer
timestamp. Once in this state, developers would have to Rebuild the
project or delete obj.

The fix here is to:

Return a bool indicating if AndroidResource.UpdateXmlResource
succeeds
Don't write the final file if AndroidResource.UpdateXmlResource
failed

This change doesn't fully fix #2043, as I think VS for Mac should
prevent builds from happening in parallel. I tried the change in the
IDE once, and it worked. However, I feel like if I repeated the steps
10 times in a row, something would eventually go wrong.

This change does, however, fix the issue for the user so a Rebuild
is not required.

Other changes:

The original temp file can use File.Copy since it is empty
We can use the ? operator for invoking logMessage

dellis1972 · 2018-08-09T15:18:05Z

src/Xamarin.Android.Build.Tasks/Tasks/ConvertResourcesCases.cs

@@ -74,10 +74,10 @@ void FixupResources (ITaskItem item, Dictionary<string, string> acwMap)
 				}
 				Log.LogDebugMessage ("  Processing: {0}   {1} > {2}", file, srcmodifiedDate, lastUpdate);
 				var tmpdest = Path.GetTempFileName ();
-				MonoAndroidHelper.CopyIfChanged (file, tmpdest);
+				File.Copy (file, tmpdest, true);


can we name the bool parameter please :) overwrite: true

Context: dotnet#2043 When parallel builds occur, we can get `obj/Debug/lp` into a broken state: Resources/layout/abc_action_menu_layout.xml warning XA1001: AndroidResgen: Warning while updating Resource XML '/var/folders/nx/rzr6lcqj6j3_vp7vtp28v2cw0000gn/T/tmp6bafd8e4.tmp': Root element is missing. Then later, `<Aapt />` fails on the empty file: obj/Debug/lp/11/jl/res/layout/abc_action_menu_layout.xml(2,0): error APT0000: Error parsing XML: no element found It appears that `ConvertResourcesCases` still copies the file, even if `AndroidResource.UpdateXmlResource` fails. Subsequent builds will continue to fail, because an empty file has been copied with a newer timestamp. Once in this state, developers would have to `Rebuild` the project or delete `obj`. The fix here is to: - Return a `bool` indicating if `AndroidResource.UpdateXmlResource` succeeds - Don't write the final file if `AndroidResource.UpdateXmlResource` failed This change doesn't fully fix dotnet#2043, as I think VS for Mac should prevent builds from happening in parallel. I tried the change in the IDE once, and it worked. However, I feel like if I repeated the steps 10 times in a row, something would eventually go wrong. This change *does*, however, fix the issue for the user so a `Rebuild` is not required. Other changes: - The original temp file can use `File.Copy` since it is empty - We can use the `?` operator for invoking `logMessage`

jonathanpeppers · 2018-08-09T19:32:05Z

I think this is green, the failure here seems unrelated:

<h3>Type Changed: System.String</h3>
<p>Removed method:</p>

<pre>
	<span class='removed removed-method breaking' data-is-breaking>public static string Concat (object, object, object, object);</span>
</pre>

</div> <!-- end type String -->

</div> <!-- end namespace System -->
</div> <!-- end topmost div -->
</div>
make[2]: *** [check] Error 1
make[1]: *** [run-api-compatibility-tests] Error 2

jonpryor · 2018-08-13T20:08:31Z

@marek-safar will be looking into the ABI breakage reported above.

marek-safar · 2018-08-13T20:26:28Z

that's one of two only (if I remember correctly) __arglist methods in whole .net which are not supported by .net core with no real usage as the feature is undocumented. I let the change in because it believe it's safe to remove (I don't think our AOT compiler support that fully either).

Context: dotnet/android#1503 Context: https://jenkins.mono-project.com/view/Xamarin.Android/job/xamarin-android/1126/ Context: mono/mono@ca9cbdc The [bump to mono:2018-04][0] introduced two "incompatible" changes which impact our API Compatibility checks: 1. `mono-api-html.exe` behavior has changed, and 2. `System.String.Concat(object, object, object, object)` was removed. `mono-api-html.exe` from mono:2018-02 and before had a bug wherein it didn't properly handle nested types. The immediate affect of this was that the `Android.App.Notification.Icon` property change which was handled/ignored in `inter-api-extra-v5.1-v6.0.txt` was seemingly ignored, resulting in an API compatibility failure: <h1>### API BREAK BETWEEN v5.1 and v6.0</h1> <h3>Type Changed: Android.App.Notification.Action.Action</h3> <p>Modified properties:</p> <pre> <div data-is-breaking> public <span class='removed removed-inline removed-breaking-inline'>int</span> <span class='added '>Android.Graphics.Drawables.Icon</span> Icon { get; <span class='removed removed-inline removed-breaking-inline'>set;</span> } </div></pre> Note that the `mono-api-html` output here is still somewhat buggy: *there is no* `Android.App.Notification.Action.Action` type -- nested type names appear to be duplicated now, instead of skipped entirely -- so it's possible that future `mono-api-html` changes may break us. Other errors were being reported as well [^0], all of which are *reasonable and correct* but weren't needed with mono:2018-02's `mono-api-html`, which is...worrying, but ¯\_(ツ)_/¯. The `System.String.Concat(object, object, object, object)` removal is a bit of a misnomer: *there is no* `String.Concat()` method which takes four `object` parameters. There *was* a `String.Concat()` method which took *three* `object` parameters and an `__arglist` parameter, and the `__arglist`-including overload was removed because [it was deemed safe to remove][1]: > @mark-safar: I let the change in because it believe it's safe to > remove (I don't think our AOT compiler support that fully either). [^0]: Brief summary of most "new" changes which `mono-api-html` from mono:2018-02 didn't need but are reported with mono:2018-04: <h1>### API BREAK BETWEEN v4.3 and v4.4</h1> <h3>Type Changed: Android.Media.RemoteControlClient.MetadataEditor</h3> <p>Modified base type:</p> <pre> <div data-is-breaking> <span class='removed removed-inline removed-breaking-inline'>Java.Lang.Object</span> <span class='added '>Android.Media.MediaMetadataEditor</span> </div></pre> <h3>Type Changed: Android.Views.Surface.OutOfResourcesException</h3> <p>Modified base type:</p> <pre> <div data-is-breaking> <span class='removed removed-inline removed-breaking-inline'>Java.Lang.Exception</span> <span class='added '>Java.Lang.RuntimeException</span> </div></pre> <h3>Type Changed: Java.Lang.ClassNotFoundException</h3> <p>Modified base type:</p> <pre> <div data-is-breaking> <span class='removed removed-inline removed-breaking-inline'>Java.Lang.Exception</span> <span class='added '>Java.Lang.ReflectiveOperationException</span> </div></pre> <h3>Type Changed: Java.Lang.IllegalAccessException</h3> <p>Modified base type:</p> <pre> <div data-is-breaking> <span class='removed removed-inline removed-breaking-inline'>Java.Lang.Exception</span> <span class='added '>Java.Lang.ReflectiveOperationException</span> </div></pre> <h3>Type Changed: Java.Lang.InstantiationException</h3> <p>Modified base type:</p> <pre> <div data-is-breaking> <span class='removed removed-inline removed-breaking-inline'>Java.Lang.Exception</span> <span class='added '>Java.Lang.ReflectiveOperationException</span> </div></pre> <h3>Type Changed: Java.Lang.NoSuchFieldException</h3> <p>Modified base type:</p> <pre> <div data-is-breaking> <span class='removed removed-inline removed-breaking-inline'>Java.Lang.Exception</span> <span class='added '>Java.Lang.ReflectiveOperationException</span> </div></pre> <h3>Type Changed: Java.Lang.NoSuchMethodException</h3> <p>Modified base type:</p> <pre> <div data-is-breaking> <span class='removed removed-inline removed-breaking-inline'>Java.Lang.Exception</span> <span class='added '>Java.Lang.ReflectiveOperationException</span> </div></pre> <h3>Type Changed: Java.Lang.Reflect.InvocationTargetException</h3> <p>Modified base type:</p> <pre> <div data-is-breaking> <span class='removed removed-inline removed-breaking-inline'>Java.Lang.Exception</span> <span class='added '>Java.Lang.ReflectiveOperationException</span> </div></pre> <h1>### API BREAK BETWEEN v4.4.87 and v5.0</h1> <h3>Type Changed: Android.OS.Bundle</h3> <p>Modified base type:</p> <pre> <div data-is-breaking> <span class='removed removed-inline removed-breaking-inline'>Java.Lang.Object</span> <span class='added '>Android.OS.BaseBundle</span> </div></pre> <h1>### API BREAK BETWEEN v5.1 and v6.0</h1> <h3>Type Changed: Android.App.Notification.Action.Action</h3> <p>Modified properties:</p> <pre> <div data-is-breaking> public <span class='removed removed-inline removed-breaking-inline'>int</span> <span class='added '>Android.Graphics.Drawables.Icon</span> Icon { get; <span class='removed removed-inline removed-breaking-inline'>set;</span> } </div></pre> [0]: dotnet/android#1503 [1]: dotnet/android#2051 (comment)

Context: #2043 When parallel builds occur, `obj/Debug/lp` can get broken: Resources/layout/abc_action_menu_layout.xml warning XA1001: AndroidResgen: Warning while updating Resource XML '/var/folders/nx/rzr6lcqj6j3_vp7vtp28v2cw0000gn/T/tmp6bafd8e4.tmp': Root element is missing. Then later, `<Aapt/>` fails on the empty file: obj/Debug/lp/11/jl/res/layout/abc_action_menu_layout.xml(2,0): error APT0000: Error parsing XML: no element found It appears that the `<ConvertResourcesCases/>` task still copies the file, even if `AndroidResource.UpdateXmlResource()` fails. Subsequent builds will continue to fail, because an empty file has been copied with a newer timestamp. Once in this state, developers would have to `Rebuild` the project or delete the `obj` directory. The fix here is to: 1. Return a `bool` indicating whether or not `AndroidResource.UpdateXmlResource()` succeeds. 2. Don't write the final file if `AndroidResource.UpdateXmlResource()` fails. This change doesn't fully fix #2043, as I think Visual Studio for Mac should prevent builds from happening in parallel. I tried the change in the IDE once, and it worked. However, I feel like if I repeated the steps 10 times in a row, something would eventually go wrong. This change *does*, however, fix the issue for the user so that a **Rebuild** is not required. Other changes: - The original temp file can use `File.Copy()` since it is empty - We can use the `?.` operator for invoking `logMessage`

jonathanpeppers requested review from jonpryor and dellis1972 August 9, 2018 15:15

dellis1972 reviewed Aug 9, 2018

View reviewed changes

jonathanpeppers force-pushed the parallel-convertresourcescases branch from b232a32 to 66318d9 Compare August 9, 2018 15:24

dellis1972 approved these changes Aug 13, 2018

View reviewed changes

jonpryor merged commit 8905dfa into dotnet:master Aug 13, 2018

jonathanpeppers deleted the parallel-convertresourcescases branch August 13, 2018 20:47

jonpryor mentioned this pull request Oct 5, 2018

Bump to mono:2018-06 #1836

Merged

github-actions bot locked and limited conversation to collaborators Feb 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Xamarin.Android.Build.Tasks] fix for parallel builds #2051

[Xamarin.Android.Build.Tasks] fix for parallel builds #2051

Uh oh!

jonathanpeppers commented Aug 9, 2018 •

edited

Loading

Uh oh!

dellis1972 Aug 9, 2018

Uh oh!

jonathanpeppers commented Aug 9, 2018 •

edited

Loading

Uh oh!

jonpryor commented Aug 13, 2018

Uh oh!

marek-safar commented Aug 13, 2018

Uh oh!

Uh oh!

[Xamarin.Android.Build.Tasks] fix for parallel builds #2051

[Xamarin.Android.Build.Tasks] fix for parallel builds #2051

Uh oh!

Conversation

jonathanpeppers commented Aug 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dellis1972 Aug 9, 2018

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers commented Aug 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonpryor commented Aug 13, 2018

Uh oh!

marek-safar commented Aug 13, 2018

Uh oh!

Uh oh!

jonathanpeppers commented Aug 9, 2018 •

edited

Loading

jonathanpeppers commented Aug 9, 2018 •

edited

Loading