Fix BuildArchive updates for existing jar entries

[Copilot]

BuildArchive could remove or preserve stale Java/JAR resources from incremental APK publishes when the previous APK already contained those entries. Duplicate jar-entry adds were skipped, but existing output entries were not distinguished from duplicates added by current build inputs.

• Archive retention and updates

  • Marks jar-sourced FilesToAddToArchive paths as retained before cleanup.
  • Keeps existing APK entries that are still part of the current build inputs.
  • Updates stale existing APK entries when the source jar entry contents change.
  • Preserves existing duplicate handling when a current packaged resource or earlier archive item already added the same path.
  • Preserves cleanup behavior for entries no longer present in FilesToAddToArchive.

• Regression coverage

  • Adds focused BuildArchive tests for existing jar-sourced APK entries.
  • Verifies stale Java resource entries are replaced with current jar contents.
  • Verifies up-to-date Java resource entries are skipped.
  • Verifies unrelated stale entries are still removed.

[simonrozsival]

@copilot The current fix prevents deletion of existing jar-sourced entries, but it still preserves stale contents forever when the source jar entry changes.

Please update BuildArchive so jar-sourced entries distinguish between:

1. entries that were present in the existing output APK and should be retained/updated, and
2. entries already added by the current build inputs, where the existing duplicate semantics should still win.

A better shape is:

var wasExistingOutputEntry = existingEntries.Remove (apk_path);

using (var stream = File.OpenRead (jar_file_path))
using (var jar = ZipArchive.Open (stream)) {
	var jar_item = jar.ReadEntry (jar_entry_name);

	if (apk.ContainsEntry (apk_path)) {
		if (!wasExistingOutputEntry) {
			// Current packaged resource or earlier archive item wins.
			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;
		}

		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);
	}

	// extract jar_item and add it as today
}

Then update the regression test so the existing APK entry has stale contents and the jar has current contents, and assert that the APK ends with the current contents. That proves the incremental bug is fixed rather than only hiding the deletion symptom. The test can still keep an unrelated stale entry to verify cleanup behavior.

[Copilot]

@copilot The current fix prevents deletion of existing jar-sourced entries, but it still preserves stale contents forever when the source jar entry changes.

Please update BuildArchive so jar-source...

Addressed in 95e7370 and 2c430e7. Jar-sourced entries now distinguish existing output entries from current-build duplicates, refresh stale existing APK contents from the jar, skip up-to-date CRC matches, and keep cleanup coverage for unrelated stale entries.

[simonrozsival]

/review

[github-actions]

✅ Android PR Reviewer completed successfully!

[github-actions]

@copilot ⚠️ Needs Changes — The fix logic is correct and well-structured. A few items to address:

Issues (1 ⚠️, 3 💡):

• ⚠️ Test field tempDirectory should be nullable (string?) for [SetUp]-initialized fields
• 💡 New test file missing #nullable enable and file-scoped namespace
• 💡 Minor perf trade-off: jar is now opened before the duplicate check (acceptable for correctness)
• 💡 Consider a test for duplicate jar items targeting the same apk_path

Positive notes:

• CRC comparison for staleness detection is consistent with the existing pattern used for ApkInputPath entries (line 120)
• Good separation of the "existing output entry" vs "current build duplicate" cases
• Regression tests cover both stale-update and up-to-date-skip paths

CI: Public checks pass. Internal Xamarin.Android-PR pipeline status should be verified separately.

Generated by Android PR Reviewer for issue #11552 · ● 7M

[github-actions]

🤖 💡 Performance — The jar is now opened unconditionally, whereas previously the apk.ContainsEntry check was before File.OpenRead/ZipArchive.Open, allowing duplicates from current-build items to be skipped without touching the jar at all. This is probably fine in practice (OS file cache, rare duplicate path), but worth noting as a trade-off for correctness.

Rule: Place cheap checks before expensive ones

[github-actions]

🤖 💡 Formatting — New files should have #nullable enable at the top and use file-scoped namespaces (namespace Xamarin.Android.Build.Tests;). Several other test fixtures in this directory follow those conventions.

Rule: Nullable reference types / File-scoped namespaces

[github-actions]

🤖 ⚠️ Nullable — tempDirectory is declared as non-nullable string but is only assigned in Setup(). Without #nullable enable this compiles, but the intent is unclear and fragile. Declare as string? tempDirectory; (consistent with the nullable pattern for [SetUp]-initialized fields per repo conventions).

Rule: Nullable reference types

[github-actions]

🤖 💡 Testing — Consider adding a test for the case where multiple jar items target the same apk_path — this exercises the preserved duplicate-handling logic (!wasExistingOutputEntry branch). The current tests only cover the existing-output-entry path.

Rule: Test edge cases