Skip to content

Commit

Permalink
[Xamarin.Android.Tools.AndroidSdk] AndroidSdkInfo validation locator (#…
Browse files Browse the repository at this point in the history 
…170)

Context: dotnet/android#7073
Context: https://github.com/xamarin/xamarin-android/blob/fdfc4c44ba65fcff9caf809bcf2d1f1a6837b1e3/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidDependenciesTests.cs#L19-L50

I was trying to figure out how xamarin-android's
`AndroidDependenciesTests.InstallAndroidDependenciesTest()` *passes*;
the test creates an empty "SDK" directory, then builds with the
`InstallAndroidDependencies` target, then builds the `Build` target,
then asserts that the used SDK directory matches the "temp" directory.

The cause of the confusion was twofold:

 1. Two targets were run, but they both wrote to the same log file,
    and thus any output from the `InstallAndroidDependencies` target
    was *lost*, which meant

 2. When reviewing the output of the `Build` target -- the *only*
    output for quite some time -- I started searching for "other
    possibilities" for why it would work, e.g. "it's not using the
    constructor parameter, but rather `monodroid-config.xml`",
    which needed to be separately investigated and discarded.

The investigation is done -- the problems were that the log file
needed to understand what was going wrong didn't exist, and that the
`platform-tools` 32.0.0 package didn't exist in the GoogleV2
manifest, and thus `platform-tools` wasn't installed, and thus `adb`
wasn't found, causing `ValidateAndroidSdkLocation()` to skip it --
but the additional contextual log information could be useful for
future investigations.

Expand the log messages provided by `AndroidSdkBase` & co. so that
we also log "where" the `loc` parameter is coming from, via a new
`locator` parameter (similar to the `locator` parameter in `JdkInfo`),
and update the "file check" logic so that we log the path of the
detected files.
  • Loading branch information
