Skip to content

Commit 5718cd2

Browse files
jonathanpeppersjonathanpeppers
authored
Fix sort ordering for ndk-bundle, add macOS support (#91)
Fixes: #92 Context: #90 (comment) The PR builds for #90 encountered an "unrelated test failure" in `AndroidSdkInfoTests.Ndk_PathInSdk()` on Windows, because Windows is non-deterministic: the test asserts that given an Android SDK directory `androidSdk` which contains the file `{androidSdk}\ndk-bundle\ndk-stack.cmd`, then this: var info = new AndroidSdkInfo (logger: null, androidSdkPath: androidSdk); will have `info.AndroidNdkPath`==`{androidSdk}\ndk-bundle`. Instead, this test would occasionally fail on CI: Ndk_PathInSdk AndroidNdkPath not found inside sdk! Expected string length 71 but was 53. Strings differ at index 3. Expected: "C:\Users\VssAdministrator\AppData\Local\Temp\tmpAE78.tmp\sdk\..." But was: "C:\Program Files (x86)\Android\android-sdk\ndk-bundle" Here, the "preferred"/system-wide NDK is being chosen over the `{androidSdk}\ndk-bundle` directory that the unit test created. The wrong directory was chosen for two reasons: 1. `AndroidSdkBase.Initialize()` would use `PreferedAndroidNdkPath` when `androidNdkPath` was null, *first*, before we checked `{androidSdk}\ndk-bundle`. 2. If `PreferedAndroidNdkPath` happened to be null, then `AndroidSdkBase.Initialize()` would try to use `AllAndroidNdks.FirstOrDefault()` as a default value, also before checking `{androidSdk}\ndk-bundle`. The problem here is that the `AllAndrdoidNdks` property uses [`Enumerable.Distinct()`][0], which returns an *unordered* list of directories. That the test ever worked at all is a minor miracle. Additionally, the support for `{androidSdk}\ndk-bundle` was Windows- specific; it didn't run on macOS. Update `AndroidSdkInfo` so that `{androidSdk}/ndk-bundle` is supported on macOS, and that `{androidSdk}/ndk-bundle` is *preferred* when the `androidNdkPath` parameter is `null`, *before* checking any other plausible default locations. This allows the `AndroidSdkInfoTests.Ndk_PathInSdk()` test to run everywhere, and work reliably. [0]: https://docs.microsoft.com/en-us/dotnet/api/system.linq.enumerable.distinct?view=netcore-3.1
1 parent 8e63795 commit 5718cd2

File tree

4 files changed

+82
-65
lines changed

4 files changed

+82
-65
lines changed

‎src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkBase.cs‎

Lines changed: 69 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,22 +10,21 @@ namespace Xamarin.Android.Tools
1010
abstract class AndroidSdkBase
1111
{
1212
string[]? allAndroidSdks;
13-
string[]? allAndroidNdks;
1413

1514
public string[] AllAndroidSdks {
1615
get {
17-
if (allAndroidSdks == null)
18-
allAndroidSdks = GetAllAvailableAndroidSdks ().Distinct ().ToArray ();
16+
if (allAndroidSdks == null) {
17+
var dirs = new List<string?> ();
18+
dirs.Add (AndroidSdkPath);
19+
dirs.AddRange (GetAllAvailableAndroidSdks ());
20+
allAndroidSdks = dirs.Where (d => ValidateAndroidSdkLocation (d))
21+
.Select (d => d!)
22+
.Distinct ()
23+
.ToArray ();
24+
}
1925
return allAndroidSdks;
2026
}
2127
}
22-
public string[] AllAndroidNdks {
23-
get {
24-
if (allAndroidNdks == null)
25-
allAndroidNdks = GetAllAvailableAndroidNdks ().Distinct ().ToArray ();
26-
return allAndroidNdks;
27-
}
28-
}
2928

3029
public readonly Action<TraceLevel, string> Logger;
3130

@@ -57,13 +56,10 @@ public AndroidSdkBase (Action<TraceLevel, string> logger)
5756

5857
public virtual void Initialize (string? androidSdkPath = null, string? androidNdkPath = null, string? javaSdkPath = null)
5958
{
60-
androidSdkPath = androidSdkPath ?? PreferedAndroidSdkPath;
61-
androidNdkPath = androidNdkPath ?? PreferedAndroidNdkPath;
62-
javaSdkPath = javaSdkPath ?? PreferedJavaSdkPath;
59+
AndroidSdkPath = GetValidPath (ValidateAndroidSdkLocation, androidSdkPath, () => PreferedAndroidSdkPath, () => GetAllAvailableAndroidSdks ());
60+
JavaSdkPath = GetValidPath (ValidateJavaSdkLocation, javaSdkPath, () => PreferedJavaSdkPath, () => GetJavaSdkPaths ());
6361

64-
AndroidSdkPath = ValidateAndroidSdkLocation (androidSdkPath) ? androidSdkPath : AllAndroidSdks.FirstOrDefault ();
65-
AndroidNdkPath = ValidateAndroidNdkLocation (androidNdkPath) ? androidNdkPath : AllAndroidNdks.FirstOrDefault ();
66-
JavaSdkPath = ValidateJavaSdkLocation (javaSdkPath) ? javaSdkPath : GetJavaSdkPath ();
62+
AndroidNdkPath = GetValidNdkPath (androidNdkPath);
6763

6864
if (!string.IsNullOrEmpty (JavaSdkPath)) {
6965
JavaBinPath = Path.Combine (JavaSdkPath, "bin");
@@ -93,11 +89,60 @@ public virtual void Initialize (string? androidSdkPath = null, string? androidNd
9389
NdkStack = GetExecutablePath (AndroidNdkPath, NdkStack);
9490
}
9591

92+
static string? GetValidPath (Func<string?, bool> pathValidator, string? ctorParam, Func<string?> getPreferredPath, Func<IEnumerable<string>> getAllPaths)
93+
{
94+
if (pathValidator (ctorParam))
95+
return ctorParam;
96+
ctorParam = getPreferredPath ();
97+
if (pathValidator (ctorParam))
98+
return ctorParam;
99+
foreach (var path in getAllPaths ()) {
100+
if (pathValidator (path))
101+
return path;
102+
}
103+
return null;
104+
}
105+
106+
string? GetValidNdkPath (string? ctorParam)
107+
{
108+
if (ValidateAndroidNdkLocation (ctorParam))
109+
return ctorParam;
110+
if (AndroidSdkPath != null) {
111+
string bundle = Path.Combine (AndroidSdkPath, "ndk-bundle");
112+
if (Directory.Exists (bundle) && ValidateAndroidNdkLocation (bundle))
113+
return bundle;
114+
}
115+
ctorParam = PreferedAndroidNdkPath;
116+
if (ValidateAndroidNdkLocation (ctorParam))
117+
return ctorParam;
118+
foreach (var path in GetAllAvailableAndroidNdks ()) {
119+
if (ValidateAndroidNdkLocation (path))
120+
return path;
121+
}
122+
return null;
123+
}
124+
96125
protected abstract IEnumerable<string> GetAllAvailableAndroidSdks ();
97-
protected abstract IEnumerable<string> GetAllAvailableAndroidNdks ();
98-
protected abstract string? GetJavaSdkPath ();
99126
protected abstract string GetShortFormPath (string path);
100127

128+
protected virtual IEnumerable<string> GetAllAvailableAndroidNdks ()
129+
{
130+
// Look in PATH
131+
foreach (var ndkStack in ProcessUtils.FindExecutablesInPath (NdkStack)) {
132+
var ndkDir = Path.GetDirectoryName (ndkStack);
133+
if (ndkDir == null)
134+
continue;
135+
yield return ndkDir;
136+
}
137+
138+
// Check for the "ndk-bundle" directory inside other SDK directories
139+
foreach (var sdk in GetAllAvailableAndroidSdks ()) {
140+
if (sdk == AndroidSdkPath)
141+
continue;
142+
yield return Path.Combine (sdk, "ndk-bundle");
143+
}
144+
}
145+
101146
public abstract void SetPreferredAndroidSdkPath (string? path);
102147
public abstract void SetPreferredJavaSdkPath (string? path);
103148
public abstract void SetPreferredAndroidNdkPath (string? path);
@@ -108,6 +153,12 @@ public string NdkHostPlatform {
108153
get { return IsNdk64Bit ? NdkHostPlatform64Bit : NdkHostPlatform32Bit; }
109154
}
110155

156+
IEnumerable<string> GetJavaSdkPaths ()
157+
{
158+
return JdkInfo.GetKnownSystemJdkInfos (Logger)
159+
.Select (jdk => jdk.HomePath);
160+
}
161+
111162
/// <summary>
112163
/// Checks that a value is the location of an Android SDK.
113164
/// </summary>

‎src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkUnix.cs‎

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -98,33 +98,15 @@ protected override IEnumerable<string> GetAllAvailableAndroidSdks ()
9898
// Strip off "platform-tools"
9999
var dir = Path.GetDirectoryName (path);
100100

101-
if (ValidateAndroidSdkLocation (dir))
102-
yield return dir;
101+
if (dir == null)
102+
continue;
103+
104+
yield return dir;
103105
}
104106

105107
// Check some hardcoded paths for good measure
106108
var macSdkPath = Path.Combine (Environment.GetFolderPath (Environment.SpecialFolder.Personal), "Library", "Android", "sdk");
107-
if (ValidateAndroidSdkLocation (macSdkPath))
108-
yield return macSdkPath;
109-
}
110-
111-
protected override string? GetJavaSdkPath ()
112-
{
113-
return JdkInfo.GetKnownSystemJdkInfos (Logger).FirstOrDefault ()?.HomePath;
114-
}
115-
116-
protected override IEnumerable<string> GetAllAvailableAndroidNdks ()
117-
{
118-
var preferedNdkPath = PreferedAndroidNdkPath;
119-
if (!string.IsNullOrEmpty (preferedNdkPath))
120-
yield return preferedNdkPath!;
121-
122-
// Look in PATH
123-
foreach (var ndkStack in ProcessUtils.FindExecutablesInPath (NdkStack)) {
124-
var ndkDir = Path.GetDirectoryName (ndkStack);
125-
if (ValidateAndroidNdkLocation (ndkDir))
126-
yield return ndkDir;
127-
}
109+
yield return macSdkPath;
128110
}
129111

130112
protected override string GetShortFormPath (string path)

‎src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkWindows.cs‎

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,7 @@ protected override IEnumerable<string> GetAllAvailableAndroidSdks ()
102102
};
103103
foreach (var basePath in paths)
104104
if (Directory.Exists (basePath))
105-
if (ValidateAndroidSdkLocation (basePath))
106-
yield return basePath;
107-
}
108-
109-
protected override string? GetJavaSdkPath ()
110-
{
111-
var jdk = JdkInfo.GetKnownSystemJdkInfos (Logger).FirstOrDefault ();
112-
return jdk?.HomePath;
105+
yield return basePath;
113106
}
114107

