[Xamarin.Android.Build.Tasks] Add support for @(AndroidMavenLibrary). #8420

jpobst · 2023-10-12T19:50:34Z

Context: #4528

Description

Adds support for the @(AndroidMavenLibrary) item group, which will automatically download the requested Java artifact version from a Maven repository and add it as an @(AndroidLibrary) for binding.

For example, to download a library from Maven Central and create an Android binding for it:

<ItemGroup>
  <AndroidMavenLibrary Include="com.google.auto.value:auto-value-annotations" Version="1.10.4" />
</ItemGroup>

Specification

The @Include format is {GroupId}:{ArtifactId}.

Any additional attributes such as Bind or Pack will be copied to the created @(AndroidLibrary) item.

Bind and Pack default to true and control the binding process as specified for @(AndroidLibrary).

Repository controls which Maven to download the artifact from. It defaults to Central for Maven Central but also supports Google for Google's Maven. Additionally, it can be given a URL to download from a custom Maven. (Note that Maven authentication is not supported.)

Examples:

<ItemGroup>
  <AndroidMavenLibrary 
    Include="androidx.core:core" 
    Version="1.9.0" 
    Repository="Google"
    Bind="false" />
</ItemGroup>

<ItemGroup>
  <AndroidMavenLibrary 
    Include="com.github.chrisbanes:PhotoView" 
    Version="2.3.0" 
    Repository="https://repository.mulesoft.org/nexus/content/repositories/public"
    Pack="false" />
</ItemGroup>

Note

There are 2 parts to #4528:

Downloading artifacts from Maven
Ensuring all dependencies specified in the POM file are met

This PR only addresses (1). (2) will be addressed in a future PR.

build-tools/installers/create-installers.targets

jonpryor · 2023-10-16T19:34:34Z

src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Bindings.Maven.targets

+
+  <PropertyGroup>
+    <!-- Maven cache directory, locations copied from XBD which should be sufficiently battle-tested -->
+    <MavenCacheDirectory Condition="'$(OS)'=='Unix' and '$(MavenCacheDirectory)'==''">$(HOME)\Library\Caches\MavenCacheDirectory\</MavenCacheDirectory>


Is this specified/controlled by MavenNet, or is this something we control? If it's something we control, should we instead use Library/Caches/dotnet-android?

It reminds me of %GRADLE_USER_HOME%.

I moved this to a Dev Drive to a location like D:\.gradle.

This is a location of our choosing. I feel like it should denote in some way that this contains Maven artifacts. We could go deeper if we also want to denote it comes from dotnet-android, like:

Library/Caches/dotnet-android/MavenCacheDirectory

Another data point to consider is what NuGet uses:

Windows: %userprofile%\.nuget\packages

Mac/Linux: ~/.nuget/packages

Too deep folder structure might cause issues on Windows. Do not forget that we need unpacking too.

Not sure how to make this work in an MSBuild context, but I think that a subdirectory of Environment.SpecialFolder. InternetCache might be best:

The directory that serves as a common repository for temporary Internet files.

For macOS this is $HOME/Llibrary/Caches.

For Windows this is Interop.Shell32.KnownFolders.InternetCache which is CSIDL_INTERNET_CACHE which is:

file system directory that serves as a common repository for temporary Internet files. A typical path is C:\Documents and Settings\username\Local Settings\Temporary Internet Files.

(which just reads like Vista, and presumably has changed since Windows 7…)

Looks like you should be able to use MSBuild property functions, a'la:

<MavenCacheDirectory Condition=" '$(MavenCacheDirectory)' == '' " >$([System.IO.Path]::Combine($([System.Environment]::GetFolderPath(SpecialFolder.InternetCache)), "dotnet-android", "Maven"))</MavenCacheDirectory>

It looks like on Windows 11, CSIDL_INTERNET_CACHE is:

C:\Users\jopobst\AppData\Local\Microsoft\Windows\INetCache

I don't like that this is hidden underneath Microsoft\Windows. If I'm looking for a dotnet-android Maven cache directory I wouldn't be checking there.

It also appears to be largely unused, my folder has a few directories totaling less than 1MB.

Sounds like what we want is:

Mac/Linux: ($HOME)/Library/Caches/dotnet-android/MavenCacheDirectory/
Windows: $(LocalAppData)\dotnet-android\MavenCacheDirectory\

I will update to these values.

@jpobst wrote:

Sounds like what we want is:

Mac/Linux: ($HOME)/Library/Caches/dotnet-android/MavenCacheDirectory/

Not quite; $(HOME)/Library is a macOS-ism, not a Unix-ism.

The closest Linux-ism would likely be $(XDG_CACHE_HOME), which defaults to $(HOME)/.cache.

@grendello: what is System.Environment.GetFolderPath(System.Environment.SpecialFolder.InternetCache)) on Linux? Looks like it should be string.Empty, which isn't helpful.

@jpobst: perhaps instead of using $(OS) we need to use the MSBuild OS property functions, using OperatingSystem.IsOsPlatform(string), with Windows first, macOS second, and "Unix" as the fallback?

build-tools/installers/create-installers.targets

