Skip to content

Commit efc5bb9

Browse files
dellis1972jonpryor
authored andcommitted
[Xamarin.Android.Build.Tasks] Resource ids must be consistent (#721)
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=57279 The DesignTimeBuilds are causing a bit of a problem. Because they are being run as soon as the project is loaded, it resulted in `_UpdateAndroidResGen` thinking that the resources are up to date. This results in the following $ find . -iname R.java | xargs grep EntryActivityStartButton ./IntuneMAMSampleAndroid/obj/AnyCPU/Debug/android/com/microsoft/intune/mam/R.java: public static final int EntryActivityStartButton=0x7f070000; ./IntuneMAMSampleAndroid/obj/AnyCPU/Debug/android/microsoftintunemamsampleandroid/microsoftintunemamsampleandroid/R.java: public static final int EntryActivityStartButton=0x7f070000; ./IntuneMAMSampleAndroid/obj/AnyCPU/Debug/android/src/android/support/v4/R.java: public static int EntryActivityStartButton=0x7f0c0050; ./IntuneMAMSampleAndroid/obj/AnyCPU/Debug/android/src/android/support/v7/appcompat/R.java: public static int EntryActivityStartButton=0x7f0c0050; ./IntuneMAMSampleAndroid/obj/AnyCPU/Debug/android/src/com/microsoft/intune/mam/R.java: public static final int EntryActivityStartButton=0x7f0c0050; ./IntuneMAMSampleAndroid/obj/AnyCPU/Debug/android/src/microsoftintunemamsampleandroid/microsoftintunemamsampleandroid/R.java: public static final int EntryActivityStartButton=0x7f0c0050; Note that the `EntryActivityStartButton` value is different, they should all be the same. This is because the `_UpdateAndroidResGen` target was NOT running. As a result of this, when the app runs a `NullReferenceException` could be thrown when the wrong resource id value is inadvertently used with e.g. `Activity.FindViewById<T>(int)`: // C# var button = FindViewById<Button>(Resource.Id.button); // `button` is actually null, as `Resource.id.button` is wrong button.Click += delegate { /* ... */ }; // throws `NullReferenceException`, as `button` is null Fortunately commit 1cd582e adds `$(DesignTimeBuild)` to the list of properties in the `@(_PropertyCacheItems)` ItemGroup. So we can now use the `$(_AndroidBuildPropertiesCache)` property to detect when we are moving from a DesignTimeBuild into a normal Build. This means the resources are then generated correctly.
1 parent 811a58c commit efc5bb9

File tree

2 files changed

+22
-4
lines changed

2 files changed

+22
-4
lines changed

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidUpdateResourcesTest.cs

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using Microsoft.Build.Framework;
1010
using System.Xml.Linq;
1111
using System.Security.Cryptography;
12+
using System.Text.RegularExpressions;
1213

1314
namespace Xamarin.Android.Build.Tests
1415
{
@@ -37,6 +38,8 @@ public void RepetitiveBuild ()
3738
[Test]
3839
public void DesignTimeBuild ([Values(false, true)] bool isRelease)
3940
{
41+
var regEx = new Regex (@"(?<type>([a-zA-Z_0-9])+)\slibrary_name=(?<value>([0-9A-Za-z])+);", RegexOptions.Compiled | RegexOptions.Multiline );
42+
4043
var path = Path.Combine (Root, "temp", $"DesignTimeBuild_{isRelease}");
4144
var cachePath = Path.Combine (path, "Cache");
4245
var envVar = new Dictionary<string, string> () {
@@ -71,8 +74,8 @@ public void DesignTimeBuild ([Values(false, true)] bool isRelease)
7174
},
7275
};
7376

74-
using (var l = CreateDllBuilder (Path.Combine (path, lib.ProjectName))) {
75-
using (var b = CreateApkBuilder (Path.Combine (path, proj.ProjectName))) {
77+
using (var l = CreateDllBuilder (Path.Combine (path, lib.ProjectName), false, false)) {
78+
using (var b = CreateApkBuilder (Path.Combine (path, proj.ProjectName), false, false)) {
7679
l.Verbosity = LoggerVerbosity.Diagnostic;
7780
Assert.IsTrue(l.Clean(lib), "Lib1 should have cleaned successfully");
7881
Assert.IsTrue (l.Build (lib), "Lib1 should have built successfully");
@@ -83,12 +86,26 @@ public void DesignTimeBuild ([Values(false, true)] bool isRelease)
8386
"first build failed");
8487
Assert.IsTrue (b.LastBuildOutput.Contains ("Skipping download of "),
8588
"failed to skip the downloading of files.");
89+
var items = new List<string> ();
90+
foreach (var file in Directory.EnumerateFiles (Path.Combine (path, proj.ProjectName, proj.IntermediateOutputPath, "android"), "R.java", SearchOption.AllDirectories)) {
91+
var matches = regEx.Matches (File.ReadAllText (file));
92+
items.AddRange (matches.Cast<System.Text.RegularExpressions.Match> ().Select(x => x.Groups ["value"].Value));
93+
}
94+
var first = items.First ();
95+
Assert.IsTrue (items.All (x => x == first), "All Items should have matching values");
8696
WaitFor (1000);
8797
b.Target = "Build";
8898
Assert.IsTrue (b.Build (proj, doNotCleanupOnUpdate: true, parameters: new string [] { "DesignTimeBuild=false" }, environmentVariables: envVar), "second build failed");
8999
Assert.IsFalse(b.Output.IsTargetSkipped ("_BuildAdditionalResourcesCache"), "_BuildAdditionalResourcesCache should have run.");
90-
Assert.IsTrue (b.LastBuildOutput.Contains($"Downloading {url}") || b.LastBuildOutput.Contains ($"reusing existing archive: {zipPath}"), $"{url} should have been downloaded.");
100+
Assert.IsTrue (b.LastBuildOutput.Contains ($"Downloading {url}") || b.LastBuildOutput.Contains ($"reusing existing archive: {zipPath}"), $"{url} should have been downloaded.");
91101
Assert.IsTrue (File.Exists (Path.Combine (extractedDir, "1", "content", "android-N", "aapt")), $"Files should have been extracted to {extractedDir}");
102+
items.Clear ();
103+
foreach (var file in Directory.EnumerateFiles (Path.Combine (path, proj.ProjectName, proj.IntermediateOutputPath, "android"), "R.java", SearchOption.AllDirectories)) {
104+
var matches = regEx.Matches (File.ReadAllText (file));
105+
items.AddRange (matches.Cast<System.Text.RegularExpressions.Match> ().Select (x => x.Groups["value"].Value));
106+
}
107+
first = items.First ();
108+
Assert.IsTrue (items.All (x => x == first), "All Items should have matching values");
92109
}
93110
}
94111
}

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1256,7 +1256,8 @@ because xbuild doesn't support framework reference assemblies.
12561256
$(MSBuildAllProjects);
12571257
@(_AndroidResourceDest);
12581258
@(_LibraryResourceDirectoryStamps);
1259-
@(_AdditonalAndroidResourceCachePaths->'%(Identity)\cache.stamp')
1259+
@(_AdditonalAndroidResourceCachePaths->'%(Identity)\cache.stamp');
1260+
$(_AndroidBuildPropertiesCache)
12601261
</_UpdateAndroidResgenInputs>
12611262
</PropertyGroup>
12621263

0 commit comments

Comments
 (0)