115108
internal static IEnumerable<JdkInfo> GetJdkInfos (Action<TraceLevel, string> logger)
@@ -223,24 +216,13 @@ private static IEnumerable<string> GetOracleJdkPaths ()
223216

224217
protected override IEnumerable<string> GetAllAvailableAndroidNdks ()
225218
{
219+
226220
var roots = new[] { RegistryEx.CurrentUser, RegistryEx.LocalMachine };
227221
var wow = RegistryEx.Wow64.Key32;
228222
var regKey = GetMDRegistryKey ();
229223

230224
Logger (TraceLevel.Info, "Looking for Android NDK...");
231225

232-
// Check for the "ndk-bundle" directory inside the SDK directories
233-
string ndk;
234-
235-
var sdks = GetAllAvailableAndroidSdks().ToList();
236-
if (!string.IsNullOrEmpty(AndroidSdkPath))
237-
sdks.Add (AndroidSdkPath!);
238-
239-
foreach(var sdk in sdks.Distinct())
240-
if (Directory.Exists(ndk = Path.Combine(sdk, "ndk-bundle")))
241-
if (ValidateAndroidNdkLocation(ndk))
242-
yield return ndk;
243-
244226
// Check for the key the user gave us in the VS/addin options
245227
foreach (var root in roots)
246228
if (CheckRegistryKeyForExecutable (root, regKey, MDREG_ANDROID_NDK, wow, ".", NdkStack))
@@ -265,6 +247,10 @@ protected override IEnumerable<string> GetAllAvailableAndroidNdks ()
265247
foreach (var dir in Directory.GetDirectories (basePath, "android-ndk-r*"))
266248
if (ValidateAndroidNdkLocation (dir))
267249
yield return dir;
250+
251+
foreach (var dir in base.GetAllAvailableAndroidNdks ()) {
252+
yield return dir;
253+
}
268254
}
269255