src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Bindings.Maven.targets

src/Xamarin.Android.Build.Tasks/Tasks/MavenDownloadTask.cs

jonathanpeppers · 2023-10-16T19:43:41Z

src/Xamarin.Android.Build.Tasks/Utilities/MavenExtensions.cs

+	static async Task<string?> TryDownloadPayload (Artifact artifact, string filename)
+	{
+		try {
+			using var src = await artifact.OpenLibraryFile (artifact.Versions.First (), Path.GetExtension (filename));


Another place MavenNet might need overload for CancellationToken?

It looks like this is basically a call to HttpClient.OpenStreamAsync, which does not have a cancelable version in netstandard2.0.

One mitigating factor is this only opens the connection, it does not download the file. So likely the only long operation here would be an HTTP connect timeout.

src/Xamarin.Android.Build.Tasks/Utilities/MavenExtensions.cs

jonathanpeppers

This looks pretty good. My remaining comments seem pretty minor. 👍

jonathanpeppers · 2023-10-30T19:29:14Z

Documentation/guides/AndroidMavenLibrary.md

+```xml
+<!-- Include format is {GroupId}:{ArtifactId} -->
+<ItemGroup>
+  <AndroidMavenLibrary Include="com.squareup.okhttp3:okhttp" Version="4.9.3" />
+</ItemGroup>


Can we show an example of how you'd copy paste from here:

https://mvnrepository.com/artifact/com.squareup.okhttp3/okhttp/4.9.3

And then turn it into our XML/MSBuild version?

https://mvnrepository.com/artifact/com.squareup.okhttp3/okhttp/4.9.3

mvnrepository is agregate repo and most packages point to MavenCentral repo.

Pobst used proper maven name for artifactId which is groupId:artifactId

https://maven.apache.org/guides/introduction/introduction-to-the-pom.html#minimal-pom

It would be nice to have FQ Maven Id with version too: groupId:artifactId:version then MsBuild Version attribute is not needed.

The rationale for a separate Version attribute is to mirror <PackageReference> and to allow MSBuild central versioning strategies to work, like:

<AndroidMavenLibrary Include="com.squareup.okhttp3:okhttp" Version="$(OKHTTP3_VERSION)" />

or

<AndroidMavenLibrary Update="com.squareup.okhttp3:okhttp" Version="5.0.0" />

I suppose we could allow either format, but let's start with this one and see if there is any demand for alternative formats.

src/Xamarin.Android.Build.Tasks/Utilities/MavenExtensions.cs

jonpryor · 2023-10-30T20:01:27Z

Thinking about:

Repository controls which Maven to download the artifact from. It defaults to Central for Maven Central but also supports Google for Google's Maven. Additionally, it can be given a URL to download from a custom Maven.

…and it feels like Central and Google are special because MavenNet treats them special. But why?

I'm partially inclined toward an "extensible" model that would use an @(AndroidMavenRepositoryInfo) item group to associate a name %(Identity) with a %(Url):

<ItemGroup>
  <AndroidMavenRepositoryInfo Identity="Google"  Url="https://maven.google.com/web/" />
  <AndroidMavenRepositoryInfo Identity="Central" Url="https://mvnrepository.com/repos/central" />
</ItemGroup>

which would allow adding new names without requiring a MavenNet update or xamarin-android rebuild.

Is this bananas?

Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>

jpobst · 2023-10-31T17:58:02Z

…and it feels like Central and Google are special because MavenNet treats them special. But why?

MavenNet does have custom code for Central and Google. I am not sure if these are purely optimizations or if they are actually required due to API differences. (How "standard" is the Maven protocol, particularly around package search?) Either way, they are probably beneficial to continue using.

While @(AndroidMavenRepositoryInfo) is an interesting idea, it feels like it may be overkill at this point.

The current design could be extended in this way in the future without breaking existing usages.

Code formatting

jonpryor · 2023-11-13T19:53:13Z

src/Xamarin.Android.Build.Tasks/Tasks/MavenDownloadTask.cs

+
+namespace Xamarin.Android.Tasks;
+
+public class MavenDownloadTask : AndroidAsyncTask


Naming? Not sure we need to use Task as a suffix. We usually don't.

jonpryor · 2023-11-14T18:59:08Z

Context: https://github.com/xamarin/xamarin-android/issues/4528

Add support for the `@(AndroidMavenLibrary)` item group, which will
download the requested Java artifact version from a Maven repository
and add it as an `@(AndroidLibrary)` for binding.

For example, to download the Maven package
[pkg:maven/com.google.auto.value/auto-value-annotations@1.10.4][0]
from Maven Central and create an Android binding for it:

	<ItemGroup>
	  <AndroidMavenLibrary Include="com.google.auto.value:auto-value-annotations" Version="1.10.4" />
	</ItemGroup>

~~ Specification ~~

The `//AndroidMavenLibrary/@Include` format is `{GroupId}:{ArtifactId}`.

Any additional attributes such as `%(Bind)` or `%(Pack)` will be copied
to the created `@(AndroidLibrary)` item.

`%(Bind)` and `%(Pack)` default to `true` and control the binding
process as specified for [`@(AndroidLibrary)`][1].

`%(Repository)` controls which Maven Repository to download artifacts
from.  Supported values include:

  * `Central` for [Maven Central][2].  This is the default value if
    `%(Repository)` is not specified.

  * `Google` for [Google's Maven][3].

  * A URL, for downloading from a custom Maven instance.
    *Note*: Maven authentication is *not* supported.

~~Examples:~~

	<ItemGroup>
	  <AndroidMavenLibrary 
	      Include="androidx.core:core" 
	      Version="1.9.0" 
	      Repository="Google"
	      Bind="false"
	  />
	  <AndroidMavenLibrary 
	      Include="com.github.chrisbanes:PhotoView" 
	      Version="2.3.0" 
	      Repository="https://repository.mulesoft.org/nexus/content/repositories/public"
	      Pack="false"
	  />
	</ItemGroup>

There are 2 parts to #4528:

 1. Downloading artifacts from Maven
 2. Ensuring all dependencies specified in the POM file are met

Only (1) is currently addressed. (2) will be addressed in the future.

[0]: https://repo1.maven.org/maven2/com/google/auto/value/auto-value-annotations/1.10.4/
[1]: https://github.com/xamarin/xamarin-android/blob/main/Documentation/guides/OneDotNetEmbeddedResources.md#msbuild-item-groups
[2]: https://repo1.maven.org/maven2/
[3]: https://maven.google.com/web/index.html

jpobst force-pushed the android-maven-library branch from 24219f5 to 693877e Compare October 12, 2023 20:34

jonpryor reviewed Oct 16, 2023

View reviewed changes

build-tools/installers/create-installers.targets Show resolved Hide resolved

jonpryor reviewed Oct 16, 2023

View reviewed changes

jonathanpeppers reviewed Oct 16, 2023

View reviewed changes

jpobst force-pushed the android-maven-library branch 7 times, most recently from b328c52 to a325863 Compare October 26, 2023 23:45

jonathanpeppers approved these changes Oct 30, 2023

View reviewed changes

jpobst and others added 9 commits October 31, 2023 07:48

[Xamarin.Android.Build.Tasks] Add support for @(AndroidMavenLibrary).

29e4d05

Apply suggestions from code review

0da40eb

Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>

Use CancellationToken where available.

167575d

Add tests.

62d33b9

Localization, third party notices

8a228ad

Add docs.

1cb9743

Update MavenNet.

7a8c6f8

Appease PoliCheck.

37f7159

Small review changes.

c71b234

jpobst force-pushed the android-maven-library branch from a325863 to c71b234 Compare October 31, 2023 17:50

jpobst marked this pull request as ready for review November 7, 2023 19:37

jpobst requested review from dellis1972, grendello and pjcollins as code owners November 7, 2023 19:37

pjcollins approved these changes Nov 7, 2023

View reviewed changes

Update Xamarin.Android.Bindings.Maven.targets

4b5d5df

Code formatting

jonpryor reviewed Nov 13, 2023

View reviewed changes

Review feedback.

183f67b

jpobst force-pushed the android-maven-library branch from cfc65eb to 1a3546d Compare November 14, 2023 23:26

Fix MSBuild error

2a0245b

jpobst force-pushed the android-maven-library branch from 1a3546d to 2a0245b Compare November 15, 2023 21:50

jonpryor merged commit 3659766 into main Nov 16, 2023
47 checks passed

jonpryor deleted the android-maven-library branch November 16, 2023 22:28

github-actions bot locked and limited conversation to collaborators Jan 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Xamarin.Android.Build.Tasks] Add support for @(AndroidMavenLibrary). #8420

[Xamarin.Android.Build.Tasks] Add support for @(AndroidMavenLibrary). #8420

jpobst commented Oct 12, 2023 •

edited

jonpryor Oct 16, 2023

jonathanpeppers Oct 16, 2023

jpobst Oct 17, 2023

moljac Oct 30, 2023

jonpryor Oct 30, 2023

jpobst Oct 31, 2023 •

edited

jonpryor Nov 13, 2023

jonathanpeppers Oct 16, 2023

jpobst Oct 17, 2023

jonathanpeppers left a comment

jonathanpeppers Oct 30, 2023

moljac Oct 30, 2023

jpobst Oct 31, 2023

jonpryor commented Oct 30, 2023

jpobst commented Oct 31, 2023

jonpryor Nov 13, 2023

jonpryor commented Nov 14, 2023


		namespace Xamarin.Android.Tasks;

		public class MavenDownloadTask : AndroidAsyncTask

[Xamarin.Android.Build.Tasks] Add support for @(AndroidMavenLibrary). #8420

[Xamarin.Android.Build.Tasks] Add support for @(AndroidMavenLibrary). #8420

Conversation

jpobst commented Oct 12, 2023 • edited

Description

Specification

Note

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpobst Oct 31, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonathanpeppers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonpryor commented Oct 30, 2023

jpobst commented Oct 31, 2023

Choose a reason for hiding this comment

jonpryor commented Nov 14, 2023

jpobst commented Oct 12, 2023 •

edited

jpobst Oct 31, 2023 •

edited