From e8223b019f4d64487029c12f7b977e30cff33a0c Mon Sep 17 00:00:00 2001 From: Brendan Zagaeski Date: Tue, 12 May 2020 21:21:12 -0700 Subject: [PATCH] [Xamarin.Android.Build.Tasks] Use Path.Combine() in Fixes: https://github.com/xamarin/xamarin-android/issues/4677 `` was always using `/` as the path separator for the `build-tools` and `platforms` output items: Task "CalculateProjectDependencies" Task Parameter:TargetFrameworkVersion=v10.0 Task Parameter:ManifestFile=C:\Source\xamarin-android\bin\TestDebug\temp\InstallAndroidDependenciesTest\Project\Properties\AndroidManifest.xml Task Parameter:BuildToolsVersion=29.0.2 Task Parameter:PlatformToolsVersion=29.0.5 Task Parameter:ToolsVersion=26.1.1 Task Parameter:NdkVersion=16.1 Task Parameter:NdkRequired=False Output Item(s): AndroidDependency= platforms/android-29 build-tools/29.0.2 Version=29.0.2 platform-tools Version=29.0.5 tools Version=26.1.1 Done executing task "CalculateProjectDependencies". This meant the commercial `` task wouldn't ever install the `build-tools` or `platforms` components on Windows because none of the candidate `IAndroidComponent.FileSystemPath` values that were available via the SDK installer matched the input `ITaskItem.ItemSpec` values. Fix this by changing `` to use `Path.Combine()` instead of `/`. Test changes: `InstallAndroidDependenciesTest` did not yet enforce the expected behavior for this bug because `` was automatically falling back to the default Android SDK installation. Update `InstallAndroidDependenciesTest` to enforce the expected behavior. To minimize download requirements for the test, add a mock `tools/source.properties` file to the target `android-sdk` directory during each test run. This reduces the total downloads from about 1 GB to under 100 MB, and, because the `tools` component isn't needed for the current default project configuration, `b.Build (proj)` still completes successfully. --- Documentation/release-notes/fix-4677.md | 6 ++++ .../Tasks/CalculateProjectDependencies.cs | 4 +-- .../AndroidDependenciesTests.cs | 29 +++++++++++++++++-- 3 files changed, 35 insertions(+), 4 deletions(-) create mode 100644 Documentation/release-notes/fix-4677.md diff --git a/Documentation/release-notes/fix-4677.md b/Documentation/release-notes/fix-4677.md new file mode 100644 index 00000000000..9a164bdc426 --- /dev/null +++ b/Documentation/release-notes/fix-4677.md @@ -0,0 +1,6 @@ +#### Application and library build and deployment + +- [GitHub 4677](https://github.com/xamarin/xamarin-android/issues/4677): + On Windows, the `InstallAndroidDependencies` MSBuild target would not yet + install the expected Android SDK Platform or Android SDK Build-Tools + components in a newly configured command line build environment. diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/CalculateProjectDependencies.cs b/src/Xamarin.Android.Build.Tasks/Tasks/CalculateProjectDependencies.cs index a19c4e820ab..515a6fd12b6 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/CalculateProjectDependencies.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/CalculateProjectDependencies.cs @@ -53,8 +53,8 @@ public override bool RunTask () manifestApiLevel = manifest.TargetSdkVersion ?? manifest.MinSdkVersion ?? DefaultMinSDKVersion; } var sdkVersion = Math.Max (targetApiLevel.Value, manifestApiLevel); - dependencies.Add (CreateAndroidDependency ($"platforms/android-{sdkVersion}", $"")); - dependencies.Add (CreateAndroidDependency ($"build-tools/{BuildToolsVersion}", BuildToolsVersion)); + dependencies.Add (CreateAndroidDependency (Path.Combine ("platforms", $"android-{sdkVersion}"), $"")); + dependencies.Add (CreateAndroidDependency (Path.Combine ("build-tools", BuildToolsVersion), BuildToolsVersion)); if (!string.IsNullOrEmpty (PlatformToolsVersion)) { dependencies.Add (CreateAndroidDependency ("platform-tools", PlatformToolsVersion)); } diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidDependenciesTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidDependenciesTests.cs index 73c328815d9..dfed65a2eea 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidDependenciesTests.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidDependenciesTests.cs @@ -1,5 +1,7 @@ using System; +using System.Collections.Generic; using System.IO; +using System.Linq; using NUnit.Framework; using Xamarin.ProjectTools; @@ -13,19 +15,42 @@ public class AndroidDependenciesTests : BaseTest [Test] public void InstallAndroidDependenciesTest () { + var path = Path.Combine (Root, "temp", TestName); + TestOutputDirectories [TestContext.CurrentContext.Test.ID] = path; if (!CommercialBuildAvailable) Assert.Ignore ("Not required on Open Source Builds"); var old = Environment.GetEnvironmentVariable ("ANDROID_SDK_PATH"); try { - string sdkPath = Path.Combine (Root, "temp", TestName, "android-sdk"); + string sdkPath = Path.Combine (path, "android-sdk"); + CreateFauxAndroidSdkDirectory (sdkPath, buildToolsVersion: "23.0.0"); + // Provide mock version info so the `tools` component won't be downloaded. + File.WriteAllText (Path.Combine (sdkPath, "tools", "source.properties"), "Pkg.Revision=99.99.99"); Environment.SetEnvironmentVariable ("ANDROID_SDK_PATH", sdkPath); var proj = new XamarinAndroidApplicationProject (); - using (var b = CreateApkBuilder ()) { + // Use `BuildHelper.CreateApkBuilder()` instead of `BaseTest.CreateApkBuilder()` so + // `android-sdk` can be placed beside the project directory rather than within it. + using (var b = BuildHelper.CreateApkBuilder (Path.Combine (path, "Project"))) { string defaultTarget = b.Target; b.Target = "InstallAndroidDependencies"; + b.ThrowOnBuildFailure = false; + Assert.IsFalse (b.Build (proj, parameters: new string [] { "AcceptAndroidSDKLicenses=false" }), "InstallAndroidDependencies should have failed."); + IEnumerable taskOutput = b.LastBuildOutput + .SkipWhile (x => !(x.StartsWith (" Detecting Android SDK in") && x.Contains (sdkPath))) + .TakeWhile (x => !x.StartsWith ("Done executing task \"InstallAndroidDependencies\"")) + .Where (x => x.StartsWith (" Dependency to be installed:")); + Assert.AreEqual (2, taskOutput.Count (), "Exactly two dependencies should be identified to be installed."); + StringAssertEx.Contains ("Dependency to be installed: Android SDK Platform", taskOutput); + StringAssertEx.Contains ("Dependency to be installed: Android SDK Build-Tools", taskOutput); + b.ThrowOnBuildFailure = true; Assert.IsTrue (b.Build (proj, parameters: new string [] { "AcceptAndroidSDKLicenses=true" }), "InstallAndroidDependencies should have succeeded."); + Assert.IsTrue (Directory.Exists (Path.Combine (sdkPath, "platforms")), "At least one platform should have been installed"); + Assert.IsTrue (Directory.Exists (Path.Combine (sdkPath, "build-tools")), "At least one Build Tools version should have been installed"); b.Target = defaultTarget; Assert.IsTrue (b.Build (proj), "build should have succeeded."); + taskOutput = b.LastBuildOutput + .SkipWhile (x => !x.StartsWith (" ResolveSdks Outputs:")) + .TakeWhile (x => !x.StartsWith ("Done executing task \"ResolveSdks\"")); + StringAssert.Contains (sdkPath, taskOutput.First (x => x.StartsWith (" AndroidSdkPath:"))); } } finally { Environment.SetEnvironmentVariable ("ANDROID_SDK_PATH", old);