Skip to content

Commit 342bc83

Browse files
committed
[build] Default to using NDK r23
Context: 70272db Context: 38b1236 Context: https://github.com/actions/virtual-environments/blob/9cf1ebd754807fba137c8ce9fa2140311609fdd6/images/win/Windows2022-Readme.md Context: actions/runner-images@2950cbf Changes: https://github.com/xamarin/xamarin-android-tools/compare/20f611202bef0fc7c1659366dd38865eb119dde5...ec346d07cf3ac7fc74d08eae4f19043b51485724 * dotnet/android-tools@ec346d0: [Xamarin.Android.Tools.AndroidSdk] Permit NDK r24 (dotnet#171) * dotnet/android-tools@47832f1: [Xamarin.Android.Tools.AndroidSdk] AndroidSdkInfo validation locator (dotnet#170) We've had an "inadvertent thinko" for the past couple of months: in commit 70272db we updated the default `$(AndroidNdkDirectory)` value to be `$(ANDROID_NDK_LATEST_HOME)`, if set. *At the time*, we think this was NDK r23. In commit 38b1236, we updated `xaprepare` to install NDK r24. The "thinko" is that commit 38b1236 was *irrelevant*, as commit 70272db was "in control". The Windows **Prepare Solution** log would contain e.g.: Checking if android-ndk-r24-windows exists in C:\Android\android-sdk\ndk\23.2.8568313 Component 'android-ndk-r24-windows' requires Pkg.Revision to be '24.0.8215888', verifying installed It also *mis-detected* NDK r23 as r24 (?!). It "worked" only by coincidence. Around 2022-Jun-09, GitHub actions updated the default NDK installed on their Windows images; previously, `$(ANDROID_NDK_LATEST_HOME)` was NDK r23 or earlier (we're not sure); *now*, it is NDK r24. This subtle change -- invisible! The build environment changed! -- *introduced* unit test failures, of two patterns. Pattern 1 is that `readelf` could not be found: System.ComponentModel.Win32Exception : An error occurred trying to start process 'C:\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\bin\readelf' with working directory 'C:\a_work\1\s\xamarin-android\bin\TestRelease\net6.0'. The system cannot find the file specified. at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo) at Xamarin.Android.Build.Tests.EnvironmentHelper.RunCommand(String executablePath, String arguments) in C:\a\_work\1\s\src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Utilities\EnvironmentHelper.cs:line 550 at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertSharedLibraryHasRequiredSymbols(String dsoPath, String readElfPath) in C:\a\_work\1\s\src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Utilities\EnvironmentHelper.cs:line 495 at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertValidEnvironmentSharedLibrary(String outputDirectoryRoot, String sdkDirectory, String ndkDirectory, String supportedAbis) in C:\a\_work\1\s\src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Utilities\EnvironmentHelper.cs:line 489 -or- System.ComponentModel.Win32Exception : The system cannot find the file specified at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo) at Xamarin.Android.Build.Tests.EnvironmentHelper.RunCommand(String executablePath, String arguments) in /Users/builder/azdo/_work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs:line 551 at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertSharedLibraryHasRequiredSymbols(String dsoPath, String readElfPath) in /Users/builder/azdo/_work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs:line 495 at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertValidEnvironmentSharedLibrary(String outputDirectoryRoot, String sdkDirectory, String ndkDirectory, String supportedAbis) in /Users/builder/azdo/_work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs:line 489 This happened because `readelf` was *removed* in NDK r24. Fix this pattern updating `EnvironmentHelper.AssertValidEnvironmentSharedLibrary()` to also look for `llvm-readelf`, the replacement for `readelf` in NDK r24+. Pattern 2 is that the `aarch64-linux-android28-clang.cmd` & related scripts on Windows don't support being run from a directory containing spaces: CC="C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\bin\armv7a-linux-androideabi19-clang.CMD" AS="C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Xamarin\Android\binutils\bin\arm-linux-androideabi-as.CMD" … [CC] "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\bin\armv7a-linux-androideabi19-clang.CMD" ^ -c -D__ANDROID_API__=19 -DANDROID -o obj\Release\bundles\armeabi-v7a\temp.o ^ -I "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\sysroot\usr\include\arm-linux-androideabi" ^ -I "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\sysroot\usr\include" ^ obj\Release\bundles\armeabi-v7a\temp.c [cc stderr] 'C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK' is not recognized as an internal or external command, [cc stderr] operable program or batch file. Fix this pattern by updating `NdkToolsWithClangNoBinutils` to override the C and C++ compiler toolnames, a'la: 71ae556.
1 parent 8070679 commit 342bc83

File tree

4 files changed

+63
-10
lines changed

4 files changed

+63
-10
lines changed

