[Xamarin.Android.Build.Tasks] Rethink default property values

[jonpryor]

Context: 5d1ac0a
Context: dotnet/android-tools@a6a23bb
Context: https://learn.microsoft.com/en-us/visualstudio/mac/what-happened-to-vs-for-mac?view=vsmac-2022

Commit 5d1ac0a had a brilliant idea:

Of particular note is dotnet/android-tools@a6a23bb:
[ed note: now dotnet/android-tools@a6a23bb]

In order to ensure that both Xamarin.Android and the macOS installer
use the same default version numbers for Android SDK components, we are
now using Xamarin.Android.Tools.Versions.props to contain default
version number information. This file is maintained in the
xamarin/xamarin-android-tools repo and will be imported via
external/xamarin-android-tools:

In the intervening four years (!), we've learned something, and the environment has changed.

What we've learned is that this approach is "bump-heavy": in order to update the default value of e.g. $(AndroidSdkPlatformVersion), we'd need to update:

1. dotnet/android-tools
2. xamarin/androidtools
3. Plus other repos that depend on (1) or (2) that are used by (4)
4. xamarin/monodroid
5. dotnet/android

This is, to put it mildly, cumbersome.

Additionally, it's no longer necessary: Visual Studio for Mac will be retired on 2024-Aug-31, so there is no longer a need to have a common file shared between the .NET for Android SDK and Visual Studio for Mac.

Instead of considering Xamarin.Android.Tools.Versions.props as the canonical source of truth, consider it as the fallback source:

1. Update Xamarin.Android.Common.targets to now optionally include a new Xamarin.Installer.Common.props file, if present. This will allow e.g. xamarin/android-sdk-installer to eventually provide default property values which are tied to the in-use "Xamarin Manifest" for package installation, e.g. $(AndroidCommandLineToolsVersion), $(JavaSdkVersion).

2. Update Xamarin.Android.Common.props.in so that the following properties are set before importing Xamarin.Android.Tools.Versions.props:

   • $(AndroidSdkBuildToolsVersion)
   • $(AndroidSdkPlatformVersion)
   • $(AndroidSdkPlatformToolsVersion)

   This will allow us to update these values when this repo ads support for these versions.

   TODO: Should these actually be located in (1)? Regardless, as Xamarin.Installer.Common.props is imported first, it can always override these values.

[jonathanpeppers]

There are a few test failures like GetDependencyWhenSDKIsMissingTest(True,True):

build-tools/@SDK_BUILD_TOOLS_VERSION@ should be a dependency.

Maybe the code that produces XABuildConfig.cs, also needs an update?

[jonpryor]

@jonathanpeppers wrote:

Maybe the code that produces XABuildConfig.cs, also needs an update?

Close, but not quite: the problem is GetExpectedBuildToolsVersion():

android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidDependenciesTests.cs

Lines 218 to 227 in c157aed

	static string GetExpectedBuildToolsVersion ()
	{
	var propsPath = Path.Combine (XABuildPaths.TopDirectory, "src", "Xamarin.Android.Build.Tasks", "Xamarin.Android.Common.props.in");
	var props = XElement.Load (propsPath);
	var AndroidSdkBuildToolsVersion = props.Elements (MSBuildXmlns + "PropertyGroup")
	.Elements (MSBuildXmlns + "AndroidSdkBuildToolsVersion")
	.FirstOrDefault ();
	return AndroidSdkBuildToolsVersion?.Value?.Trim ();
	}
	}

GetExpectedBuildToolsVersion() parses Xamarin.Android.Common.props.in to find $(AndroidSdkBuildToolsVersion), and with this commit $(AndroidSdkBuildToolsVersion) within Xamarin.Android.Common.props.in is @SDK_BUILD_TOOLS_VERSION@.

What confuses me is, how did this ever work? Before this commit, Xamarin.Android.Common.props.in did not contain $(AndroidSdkBuildToolsVersion); that's instead within Xamarin.Android.Tools.Versions.props. How does GetExpectedBuildToolsVersion() return a non-empty string value?

…or it does return the empty string, which Just Happens™ to pass the unit tests, as build-tools/{empty-string} will always exist…

[pjcollins]

I think part of this goal would be to eventually remove Xamarin.Android.Tools.Versions.props from dotnet/android-tools? I think it would make sense to remove this import now, and update the properties above to also include the other properties in that file:

    <AndroidCommandLineToolsVersion>
    <AndroidSdkEmulatorVersion>
    <AndroidNdkVersion>
    <JavaSdkVersion>

We could base these off other values already defined in Configuration.props as well, to continue to improve our own build and test coverage against the versions of these tools that we're recommending:

android/Configuration.props

Line 162 in 64bb147

<CommandLineToolsFolder Condition=" '$(CommandLineToolsFolder)' == '' ">11.0</CommandLineToolsFolder>

android/Configuration.props

Line 166 in 64bb147

<EmulatorVersion Condition=" '$(EmulatorVersion)' == '' ">9364964</EmulatorVersion>

NDK and JDK versions may be a bit more tricky as we'd potentially need to introduce new props for these and also pass them to xaprepare:

android/build-tools/xaprepare/xaprepare/ConfigAndData/BuildAndroidPlatforms.cs

Line 9 in 05bc90a

public const string AndroidNdkPkgRevision = "27.0.12077973";

android/build-tools/xaprepare/xaprepare/ConfigAndData/Configurables.cs

Line 21 in 553e927

const string MicrosoftOpenJDK17Release = "17.0.11.9";

[jonpryor]

/azp run

[pjcollins]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jonpryor]

PR build fails with

      "D:\a\_work\1\a\TestRelease\08-14_21.52.23\temp\InstallAndroidDependenciesTestGoogleV2\UnnamedProject.csproj" (InstallAndroidDependencies target) (1:7) ->
       (InstallAndroidDependencies target) -> 
         D:\a\_work\1\s\bin\Release\dotnet\packs\Microsoft.Android.Sdk.Windows\35.0.0-ci.pr.gh9155.59\tools\Xamarin.Installer.Common.targets(19,3): error : Unable to find a Java SDK with version `17.0.11.9`. Please set `$(JavaSdkVersion)` to a valid version. [D:\a\_work\1\a\TestRelease\08-14_21.52.23\temp\InstallAndroidDependenciesTestGoogleV2\UnnamedProject.csproj]

Looks like we'll need to downgrade our JDK version to what android-sdk-installer supports.