[Xamarin.Android.Build.Tasks] Enable POM verification features. #8649

jpobst · 2024-01-18T19:14:16Z

Fixes: #4528
Context: #8420

Building on our previous work enabling <AndroidMavenLibrary>, this adds the second piece we would like to support: using POM files to ensure all required Java dependencies are fulfilled. This is a critical step that users often miss or make mistakes, resulting in non-functional bindings.

The commit contains complete documentation, so only a summary is provided here.

Adding the following <AndroidMavenLibrary> to a binding project:

<ItemGroup>
  <AndroidMavenLibrary Include="com.squareup.okhttp3:okhttp" Version="4.9.3" />
</ItemGroup>

Will produce the following errors:

error XA4242: Java dependency 'com.squareup.okio:okio:2.8.0' is not satisfied. Microsoft maintains the NuGet package 'Square.OkIO' that could fulfill this dependency.
error XA4242: Java dependency 'org.jetbrains.kotlin:kotlin-stdlib:1.4.10' is not satisfied. Microsoft maintains the NuGet package 'Xamarin.Kotlin.StdLib' that could fulfill this dependency.

This is accomplished by automatically downloading the POM file (and any needed parent/imported POM files) for the specified artifact and using them to determine all required dependencies.

There are many documented ways of fixing these errors, the best way is adding the latest versions of the suggested NuGet packages:

<ItemGroup>
  <PackageReference Include="Square.OkIO" Version="3.6.0.1" />
  <PackageReference Include="Xamarin.Kotlin.StdLib" Version="1.9.22" />
</ItemGroup>

Note this commit replaces our previous usage of MavenNet with Java.Interop.Tools.Maven. Thus it removes the MavenNet TPN.

Some future concerns:

Can we automatically determine which dependencies are met by a <ProjectReference>? Today, the user must manually add the attributes JavaArtifact and JavaVersion to the <ProjectReference> to specify this.
We use the link https://aka.ms/ms-nuget-packages to download https://raw.githubusercontent.com/jpobst/Prototype.Android.MavenBindings/main/microsoft-packages.json, which is used to provide NuGet suggestions. We need to figure out a permanent home for this file and a process for generating it. Luckily we can ship a preview for now and change the aka.ms link to point to the permanent home later.

jonathanpeppers · 2024-02-12T16:15:22Z

...ld.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Bindings.JavaDependencyVerification.targets

+  <UsingTask TaskName="Xamarin.Android.Tasks.JavaDependencyVerification" AssemblyFile="Xamarin.Android.Build.Tasks.dll" />
+
+  <Target Name="_VerifyJavaDependencies"
+      Condition=" '@(AndroidLibrary->Count())' != '0' "


Even though we "strongly suggest" people use the new item name, there are some using the old one.

Can this check $(_AndroidGenerateManagedBindings) instead?

https://github.com/xamarin/xamarin-android/blob/b812506b9c9dab88a5bc48f561efc7290309a401/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Bindings.Core.targets#L47-L53

As written, none of these new features will function on the "legacy" item names.

For example:

 <AndroidLibrary Include="mylib.jar" Pack="false" JavaArtifact="com.example.mylib" JavaVersion="1.0.0" />  <InputJar Include="mylib.jar" JavaArtifact="com.example.mylib" JavaVersion="1.0.0" />

Running it on a project that does not have <AndroidLibrary> will waste time.

Maybe we should take a stronger stance and add a deprecation warning for the legacy items in .NET 9 and remove them in .NET 10?

We can begin taking a stance and start warning on usage of legacy item names. That doesn't require that we remove support for them entirely, but it's required before we remove support for them entirely…

jonathanpeppers · 2024-02-12T16:17:14Z

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

