Skip to content

Fix sort ordering for ndk-bundle, add macOS support #91

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 14, 2020
Merged

Fix sort ordering for ndk-bundle, add macOS support #91

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f8c43c9
Fix sort ordering for ndk-bundle, add macOS support
jonathanpeppers Jun 30, 2020
9232161
Merge remote-tracking branch 'origin/master' into windows-ndk-bundle
jonpryor Jul 1, 2020
2917561
[Xamarin.Android.Tools.AndroidSdk] Cleanup path validation logic
jonpryor Jul 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 69 additions & 18 deletions src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,21 @@ namespace Xamarin.Android.Tools
abstract class AndroidSdkBase
{
string[]? allAndroidSdks;
string[]? allAndroidNdks;

public string[] AllAndroidSdks {
get {
if (allAndroidSdks == null)
allAndroidSdks = GetAllAvailableAndroidSdks ().Distinct ().ToArray ();
if (allAndroidSdks == null) {
var dirs = new List<string?> ();
dirs.Add (AndroidSdkPath);
dirs.AddRange (GetAllAvailableAndroidSdks ());
allAndroidSdks = dirs.Where (d => ValidateAndroidSdkLocation (d))
.Select (d => d!)
.Distinct ()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonpryor will Distinct() lose the ordering again? or does the ordering not matter here?

Copy link
Contributor

@jonpryor jonpryor Jul 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it will lose the ordering again, but this is only accessed from AndroidSdkInfo.AllAndroidSdks, which "nobody" should be using, and which always had .Distinct() semantics, so anybody using that method shouldn't see anything bizarre.

I hope.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed we're only using GetAllAvailableAndroidSdks (and also every time we use it we're using Distinct too :)

.ToArray ();
}
return allAndroidSdks;
}
}
public string[] AllAndroidNdks {
get {
if (allAndroidNdks == null)
allAndroidNdks = GetAllAvailableAndroidNdks ().Distinct ().ToArray ();
return allAndroidNdks;
}
}

public readonly Action<TraceLevel, string> Logger;

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

public virtual void Initialize (string? androidSdkPath = null, string? androidNdkPath = null, string? javaSdkPath = null)
{
androidSdkPath = androidSdkPath ?? PreferedAndroidSdkPath;
androidNdkPath = androidNdkPath ?? PreferedAndroidNdkPath;
javaSdkPath = javaSdkPath ?? PreferedJavaSdkPath;
AndroidSdkPath = GetValidPath (ValidateAndroidSdkLocation, androidSdkPath, () => PreferedAndroidSdkPath, () => GetAllAvailableAndroidSdks ());
JavaSdkPath = GetValidPath (ValidateJavaSdkLocation, javaSdkPath, () => PreferedJavaSdkPath, () => GetJavaSdkPaths ());

AndroidSdkPath = ValidateAndroidSdkLocation (androidSdkPath) ? androidSdkPath : AllAndroidSdks.FirstOrDefault ();
AndroidNdkPath = ValidateAndroidNdkLocation (androidNdkPath) ? androidNdkPath : AllAndroidNdks.FirstOrDefault ();
JavaSdkPath = ValidateJavaSdkLocation (javaSdkPath) ? javaSdkPath : GetJavaSdkPath ();
AndroidNdkPath = GetValidNdkPath (androidNdkPath);

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

static string? GetValidPath (Func<string?, bool> pathValidator, string? ctorParam, Func<string?> getPreferredPath, Func<IEnumerable<string>> getAllPaths)
{
if (pathValidator (ctorParam))
return ctorParam;
ctorParam = getPreferredPath ();
if (pathValidator (ctorParam))
return ctorParam;
foreach (var path in getAllPaths ()) {
if (pathValidator (path))
return path;
}
return null;
}

string? GetValidNdkPath (string? ctorParam)
{
if (ValidateAndroidNdkLocation (ctorParam))
return ctorParam;
if (AndroidSdkPath != null) {
string bundle = Path.Combine (AndroidSdkPath, "ndk-bundle");
if (Directory.Exists (bundle) && ValidateAndroidNdkLocation (bundle))
return bundle;
}
ctorParam = PreferedAndroidNdkPath;
if (ValidateAndroidNdkLocation (ctorParam))
return ctorParam;
foreach (var path in GetAllAvailableAndroidNdks ()) {
if (ValidateAndroidNdkLocation (path))
return path;
}
return null;
}

protected abstract IEnumerable<string> GetAllAvailableAndroidSdks ();
protected abstract IEnumerable<string> GetAllAvailableAndroidNdks ();
protected abstract string? GetJavaSdkPath ();
protected abstract string GetShortFormPath (string path);

protected virtual IEnumerable<string> GetAllAvailableAndroidNdks ()
{
// Look in PATH
foreach (var ndkStack in ProcessUtils.FindExecutablesInPath (NdkStack)) {
var ndkDir = Path.GetDirectoryName (ndkStack);
if (ndkDir == null)
continue;
yield return ndkDir;
}

// Check for the "ndk-bundle" directory inside other SDK directories
foreach (var sdk in GetAllAvailableAndroidSdks ()) {
if (sdk == AndroidSdkPath)
continue;
yield return Path.Combine (sdk, "ndk-bundle");
}
}

public abstract void SetPreferredAndroidSdkPath (string? path);
public abstract void SetPreferredJavaSdkPath (string? path);
public abstract void SetPreferredAndroidNdkPath (string? path);
Expand All @@ -108,6 +153,12 @@ public string NdkHostPlatform {
get { return IsNdk64Bit ? NdkHostPlatform64Bit : NdkHostPlatform32Bit; }
}

IEnumerable<string> GetJavaSdkPaths ()
{
return JdkInfo.GetKnownSystemJdkInfos (Logger)
.Select (jdk => jdk.HomePath);
}

/// <summary>
/// Checks that a value is the location of an Android SDK.
/// </summary>
Expand Down
28 changes: 5 additions & 23 deletions src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkUnix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,33 +98,15 @@ protected override IEnumerable<string> GetAllAvailableAndroidSdks ()
// Strip off "platform-tools"
var dir = Path.GetDirectoryName (path);

if (ValidateAndroidSdkLocation (dir))
yield return dir;
if (dir == null)
continue;

yield return dir;
}