270256
protected override string GetShortFormPath (string path)

‎tests/Xamarin.Android.Tools.AndroidSdk-Tests/AndroidSdkInfoTests.cs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,6 @@ public void Constructor_Paths ()
6767
[Test]
6868
public void Ndk_PathInSdk()
6969
{
70-
if (!OS.IsWindows)
71-
Assert.Ignore("This only works in Windows");
72-
7370
CreateSdks(out string root, out string jdk, out string ndk, out string sdk);
7471

7572
var logs = new StringWriter();
@@ -79,10 +76,11 @@ public void Ndk_PathInSdk()
7976

8077
try
8178
{
79+
var extension = OS.IsWindows ? ".cmd" : "";
8280
var ndkPath = Path.Combine(sdk, "ndk-bundle");
8381
Directory.CreateDirectory(ndkPath);
8482
Directory.CreateDirectory(Path.Combine(ndkPath, "toolchains"));
85-
File.WriteAllText(Path.Combine(ndkPath, "ndk-stack.cmd"), "");
83+
File.WriteAllText(Path.Combine(ndkPath, $"ndk-stack{extension}"), "");
8684

8785
var info = new AndroidSdkInfo(logger, androidSdkPath: sdk, androidNdkPath: null, javaSdkPath: jdk);
8886

0 commit comments

Comments
 (0)