Fix BuildArchive updates for existing jar entries

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

@@ -158,8 +158,10 @@ public override bool RunTask ()
 				// ItemSpec for these will be "<jarfile>#<entrypath>
 				// eg: "obj/myjar.jar#myfile.txt"
 				var jar_file_path = disk_path.Substring (0, disk_path.Length - (jar_entry_name.Length + 1));
+				var wasExistingOutputEntry = existingEntries.Remove (apk_path);
+				var hasApkEntry = apk.ContainsEntry (apk_path);
 
-				if (apk.ContainsEntry (apk_path)) {
+				if (hasApkEntry && !wasExistingOutputEntry) {
 					Log.LogDebugMessage ("Failed to add jar entry {0} from {1}: the same file already exists in the apk", jar_entry_name, Path.GetFileName (jar_file_path));
 					continue;
 				}
@@ -168,6 +170,22 @@ public override bool RunTask ()
 				using (var jar = ZipArchive.Open (stream)) {
 					var jar_item = jar.ReadEntry (jar_entry_name);
 
+					if (jar_item == null) {
+						Log.LogDebugMessage ("Failed to add jar entry {0} from {1}: entry not found in jar.", jar_entry_name, jar_file_path);
+						if (wasExistingOutputEntry)
+							existingEntries.Add (apk_path);
+						continue;
+					}
+
+					if (hasApkEntry) {
+						if (apk.GetEntry (apk_path).CRC == jar_item.CRC) {
+							Log.LogDebugMessage ("Skipping {0} from {1} as it is up to date.", jar_entry_name, jar_file_path);
+							continue;
+						}
+
+						apk.DeleteEntry (apk_path);
+					}
+
 					byte [] data;
 					var d = MemoryStreamPool.Shared.Rent ();
 

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

@@ -0,0 +1,165 @@
+#nullable enable
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Text;
+using Microsoft.Android.Build.Tasks;
+using Microsoft.Build.Framework;
+using Microsoft.Build.Utilities;
+using NUnit.Framework;
+using Xamarin.Android.Tasks;
+using Xamarin.Tools.Zip;
+
+namespace Xamarin.Android.Build.Tests;
+
+[TestFixture]
+public class BuildArchiveTests
+{
+	string? tempDirectory;
+
+	string TempDirectory => tempDirectory ?? throw new InvalidOperationException ("Setup has not run.");
+
+	[SetUp]
+	public void Setup ()
+	{
+		tempDirectory = Path.Combine (Path.GetTempPath (), Path.GetRandomFileName ());
+		Directory.CreateDirectory (tempDirectory);
+	}
+
+	[TearDown]
+	public void TearDown ()
+	{
+		if (!tempDirectory.IsNullOrEmpty () && Directory.Exists (tempDirectory))
+			Directory.Delete (tempDirectory, recursive: true);
+	}
+
+	[Test]
+	public void ExistingJavaArchiveEntriesAreUpdated ()
+	{
+		var apk = Path.Combine (TempDirectory, "app.apk");
+		var jar = Path.Combine (TempDirectory, "classes.jar");
+
+		CreateArchive (apk, ("commonMain/default/manifest", "existing"), ("stale.txt", "stale"));
+		CreateArchive (jar, ("commonMain/default/manifest", "current"));
+
+		var item = new TaskItem ($"{jar}#commonMain/default/manifest");
+		item.SetMetadata ("ArchivePath", "commonMain/default/manifest");
+		item.SetMetadata ("JavaArchiveEntry", "commonMain/default/manifest");
+
+		var task = new BuildArchive {
+			BuildEngine = new MockBuildEngine (TestContext.Out),
+			ApkOutputPath = apk,
+			FilesToAddToArchive = new ITaskItem [] { item },
+		};
+
+		Assert.IsTrue (task.RunTask (), "task should have succeeded");
+
+		using (var archive = ZipArchive.Open (apk, FileMode.Open)) {
+			archive.AssertEntryContents (apk, "commonMain/default/manifest", "current");
+			archive.AssertDoesNotContainEntry (apk, "stale.txt");
+		}
+	}
+
+	[Test]
+	public void ExistingJavaArchiveEntriesAreSkippedWhenUpToDate ()
+	{
+		var apk = Path.Combine (TempDirectory, "app.apk");
+		var jar = Path.Combine (TempDirectory, "classes.jar");
+
+		CreateArchive (apk, ("commonMain/default/manifest", "current"));
+		CreateArchive (jar, ("commonMain/default/manifest", "current"));
+
+		var item = new TaskItem ($"{jar}#commonMain/default/manifest");
+		item.SetMetadata ("ArchivePath", "commonMain/default/manifest");
+		item.SetMetadata ("JavaArchiveEntry", "commonMain/default/manifest");
+		var messages = new List<BuildMessageEventArgs> ();
+
+		var task = new BuildArchive {
+			BuildEngine = new MockBuildEngine (TestContext.Out, messages: messages),
+			ApkOutputPath = apk,
+			FilesToAddToArchive = new ITaskItem [] { item },
+		};
+
+		Assert.IsTrue (task.RunTask (), "task should have succeeded");
+
+		Assert.That (messages, Has.Some.Property (nameof (BuildMessageEventArgs.Message)).EqualTo ($"Skipping commonMain/default/manifest from {jar} as it is up to date."));
+
+		using (var archive = ZipArchive.Open (apk, FileMode.Open)) {
+			archive.AssertEntryContents (apk, "commonMain/default/manifest", "current");
+		}
+	}
+
+	[Test]
+	public void DuplicateJavaArchiveEntriesKeepFirstCurrentBuildItem ()
+	{
+		var apk = Path.Combine (TempDirectory, "app.apk");
+		var firstJar = Path.Combine (TempDirectory, "first.jar");
+		var secondJar = Path.Combine (TempDirectory, "second.jar");
+
+		CreateArchive (apk, ("stale.txt", "stale"));
+		CreateArchive (firstJar, ("commonMain/default/manifest", "first"));
+		CreateArchive (secondJar, ("commonMain/default/manifest", "second"));
+
+		var firstItem = new TaskItem ($"{firstJar}#commonMain/default/manifest");
+		firstItem.SetMetadata ("ArchivePath", "commonMain/default/manifest");
+		firstItem.SetMetadata ("JavaArchiveEntry", "commonMain/default/manifest");
+		var secondItem = new TaskItem ($"{secondJar}#commonMain/default/manifest");
+		secondItem.SetMetadata ("ArchivePath", "commonMain/default/manifest");
+		secondItem.SetMetadata ("JavaArchiveEntry", "commonMain/default/manifest");
+		var messages = new List<BuildMessageEventArgs> ();
+
+		var task = new BuildArchive {
+			BuildEngine = new MockBuildEngine (TestContext.Out, messages: messages),
+			ApkOutputPath = apk,
+			FilesToAddToArchive = new ITaskItem [] { firstItem, secondItem },
+		};
+
+		Assert.IsTrue (task.RunTask (), "task should have succeeded");
+
+		Assert.That (messages, Has.Some.Property (nameof (BuildMessageEventArgs.Message)).EqualTo ("Failed to add jar entry commonMain/default/manifest from second.jar: the same file already exists in the apk"));
+
+		using (var archive = ZipArchive.Open (apk, FileMode.Open)) {
+			archive.AssertEntryContents (apk, "commonMain/default/manifest", "first");
+			archive.AssertDoesNotContainEntry (apk, "stale.txt");
+		}
+	}
+
+	[Test]
+	public void MissingJarEntryIsSkippedAndExistingOutputEntryIsRemoved ()
+	{
+		var apk = Path.Combine (TempDirectory, "app.apk");
+		var jar = Path.Combine (TempDirectory, "classes.jar");
+
+		CreateArchive (apk, ("commonMain/default/manifest", "existing"));
+		CreateArchive (jar, ("other-entry.txt", "contents"));
+
+		var item = new TaskItem ($"{jar}#commonMain/default/manifest");
+		item.SetMetadata ("ArchivePath", "commonMain/default/manifest");
+		item.SetMetadata ("JavaArchiveEntry", "commonMain/default/manifest");
+		var messages = new List<BuildMessageEventArgs> ();
+
+		var task = new BuildArchive {
+			BuildEngine = new MockBuildEngine (TestContext.Out, messages: messages),
+			ApkOutputPath = apk,
+			FilesToAddToArchive = new ITaskItem [] { item },
+		};
+
+		Assert.IsTrue (task.RunTask (), "task should have succeeded");
+
+		Assert.That (messages, Has.Some.Property (nameof (BuildMessageEventArgs.Message)).EqualTo ($"Failed to add jar entry commonMain/default/manifest from {jar}: entry not found in jar."));
+
+		using (var archive = ZipArchive.Open (apk, FileMode.Open)) {
+			archive.AssertDoesNotContainEntry (apk, "commonMain/default/manifest");
+		}
+	}
+
+	static void CreateArchive (string path, params (string name, string contents) [] entries)
+	{
+		using (var stream = File.Create (path))
+		using (var archive = ZipArchive.Create (stream)) {
+			foreach (var entry in entries) {
+				archive.AddEntry (entry.name, entry.contents, encoding: Encoding.UTF8);
+			}
+		}
+	}
+}