external/xamarin-android-tools

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,11 @@ public static void AssertValidEnvironmentSharedLibrary (string outputDirectoryRo
486486
Assert.IsTrue (File.Exists (envSharedLibrary), $"Application environment SharedLibrary '{envSharedLibrary}' must exist");
487487

488488
// API level doesn't matter in this case
489-
AssertSharedLibraryHasRequiredSymbols (envSharedLibrary, ndk.GetToolPath ("readelf", arch, 0));
489+
var readelf = ndk.GetToolPath ("readelf", arch, 0);
490+
if (!File.Exists (readelf)) {
491+
readelf = ndk.GetToolPath ("llvm-readelf", arch, 0);
492+
}
493+
AssertSharedLibraryHasRequiredSymbols (envSharedLibrary, readelf);
490494
}
491495
}
492496

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Android/AndroidSdkResolver.cs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,6 @@ static string GetPathFromRegistry (string valueName)
2222
public static string GetAndroidSdkPath ()
2323
{
2424
var sdkPath = Environment.GetEnvironmentVariable ("TEST_ANDROID_SDK_PATH");
25-
if (String.IsNullOrEmpty (sdkPath))
26-
sdkPath = Environment.GetEnvironmentVariable ("ANDROID_SDK_PATH");
27-
if (String.IsNullOrEmpty (sdkPath))
28-
sdkPath = GetPathFromRegistry ("AndroidSdkDirectory");
2925
if (String.IsNullOrEmpty (sdkPath))
3026
sdkPath = Environment.GetEnvironmentVariable ("ANDROID_SDK_ROOT");
3127
if (String.IsNullOrEmpty (sdkPath))
@@ -38,10 +34,6 @@ public static string GetAndroidSdkPath ()
3834
public static string GetAndroidNdkPath ()
3935
{
4036
var ndkPath = Environment.GetEnvironmentVariable ("TEST_ANDROID_NDK_PATH");
41-
if (String.IsNullOrEmpty (ndkPath))
42-
ndkPath = Environment.GetEnvironmentVariable ("ANDROID_NDK_PATH");
43-
if (String.IsNullOrEmpty (ndkPath))
44-
ndkPath = GetPathFromRegistry ("AndroidNdkDirectory");
4537
if (String.IsNullOrEmpty (ndkPath))
4638
ndkPath = Environment.GetEnvironmentVariable ("ANDROID_NDK_LATEST_HOME");
4739
if (String.IsNullOrEmpty (ndkPath))

src/Xamarin.Android.Build.Tasks/Utilities/NdkTools/WithClangNoBinutils.cs

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.IO;
33
using System.Collections.Generic;
4+
using System.Text;
45

56
using Microsoft.Build.Utilities;
67
using Xamarin.Android.Tools;
@@ -14,6 +15,41 @@ public NdkToolsWithClangNoBinutils (string androidNdkPath, NdkVersion version, T
1415
{
1516
NdkToolNames[NdkToolKind.Linker] = "ld";
1617
NoBinutils = true;
18+
19+
NeedClangWorkaround = IsWindows;
20+
if (NeedClangWorkaround) {
21+
//
22+
// NDK r23 bug:
23+
//
24+
// The llvm toolchain directory contains a selection of shell scripts (both Unix and Windows)
25+
// which call `clang/clang++` with different `-target` parameters depending on both the target
26+
// architecture and API level. For instance, the clang/clang++ compilers targetting aarch64 on API level
27+
// 28 will have the following Unix shell scripts present in the toolchain `bin` directory:
28+
//
29+
// aarch64-linux-android28-clang.cmd
30+
// aarch64-linux-android28-clang++.cmd
31+
//
32+
// However, the Windows version of the NDK has a bug where there these scripts don't properly deal with
33+
// spaces, which breaks the `BuildAotApplicationAndBundleAndÜmläüts()` unit tests, as it attempts to
34+
// place the NDK into a directory containing spaces. The result is:
35+
//
36+
// CC="C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\bin\armv7a-linux-androideabi19-clang.CMD"
37+
// AS="C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Xamarin\Android\binutils\bin\arm-linux-androideabi-as.CMD"
38+
// …
39+
// [CC] "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\bin\armv7a-linux-androideabi19-clang.CMD" ^
40+
// -c -D__ANDROID_API__=19 -DANDROID -o obj\Release\bundles\armeabi-v7a\temp.o ^
41+
// -I "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\sysroot\usr\include\arm-linux-androideabi" ^
42+
// -I "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\sysroot\usr\include" ^
43+
// obj\Release\bundles\armeabi-v7a\temp.c
44+
// [cc stderr] 'C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK' is not recognized as an internal or external command,
45+
// [cc stderr] operable program or batch file.
46+
//
47+
// The code below tries to rectify the situation by special-casing the compiler tool handling to
48+
// return path to the actual .exe instead of the CMD.
49+
//
50+
NdkToolNames[NdkToolKind.CompilerC] = "clang.exe";
51+
NdkToolNames[NdkToolKind.CompilerCPlusPlus] = "clang++.exe";
52+
}
1753
}
1854

1955
public override string GetToolPath (NdkToolKind kind, AndroidTargetArch arch, int apiLevel)
@@ -52,5 +88,26 @@ string GetEmbeddedToolPath (NdkToolKind kind, AndroidTargetArch arch)
5288

5389
return GetExecutablePath (Path.Combine (binutilsDir, $"{triple}-{toolName}"), mustExist: true);
5490
}
91+
92+
public override string GetCompilerTargetParameters (AndroidTargetArch arch, int apiLevel, bool forCPlusPlus = false)
93+
{
94+
if (!NeedClangWorkaround) {
95+
return base.GetCompilerTargetParameters (arch, apiLevel, forCPlusPlus);
96+
}
97+
98+
var sb = new StringBuilder ();
99+
sb.Append ("--target=").Append (GetCompilerTriple (arch)).Append (apiLevel);
100+
sb.Append (" -fno-addrsig");
101+
102+
if (arch == AndroidTargetArch.X86) {
103+
sb.Append (" -mstackrealign");
104+
}
105+
106+
if (forCPlusPlus) {
107+
sb.Append (" -stdlib=libc++");
108+
}
109+
110+
return sb.ToString ();
111+
}
55112
}
56113
}

0 commit comments

Comments
 (0)