Skip to content

Commit 8a95d55

Browse files
dellis1972jonpryor
authored andcommitted
[Xamarin.Android.Build.Tasks] Fix Removal of Non Duplicate elements (dotnet#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 b18f6d4 commit 8a95d55

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
@@ -512,7 +512,56 @@ public void MergeLibraryManifest ()
512512
References = {
513513
new BuildItem.ProjectReference ("..\\Binding1\\Binding1.csproj", lib.ProjectGuid)
514514
},
515+
Packages = {
516+
KnownPackages.SupportMediaCompat_25_4_0_1,
517+
KnownPackages.SupportFragment_25_4_0_1,
518+
KnownPackages.SupportCoreUtils_25_4_0_1,
519+
KnownPackages.SupportCoreUI_25_4_0_1,
520+
KnownPackages.SupportCompat_25_4_0_1,
521+
KnownPackages.AndroidSupportV4_25_4_0_1,
522+
KnownPackages.SupportV7AppCompat_25_4_0_1,
523+
},
515524
};
525+
proj.Sources.Add (new BuildItem.Source ("TestActivity1.cs") {
526+
TextContent = () => @"using System;
527+
using System.Collections.Generic;
528+
using System.Linq;
529+
using System.Text;
530+
531+
using Android.App;
532+
using Android.Content;
533+
using Android.OS;
534+
using Android.Runtime;
535+
using Android.Views;
536+
using Android.Widget;
537+
using Android.Support.V4.App;
538+
using Android.Util;
539+
[Activity (Label = ""TestActivity1"")]
540+
[IntentFilter (new[]{Intent.ActionMain}, Categories = new[]{ ""com.xamarin.sample"" })]
541+
public class TestActivity1 : FragmentActivity {
542+
}
543+
",
544+
});
545+
proj.Sources.Add (new BuildItem.Source ("TestActivity2.cs") {
546+
TextContent = () => @"using System;
547+
using System.Collections.Generic;
548+
using System.Linq;
549+
using System.Text;
550+
551+
using Android.App;
552+
using Android.Content;
553+
using Android.OS;
554+
using Android.Runtime;
555+
using Android.Views;
556+
using Android.Widget;
557+
using Android.Support.V4.App;
558+
using Android.Util;
559+
[Activity (Label = ""TestActivity2"")]
560+
[IntentFilter (new[]{Intent.ActionMain}, Categories = new[]{ ""com.xamarin.sample"" })]
561+
public class TestActivity2 : FragmentActivity {
562+
}
563+
",
564+
});
516565
using (var libbuilder = CreateDllBuilder (Path.Combine (path, "Binding1"))) {
517566
Assert.IsTrue (libbuilder.Build (lib), "Build should have succeeded.");
518567
using (var builder = CreateApkBuilder (Path.Combine (path, "App1"))) {
@@ -526,6 +575,15 @@ public void MergeLibraryManifest ()
526575
".internal.FacebookInitProvider was not replaced with com.xamarin.test.internal.FacebookInitProvider");
527576
Assert.AreEqual (manifest.IndexOf ("meta-data", StringComparison.OrdinalIgnoreCase),
528577
manifest.LastIndexOf ("meta-data", StringComparison.OrdinalIgnoreCase), "There should be only one meta-data element");
578+
579+
var doc = XDocument.Parse (manifest);
580+
var ns = XNamespace.Get ("http://schemas.android.com/apk/res/android");
581+
582+
var activities = doc.Element ("manifest")?.Element ("application")?.Elements ("activity");
583+
var e = activities.FirstOrDefault (x => x.Attribute (ns.GetName ("label"))?.Value == "TestActivity2");
584+
Assert.IsNotNull (e, "Manifest should contain an activity for TestActivity2");
585+
Assert.IsNotNull (e.Element ("intent-filter"), "TestActivity2 should have an intent-filter");
586+
Assert.IsNotNull (e.Element ("intent-filter").Element ("action"), "TestActivity2 should have an intent-filter/action");
529587
}
530588
}
531589
}

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

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

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

0 commit comments

Comments
 (0)