From 0cdd106309b0ec191efdced062cb0bbd4b3c3af1 Mon Sep 17 00:00:00 2001 From: Bret Johnson Date: Wed, 6 May 2020 02:59:25 -0400 Subject: [PATCH 01/12] Write resources at the end of the APK, to optimize delta install --- .../Tasks/BuildApk.cs | 79 ++++++++++--------- .../Utilities/ZipArchiveEx.cs | 18 ++++- 2 files changed, 57 insertions(+), 40 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs b/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs index d1075ef0376..3715687160c 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs @@ -122,46 +122,14 @@ void ExecuteWithAbi (string [] supportedAbis, string apkInputPath, string apkOut } ArchiveFileList files = new ArchiveFileList (); - bool refresh = true; - if (apkInputPath != null && File.Exists (apkInputPath) && !File.Exists (apkOutputPath)) { - Log.LogDebugMessage ($"Copying {apkInputPath} to {apkInputPath}"); - File.Copy (apkInputPath, apkOutputPath, overwrite: true); - refresh = false; - } + using (var notice = Assembly.GetExecutingAssembly ().GetManifestResourceStream ("NOTICE.txt")) using (var apk = new ZipArchiveEx (apkOutputPath, File.Exists (apkOutputPath) ? FileMode.Open : FileMode.Create )) { - if (refresh) { - for (long i = 0; i < apk.Archive.EntryCount; i++) { - ZipEntry e = apk.Archive.ReadEntry ((ulong) i); - Log.LogDebugMessage ($"Registering item {e.FullName}"); - existingEntries.Add (e.FullName); - } + for (long i = 0; i < apk.Archive.EntryCount; i++) { + ZipEntry e = apk.Archive.ReadEntry ((ulong) i); + Log.LogDebugMessage ($"Registering item {e.FullName}"); + existingEntries.Add (e.FullName); } - if (apkInputPath != null && File.Exists (apkInputPath) && refresh) { - var lastWriteOutput = File.Exists (apkOutputPath) ? File.GetLastWriteTimeUtc (apkOutputPath) : DateTime.MinValue; - var lastWriteInput = File.GetLastWriteTimeUtc (apkInputPath); - using (var packaged = new ZipArchiveEx (apkInputPath, FileMode.Open)) { - foreach (var entry in packaged.Archive) { - Log.LogDebugMessage ($"Deregistering item {entry.FullName}"); - existingEntries.Remove (entry.FullName); - if (lastWriteInput <= lastWriteOutput) - continue; - if (apk.Archive.ContainsEntry (entry.FullName)) { - ZipEntry e = apk.Archive.ReadEntry (entry.FullName); - // check the CRC values as the ModifiedDate is always 01/01/1980 in the aapt generated file. - if (entry.CRC == e.CRC) { - Log.LogDebugMessage ($"Skipping {entry.FullName} from {apkInputPath} as its up to date."); - continue; - } - } - var ms = new MemoryStream (); - entry.Extract (ms); - Log.LogDebugMessage ($"Refreshing {entry.FullName} from {apkInputPath}"); - apk.Archive.AddStream (ms, entry.FullName, compressionMethod: entry.CompressionMethod); - } - } - } - apk.FixupWindowsPathSeparators ((a, b) => Log.LogDebugMessage ($"Fixing up malformed entry `{a}` -> `{b}`")); string noticeName = RootPath + "NOTICE"; existingEntries.Remove (noticeName); if (!apk.Archive.ContainsEntry (noticeName)) @@ -248,6 +216,43 @@ void ExecuteWithAbi (string [] supportedAbis, string apkInputPath, string apkOut count = 0; } } + + HashSet deletedEntries = new HashSet (); + if (apkInputPath != null && File.Exists (apkInputPath)) { + var lastWriteOutput = File.Exists (apkOutputPath) ? File.GetLastWriteTimeUtc (apkOutputPath) : DateTime.MinValue; + var lastWriteInput = File.GetLastWriteTimeUtc (apkInputPath); + using (var packaged = new ZipArchiveEx (apkInputPath, FileMode.Open)) { + foreach (var entry in packaged.Archive) { + Log.LogDebugMessage ($"Deregistering item {entry.FullName}"); + existingEntries.Remove (entry.FullName); + if (lastWriteInput <= lastWriteOutput) + continue; + if (apk.Archive.ContainsEntry (entry.FullName)) { + ZipEntry e = apk.Archive.ReadEntry (entry.FullName); + // check the CRC values as the ModifiedDate is always 01/01/1980 in the aapt generated file. + if (entry.CRC == e.CRC) { + Log.LogDebugMessage ($"Skipping {entry.FullName} from {apkInputPath} as its up to date."); + continue; + } + } + var ms = new MemoryStream (); + entry.Extract (ms); + Log.LogDebugMessage ($"Refreshing {entry.FullName} from {apkInputPath}"); + + // Force the modified resource to move to the end of the file, by deleting it first so that AddStream adds it + // back with a new index. Keeping modified resources toward the end optimizes the delta install for the typical + // dev scenario where the user is editing a few resources but most of the APK contents (e.g. the native libs) + // don't change and we want to keep their byte offset in the APK fixed. Delta install need not update APK + // contents that don't change and don't move. + //apk.Archive.DeleteEntry (entry); + //deletedEntries.Add (entry.Index); + + apk.Archive.AddStream (ms, entry.FullName, compressionMethod: entry.CompressionMethod); + } + } + } + apk.FixupWindowsPathSeparators (deletedEntries, (a, b) => Log.LogDebugMessage ($"Fixing up malformed entry `{a}` -> `{b}`")); + // Clean up Removed files. foreach (var entry in existingEntries) { Log.LogDebugMessage ($"Removing {entry} as it is not longer required."); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/ZipArchiveEx.cs b/src/Xamarin.Android.Build.Tasks/Utilities/ZipArchiveEx.cs index a1d55647cb5..b3a76d9c6e9 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/ZipArchiveEx.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/ZipArchiveEx.cs @@ -1,4 +1,5 @@ -using System; +using System; +using System.Collections.Generic; using System.IO; using System.Linq; using Xamarin.Tools.Zip; @@ -113,10 +114,21 @@ public void AddDirectory (string folder, string folderInArchive, CompressionMeth /// /// HACK: aapt2 is creating zip entries on Windows such as `assets\subfolder/asset2.txt` /// - public void FixupWindowsPathSeparators (Action onRename) + public void FixupWindowsPathSeparators (HashSet deletedEntries, Action onRename) { bool modified = false; - foreach (var entry in zip) { + + long entryCount = zip.EntryCount; + if (entryCount < 0) + return; + + for (ulong i = 0; i < (ulong) entryCount; ++i) { + // If an entry index has been deleted then ReadEntry will throw a "Entry has been deleted" ZipException. + // So we skip deleted entries instead + if (deletedEntries.Contains (i)) + continue; + + var entry = zip.ReadEntry (i); if (entry.FullName.Contains ('\\')) { var name = entry.FullName.Replace ('\\', '/'); onRename?.Invoke (entry.FullName, name); From f53a5c7fc66f4d2d3a61df7f08d4c33ccc4e6418 Mon Sep 17 00:00:00 2001 From: Bret Johnson Date: Wed, 6 May 2020 23:34:43 -0400 Subject: [PATCH 02/12] Fixup resource update Fix to delete the resource with the output APK zip index, not the input APK Updated logging, including separate message for added vs updated resources --- .../Tasks/BuildApk.cs | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs b/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs index 3715687160c..8135a819822 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs @@ -227,7 +227,9 @@ void ExecuteWithAbi (string [] supportedAbis, string apkInputPath, string apkOut existingEntries.Remove (entry.FullName); if (lastWriteInput <= lastWriteOutput) continue; - if (apk.Archive.ContainsEntry (entry.FullName)) { + + long entryIndexInOutput = -1; + if (apk.Archive.ContainsEntry (entry.FullName, out entryIndexInOutput)) { ZipEntry e = apk.Archive.ReadEntry (entry.FullName); // check the CRC values as the ModifiedDate is always 01/01/1980 in the aapt generated file. if (entry.CRC == e.CRC) { @@ -235,17 +237,22 @@ void ExecuteWithAbi (string [] supportedAbis, string apkInputPath, string apkOut continue; } } + var ms = new MemoryStream (); entry.Extract (ms); - Log.LogDebugMessage ($"Refreshing {entry.FullName} from {apkInputPath}"); - - // Force the modified resource to move to the end of the file, by deleting it first so that AddStream adds it - // back with a new index. Keeping modified resources toward the end optimizes the delta install for the typical - // dev scenario where the user is editing a few resources but most of the APK contents (e.g. the native libs) - // don't change and we want to keep their byte offset in the APK fixed. Delta install need not update APK - // contents that don't change and don't move. - //apk.Archive.DeleteEntry (entry); - //deletedEntries.Add (entry.Index); + + if (entryIndexInOutput != -1) { + Log.LogDebugMessage ($"Refreshing {entry.FullName} from {apkInputPath}"); + + // Force the modified resource to move to the end of the file, by deleting it first so that AddStream adds it + // back with a new index. Keeping modified resources toward the end optimizes the delta install for the typical + // dev scenario where the user is editing a few resources but most of the APK contents (e.g. the native libs) + // don't change and we want to keep their byte offset in the APK fixed. Delta install need not update APK + // contents that don't change and don't move. + apk.Archive.DeleteEntry ((ulong) entryIndexInOutput); + deletedEntries.Add ((ulong) entryIndexInOutput); + } + else Log.LogDebugMessage ($"Adding {entry.FullName} from {apkInputPath}"); apk.Archive.AddStream (ms, entry.FullName, compressionMethod: entry.CompressionMethod); } From 5fc6087f7de2c0ee1317a24373b2a0a65135a0d5 Mon Sep 17 00:00:00 2001 From: Bret Johnson Date: Thu, 7 May 2020 00:30:57 -0400 Subject: [PATCH 03/12] Fix to get the file timestamps before output is modified --- src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs b/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs index 8135a819822..43fbcb5f271 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs @@ -125,6 +125,15 @@ void ExecuteWithAbi (string [] supportedAbis, string apkInputPath, string apkOut using (var notice = Assembly.GetExecutingAssembly ().GetManifestResourceStream ("NOTICE.txt")) using (var apk = new ZipArchiveEx (apkOutputPath, File.Exists (apkOutputPath) ? FileMode.Open : FileMode.Create )) { + bool apkInputPathExists = apkInputPath != null && File.Exists (apkInputPath); + + DateTime lastWriteOutput = DateTime.MinValue; + DateTime lastWriteInput = DateTime.MinValue; + if (apkInputPathExists) { + lastWriteOutput = File.Exists (apkOutputPath) ? File.GetLastWriteTimeUtc (apkOutputPath) : DateTime.MinValue; + lastWriteInput = File.GetLastWriteTimeUtc (apkInputPath); + } + for (long i = 0; i < apk.Archive.EntryCount; i++) { ZipEntry e = apk.Archive.ReadEntry ((ulong) i); Log.LogDebugMessage ($"Registering item {e.FullName}"); @@ -218,9 +227,7 @@ void ExecuteWithAbi (string [] supportedAbis, string apkInputPath, string apkOut } HashSet deletedEntries = new HashSet (); - if (apkInputPath != null && File.Exists (apkInputPath)) { - var lastWriteOutput = File.Exists (apkOutputPath) ? File.GetLastWriteTimeUtc (apkOutputPath) : DateTime.MinValue; - var lastWriteInput = File.GetLastWriteTimeUtc (apkInputPath); + if (apkInputPathExists) { using (var packaged = new ZipArchiveEx (apkInputPath, FileMode.Open)) { foreach (var entry in packaged.Archive) { Log.LogDebugMessage ($"Deregistering item {entry.FullName}"); From 968f4eb41c3a92fdc8106a34615eb1f97c6a24db Mon Sep 17 00:00:00 2001 From: Bret Johnson Date: Mon, 11 May 2020 16:38:55 -0400 Subject: [PATCH 04/12] Add braces, per code style conventions --- src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs b/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs index 43fbcb5f271..946d23b5cd1 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs @@ -250,7 +250,7 @@ void ExecuteWithAbi (string [] supportedAbis, string apkInputPath, string apkOut if (entryIndexInOutput != -1) { Log.LogDebugMessage ($"Refreshing {entry.FullName} from {apkInputPath}"); - + // Force the modified resource to move to the end of the file, by deleting it first so that AddStream adds it // back with a new index. Keeping modified resources toward the end optimizes the delta install for the typical // dev scenario where the user is editing a few resources but most of the APK contents (e.g. the native libs) @@ -258,8 +258,9 @@ void ExecuteWithAbi (string [] supportedAbis, string apkInputPath, string apkOut // contents that don't change and don't move. apk.Archive.DeleteEntry ((ulong) entryIndexInOutput); deletedEntries.Add ((ulong) entryIndexInOutput); + } else { + Log.LogDebugMessage ($"Adding {entry.FullName} from {apkInputPath}"); } - else Log.LogDebugMessage ($"Adding {entry.FullName} from {apkInputPath}"); apk.Archive.AddStream (ms, entry.FullName, compressionMethod: entry.CompressionMethod); } From abd6e73e7289c27cbbf221dde448e719e342081a Mon Sep 17 00:00:00 2001 From: Bret Johnson Date: Tue, 12 May 2020 14:01:25 -0400 Subject: [PATCH 05/12] Add test to verify updated resources at end of zip --- .../IncrementalBuildTest.cs | 92 ++++++++++++++++++- 1 file changed, 87 insertions(+), 5 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs index c1d6353aac5..f424eff4bfc 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs @@ -8,6 +8,7 @@ using System.Text; using Xamarin.Android.Tasks; using Xamarin.ProjectTools; +using Xamarin.Tools.Zip; namespace Xamarin.Android.Build.Tests { @@ -1088,21 +1089,102 @@ public void AndroidResourceChange () using (var builder = CreateApkBuilder ()) { Assert.IsTrue (builder.Build (proj), "first build should succeed"); - // AndroidResource change - proj.LayoutMain += $"{Environment.NewLine}"; + // Change the AndroidResource body; just adding a comment isn't enough to trigger an APK update + string orientationTag = "android:orientation=\"vertical\""; + proj.LayoutMain = proj.LayoutMain.Replace (orientationTag, $"{orientationTag} android:paddingLeft=\"16dp\""); proj.Touch ("Resources\\layout\\Main.axml"); Assert.IsTrue (builder.Build (proj), "second build should succeed"); - var targets = new [] { + AssertNonResourceTargetsSkipped (builder); + } + } + + void AssertNonResourceTargetsSkipped(ProjectBuilder builder) + { + var targets = new [] { "_ResolveLibraryProjectImports", "_GenerateJavaStubs", "_CompileJava", "_CompileToDalvik", }; - foreach (var target in targets) { - Assert.IsTrue (builder.Output.IsTargetSkipped (target), $"`{target}` should be skipped!"); + foreach (var target in targets) { + Assert.IsTrue (builder.Output.IsTargetSkipped (target), $"`{target}` should be skipped!"); + } + } + + [Test] + public void AndroidChangedResourcesAtEnd () + { + var path = Path.Combine (Root, "temp", TestName); + + var proj = new XamarinAndroidApplicationProject (); + using (var builder = CreateApkBuilder ()) { + Assert.IsTrue (builder.Build (proj), "first build should succeed"); + + string signedApkPath = Path.Combine (path, proj.OutputPath, "UnnamedProject.UnnamedProject-Signed.apk"); + + // Initial resources should at the end of the zip + using (var zip = ZipArchive.Open (signedApkPath, FileMode.Open)) { + AssertZipEntryHasPosition (zip, "res/layout/main.xml", 32); + AssertZipEntryHasPosition (zip, "resources.arsc", 33); + } + + AddLayout2 (proj); + Assert.IsTrue (builder.Build (proj), "second build should succeed"); + //TODO: This fails currently as at least _ResolveLibraryProjectImports is run again; bug? + //AssertNonResourceTargetsSkipped (builder); + + // Added resources should go at the end of the zip + using (var zip = ZipArchive.Open (signedApkPath, FileMode.Open)) { + AssertZipEntryHasPosition (zip, "res/layout/main.xml", 32); + AssertZipEntryHasPosition (zip, "res/layout/layout2.xml", 33); + AssertZipEntryHasPosition (zip, "resources.arsc", 34); + } + + string orientationTag = "android:orientation=\"vertical\""; + proj.LayoutMain = proj.LayoutMain.Replace (orientationTag, $"{orientationTag} android:paddingLeft=\"16dp\""); + proj.Touch ("Resources\\layout\\Main.axml"); + Assert.IsTrue (builder.Build (proj), "third build should succeed"); + AssertNonResourceTargetsSkipped (builder); + + // Updated resources should go at the end of the zip + using (var zip = ZipArchive.Open (signedApkPath, FileMode.Open)) { + AssertZipEntryHasPosition (zip, "res/layout/layout2.xml", 32); + AssertZipEntryHasPosition (zip, "resources.arsc", 33); + AssertZipEntryHasPosition (zip, "res/layout/main.xml", 34); + } + } + } + + void AddLayout2 (XamarinAndroidApplicationProject proj) + { + const string layout2 = @" + +