Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[XABT] Add ArtifactFilename metadata for @(AndroidMavenLibrary) item. #9479

Merged
merged 1 commit into from
Nov 26, 2024
Merged

[XABT] Add ArtifactFilename metadata for @(AndroidMavenLibrary) item. #9479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 2 additions & 6 deletions src/Xamarin.Android.Build.Tasks/Properties/Resources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -985,15 +985,11 @@ To use a custom JDK path for a command line build, set the 'JavaSdkDirectory' MS
</data>
<data name="XA4236" xml:space="preserve">
<value>Cannot download Maven artifact '{0}:{1}'.
- {2}: {3}
- {4}: {5}</value>
{2}</value>
<comment>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</comment>
{2} - The filenames we tried to download and the HttpClient reported download exception messages</comment>
</data>
<data name="XA4237" xml:space="preserve">
<value>Cannot download POM file for Maven artifact '{0}'.
Expand Down
5 changes: 4 additions & 1 deletion src/Xamarin.Android.Build.Tasks/Tasks/MavenDownload.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
34 changes: 33 additions & 1 deletion ...Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/MavenDownloadTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,14 +170,46 @@ 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<BuildErrorEventArgs> ());
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);

if (version is not null)
item.SetMetadata ("Version", version);
if (repository is not null)
item.SetMetadata ("Repository", repository);
if (artifactFilename is not null)
item.SetMetadata ("ArtifactFilename", artifactFilename);

return item;
}
Expand Down
4 changes: 3 additions & 1 deletion src/Xamarin.Android.Build.Tasks/Utilities/ITaskItemExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -23,7 +24,8 @@ public static IEnumerable<XElement> ToXElements (this ICollection<ITaskItem> 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);

Expand Down
49 changes: 31 additions & 18 deletions src/Xamarin.Android.Build.Tasks/Utilities/MavenExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -130,45 +131,57 @@ public static bool TryParseJavaArtifacts (this ITaskItem task, string type, Task
}

// Returns artifact output path
public static async Task<string?> DownloadPayload (CachedMavenRepository repository, Artifact artifact, string cacheDir, TaskLoggingHelper log, CancellationToken cancellationToken)
public static async Task<string?> 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<string> ();

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"));
Comment on lines +145 to +146
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have to do this here, but is there some manifest file that we can download to know the exact file name? It seems odd they can choose random names. How does Java tooling know which file to download?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good question. There doesn't appear to be anything in the .pom that gives the file name: https://repo1.maven.org/maven2/com/facebook/react/react-android/0.76.0/react-android-0.76.0.pom

It looks like React has its own Gradle plugin you need to use, perhaps it knows how to resolve their files?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case of react-android, there appears to be a react-android-0.76.0.module file, which is a JSON file which mentions the "real" filenames:

{
  …
  "component": {
    "group": "com.facebook.react",
    "module": "react-android",
    "version": "0.76.0",
    "attributes": {
      "org.gradle.status": "release"
    }
  },
  …
  "variants": [
    {
      "name": "debugVariantDefaultApiPublication",
      "attributes": {
        "com.android.build.api.attributes.BuildTypeAttr": "debug",
        "org.gradle.category": "library",
        "org.gradle.dependency.bundling": "external",
        "org.gradle.libraryelements": "aar",
        "org.gradle.usage": "java-api"
      },
      …
      "files": [
        {
          "name": "react-android-0.76.0-debug.aar",
          "url": "react-android-0.76.0-debug.aar",
          "size": 204696650,
          "sha512": "736141a49db0dbadf34379b832ea858642be172d0d302edca7edc76ae34c93846b98167989854cc3da954b4ac32946f0ca3fc55bd5f7b6b8df17483e04eb342e",
          "sha256": "a131374a0e4aea0bf131746d615c8ba5fcafedf79e809d864d2b2a619714c205",
          "sha1": "a78b8bb37b808263e6757f99ef82f3d75823af0f",
          "md5": "70afd54443016d5b9c2c5d1c7ac9342b"
        }
      ]
    },
   …
   {
      "name": "releaseVariantDefaultRuntimePublication",
      "attributes": {
        "com.android.build.api.attributes.BuildTypeAttr": "release",
        "org.gradle.category": "library",
        "org.gradle.dependency.bundling": "external",
        "org.gradle.libraryelements": "aar",
        "org.gradle.usage": "java-runtime"
      },
      …
      "files": [
        {
          "name": "react-android-0.76.0-release.aar",
          "url": "react-android-0.76.0-release.aar",
          "size": 128739900,
          "sha512": "bbd62ba818768ab4ef05cdbb463a63cf208933c23419b44f692284f30dc5af5acfa4bb50eb48015b77132c7cb81c33793d3d706dafb30820d9b478ce1d0d18d4",
          "sha256": "ce57d01ab360711f768a2aa46a86f153e3edaf8cf20da25237861bef21c41173",
          "sha1": "f766f97dec77212133f8b3a1b4615d9b5d41ffe5",
          "md5": "7cbeccd4a237fae95035f1bcc2f25f0f"
        }
      ]
    },
  …

Which certainly looks interesting! The problems include:

  • What is a .module file? Documentation?
  • How widespread is this construct used?
  • What (and how) should this info be used or surfaced? A %(BuildTypeAttr) metadata which is used to key off of com.android.build.api.attributes.BuildTypeAttr?
  • Is this something we want to support?

%(ArtifactFilename) is a good workaround for now.

}

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

// Return value is download error message, null represents success (async methods cannot have out parameters)
static async Task<string?> 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) {
Expand Down
Loading