Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] Support cmdline-tools (#4778)
Browse files Browse the repository at this point in the history
Context: #4567

The Android SDK `lint` utility is now in *two* Android SDK packages:
the `tools` package, and the `cmdline-tools` package, e.g. as of commit
d99facb, *both* of these exist on e.g. macOS:

  * `$HOME/android-toolchain/sdk/tools/bin/lint`
  * `$HOME/android-toolchain/sdk/cmdline-tools/1.0/bin/lint`

There are two important differences of interest:

 1. `tools/bin/lint` will not run on OpenJDK 11.
 2. `cmdline-tools/1.0/bin/lint` has a *lower version* than
    `tools/bin/lint`.  *Much* lower:

        % ~/android-toolchain/sdk/tools/bin/lint --version
        lint: version 26.1.1
        % ~/android-toolchain/sdk/cmdline-tools/1.0/bin/lint --version
        lint: version 3.6.0

(1) means that we can't use the previous `lint` in an OpenJDK11 world,
and thus we should migrate to the cmdline-tools `lint`.

Support this by adding a new `$(AndroidCommandLineToolsVersion)`
MSBuild property to `Xamarin.Android.Common.props.in` and
`Microsoft.Android.Sdk.props`, which allows controlling which
`cmdline-tools` package version to use, if more than one is present.
The default value is `1.0`.

(2) means that the `<Lint/>` Task is *broken* when using the
cmdline-tools version of `lint`, becasuse certain `lint --disable`
flags are protected by version checks, which are now all "wrong" when
we "expect" >= 26.1.1 and instead get 3.6.0.

Attempt to address (2) by assuming that we're using the cmdline-tools
package if `ToolPath` contains `cmdline-tools`, in which case we treat
`lint` as newer than any previous version.

(This may be less than ideal.)
  • Loading branch information
jonpryor committed Jun 8, 2020
1 parent b9fe2c5 commit 3426b32
Show file tree
Hide file tree
Showing 10 changed files with 32 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ called for "legacy" projects in Xamarin.Android.Legacy.targets.
AndroidUseAapt2="$(AndroidUseAapt2)"
AotAssemblies="$(AotAssemblies)"
Aapt2ToolPath="$(Aapt2ToolPath)"
CommandLineToolsVersion="$(AndroidCommandLineToolsVersion)"
SequencePointsMode="$(_AndroidSequencePointsMode)"
ProjectFilePath="$(MSBuildProjectFullPath)"
LintToolPath="$(LintToolPath)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

<PropertyGroup>
<UsingAndroidNETSdk>true</UsingAndroidNETSdk>
<AndroidCommandLineToolsVersion Condition=" '$(AndroidCommandLineToolsVersion)' == '' ">1.0</AndroidCommandLineToolsVersion>
<LatestSupportedJavaVersion Condition=" '$(LatestSupportedJavaVersion)' == '' ">1.8.0</LatestSupportedJavaVersion>
<MinimumSupportedJavaVersion Condition=" '$(MinimumSupportedJavaVersion)' == '' ">1.8.0</MinimumSupportedJavaVersion>
<EnableDefaultOutputPaths Condition=" '$(EnableDefaultOutputPaths)' == '' And '$(OS)' != 'Windows_NT' ">false</EnableDefaultOutputPaths>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ public class CalculateProjectDependencies : AndroidTask

const int DefaultMinSDKVersion = 11;

public string CommandLineToolsVersion { get; set; }

[Required]
public string TargetFrameworkVersion { get; set; }

Expand Down Expand Up @@ -58,6 +60,9 @@ public override bool RunTask ()
if (!string.IsNullOrEmpty (PlatformToolsVersion)) {
dependencies.Add (CreateAndroidDependency ("platform-tools", PlatformToolsVersion));
}
if (!string.IsNullOrEmpty (CommandLineToolsVersion)) {
dependencies.Add (CreateAndroidDependency ($"cmdline-tools/{CommandLineToolsVersion}", CommandLineToolsVersion));
}
if (!string.IsNullOrEmpty (ToolsVersion)) {
dependencies.Add (CreateAndroidDependency ("tools", ToolsVersion));
}
Expand Down
6 changes: 4 additions & 2 deletions src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -195,17 +195,19 @@ public override bool RunTask ()
return false;
}

bool fromCmdlineTools = ToolPath.IndexOf ("cmdline-tools", StringComparison.OrdinalIgnoreCase) >= 0;

