Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] fix for UseLatest when API level not in…
Browse files Browse the repository at this point in the history 
…stalled (#2018)

Fixes: #2007

Apparently there is a situation we aren't doing the right thing for:

  - `$(TargetFrameworkVersion)`=v9.0
  - `$(AndroidUseLatestPlatformSdk)`=True
  - API 28 is not installed

In this case, we *should* be downgrading `$(TargetFrameworkVersion)`
to `v8.1`.  Instead we get:

	ResolveSdksTask Outputs:
	    AndroidApiLevel: 27
	    AndroidApiLevelName: 27
	    TargetFrameworkVersion: v9.0

This puts us in a weird state, since we would use `.jar` files for
different API levels:

  - `platforms/android-27/android.jar`
  - `xbuild-frameworks/MonoAndroid/v9.0/mono.android.jar`

This in turn causes `proguard` (and likely other things) to fail:

	PROGUARD : warning : there were 38 unresolved references to classes or interfaces.
	    You may need to add missing library jars or update their versions.
	    If your code works fine without the missing classes, you can suppress
	    the warnings with '-dontwarn' options.
	    (http://proguard.sourceforge.net/manual/troubleshooting.html#unresolvedclass)

The fix here is somewhat complicated:

 1. Starting at `maxSupported` API level, check to see if:
 2. An Android API directory corresponding to `maxSupported` exists,
    e.g. `platforms/android-28`, and
 3. A `$(TargetFrameworkVersion)` directory corresponding to (2)
    exists such as `MonoAndroid/v9.0`.
 4. Decrement `maxSupported` until a value which satisfies *both*
    (2) and (3) is found.

Add a parameterized test cases verifying that the right thing is
happening when `$(AndroidUseLatestPlatformSdk)`=True and different
API levels are actually installed.

Other changes:

  - Refactored a bit to not call `int.TryParse()` when not needed. We
    could just use the `int` value and call `ToString()` in a better
    order.
  - One existing test case had an incorrect failure message; fixed.

Finally, remove `$(_SupportedApiLevel)`.  This MSBuild property
appears to be vestigial, and was causing confusion but had no real
benefit, i.e. as `$(_SupportedApiLevel)` is used to generate the
`ANDROID_X` values in `<GetAndroidDefineConstants/>`, what possible
value is there in allowing it to be *higher* than
`$(TargetFrameworkVersion)`, which controls which `Mono.Android.dll`
the source code is compiled against?

It appears that we can just remove `$(_SupportedApiLevel)`, and use
`$(_AndroidApiLevel)` in its place.  I also checked private repos,
and could find nothing that is using this property.
  • Loading branch information
@jonathanpeppers @jonpryor
jonathanpeppers authored and jonpryor committed Aug 6, 2018
1 parent 7ef79b7 commit 414538b
Show file tree
Hide file tree
Showing 4 changed files with 155 additions and 22 deletions.
2 changes: 1 addition & 1 deletion Documentation/guides/BuildProcess.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ when packaing Release applications.
allows the developer to define custom items to use with the
`AndroidVersionCodePattern`. They are in the form of a `key=value`
pair. All items in the `value` should be integer values. For
example: `screen=23;target=$(_SupportedApiLevel)`. As you can see
example: `screen=23;target=$(_AndroidApiLevel)`. As you can see
you can make use of existing or custom MSBuild properties in the
string.

Expand Down
37 changes: 21 additions & 16 deletions src/Xamarin.Android.Build.Tasks/Tasks/ResolveAndroidTooling.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ public class ResolveAndroidTooling : Task
[Output]
public string AndroidApiLevelName { get; set; }

[Output]
public string SupportedApiLevel { get; set; }

[Output]
public string AndroidSdkBuildToolsPath { get; set; }

Expand Down Expand Up @@ -208,7 +205,6 @@ public override bool Execute ()
Log.LogDebugMessage ($" {nameof (TargetFrameworkVersion)}: {TargetFrameworkVersion}");
Log.LogDebugMessage ($" {nameof (AndroidApiLevel)}: {AndroidApiLevel}");
Log.LogDebugMessage ($" {nameof (AndroidApiLevelName)}: {AndroidApiLevelName}");
Log.LogDebugMessage ($" {nameof (SupportedApiLevel)}: {SupportedApiLevel}");
Log.LogDebugMessage ($" {nameof (AndroidSdkBuildToolsPath)}: {AndroidSdkBuildToolsPath}");
Log.LogDebugMessage ($" {nameof (AndroidSdkBuildToolsBinPath)}: {AndroidSdkBuildToolsBinPath}");
Log.LogDebugMessage ($" {nameof (ZipAlignPath)}: {ZipAlignPath}");
Expand Down Expand Up @@ -272,24 +268,36 @@ bool ValidateApiLevels ()
(string.IsNullOrWhiteSpace (AndroidApiLevel) && string.IsNullOrWhiteSpace (TargetFrameworkVersion));

if (UseLatestAndroidPlatformSdk) {
AndroidApiLevel = GetMaxInstalledApiLevel ().ToString ();
SupportedApiLevel = GetMaxStableApiLevel ().ToString ();
int maxInstalled, maxSupported = 0;
if (int.TryParse (AndroidApiLevel, out maxInstalled) && int.TryParse (SupportedApiLevel, out maxSupported) && maxInstalled > maxSupported) {
Log.LogDebugMessage ($"API Level {AndroidApiLevel} is greater than the maximum supported API level of {SupportedApiLevel}. " +
int maxInstalled = GetMaxInstalledApiLevel ();
int maxSupported = GetMaxStableApiLevel ();
AndroidApiLevel = maxInstalled.ToString ();
if (maxInstalled > maxSupported) {
Log.LogDebugMessage ($"API Level {maxInstalled} is greater than the maximum supported API level of {maxSupported}. " +
"Support for this API will be added in a future release.");
AndroidApiLevel = SupportedApiLevel;
}
if (!string.IsNullOrWhiteSpace (TargetFrameworkVersion)) {
var userSelected = MonoAndroidHelper.SupportedVersions.GetApiLevelFromFrameworkVersion (TargetFrameworkVersion);
// overwrite using user version only if it is
// above the maxStableApi and a valid apiLevel.
if (userSelected != null && userSelected > maxSupported && userSelected <= maxInstalled) {
maxInstalled =
maxSupported = userSelected.Value;
AndroidApiLevel = userSelected.ToString ();
SupportedApiLevel = userSelected.ToString ();
}
}
TargetFrameworkVersion = GetTargetFrameworkVersionFromApiLevel ();

for (int apiLevel = maxSupported; apiLevel >= MonoAndroidHelper.SupportedVersions.MinStableVersion.ApiLevel; apiLevel--) {
var id = MonoAndroidHelper.SupportedVersions.GetIdFromApiLevel (apiLevel);
var apiPlatformDir = MonoAndroidHelper.AndroidSdk.TryGetPlatformDirectoryFromApiLevel (id, MonoAndroidHelper.SupportedVersions);
if (apiPlatformDir != null && Directory.Exists (apiPlatformDir)) {
var targetFramework = MonoAndroidHelper.SupportedVersions.GetFrameworkVersionFromId (id);
if (targetFramework != null && MonoAndroidHelper.SupportedVersions.InstalledBindingVersions.Any (b => b.FrameworkVersion == targetFramework)) {
AndroidApiLevel = apiLevel.ToString ();
TargetFrameworkVersion = targetFramework;
break;
}
}
}
return TargetFrameworkVersion != null;
}

Expand All @@ -303,13 +311,11 @@ bool ValidateApiLevels ()
return false;
}
AndroidApiLevel = MonoAndroidHelper.SupportedVersions.GetApiLevelFromId (id).ToString ();
SupportedApiLevel = AndroidApiLevel;
return true;
}

if (!string.IsNullOrWhiteSpace (AndroidApiLevel)) {
AndroidApiLevel = AndroidApiLevel.Trim ();
SupportedApiLevel = GetMaxSupportedApiLevel (AndroidApiLevel);
TargetFrameworkVersion = GetTargetFrameworkVersionFromApiLevel ();
return TargetFrameworkVersion != null;
}
Expand Down Expand Up @@ -369,8 +375,7 @@ string GetMaxSupportedApiLevel (string apiLevel)

string GetTargetFrameworkVersionFromApiLevel ()
{
string targetFramework = MonoAndroidHelper.SupportedVersions.GetFrameworkVersionFromId (SupportedApiLevel) ??
MonoAndroidHelper.SupportedVersions.GetFrameworkVersionFromId (AndroidApiLevel);
string targetFramework = MonoAndroidHelper.SupportedVersions.GetFrameworkVersionFromId (AndroidApiLevel);
if (targetFramework != null)
return targetFramework;
Log.LogCodedError ("XA0000",
Expand Down
135 changes: 132 additions & 3 deletions src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/ResolveSdksTaskTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public void UseLatestAndroidSdk (string buildtools, string jdk, ApiInfo[] apis,
SequencePointsMode = "None",
};
Assert.AreEqual (expectedTaskResult, resolveSdks.Execute () && validateJavaVersion.Execute () && androidTooling.Execute (), $"Tasks should have {(expectedTaskResult ? "succeeded" : "failed" )}.");
Assert.AreEqual (expectedTargetFramework, androidTooling.TargetFrameworkVersion, $"TargetFrameworkVersion should be {expectedTargetFramework} but was {targetFrameworkVersion}");
Assert.AreEqual (expectedTargetFramework, androidTooling.TargetFrameworkVersion, $"TargetFrameworkVersion should be {expectedTargetFramework} but was {androidTooling.TargetFrameworkVersion}");
if (!string.IsNullOrWhiteSpace (expectedError)) {
Assert.AreEqual (1, errors.Count (), "An error should have been raised.");
Assert.AreEqual (expectedError, errors [0].Code, $"Expected error code {expectedError} but found {errors [0].Code}");
Expand Down Expand Up @@ -240,7 +240,7 @@ public void ResolveSdkTiming ()
UseLatestAndroidPlatformSdk = false,
AotAssemblies = false,
SequencePointsMode = "None",
};;
};
var start = DateTime.UtcNow;
Assert.IsTrue (resolveSdks.Execute (), "ResolveSdks should succeed!");
Assert.IsTrue (validateJavaVersion.Execute (), "ValidateJavaVersion should succeed!");
Expand All @@ -250,7 +250,6 @@ public void ResolveSdkTiming ()
Assert.AreEqual (androidTooling.AndroidApiLevel, "26", "AndroidApiLevel should be 26");
Assert.AreEqual (androidTooling.TargetFrameworkVersion, "v8.0", "TargetFrameworkVersion should be v8.0");
Assert.AreEqual (androidTooling.AndroidApiLevelName, "26", "AndroidApiLevelName should be 26");
Assert.AreEqual (androidTooling.SupportedApiLevel, "26", "SupportedApiLevel should be 26");
Assert.NotNull (resolveSdks.ReferenceAssemblyPaths, "ReferenceAssemblyPaths should not be null.");
Assert.AreEqual (resolveSdks.ReferenceAssemblyPaths.Length, 1, "ReferenceAssemblyPaths should have 1 entry.");
Assert.AreEqual (resolveSdks.ReferenceAssemblyPaths[0], Path.Combine (referencePath, "MonoAndroid"), $"ReferenceAssemblyPaths should be {Path.Combine (referencePath, "MonoAndroid")}.");
Expand Down Expand Up @@ -278,5 +277,135 @@ public void ResolveSdkTiming ()
Assert.AreEqual (validateJavaVersion.MinimumRequiredJdkVersion, "1.8", "MinimumRequiredJdkVersion should be 1.8");
Directory.Delete (Path.Combine (Root, path), recursive: true);
}

static object [] TargetFrameworkPairingParameters = new [] {
//We support 28, but only 27 is installed
new object [] {
"Older API Installed", //description
// androidSdks
new [] {
new ApiInfo () { Id = "27", Level = 27, Name = "Oreo", FrameworkVersion = "v8.1", Stable = true },
},
// targetFrameworks
new ApiInfo [] {
new ApiInfo () { Id = "27", Level = 27, Name = "Oreo", FrameworkVersion = "v8.1", Stable = true },
new ApiInfo () { Id = "28", Level = 28, Name = "P", FrameworkVersion = "v9.0", Stable = true },
},
null, //userSelected
"27", //androidApiLevel
"27", //androidApiLevelName
"v8.1", //targetFrameworkVersion
},
//28 is installed but we only support 27
new object [] {
"Newer API Installed", //description
// androidSdks
new [] {
new ApiInfo () { Id = "27", Level = 27, Name = "Oreo", FrameworkVersion = "v8.1", Stable = true },
new ApiInfo () { Id = "28", Level = 28, Name = "P", FrameworkVersion = "v9.0", Stable = true },
},
// targetFrameworks
new ApiInfo [] {
new ApiInfo () { Id = "27", Level = 27, Name = "Oreo", FrameworkVersion = "v8.1", Stable = true },
},
null, //userSelected
"27", //androidApiLevel
"27", //androidApiLevelName
"v8.1", //targetFrameworkVersion
},
//A paired downgrade to API 26
new object [] {
"Paired Downgrade", //description
// androidSdks
new [] {
new ApiInfo () { Id = "26", Level = 26, Name = "Oreo", FrameworkVersion = "v8.0", Stable = true },
new ApiInfo () { Id = "27", Level = 27, Name = "Oreo", FrameworkVersion = "v8.1", Stable = true },
},
// targetFrameworks
new ApiInfo [] {
new ApiInfo () { Id = "26", Level = 26, Name = "Oreo", FrameworkVersion = "v8.0", Stable = true },
new ApiInfo () { Id = "28", Level = 28, Name = "P", FrameworkVersion = "v9.0", Stable = true },
},
null, //userSelected
"26", //androidApiLevel
"26", //androidApiLevelName
"v8.0", //targetFrameworkVersion
},
//A new API level 28 is not stable yet
new object [] {
"New Unstable API", //description
// androidSdks
new [] {
new ApiInfo () { Id = "26", Level = 26, Name = "Oreo", FrameworkVersion = "v8.0", Stable = true },
new ApiInfo () { Id = "27", Level = 27, Name = "Oreo", FrameworkVersion = "v8.1", Stable = true },
new ApiInfo () { Id = "28", Level = 28, Name = "P", FrameworkVersion = "v9.0", Stable = false },
},
// targetFrameworks
new ApiInfo [] {
new ApiInfo () { Id = "26", Level = 26, Name = "Oreo", FrameworkVersion = "v8.0", Stable = true },
new ApiInfo () { Id = "27", Level = 27, Name = "Oreo", FrameworkVersion = "v8.1", Stable = true },
new ApiInfo () { Id = "28", Level = 28, Name = "P", FrameworkVersion = "v9.0", Stable = false },
},
null, //userSelected
"27", //androidApiLevel
"27", //androidApiLevelName
"v8.1", //targetFrameworkVersion
},
//User selected a new API level 28 is not stable yet
new object [] {
"User Selected Unstable API", //description
// androidSdks
new [] {
new ApiInfo () { Id = "26", Level = 26, Name = "Oreo", FrameworkVersion = "v8.0", Stable = true },
new ApiInfo () { Id = "27", Level = 27, Name = "Oreo", FrameworkVersion = "v8.1", Stable = true },
new ApiInfo () { Id = "28", Level = 28, Name = "P", FrameworkVersion = "v9.0", Stable = false },
},
// targetFrameworks
new ApiInfo [] {
new ApiInfo () { Id = "26", Level = 26, Name = "Oreo", FrameworkVersion = "v8.0", Stable = true },
new ApiInfo () { Id = "27", Level = 27, Name = "Oreo", FrameworkVersion = "v8.1", Stable = true },
new ApiInfo () { Id = "28", Level = 28, Name = "P", FrameworkVersion = "v9.0", Stable = false },
},
"v9.0", //userSelected
"28", //androidApiLevel
"28", //androidApiLevelName
"v9.0", //targetFrameworkVersion
},
};

[Test]
[TestCaseSource (nameof (TargetFrameworkPairingParameters))]
public void TargetFrameworkPairing (string description, ApiInfo[] androidSdk, ApiInfo[] targetFrameworks, string userSelected, string androidApiLevel, string androidApiLevelName, string targetFrameworkVersion)
{
var path = Path.Combine ("temp", $"{nameof (TargetFrameworkPairing)}_{description}");
var androidSdkPath = CreateFauxAndroidSdkDirectory (Path.Combine (path, "android-sdk"), "26.0.3", androidSdk);
string javaExe = string.Empty;
string javacExe;
var javaPath = CreateFauxJavaSdkDirectory (Path.Combine (path, "jdk"), "1.8.0", out javaExe, out javacExe);
var referencePath = CreateFauxReferencesDirectory (Path.Combine (path, "references"), targetFrameworks);
IBuildEngine engine = new MockBuildEngine (TestContext.Out);
var resolveSdks = new ResolveSdks {
BuildEngine = engine,
AndroidSdkPath = androidSdkPath,
AndroidNdkPath = androidSdkPath,
JavaSdkPath = javaPath,
ReferenceAssemblyPaths = new [] {
Path.Combine (referencePath, "MonoAndroid"),
},
};
var androidTooling = new ResolveAndroidTooling {
BuildEngine = engine,
AndroidSdkPath = androidSdkPath,
AndroidNdkPath = androidSdkPath,
UseLatestAndroidPlatformSdk = true,
TargetFrameworkVersion = userSelected,
};
Assert.IsTrue (resolveSdks.Execute (), "ResolveSdks should succeed!");
Assert.IsTrue (androidTooling.Execute (), "ResolveAndroidTooling should succeed!");
Assert.AreEqual (androidApiLevel, androidTooling.AndroidApiLevel, $"AndroidApiLevel should be {androidApiLevel}");
Assert.AreEqual (androidApiLevelName, androidTooling.AndroidApiLevelName, $"AndroidApiLevelName should be {androidApiLevelName}");
Assert.AreEqual (targetFrameworkVersion, androidTooling.TargetFrameworkVersion, $"TargetFrameworkVersion should be {targetFrameworkVersion}");
Directory.Delete (Path.Combine (Root, path), recursive: true);
}
}
}
3 changes: 1 addition & 2 deletions src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets
View file
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,6 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved.
<Output TaskParameter="TargetFrameworkVersion" PropertyName="TargetFrameworkVersion" />
<Output TaskParameter="AndroidApiLevel" PropertyName="_AndroidApiLevel" Condition="'$(_AndroidApiLevel)' == ''" />
<Output TaskParameter="AndroidApiLevelName" PropertyName="_AndroidApiLevelName" />
<Output TaskParameter="SupportedApiLevel" PropertyName="_SupportedApiLevel" />
<Output TaskParameter="AndroidSdkBuildToolsPath" PropertyName="AndroidSdkBuildToolsPath" Condition="'$(AndroidSdkBuildToolsPath)' == ''" />
<Output TaskParameter="AndroidSdkBuildToolsBinPath" PropertyName="AndroidSdkBuildToolsBinPath" Condition="'$(AndroidSdkBuildToolsBinPath)' == ''" />
<Output TaskParameter="ZipAlignPath" PropertyName="ZipAlignToolPath" Condition="'$(ZipAlignToolPath)' == ''" />
Expand Down Expand Up @@ -1008,7 +1007,7 @@ because xbuild doesn't support framework reference assemblies.
</CreateProperty>

<!-- Get the defined constants for this API Level -->
<GetAndroidDefineConstants AndroidApiLevel="$(_SupportedApiLevel)" ProductVersion="$(MonoAndroidVersion)">
<GetAndroidDefineConstants AndroidApiLevel="$(_AndroidApiLevel)" ProductVersion="$(MonoAndroidVersion)">
<Output TaskParameter="AndroidDefineConstants" ItemName="AndroidDefineConstants" />
</GetAndroidDefineConstants>

Expand Down

0 comments on commit 414538b

Please sign in to comment.