@jonpryor
jonpryor authored Jun 13, 2022
1 parent 20f6112 commit 47832f1
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 21 deletions.
50 changes: 33 additions & 17 deletions src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public string[] AllAndroidSdks {
var dirs = new List<string?> ();
dirs.Add (AndroidSdkPath);
dirs.AddRange (GetAllAvailableAndroidSdks ());
allAndroidSdks = dirs.Where (d => ValidateAndroidSdkLocation (d))
allAndroidSdks = dirs.Where (d => ValidateAndroidSdkLocation ("AllAndroidSdks", d))
.Select (d => d!)
.Distinct ()
.ToArray ();
Expand Down Expand Up @@ -102,15 +102,15 @@ 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)
static string? GetValidPath (Func<string, string?, bool> pathValidator, string? ctorParam, Func<string?> getPreferredPath, Func<IEnumerable<string>> getAllPaths)
{
if (pathValidator (ctorParam))
if (pathValidator ("constructor param", ctorParam))
return ctorParam;
ctorParam = getPreferredPath ();
if (pathValidator (ctorParam))
if (pathValidator ("preferred path", ctorParam))
return ctorParam;
foreach (var path in getAllPaths ()) {
if (pathValidator (path)) {
if (pathValidator ("all paths", path)) {
return path;
}
}
Expand All @@ -119,18 +119,18 @@ public virtual void Initialize (string? androidSdkPath = null, string? androidNd

string? GetValidNdkPath (string? ctorParam)
{
if (ValidateAndroidNdkLocation (ctorParam))
if (ValidateAndroidNdkLocation ("constructor param", ctorParam))
return ctorParam;
if (AndroidSdkPath != null) {
string bundle = FindBestNDK (AndroidSdkPath);
if (Directory.Exists (bundle) && ValidateAndroidNdkLocation (bundle))
if (Directory.Exists (bundle) && ValidateAndroidNdkLocation ("within Android SDK", bundle))
return bundle;
}
ctorParam = PreferedAndroidNdkPath;
if (ValidateAndroidNdkLocation (ctorParam))
if (ValidateAndroidNdkLocation ("preferred path", ctorParam))
return ctorParam;
foreach (var path in GetAllAvailableAndroidNdks ()) {
if (ValidateAndroidNdkLocation (path))
if (ValidateAndroidNdkLocation ("all paths", path))
return path;
}
return null;
Expand Down Expand Up @@ -255,31 +255,47 @@ IEnumerable<string> GetJavaSdkPaths ()
/// <summary>
/// Checks that a value is the location of an Android SDK.
/// </summary>
public bool ValidateAndroidSdkLocation ([NotNullWhen (true)] string? loc)
public bool ValidateAndroidSdkLocation (string locator, [NotNullWhen (true)] string? loc)
{
bool result = !string.IsNullOrEmpty (loc) && ProcessUtils.FindExecutablesInDirectory (Path.Combine (loc, "platform-tools"), Adb).Any ();
Logger (TraceLevel.Verbose, $"{nameof (ValidateAndroidSdkLocation)}: `{loc}`, result={result}");
bool result = !string.IsNullOrEmpty (loc);
if (result) {
bool foundAdb = false;
foreach (var p in ProcessUtils.FindExecutablesInDirectory (Path.Combine (loc!, "platform-tools"), Adb)) {
Logger (TraceLevel.Verbose, $"{nameof (ValidateAndroidSdkLocation)}: for locator={locator}, path=`{loc}`, found adb `{p}`");
foundAdb = true;
}
result = foundAdb;
}
Logger (TraceLevel.Verbose, $"{nameof (ValidateAndroidSdkLocation)}: for locator={locator}, path=`{loc}`, result={result}");
return result;
}

/// <summary>
/// Checks that a value is the location of a Java SDK.
/// </summary>
public virtual bool ValidateJavaSdkLocation ([NotNullWhen (true)] string? loc)
public virtual bool ValidateJavaSdkLocation (string locator, [NotNullWhen (true)] string? loc)
{
bool result = !string.IsNullOrEmpty (loc) && ProcessUtils.FindExecutablesInDirectory (Path.Combine (loc, "bin"), JarSigner).Any ();
Logger (TraceLevel.Verbose, $"{nameof (ValidateJavaSdkLocation)}: `{loc}`, result={result}");
bool result = !string.IsNullOrEmpty (loc);
if (result) {
bool foundSigner = false;
foreach (var p in ProcessUtils.FindExecutablesInDirectory (Path.Combine (loc!, "bin"), JarSigner)) {
Logger (TraceLevel.Verbose, $"{nameof (ValidateJavaSdkLocation)}: for locator={locator}, path=`{loc}`, found jarsigner `{p}`");
foundSigner = true;
}
result = foundSigner;
}
Logger (TraceLevel.Verbose, $"{nameof (ValidateJavaSdkLocation)}: locator={locator}, path=`{loc}`, result={result}");
return result;
}

/// <summary>
/// Checks that a value is the location of an Android SDK.
/// </summary>
public bool ValidateAndroidNdkLocation ([NotNullWhen (true)] string? loc)
public bool ValidateAndroidNdkLocation (string locator, [NotNullWhen (true)] string? loc)
{
bool result = !string.IsNullOrEmpty (loc) &&
ProcessUtils.FindExecutablesInDirectory (loc!, NdkStack).Any ();
Logger (TraceLevel.Verbose, $"{nameof (ValidateAndroidNdkLocation)}: `{loc}`, result={result}");
Logger (TraceLevel.Verbose, $"{nameof (ValidateAndroidNdkLocation)}: for locator={locator}, path=`{loc}`, result={result}");
return result;
}

Expand Down
6 changes: 3 additions & 3 deletions src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkUnix.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public override string? PreferedAndroidSdkPath {
if (androidEl != null) {
var path = (string?)androidEl.Attribute ("path");

if (ValidateAndroidSdkLocation (path))
if (ValidateAndroidSdkLocation ("preferred path", path))
return path;
}
return null;
Expand All @@ -66,7 +66,7 @@ public override string? PreferedAndroidNdkPath {
if (androidEl != null) {
var path = (string?)androidEl.Attribute ("path");

if (ValidateAndroidNdkLocation (path))
if (ValidateAndroidNdkLocation ("preferred path", path))
return path;
}
return null;
Expand All @@ -81,7 +81,7 @@ public override string? PreferedJavaSdkPath {
if (javaEl != null) {
var path = (string?)javaEl.Attribute ("path");

if (ValidateJavaSdkLocation (path))
if (ValidateJavaSdkLocation ("preferred path", path))
return path;
}
return null;
Expand Down
2 changes: 1 addition & 1 deletion src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkWindows.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ protected override IEnumerable<string> GetAllAvailableAndroidNdks ()
foreach (var basePath in new string [] {xamarin_private, android_default, vs_default, vs_default32bit, vs_2017_default, cdrive_default})
if (Directory.Exists (basePath))
foreach (var dir in Directory.GetDirectories (basePath, "android-ndk-r*"))
if (ValidateAndroidNdkLocation (dir))
if (ValidateAndroidNdkLocation ("Windows known NDK path", dir))
yield return dir;

foreach (var dir in base.GetAllAvailableAndroidNdks ()) {
Expand Down

0 comments on commit 47832f1

Please sign in to comment.