Skip to content

Commit 86d2f30

Browse files
dellis1972jonpryor
authored andcommitted
[Xamarin.Android.Build.Tasks] Fix Removal of Non Duplicate elements (#856)
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=59473 Commit d079a42 went a bit too far when removing duplicates. It removed *any* duplicate entry that appeared anywhere in the document. What we really wanted to remove full duplicates that exist at the same level as the current element in the document. For example: ```xml <foo> <bar name="bar1"> <bar2 /> </bar> <bar name="bar2"> <bar2 /> </bar> <dupe/> <dupe/> </foo> ``` `<bar2/>` should *not* be removed but one of the `<dupe/>` values should. This commit reworks `RemoveDuplicates()` code to handle the correct logic. It also adds a unit test for this exact senario.
1 parent 347fedf commit 86d2f30

File tree

2 files changed

+68
-3
lines changed

2 files changed

+68
-3
lines changed

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,56 @@ public void MergeLibraryManifest ()
544544
References = {
545545
new BuildItem.ProjectReference ("..\\Binding1\\Binding1.csproj", lib.ProjectGuid)
546546
},
547+
Packages = {
548+
KnownPackages.SupportMediaCompat_25_4_0_1,
549+
KnownPackages.SupportFragment_25_4_0_1,
550+
KnownPackages.SupportCoreUtils_25_4_0_1,
551+
KnownPackages.SupportCoreUI_25_4_0_1,
552+
KnownPackages.SupportCompat_25_4_0_1,
553+
KnownPackages.AndroidSupportV4_25_4_0_1,
554+
KnownPackages.SupportV7AppCompat_25_4_0_1,
555+
},
547556
};
557+
proj.Sources.Add (new BuildItem.Source ("TestActivity1.cs") {
558+
TextContent = () => @"using System;
559+
using System.Collections.Generic;
560+
using System.Linq;
561+
using System.Text;
562+
563+
using Android.App;
564+
using Android.Content;
565+
using Android.OS;
566+
using Android.Runtime;
567+
using Android.Views;
568+
using Android.Widget;
569+
using Android.Support.V4.App;
570+
using Android.Util;
571+
[Activity (Label = ""TestActivity1"")]
572+
[IntentFilter (new[]{Intent.ActionMain}, Categories = new[]{ ""com.xamarin.sample"" })]
573+
public class TestActivity1 : FragmentActivity {
574+
}
575+
",
576+
});
577+
proj.Sources.Add (new BuildItem.Source ("TestActivity2.cs") {
578+
TextContent = () => @"using System;
579+
using System.Collections.Generic;
580+
using System.Linq;
581+
using System.Text;
582+
583+
using Android.App;
584+
using Android.Content;
585+
using Android.OS;
586+
using Android.Runtime;
587+
using Android.Views;
588+
using Android.Widget;
589+
using Android.Support.V4.App;
590+
using Android.Util;
591+
[Activity (Label = ""TestActivity2"")]
592+
[IntentFilter (new[]{Intent.ActionMain}, Categories = new[]{ ""com.xamarin.sample"" })]
593+
public class TestActivity2 : FragmentActivity {
594+
}
595+
",
596+
});
548597
using (var libbuilder = CreateDllBuilder (Path.Combine (path, "Binding1"))) {
549598
Assert.IsTrue (libbuilder.Build (lib), "Build should have succeeded.");
550599
using (var builder = CreateApkBuilder (Path.Combine (path, "App1"))) {
@@ -558,6 +607,15 @@ public void MergeLibraryManifest ()
558607
".internal.FacebookInitProvider was not replaced with com.xamarin.test.internal.FacebookInitProvider");
559608
Assert.AreEqual (manifest.IndexOf ("meta-data", StringComparison.OrdinalIgnoreCase),
560609
manifest.LastIndexOf ("meta-data", StringComparison.OrdinalIgnoreCase), "There should be only one meta-data element");
610+
611+
var doc = XDocument.Parse (manifest);
612+
var ns = XNamespace.Get ("http://schemas.android.com/apk/res/android");
613+
614+
var activities = doc.Element ("manifest")?.Element ("application")?.Elements ("activity");
615+
var e = activities.FirstOrDefault (x => x.Attribute (ns.GetName ("label"))?.Value == "TestActivity2");
616+
Assert.IsNotNull (e, "Manifest should contain an activity for TestActivity2");
617+
Assert.IsNotNull (e.Element ("intent-filter"), "TestActivity2 should have an intent-filter");
618+
Assert.IsNotNull (e.Element ("intent-filter").Element ("action"), "TestActivity2 should have an intent-filter/action");
561619
}
562620
}
563621
}

src/Xamarin.Android.Build.Tasks/Utilities/ManifestDocument.cs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -402,11 +402,18 @@ void MergeLibraryManifest (string mergedManifest)
402402
}
403403
}
404404

405+
public IEnumerable<XElement> ResolveDuplicates (IEnumerable<XElement> elements)
406+
{
407+
foreach (var e in elements)
408+
foreach (var d in ResolveDuplicates (e.Elements ()))
409+
yield return d;
410+
foreach (var d in elements.GroupBy (x => x.ToFullString ()).SelectMany (x => x.Skip (1)))
411+
yield return d;
412+
}
413+
405414
void RemoveDuplicateElements ()
406415
{
407-
var duplicates = doc.Descendants ()
408-
.GroupBy (x => x.ToFullString ())
409-
.SelectMany (x => x.Skip (1));
416+
var duplicates = ResolveDuplicates (doc.Elements ());
410417
foreach (var duplicate in duplicates)
411418
duplicate.Remove ();
412419

0 commit comments

Comments
 (0)