// Check some hardcoded paths for good measure
var macSdkPath = Path.Combine (Environment.GetFolderPath (Environment.SpecialFolder.Personal), "Library", "Android", "sdk");
if (ValidateAndroidSdkLocation (macSdkPath))
yield return macSdkPath;
}

protected override string? GetJavaSdkPath ()
{
return JdkInfo.GetKnownSystemJdkInfos (Logger).FirstOrDefault ()?.HomePath;
}

protected override IEnumerable<string> GetAllAvailableAndroidNdks ()
{
var preferedNdkPath = PreferedAndroidNdkPath;
if (!string.IsNullOrEmpty (preferedNdkPath))
yield return preferedNdkPath!;

// Look in PATH
foreach (var ndkStack in ProcessUtils.FindExecutablesInPath (NdkStack)) {
var ndkDir = Path.GetDirectoryName (ndkStack);
if (ValidateAndroidNdkLocation (ndkDir))
yield return ndkDir;
}
yield return macSdkPath;
}

protected override string GetShortFormPath (string path)
Expand Down
26 changes: 6 additions & 20 deletions src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkWindows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,7 @@ protected override IEnumerable<string> GetAllAvailableAndroidSdks ()
};
foreach (var basePath in paths)
if (Directory.Exists (basePath))
if (ValidateAndroidSdkLocation (basePath))
yield return basePath;
}

protected override string? GetJavaSdkPath ()
{
var jdk = JdkInfo.GetKnownSystemJdkInfos (Logger).FirstOrDefault ();
return jdk?.HomePath;
yield return basePath;
}

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

protected override IEnumerable<string> GetAllAvailableAndroidNdks ()
{

var roots = new[] { RegistryEx.CurrentUser, RegistryEx.LocalMachine };
var wow = RegistryEx.Wow64.Key32;
var regKey = GetMDRegistryKey ();

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

// Check for the "ndk-bundle" directory inside the SDK directories
string ndk;

var sdks = GetAllAvailableAndroidSdks().ToList();
if (!string.IsNullOrEmpty(AndroidSdkPath))
sdks.Add (AndroidSdkPath!);

foreach(var sdk in sdks.Distinct())
if (Directory.Exists(ndk = Path.Combine(sdk, "ndk-bundle")))
if (ValidateAndroidNdkLocation(ndk))
yield return ndk;

// Check for the key the user gave us in the VS/addin options
foreach (var root in roots)
if (CheckRegistryKeyForExecutable (root, regKey, MDREG_ANDROID_NDK, wow, ".", NdkStack))
Expand All @@ -265,6 +247,10 @@ protected override IEnumerable<string> GetAllAvailableAndroidNdks ()
foreach (var dir in Directory.GetDirectories (basePath, "android-ndk-r*"))
if (ValidateAndroidNdkLocation (dir))
yield return dir;

foreach (var dir in base.GetAllAvailableAndroidNdks ()) {
yield return dir;
}
}

protected override string GetShortFormPath (string path)
Expand Down
6 changes: 2 additions & 4 deletions tests/Xamarin.Android.Tools.AndroidSdk-Tests/AndroidSdkInfoTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@ public void Constructor_Paths ()
[Test]
public void Ndk_PathInSdk()
{
if (!OS.IsWindows)
Assert.Ignore("This only works in Windows");

CreateSdks(out string root, out string jdk, out string ndk, out string sdk);

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

try
{
var extension = OS.IsWindows ? ".cmd" : "";
var ndkPath = Path.Combine(sdk, "ndk-bundle");
Directory.CreateDirectory(ndkPath);
Directory.CreateDirectory(Path.Combine(ndkPath, "toolchains"));
File.WriteAllText(Path.Combine(ndkPath, "ndk-stack.cmd"), "");
File.WriteAllText(Path.Combine(ndkPath, $"ndk-stack{extension}"), "");

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

Expand Down