+			try {
+				var http = new HttpClient ();
+				var json = await http.GetStringAsync ("https://aka.ms/ms-nuget-packages");
+				var outfile = Path.Combine (MavenCacheDirectory, $"microsoft-packages-{DateTime.Today:yyyyMMdd}.json");


Should we actually put this file in obj, so if users do a Clean it will download again?

It feels like this file is similar to a NuGet artifact and should be shared and cached across projects. If you are working without internet, it would also be nice if you got to keep using one you previously downloaded.

This logic will attempt to download a new one each day, but will keep the previous one if it can't get a new one.

I don't think this file will change extremely frequently as it does not include version numbers. It will only change when we bind a new AndroidX/GPS library.

I'm concerned about this "per-day" file:

The filesystem is a machine-global, barely concurrent-friendly, "database" with which it's easy to cause "spooky action at a distance".

MavenCacheDirectory is $(MavenCacheDirectory), which is e.g. $(LocalAppData)\dotnet-android\MavenCacheDirectory\.

What happens if, through some terrible confluence of events, two projects are built at the same time, on the same machine, for the first time "today"? Depending on timing, they may each see that today's microsoft-packages-*.json doesn't exist, each attempt to create it, and one of them will "lose", possibly causing some variation on our favorite "file share exception" or whatever.

This feels like a potential source of build un-reliability.

This also feels like an impediment to future changes: we can't change the file format. If we have both .NET 9 and .NET 10 installed on a given machine, and these files are stored "machine global", then there is an implicit requirement that .NET 9 be able to consume .NET 10 output, i.e. the file format either can't change, or the file format needs to be "extensible".

I'd feel much "safer" if this file were stored in $(IntermediateOutputPath). Lower risk of file share -related errors due to concurrent project builds, no chance of file format-related mismatches.

What happens if, through some terrible confluence of events, two projects are built at the same time, on the same machine, for the first time "today"? Depending on timing, they may each see that today's microsoft-packages-*.json doesn't exist, each attempt to create it, and one of them will "lose", possibly causing some variation on our favorite "file share exception" or whatever.

If this scenario were to happen, the "losing" project would log a build message saying it couldn't download the file, and any dependency verification errors would not provide NuGet recommendations. On subsequent builds the NuGet recommendations would be available.

This file is considered "optional" and simply enhances the error messages. However failure is a possible outcome (like if the user doesn't have internet) that is handled and still provides the core experience.

i.e. the file format either can't change, or the file format needs to be "extensible"

The file format is JSON, making it easily extensible if desired in the future. If it needed to be extended somehow that JSON couldn't handle we would likely just rename the new version to something like microsoft-packages-v2.json.

I'd feel much "safer" if this file were stored in $(IntermediateOutputPath). Lower risk of file share -related errors due to concurrent project builds, no chance of file format-related mismatches.

I really like that we're only downloading this file once per day no matter how many projects one is using, and that any project can use the machine-wide one if internet isn't available. But we can change it if we still have strong concerns.

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

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

jonpryor · 2024-02-14T21:53:01Z

I think that "somewhere" we need a "rewrite"/"update" of the Binding a Java Library docs, and a new Documentation/guides/binding-bytecode or something which describes how binding Android "stuff" will work in an @(AndroidMavenLibrary) world order.

(Also, Documentation/guides/building-apps/build-items.md needs to be updated with the new Build actions introduced here.)

What currently exists in this PR "jumps in" without providing broader context. For example:

The binding process ignores API that requires missing dependencies, so this can result in large portions of desired API not being bound.

I believe that the way to "know" this is happening is via BINDINGSGENERATOR and BG* warnings, but this won't be obvious to newcomers.

Note: the preferred way of interacting with this system is to use which will automatically download and include any needed POM files.

Does this mean that NuGet packages will contain .pom files? If so, what is the structure of the NuGet package and where will they reside?

error : Java dependency 'androidx.collection:collection' version '1.0.0' is not satisfied.

A more actionable error message would be useful. Could a "suggested" <AndroidMavenLibrary/> be part of the error message? NuGet package lookup (which admittedly would mean network communications to nuget.org to even check if it exists, but could be useful.)

jpobst · 2024-02-15T21:05:13Z

I think that "somewhere" we need a "rewrite"/"update" of the Binding a Java Library docs, and a new Documentation/guides/binding-bytecode or something which describes how binding Android "stuff" will work in an @(AndroidMavenLibrary) world order.

Agreed, it's on my list for the .NET 9 timeframe. However we still don't have a place for our dotnet documentation to go so I have it on hold until that exists.

Note: the preferred way of interacting with this system is to use which will automatically download and include any needed POM files.

Removed "and include" as that could be interpreted that the .pom might be included in the output. They are simply "included" in the build to enable verification and will not be included in any output.

A more actionable error message would be useful. Could a "suggested" be part of the error message? NuGet package lookup (which admittedly would mean network communications to nuget.org to even check if it exists, but could be useful.)

This is not the best example, as we do maintain a binding for androidx.collection:collection, so this message would actually suggest the NuGet we maintain:

error XA4242: Java dependency 'androidx.collection:collection:1.0.0' is not satisfied. Microsoft maintains the NuGet package 'Xamarin.AndroidX.Collection' that could fulfill this dependency.

In cases where Microsoft does not maintain a binding for the library I'm not sure we can provide an accurate suggestion, as there are many ways to fulfill the dependency and we do not know the best way for the user's scenario. We could add an aka.ms link to the error message pointing to the guide to resolving the issue:

https://github.com/xamarin/xamarin-android/blob/cd6fe2fb78a022f612e463d4714a085600d70d25/Documentation/guides/ResolvingJavaDependencies.md

Context: dotnet/android#8649 Add a new `Java.Interop.Tools.Maven.dll` assembly that contains functionality for: - Retrieving artifacts from Maven - Parsing and understanding POM files This assembly is used by dotnet/android#8649 and replaces our previous usage of [`MavenNet.dll`][0]. `Java.Interop.Tools.Maven.dll` contains a binding of [`maven-4.0.0.xsd`][1], generated via [dotnet-xscgen][2]. To update the binding: # Install the binding utility % dotnet tool install -g dotnet-xscgen # Grab the version of `maven*.xsd` you want to bind/update to % curl -o maven-4.0.0.xsd https://maven.apache.org/xsd/maven-4.0.0.xsd # `dotnet tool install` installs into e.g. `$HOME/.dotnet/tools`, which might not be in `$PATH`… # Run the `UpdateProjectSchema` target % PATH=$HOME/.dotnet/tools:$PATH dotnet build -t:UpdateProjectSchema \ src/Java.Interop.Tools.Maven/Java.Interop.Tools.Maven.csproj \ -p:MavenXsd=`pwd`/maven-4.0.0.xsd [0]: https://www.nuget.org/packages/MavenNet [1]: https://maven.apache.org/xsd/maven-4.0.0.xsd [2]: https://www.nuget.org/packages/dotnet-xscgen

Documentation/guides/building-apps/build-items.md

jonpryor · 2024-03-19T19:22:47Z

Documentation/guides/messages/xa4239.md

+
+The specified Maven repository is invalid.
+
+For example the following Maven repository must be specified with `https://`:


This requirement should be part of the %(AndroidMavenLibrary.Repository) documentation. :-)

Additionally, what other constraints/requirements/etc. are there? Can a "full URL prefix" be used a'la https://example.com/my/maven/library/path?

I'm not exactly sure what a "full URL prefix" is. The URL should point to a Maven repository, like https://dl.google.com/android/maven2/ or https://repo1.maven.org/maven2/.

Fix formatting.

jonpryor · 2024-03-20T19:03:57Z

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

+		if (suggestion is string nuget)
+			log.LogCodedError ("XA4242", Properties.Resources.XA4242, artifact_spec, nuget);
+		else
+			log.LogCodedError ("XA4242", Properties.Resources.XA4241, artifact_spec);


I think this should be "XA4241", not "XA4242", based on the Properties.Resources.XA4241 property name.

jonpryor · 2024-03-20T19:38:12Z

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

+		if (!all_files.Any (x => x.IsToday)) {
+			// No file for today, download a new one
+			try {
+				using var http = new HttpClient ();


HttpClient instances should be in static fields; see also:

[Java.Interop.Tools.Maven] Initial commit. java-interop#1179 (comment)

https://learn.microsoft.com/en-us/dotnet/fundamentals/networking/http/httpclient-guidelines#recommended-use

https://learn.microsoft.com/en-us/dotnet/fundamentals/runtime-libraries/system-net-http-httpclient#instancing

* main: [ci] Use managed identity for ApiScan (#8823) [Xamarin.Android.Build.Tasks] DTBs should not rm generator output (#8706) [Xamarin.Android.Build.Tasks] Bump to NuGet 6.7.1 (#8833) $(AndroidPackVersionSuffix)=preview.4; net9 is 34.99.0.preview.4 (#8831) Localized file check-in by OneLocBuild Task (#8824) [Xamarin.Android.Build.Tasks] Enable POM verification features. (#8649) [runtime] Optionally disable inlining (#8798) Fix assembly count when satellite assemblies are present (#8790) [One .NET] new "greenfield" projects are trimmed by default (#8805) Localized file check-in by OneLocBuild Task (#8819) LEGO: Merge pull request 8820

* main: [ci] Use managed identity for ApiScan (#8823) [Xamarin.Android.Build.Tasks] DTBs should not rm generator output (#8706) [Xamarin.Android.Build.Tasks] Bump to NuGet 6.7.1 (#8833) $(AndroidPackVersionSuffix)=preview.4; net9 is 34.99.0.preview.4 (#8831) Localized file check-in by OneLocBuild Task (#8824) [Xamarin.Android.Build.Tasks] Enable POM verification features. (#8649) [runtime] Optionally disable inlining (#8798) Fix assembly count when satellite assemblies are present (#8790) [One .NET] new "greenfield" projects are trimmed by default (#8805) Localized file check-in by OneLocBuild Task (#8819) LEGO: Merge pull request 8820 LEGO: Merge pull request 8818 Bump to dotnet/installer@b40c44502d 9.0.100-preview.3.24165.20 (#8817) Bump com.android.tools:r8 from 8.2.47 to 8.3.37 (#8816) [Mono.Android] Prevent NullPointerException in TranslateStackTrace (#8795) Localized file check-in by OneLocBuild Task (#8815) [Xamarin.Android.Build.Tasks] Make all assemblies RID-specific (#8478) Localized file check-in by OneLocBuild Task (#8813)

* main: Bump to dotnet/installer@dc43d363d2 9.0.100-preview.4.24175.5 (#8828) [Xamarin.Android.Build.Tasks] Update to newer ILRepack which supports debug files. (#8839) Bump 'NuGet.*' and 'Newtonsoft.Json' NuGet versions. (#8825) Localized file check-in by OneLocBuild Task (#8844) [LayoutBindings] Fix '[Preserve]' is obsolete warnings (#8529) LEGO: Merge pull request 8837 [ci] Use managed identity for ApiScan (#8823) [Xamarin.Android.Build.Tasks] DTBs should not rm generator output (#8706) [Xamarin.Android.Build.Tasks] Bump to NuGet 6.7.1 (#8833) $(AndroidPackVersionSuffix)=preview.4; net9 is 34.99.0.preview.4 (#8831) Localized file check-in by OneLocBuild Task (#8824) [Xamarin.Android.Build.Tasks] Enable POM verification features. (#8649) [runtime] Optionally disable inlining (#8798) Fix assembly count when satellite assemblies are present (#8790) [One .NET] new "greenfield" projects are trimmed by default (#8805) Localized file check-in by OneLocBuild Task (#8819) LEGO: Merge pull request 8820

jpobst force-pushed the java-dependency-verification branch 6 times, most recently from 29098c7 to 49bf4a5 Compare January 25, 2024 20:31

jpobst force-pushed the java-dependency-verification branch 3 times, most recently from 651997c to 1659801 Compare February 8, 2024 18:36

jpobst mentioned this pull request Feb 8, 2024

[Java.Interop.Tools.Maven] Initial commit. dotnet/java-interop#1179

Merged

jpobst force-pushed the java-dependency-verification branch 3 times, most recently from e4881ef to cfceb52 Compare February 9, 2024 21:41

jpobst marked this pull request as ready for review February 10, 2024 01:22

jpobst requested review from dellis1972, jonathanpeppers, grendello and jonpryor as code owners February 10, 2024 01:22

jpobst marked this pull request as draft February 10, 2024 01:24

jonathanpeppers reviewed Feb 12, 2024

View reviewed changes

jpobst force-pushed the java-dependency-verification branch from b73b3f3 to 0026d5f Compare February 15, 2024 20:38

jpobst force-pushed the java-dependency-verification branch 4 times, most recently from 9797f91 to 516fd07 Compare March 13, 2024 18:13

jpobst marked this pull request as ready for review March 13, 2024 20:56

jonpryor reviewed Mar 19, 2024

View reviewed changes

Documentation/guides/building-apps/build-items.md Show resolved Hide resolved

jonpryor reviewed Mar 19, 2024

View reviewed changes

Documentation/guides/building-apps/build-items.md Show resolved Hide resolved

jonpryor reviewed Mar 19, 2024

View reviewed changes

jpobst force-pushed the java-dependency-verification branch from 1313ab5 to b5729ff Compare March 19, 2024 23:02

jpobst added 8 commits March 19, 2024 13:44

[Xamarin.Android.Build.Tasks] Enable POM verification features.

3df037d

Flush.

4605751

Flush.

655d753

Begin error documentation.

e5dc999

Error documentation.

3d28c09

Address review feedback.

0dd3021

Add more documentation.

b3efd14

Track Java.Interop.Tools.Maven changes.

ccd9113

jpobst force-pushed the java-dependency-verification branch 2 times, most recently from 2b7e197 to d1d938b Compare March 19, 2024 23:58

Address docs review feedback.

6996ccc

jpobst force-pushed the java-dependency-verification branch from d1d938b to 6996ccc Compare March 20, 2024 00:01

Update ResolvingJavaDependencies.md

f86d3e7

Fix formatting.

jonpryor reviewed Mar 20, 2024

View reviewed changes

jpobst and others added 2 commits March 20, 2024 11:02

Review feedback.

e132070

Fix XML formatting.

45c6feb

jonpryor merged commit 4696bf3 into main Mar 22, 2024
2 checks passed

jonpryor deleted the java-dependency-verification branch March 22, 2024 19:09

jpobst mentioned this pull request Apr 17, 2024

Binding: Vision #5352

Closed

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Xamarin.Android.Build.Tasks] Enable POM verification features. #8649

[Xamarin.Android.Build.Tasks] Enable POM verification features. #8649

jpobst commented Jan 18, 2024 •

edited

Loading

jonathanpeppers Feb 12, 2024

jpobst Feb 15, 2024

jonpryor Mar 20, 2024

jonathanpeppers Feb 12, 2024

jpobst Feb 15, 2024

jonpryor Mar 20, 2024

jpobst Mar 20, 2024 •

edited

Loading

jonpryor commented Feb 14, 2024

jpobst commented Feb 15, 2024 •

edited

Loading

jonpryor Mar 19, 2024

jpobst Mar 20, 2024

jonpryor Mar 20, 2024

jonpryor Mar 20, 2024


		The specified Maven repository is invalid.

		For example the following Maven repository must be specified with `https://`:

[Xamarin.Android.Build.Tasks] Enable POM verification features. #8649

[Xamarin.Android.Build.Tasks] Enable POM verification features. #8649

Conversation

jpobst commented Jan 18, 2024 • edited Loading

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

Choose a reason for hiding this comment

jpobst Mar 20, 2024 • edited Loading

Choose a reason for hiding this comment

jonpryor commented Feb 14, 2024

jpobst commented Feb 15, 2024 • edited Loading

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 commented Jan 18, 2024 •

edited

Loading

jpobst Mar 20, 2024 •

edited

Loading

jpobst commented Feb 15, 2024 •

edited

Loading