Version lintToolVersion = GetLintVersion (GenerateFullPathToTool ());
Log.LogDebugMessage (" LintVersion: {0}", lintToolVersion);
foreach (var issue in DisabledIssuesByVersion) {
if (lintToolVersion >= issue.Value) {
if (fromCmdlineTools || lintToolVersion >= issue.Value) {
if (string.IsNullOrEmpty (DisabledIssues) || !DisabledIssues.Contains (issue.Key))
DisabledIssues = issue.Key + (!string.IsNullOrEmpty (DisabledIssues) ? "," + DisabledIssues : "");
}
}

foreach (var issue in DisabledIssuesByVersion) {
if (lintToolVersion < issue.Value) {
if (!fromCmdlineTools || (lintToolVersion < issue.Value)) {
DisabledIssues = CleanIssues (issue.Key, lintToolVersion, DisabledIssues, nameof (DisabledIssues));
EnabledIssues = CleanIssues (issue.Key, lintToolVersion, EnabledIssues, nameof (EnabledIssues) );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public class ResolveAndroidTooling : AndroidTask

public string AndroidSdkBuildToolsVersion { get; set; }

public string CommandLineToolsVersion { get; set; }

public string ProjectFilePath { get; set; }

public string SequencePointsMode { get; set; }
Expand Down Expand Up @@ -80,8 +82,13 @@ public override bool RunTask ()
string toolsZipAlignPath = Path.Combine (AndroidSdkPath, "tools", ZipAlign);
bool findZipAlign = (string.IsNullOrEmpty (ZipAlignPath) || !Directory.Exists (ZipAlignPath)) && !File.Exists (toolsZipAlignPath);

var commandLineToolsDir = MonoAndroidHelper.AndroidSdk.GetCommandLineToolsPaths (CommandLineToolsVersion)
.FirstOrDefault () ?? "";

var lintPaths = new string [] {
LintToolPath ?? string.Empty,
commandLineToolsDir,
Path.Combine (commandLineToolsDir, "bin"),
Path.Combine (AndroidSdkPath, "tools"),
Path.Combine (AndroidSdkPath, "tools", "bin"),
};
Expand Down
9 changes: 9 additions & 0 deletions src/Xamarin.Android.Build.Tasks/Tasks/ResolveSdksTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ public class ResolveSdks : AndroidTask

public string [] ReferenceAssemblyPaths { get; set; }

public string CommandLineToolsVersion { get; set; }

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

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

Expand Down Expand Up @@ -94,6 +99,10 @@ public override bool RunTask ()
AndroidSdkPath = MonoAndroidHelper.AndroidSdk.AndroidSdkPath;
JavaSdkPath = MonoAndroidHelper.AndroidSdk.JavaSdkPath;

CommandLineToolsPath = MonoAndroidHelper.AndroidSdk.GetCommandLineToolsPaths (CommandLineToolsVersion)
.FirstOrDefault () ??
Path.Combine (AndroidSdkPath, "tools");

if (string.IsNullOrEmpty (AndroidSdkPath)) {
Log.LogCodedError ("XA5300", Properties.Resources.XA5300_Android_SDK);
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<YieldDuringToolExecution Condition="'$(YieldDuringToolExecution)' == ''">true</YieldDuringToolExecution>
<LatestSupportedJavaVersion Condition="'$(LatestSupportedJavaVersion)' == ''">1.8.0</LatestSupportedJavaVersion>
<MinimumSupportedJavaVersion Condition="'$(MinimumSupportedJavaVersion)' == ''">1.6.0</MinimumSupportedJavaVersion>
<AndroidCommandLineToolsVersion Condition=" '$(AndroidCommandLineToolsVersion)' == '' ">1.0</AndroidCommandLineToolsVersion>
<AndroidVersionCodePattern Condition=" '$(AndroidUseLegacyVersionCode)' != 'True' And '$(AndroidVersionCodePattern)' == '' ">{abi}{versionCode:D5}</AndroidVersionCodePattern>
<AndroidResourceGeneratorTargetName>UpdateGeneratedFiles</AndroidResourceGeneratorTargetName>
<AndroidUseAapt2 Condition=" '$(AndroidUseAapt2)' == '' ">True</AndroidUseAapt2>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -692,10 +692,6 @@ because xbuild doesn't support framework reference assemblies.

<!-- Misc paths -->

<CreateProperty Value="$(_AndroidSdkDirectory)tools\">
<Output TaskParameter="Value" PropertyName="_AndroidToolsDirectory"/>
</CreateProperty>

<CreateProperty Value="$(_AndroidSdkDirectory)platform-tools\">
<Output TaskParameter="Value" PropertyName="_AndroidPlatformToolsDirectory"/>
</CreateProperty>
Expand Down Expand Up @@ -2991,6 +2987,7 @@ because xbuild doesn't support framework reference assemblies.
<Error Text="AndroidManifest file does not exist" Condition="'$(_ProjectAndroidManifest)'!='' And !Exists ('$(_ProjectAndroidManifest)')"/>
<CalculateProjectDependencies
TargetFrameworkVersion="$(TargetFrameworkVersion)"
CommandLineToolsVersion="$(AndroidCommandLineToolsVersion)"
ManifestFile="$(_ProjectAndroidManifest)"
BuildToolsVersion="$(AndroidSdkBuildToolsVersion)"
PlatformToolsVersion="$(AndroidSdkPlatformToolsVersion)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ projects. .NET 5 projects will not import this file.
AndroidApplication="$(AndroidApplication)"
AndroidSdkPath="$(_AndroidSdkDirectory)"
AndroidSdkBuildToolsVersion="$(AndroidSdkBuildToolsVersion)"
CommandLineToolsVersion="$(AndroidCommandLineToolsVersion)"
UseLatestAndroidPlatformSdk="$(AndroidUseLatestPlatformSdk)"
AndroidUseAapt2="$(AndroidUseAapt2)"
AotAssemblies="$(AotAssemblies)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,12 @@ projects.
</PropertyGroup>
<ResolveSdks
ContinueOnError="$(_AndroidAllowMissingSdkTooling)"
CommandLineToolsVersion="$(AndroidCommandLineToolsVersion)"
AndroidSdkPath="$(AndroidSdkDirectory)"
AndroidNdkPath="$(AndroidNdkDirectory)"
JavaSdkPath="$(JavaSdkDirectory)"
ReferenceAssemblyPaths="$(_XATargetFrameworkDirectories)">
<Output TaskParameter="CommandLineToolsPath" PropertyName="_AndroidToolsDirectory" />
<Output TaskParameter="AndroidNdkPath" PropertyName="AndroidNdkDirectory" Condition=" '$(AndroidNdkDirectory)' == '' " />
<Output TaskParameter="AndroidSdkPath" PropertyName="AndroidSdkDirectory" Condition=" '$(AndroidSdkDirectory)' == '' " />
<Output TaskParameter="JavaSdkPath" PropertyName="JavaSdkDirectory" Condition=" '$(JavaSdkDirectory)' == '' " />
Expand Down

0 comments on commit 3426b32

Please sign in to comment.