From bbb48c59ecd0ea02adc16718c9ffc6936ec0f257 Mon Sep 17 00:00:00 2001 From: Jonathan Pobst Date: Thu, 31 Oct 2024 11:31:42 -1000 Subject: [PATCH] [XABT] Add `ArtifactFilename` metadata for `AndroidMavenLibrary` item. --- .../Properties/Resources.Designer.cs | 3 +- .../Properties/Resources.resx | 8 +-- .../Tasks/MavenDownload.cs | 5 +- .../Tasks/MavenDownloadTests.cs | 34 ++++++++++++- .../Utilities/ITaskItemExtensions.cs | 4 +- .../Utilities/MavenExtensions.cs | 49 ++++++++++++------- 6 files changed, 74 insertions(+), 29 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs b/src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs index d689abddf20..07aec73df1d 100644 --- a/src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs +++ b/src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs @@ -1282,8 +1282,7 @@ public static string XA4235 { /// /// Looks up a localized string similar to Cannot download Maven artifact '{0}:{1}'. - ///- {2}: {3} - ///- {4}: {5}. + ///{2}. /// public static string XA4236 { get { diff --git a/src/Xamarin.Android.Build.Tasks/Properties/Resources.resx b/src/Xamarin.Android.Build.Tasks/Properties/Resources.resx index 5703c402b83..9a4d896e1e7 100644 --- a/src/Xamarin.Android.Build.Tasks/Properties/Resources.resx +++ b/src/Xamarin.Android.Build.Tasks/Properties/Resources.resx @@ -985,15 +985,11 @@ To use a custom JDK path for a command line build, set the 'JavaSdkDirectory' MS Cannot download Maven artifact '{0}:{1}'. -- {2}: {3} -- {4}: {5} +{2} The following are literal names and should not be translated: Maven {0} - Maven artifact group id {1} - Maven artifact id -{2} - The .jar filename we tried to download -{3} - The HttpClient reported download exception message -{4} - The .aar filename we tried to download -{5} - The HttpClient provided download exception message +{2} - The filenames we tried to download and the HttpClient reported download exception messages Cannot download POM file for Maven artifact '{0}'. diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/MavenDownload.cs b/src/Xamarin.Android.Build.Tasks/Tasks/MavenDownload.cs index 81010e803bc..c746d26c5a1 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/MavenDownload.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/MavenDownload.cs @@ -87,8 +87,11 @@ public async override System.Threading.Tasks.Task RunTaskAsync () if (repository is null) return null; + // Allow user to override the Maven filename of the artifact + var maven_override_filename = item.GetMetadataOrDefault ("ArtifactFilename", null); + // Download artifact - var artifact_file = await MavenExtensions.DownloadPayload (repository, artifact, MavenCacheDirectory, Log, CancellationToken); + var artifact_file = await MavenExtensions.DownloadPayload (repository, artifact, MavenCacheDirectory, maven_override_filename, Log, CancellationToken); if (artifact_file is null) return null; diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/MavenDownloadTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/MavenDownloadTests.cs index 749dadd35d9..55d81bb69c3 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/MavenDownloadTests.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/MavenDownloadTests.cs @@ -170,7 +170,37 @@ public async Task MavenGoogleSuccess () } } - ITaskItem CreateMavenTaskItem (string name, string? version, string? repository = null) + [Test] + public async Task ArtifactFilenameOverride () + { + // Technically the artifact is 'react-android-0.76.1-release.aar' but we're going to override the filename to + // 'react-android-0.76.1.module' and download it instead for this test because the real .aar is 120+ MB. + var temp_cache_dir = Path.Combine (Path.GetTempPath (), Guid.NewGuid ().ToString ()); + + try { + var engine = new MockBuildEngine (TestContext.Out, new List ()); + var task = new MavenDownload { + BuildEngine = engine, + MavenCacheDirectory = temp_cache_dir, + AndroidMavenLibraries = [CreateMavenTaskItem ("com.facebook.react:react-android", "0.76.1", artifactFilename: "react-android-0.76.1.module")], + }; + + await task.RunTaskAsync (); + + Assert.AreEqual (0, engine.Errors.Count); + Assert.AreEqual (1, task.ResolvedAndroidMavenLibraries?.Length); + + var output_item = task.ResolvedAndroidMavenLibraries! [0]; + + Assert.AreEqual ("com.facebook.react:react-android:0.76.1", output_item.GetMetadata ("JavaArtifact")); + Assert.True (output_item.ItemSpec.EndsWith (Path.Combine ("0.76.1", "react-android-0.76.1.module"), StringComparison.OrdinalIgnoreCase)); + Assert.AreEqual (Path.Combine (temp_cache_dir, "central", "com.facebook.react", "react-android", "0.76.1", "react-android-0.76.1.pom"), output_item.GetMetadata ("Manifest")); + } finally { + DeleteTempDirectory (temp_cache_dir); + } + } + + ITaskItem CreateMavenTaskItem (string name, string? version, string? repository = null, string? artifactFilename = null) { var item = new TaskItem (name); @@ -178,6 +208,8 @@ ITaskItem CreateMavenTaskItem (string name, string? version, string? repository item.SetMetadata ("Version", version); if (repository is not null) item.SetMetadata ("Repository", repository); + if (artifactFilename is not null) + item.SetMetadata ("ArtifactFilename", artifactFilename); return item; } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/ITaskItemExtensions.cs b/src/Xamarin.Android.Build.Tasks/Utilities/ITaskItemExtensions.cs index f91af2d232f..a04c8203548 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/ITaskItemExtensions.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/ITaskItemExtensions.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Xml.Linq; using Microsoft.Android.Build.Tasks; @@ -23,7 +24,8 @@ public static IEnumerable ToXElements (this ICollection ite } } - public static string GetMetadataOrDefault (this ITaskItem item, string name, string defaultValue) + [return: NotNullIfNotNull (nameof (defaultValue))] + public static string? GetMetadataOrDefault (this ITaskItem item, string name, string? defaultValue) { var value = item.GetMetadata (name); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MavenExtensions.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MavenExtensions.cs index 7782f36552b..2a44701778a 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MavenExtensions.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MavenExtensions.cs @@ -5,6 +5,7 @@ using System.Diagnostics.CodeAnalysis; using System.IO; using System.Linq; +using System.Text; using System.Threading; using System.Threading.Tasks; using Java.Interop.Tools.Maven.Models; @@ -77,7 +78,7 @@ public static bool TryParseArtifacts (string id, TaskLoggingHelper log, out List return result; } - public static bool TryParseJavaArtifact (this ITaskItem task, string type, TaskLoggingHelper log, [NotNullWhen (true)]out Artifact? artifact, out bool attributesSpecified) + public static bool TryParseJavaArtifact (this ITaskItem task, string type, TaskLoggingHelper log, [NotNullWhen (true)] out Artifact? artifact, out bool attributesSpecified) { var result = TryParseJavaArtifacts (task, type, log, out var artifacts, out attributesSpecified); @@ -130,30 +131,44 @@ public static bool TryParseJavaArtifacts (this ITaskItem task, string type, Task } // Returns artifact output path - public static async Task DownloadPayload (CachedMavenRepository repository, Artifact artifact, string cacheDir, TaskLoggingHelper log, CancellationToken cancellationToken) + public static async Task DownloadPayload (CachedMavenRepository repository, Artifact artifact, string cacheDir, string? mavenOverrideFilename, TaskLoggingHelper log, CancellationToken cancellationToken) { var output_directory = Path.Combine (cacheDir, repository.Name, artifact.GroupId, artifact.Id, artifact.Version); Directory.CreateDirectory (output_directory); - var filename = $"{artifact.Id}-{artifact.Version}"; - var jar_filename = Path.Combine (output_directory, Path.Combine ($"{filename}.jar")); - var aar_filename = Path.Combine (output_directory, Path.Combine ($"{filename}.aar")); + var files_to_check = new List (); + + if (mavenOverrideFilename.HasValue ()) { + files_to_check.Add (Path.Combine (output_directory, mavenOverrideFilename)); + } else { + files_to_check.Add (Path.Combine (output_directory, $"{artifact.Id}-{artifact.Version}.jar")); + files_to_check.Add (Path.Combine (output_directory, $"{artifact.Id}-{artifact.Version}.aar")); + } // We don't need to redownload if we already have a cached copy - if (File.Exists (jar_filename)) - return jar_filename; + foreach (var file in files_to_check) { + if (File.Exists (file)) + return file; + } - if (File.Exists (aar_filename)) - return aar_filename; + // Try to download the file from Maven + var results = new List<(string file, string error)> (); - if (await TryDownloadPayload (repository, artifact, jar_filename, cancellationToken) is not string jar_error) - return jar_filename; + foreach (var file in files_to_check) { + if (await TryDownloadPayload (repository, artifact, Path.GetFileName (file), cancellationToken) is not string error) + return file; - if (await TryDownloadPayload (repository, artifact, aar_filename, cancellationToken) is not string aar_error) - return aar_filename; + results.Add ((file, error)); + } - log.LogCodedError ("XA4236", Properties.Resources.XA4236, artifact.GroupId, artifact.Id, Path.GetFileName (jar_filename), jar_error, Path.GetFileName (aar_filename), aar_error); + // Couldn't download the artifact, construct an error message for the user + var error_builder = new StringBuilder (); + + foreach (var error in results) + error_builder.AppendLine ($"- {Path.GetFileName (error.file)}: {error.error}"); + + log.LogCodedError ("XA4236", Properties.Resources.XA4236, artifact.GroupId, artifact.Id, error_builder.ToString ().TrimEnd ()); return null; } @@ -161,14 +176,12 @@ public static bool TryParseJavaArtifacts (this ITaskItem task, string type, Task // Return value is download error message, null represents success (async methods cannot have out parameters) static async Task TryDownloadPayload (CachedMavenRepository repository, Artifact artifact, string filename, CancellationToken cancellationToken) { - var maven_filename = $"{artifact.Id}-{artifact.Version}{Path.GetExtension (filename)}"; - try { - if ((await repository.GetFilePathAsync (artifact, maven_filename, cancellationToken)) is string path) { + if ((await repository.GetFilePathAsync (artifact, filename, cancellationToken)) is string path) { return null; } else { // This probably(?) cannot be hit, everything should come back as an exception - return $"Could not download {maven_filename}"; + return $"Could not download {filename}"; } } catch (Exception ex) {