From c43ed51342f215ad285678fbe39281f1804ef300 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Thu, 13 Apr 2023 11:00:45 +0100 Subject: [PATCH 1/5] [Xamarin.Android.Build.Tasks] Bump ZipFlushFilesLimit Context https://i.azdo.io/1782014 On Windows customers are seeing the following error ``` Error XABLD7000: Xamarin.Tools.Zip.ZipException: Renaming temporary file failed: Permission denied at Xamarin.Tools.Zip.ZipArchive.Close() in /Users/runner/work/1/s/LibZipSharp/Xamarin.Tools.Zip/ZipArchive.cs:line 939 ``` We have traced this to the use of the `MoveFileExW`/`MoveFileExA` API. When libzip is trying to move its temp file to the final location, Windows is raising this error. ``` Renaming temporary file failed: Permission denied ``` Turning off Anti Virus seems to help, however adding an exclusion does not. This is very confusing. So we are unsure why this error is being raised. The process has the correct permissions and the file being moved is in the same directory, so its not a TEMP folder issue. So perhaps its the number of temp files we create? Part of the `BuildApk` system is that as we add files we `Flush` the zip file to commit those changes to disk. This is partly to work around how libzip works. It does not write any data to the main file until `zip_close` is called. So to work around issues around too many files being open [1], we added this flush. The limit of 50 files was picked out of a hat. So lets try pushing the limit up a bit to see if that helps. [1] https://github.com/xamarin/xamarin-android/commit/9166e0363417d6d121de37a765db8f2ba7cece7b --- src/Xamarin.Android.Build.Tasks/Utilities/ZipArchiveEx.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/ZipArchiveEx.cs b/src/Xamarin.Android.Build.Tasks/Utilities/ZipArchiveEx.cs index 3f22095e973..4610e48fc72 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/ZipArchiveEx.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/ZipArchiveEx.cs @@ -8,8 +8,8 @@ namespace Xamarin.Android.Tasks public class ZipArchiveEx : IDisposable { - public static int ZipFlushSizeLimit = 50 * 1024 * 1024; - public static int ZipFlushFilesLimit = 50; + public static int ZipFlushSizeLimit = 100 * 1024 * 1024; + public static int ZipFlushFilesLimit = 512; ZipArchive zip; string archive; From d9c0dfed470ccf6ea2246df2d551c125d43b29d9 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Fri, 14 Apr 2023 15:23:48 +0100 Subject: [PATCH 2/5] Add ability to override from MSBuild --- .../Tasks/BuildApk.cs | 40 +++++++------------ .../Utilities/ZipArchiveEx.cs | 24 +++++++++++ .../Xamarin.Android.Common.targets | 4 ++ 3 files changed, 43 insertions(+), 25 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs b/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs index 1fc8295b0c1..5e2dbb48a5c 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs @@ -95,6 +95,10 @@ public class BuildApk : AndroidTask public bool UseAssemblyStore { get; set; } + public string ZipFlushFilesLimit { get; set; } + + public string ZipFlushSizeLimit { get; set; } + [Required] public string ProjectFullPath { get; set; } @@ -206,7 +210,6 @@ void ExecuteWithAbi (string [] supportedAbis, string apkInputPath, string apkOut AddFileToArchiveIfNewer (apk, RuntimeConfigBinFilePath, $"{AssembliesPath}rc.bin", compressionMethod: UncompressedMethod); } - int count = 0; foreach (var file in files) { var item = Path.Combine (file.archivePath.Replace (Path.DirectorySeparatorChar, '/')); existingEntries.Remove (item); @@ -216,12 +219,7 @@ void ExecuteWithAbi (string [] supportedAbis, string apkInputPath, string apkOut continue; } Log.LogDebugMessage ("\tAdding {0}", file.filePath); - apk.Archive.AddFile (file.filePath, item, compressionMethod: compressionMethod); - count++; - if (count >= ZipArchiveEx.ZipFlushFilesLimit) { - apk.Flush(); - count = 0; - } + apk.AddFileAndFlush (file.filePath, item, compressionMethod: compressionMethod); } var jarFiles = (JavaSourceFiles != null) ? JavaSourceFiles.Where (f => f.ItemSpec.EndsWith (".jar", StringComparison.OrdinalIgnoreCase)) : null; @@ -236,7 +234,6 @@ void ExecuteWithAbi (string [] supportedAbis, string apkInputPath, string apkOut var jarFilePaths = libraryProjectJars.Concat (jarFiles != null ? jarFiles.Select (j => j.ItemSpec) : Enumerable.Empty ()); jarFilePaths = MonoAndroidHelper.DistinctFilesByContent (jarFilePaths); - count = 0; foreach (var jarFile in jarFilePaths) { using (var stream = File.OpenRead (jarFile)) using (var jar = ZipArchive.Open (stream)) { @@ -278,14 +275,9 @@ void ExecuteWithAbi (string [] supportedAbis, string apkInputPath, string apkOut data = d.ToArray (); } Log.LogDebugMessage ($"Adding {path} from {jarFile} as the archive file is out of date."); - apk.Archive.AddEntry (data, path); + apk.AddEntryAndFlush (data, path); } } - count++; - if (count >= ZipArchiveEx.ZipFlushFilesLimit) { - apk.Flush(); - count = 0; - } } // Clean up Removed files. foreach (var entry in existingEntries) { @@ -302,6 +294,12 @@ void ExecuteWithAbi (string [] supportedAbis, string apkInputPath, string apkOut public override bool RunTask () { + if (int.TryParse (ZipFlushFilesLimit, out int flushFilesLimit)) { + ZipArchiveEx.ZipFlushFilesLimit = flushFilesLimit; + } + if (int.TryParse (ZipFlushSizeLimit, out int flushSizeLimit)) { + ZipArchiveEx.ZipFlushSizeLimit = flushSizeLimit; + } Aot.TryGetSequencePointsMode (AndroidSequencePointsMode, out sequencePointsMode); var outputFiles = new List (); @@ -377,7 +375,6 @@ void AddAssemblies (ZipArchiveEx apk, bool debug, bool compress, IDictionary= ZipArchiveEx.ZipFlushFilesLimit) { - apk.Flush(); - count = 0; - } } } } @@ -554,7 +544,7 @@ bool AddFileToArchiveIfNewer (ZipArchiveEx apk, string file, string inArchivePat return false; } Log.LogDebugMessage ($"Adding {file} as the archive file is out of date."); - apk.Archive.AddFile (file, inArchivePath, compressionMethod: compressionMethod); + apk.AddFileAndFlush (file, inArchivePath, compressionMethod: compressionMethod); return true; } @@ -578,7 +568,7 @@ void AddAssemblyConfigEntry (ZipArchiveEx apk, string assemblyPath, string confi source.CopyTo (dest); dest.WriteByte (0); dest.Position = 0; - apk.Archive.AddEntry (inArchivePath, dest, compressionMethod); + apk.AddEntryAndFlush (inArchivePath, dest, compressionMethod); } } @@ -625,7 +615,7 @@ void AddNativeLibraryToArchive (ZipArchiveEx apk, string abi, string filesystemP return; } Log.LogDebugMessage ($"Adding native library: {filesystemPath} (APK path: {archivePath})"); - apk.Archive.AddEntry (archivePath, File.OpenRead (filesystemPath), compressionMethod); + apk.AddEntryAndFlush (archivePath, File.OpenRead (filesystemPath), compressionMethod); } void AddRuntimeLibraries (ZipArchiveEx apk, string [] supportedAbis) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/ZipArchiveEx.cs b/src/Xamarin.Android.Build.Tasks/Utilities/ZipArchiveEx.cs index 4610e48fc72..b7033cb6eee 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/ZipArchiveEx.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/ZipArchiveEx.cs @@ -70,6 +70,30 @@ void AddFileAndFlush (string filename, long fileLength, string archiveFileName, } } + public void AddFileAndFlush (string filename, string archiveFileName, CompressionMethod compressionMethod) + { + var fi = new FileInfo (filename); + AddFileAndFlush (filename, fi.Length, archiveFileName, compressionMethod); + } + + public void AddEntryAndFlush (byte[] data, string archiveFileName) + { + filesWrittenTotalSize += data.Length; + zip.AddEntry (data, archiveFileName); + if ((filesWrittenTotalSize >= ZipArchiveEx.ZipFlushSizeLimit || filesWrittenTotalCount >= ZipArchiveEx.ZipFlushFilesLimit) && AutoFlush) { + Flush (); + } + } + + public void AddEntryAndFlush (string archiveFileName, Stream data, CompressionMethod method) + { + filesWrittenTotalSize += data.Length; + zip.AddEntry (archiveFileName, data, method); + if ((filesWrittenTotalSize >= ZipArchiveEx.ZipFlushSizeLimit || filesWrittenTotalCount >= ZipArchiveEx.ZipFlushFilesLimit) && AutoFlush) { + Flush (); + } + } + void AddFiles (string folder, string folderInArchive, CompressionMethod method) { foreach (string fileName in Directory.GetFiles (folder, "*.*", SearchOption.TopDirectoryOnly)) { diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index 6d0af62a5d8..6259e2b3341 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -2152,6 +2152,8 @@ because xbuild doesn't support framework reference assemblies. CheckedBuild="$(_AndroidCheckedBuild)" RuntimeConfigBinFilePath="$(_BinaryRuntimeConfigPath)" ExcludeFiles="@(AndroidPackagingOptionsExclude)" + ZipFlushFilesLimit="$(_ZipFlushFilesLimit)" + ZipFlushSizeLimit="$(_ZipFlushSizeLimit)" UseAssemblyStore="$(AndroidUseAssemblyStore)"> @@ -2185,6 +2187,8 @@ because xbuild doesn't support framework reference assemblies. CheckedBuild="$(_AndroidCheckedBuild)" RuntimeConfigBinFilePath="$(_BinaryRuntimeConfigPath)" ExcludeFiles="@(AndroidPackagingOptionsExclude)" + ZipFlushFilesLimit="$(_ZipFlushFilesLimit)" + ZipFlushSizeLimit="$(_ZipFlushSizeLimit)" UseAssemblyStore="$(AndroidUseAssemblyStore)"> From 99e8a95d7bffea5cd57b1769367ca0e65355315e Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Mon, 17 Apr 2023 10:16:42 +0100 Subject: [PATCH 3/5] Convert to using Properties --- src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs | 12 ++++++------ .../Utilities/ZipArchiveEx.cs | 13 ++++++++----- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs b/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs index 5e2dbb48a5c..fbcaa1e0944 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs @@ -140,6 +140,12 @@ void ExecuteWithAbi (string [] supportedAbis, string apkInputPath, string apkOut refresh = false; } using (var apk = new ZipArchiveEx (apkOutputPath, File.Exists (apkOutputPath) ? FileMode.Open : FileMode.Create )) { + if (int.TryParse (ZipFlushFilesLimit, out int flushFilesLimit)) { + apk.ZipFlushFilesLimit = flushFilesLimit; + } + if (int.TryParse (ZipFlushSizeLimit, out int flushSizeLimit)) { + apk.ZipFlushSizeLimit = flushSizeLimit; + } if (refresh) { for (long i = 0; i < apk.Archive.EntryCount; i++) { ZipEntry e = apk.Archive.ReadEntry ((ulong) i); @@ -294,12 +300,6 @@ void ExecuteWithAbi (string [] supportedAbis, string apkInputPath, string apkOut public override bool RunTask () { - if (int.TryParse (ZipFlushFilesLimit, out int flushFilesLimit)) { - ZipArchiveEx.ZipFlushFilesLimit = flushFilesLimit; - } - if (int.TryParse (ZipFlushSizeLimit, out int flushSizeLimit)) { - ZipArchiveEx.ZipFlushSizeLimit = flushSizeLimit; - } Aot.TryGetSequencePointsMode (AndroidSequencePointsMode, out sequencePointsMode); var outputFiles = new List (); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/ZipArchiveEx.cs b/src/Xamarin.Android.Build.Tasks/Utilities/ZipArchiveEx.cs index b7033cb6eee..90a8371aad7 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/ZipArchiveEx.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/ZipArchiveEx.cs @@ -8,8 +8,8 @@ namespace Xamarin.Android.Tasks public class ZipArchiveEx : IDisposable { - public static int ZipFlushSizeLimit = 100 * 1024 * 1024; - public static int ZipFlushFilesLimit = 512; + const int DEFAULT_FLUSH_SIZE_LIMIT = 100 * 1024 * 1024; + const int DEFAULT_FLUSH_FILES_LIMIT = 512; ZipArchive zip; string archive; @@ -24,6 +24,9 @@ public ZipArchive Archive { public bool CreateDirectoriesInZip { get; set; } = true; + public int ZipFlushSizeLimit { get; set; } = DEFAULT_FLUSH_SIZE_LIMIT; + public int ZipFlushFilesLimit { get; set; } = DEFAULT_FLUSH_FILES_LIMIT; + public ZipArchiveEx (string archive) : this (archive, FileMode.CreateNew) { } @@ -65,7 +68,7 @@ void AddFileAndFlush (string filename, long fileLength, string archiveFileName, { filesWrittenTotalSize += fileLength; zip.AddFile (filename, archiveFileName, compressionMethod: compressionMethod); - if ((filesWrittenTotalSize >= ZipArchiveEx.ZipFlushSizeLimit || filesWrittenTotalCount >= ZipArchiveEx.ZipFlushFilesLimit) && AutoFlush) { + if ((filesWrittenTotalSize >= ZipFlushSizeLimit || filesWrittenTotalCount >= ZipFlushFilesLimit) && AutoFlush) { Flush (); } } @@ -80,7 +83,7 @@ public void AddEntryAndFlush (byte[] data, string archiveFileName) { filesWrittenTotalSize += data.Length; zip.AddEntry (data, archiveFileName); - if ((filesWrittenTotalSize >= ZipArchiveEx.ZipFlushSizeLimit || filesWrittenTotalCount >= ZipArchiveEx.ZipFlushFilesLimit) && AutoFlush) { + if ((filesWrittenTotalSize >= ZipFlushSizeLimit || filesWrittenTotalCount >= ZipFlushFilesLimit) && AutoFlush) { Flush (); } } @@ -89,7 +92,7 @@ public void AddEntryAndFlush (string archiveFileName, Stream data, CompressionMe { filesWrittenTotalSize += data.Length; zip.AddEntry (archiveFileName, data, method); - if ((filesWrittenTotalSize >= ZipArchiveEx.ZipFlushSizeLimit || filesWrittenTotalCount >= ZipArchiveEx.ZipFlushFilesLimit) && AutoFlush) { + if ((filesWrittenTotalSize >= ZipFlushSizeLimit || filesWrittenTotalCount >= ZipFlushFilesLimit) && AutoFlush) { Flush (); } } From d2d77b94e6cc2411fba5beae0969ef6cd795e120 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Mon, 17 Apr 2023 11:14:53 +0100 Subject: [PATCH 4/5] Add a test --- .../PackagingTest.cs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs index 8754f7d79e7..d32c8f40806 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs @@ -962,6 +962,32 @@ public void CheckExcludedFilesAreMissing () } } + [Test] + [TestCase (1, -1)] + [TestCase (5, -1)] + [TestCase (50, -1)] + [TestCase (100, -1)] + [TestCase (512, -1)] + [TestCase (1024, -1)] + [TestCase (-1, 1)] + [TestCase (-1, 5)] + [TestCase (-1, 10)] + [TestCase (-1, 100)] + [TestCase (-1, 200)] + public void BuildApkWithZipFlushLimits (int filesLimit, int sizeLimit) + { + var proj = new XamarinAndroidApplicationProject { + IsRelease = true, + }; + if (filesLimit > 0) + proj.SetProperty ("_ZipFlushFilesLimit", filesLimit.ToString ()); + if (sizeLimit > 0) + proj.SetProperty ("_ZipFlushSizeLimit", (sizeLimit * 1024 * 1024).ToString ()); + using (var b = CreateApkBuilder ()) { + Assert.IsTrue (b.Build (proj), "Build should have succeeded."); + } + } + [Test] public void ExtractNativeLibsTrue () { From c15c16e20b60336cd8fc90a4b8681447fb4d8cf7 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Mon, 17 Apr 2023 13:55:59 +0100 Subject: [PATCH 5/5] Test Debug Config --- .../Xamarin.Android.Build.Tests/PackagingTest.cs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs index d32c8f40806..e4f349982f7 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs @@ -977,14 +977,28 @@ public void CheckExcludedFilesAreMissing () public void BuildApkWithZipFlushLimits (int filesLimit, int sizeLimit) { var proj = new XamarinAndroidApplicationProject { - IsRelease = true, + IsRelease = false, + PackageReferences = { + KnownPackages.SupportDesign_27_0_2_1, + KnownPackages.SupportV7CardView_27_0_2_1, + KnownPackages.AndroidSupportV4_27_0_2_1, + KnownPackages.SupportCoreUtils_27_0_2_1, + KnownPackages.SupportMediaCompat_27_0_2_1, + KnownPackages.SupportFragment_27_0_2_1, + KnownPackages.SupportCoreUI_27_0_2_1, + KnownPackages.SupportCompat_27_0_2_1, + KnownPackages.SupportV7AppCompat_27_0_2_1, + KnownPackages.SupportV7MediaRouter_27_0_2_1, + }, }; + proj.SetProperty ("EmbedAssembliesIntoApk", "true"); if (filesLimit > 0) proj.SetProperty ("_ZipFlushFilesLimit", filesLimit.ToString ()); if (sizeLimit > 0) proj.SetProperty ("_ZipFlushSizeLimit", (sizeLimit * 1024 * 1024).ToString ()); using (var b = CreateApkBuilder ()) { Assert.IsTrue (b.Build (proj), "Build should have succeeded."); + } }