Skip to content

[.NET 5] fix warning for $(TFV) of 5.0 #4598

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 1 commit into from
Apr 23, 2020
Merged

[.NET 5] fix warning for $(TFV) of 5.0 #4598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 12 additions & 3 deletions ...n.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.Tooling.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ called for "legacy" projects in Xamarin.Android.Legacy.targets.

<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

<UsingTask TaskName="Xamarin.Android.Tasks.ValidateJavaVersion" AssemblyFile="$(_XamarinAndroidBuildTasksAssembly)" />
<UsingTask TaskName="Xamarin.Android.Tasks.ResolveAndroidTooling" AssemblyFile="$(_XamarinAndroidBuildTasksAssembly)" />
<UsingTask TaskName="Xamarin.Android.Tasks.RuntimeIdentifierToAbi" AssemblyFile="$(_XamarinAndroidBuildTasksAssembly)" />
<UsingTask TaskName="Xamarin.Android.Tasks.CheckGoogleSdkRequirements" AssemblyFile="$(_XamarinAndroidBuildTasksAssembly)" />
<UsingTask TaskName="Xamarin.Android.Tasks.ResolveAndroidTooling" AssemblyFile="$(_XamarinAndroidBuildTasksAssembly)" />
<UsingTask TaskName="Xamarin.Android.Tasks.RuntimeIdentifierToAbi" AssemblyFile="$(_XamarinAndroidBuildTasksAssembly)" />
<UsingTask TaskName="Xamarin.Android.Tasks.ValidateJavaVersion" AssemblyFile="$(_XamarinAndroidBuildTasksAssembly)" />

<Target Name="_ResolveAndroidTooling">
<ValidateJavaVersion
Expand Down Expand Up @@ -61,4 +62,12 @@ called for "legacy" projects in Xamarin.Android.Legacy.targets.
</RuntimeIdentifierToAbi>
</Target>

<Target Name="_CheckGoogleSdkRequirements"
Condition="Exists('$(IntermediateOutputPath)android\AndroidManifest.xml') And '$(AndroidEnableGooglePlayStoreChecks)' == 'true' ">
<CheckGoogleSdkRequirements
ApiLevel="$(_AndroidApiLevel)"
ManifestFile="$(IntermediateOutputPath)android\AndroidManifest.xml"
/>
</Target>

</Project>
13 changes: 11 additions & 2 deletions src/Xamarin.Android.Build.Tasks/Tasks/CheckGoogleSdkRequirements.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,26 @@ public class CheckGoogleSdkRequirements : AndroidTask
{
public override string TaskPrefix => "CGS";

[Required]
/// <summary>
/// This will be blank for .NET 5 builds
/// </summary>
public string TargetFrameworkVersion { get; set; }

/// <summary>
/// This is used instead of TargetFrameworkVersion for .NET 5 builds
/// </summary>
public int ApiLevel { get; set; }
Copy link

@Nirmal4G Nirmal4G Apr 21, 2020

Choose a reason for hiding this comment

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

Suggested change
public int ApiLevel { get; set; }
public int TargetApiLevel { get; set; }

This is more clear in intent than the previous name.

Copy link
Contributor

@jonpryor jonpryor Apr 22, 2020

Choose a reason for hiding this comment

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

Should this instead be string TargetAndroidId (plus associated chicanery and conversions), so that it can “reasonably” support preview API levels, such as this year’s current R (and presumably next year’s S, and...)?

AndroidManifest.xml has historically allowed //uses-sdk/@android:targetSdkVersion to contain values such as R, and we’ve historically allowed $(TargetFrameworkVersion) to be e.g. v10.0.99 to use the preview API level, and we need some form of equivalent in .NET 5, especially since we’ll be losing $(TargetFrameworkVersion)...

Copy link
Member Author

@jonathanpeppers jonathanpeppers Apr 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@jonp for .NET 5 the incoming value here will be $(_AndroidApiLevel), which is currently coming from:

https://github.com/xamarin/xamarin-android/blob/3c94f6f9ae7902919620c1ca26d7ad77ff080620/src/Xamarin.Android.Build.Tasks/Tasks/ResolveAndroidTooling.cs#L188

I don't think we have a way to set this to an unstable API level yet in .NET 5, but the value would be set to 30 instead of R.

I think the name ApiLevel is OK, because we've used this for other tasks that take $(_AndroidApiLevel) as input:

https://github.com/xamarin/xamarin-android/blob/3c94f6f9ae7902919620c1ca26d7ad77ff080620/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets#L810

https://github.com/xamarin/xamarin-android/blob/3c94f6f9ae7902919620c1ca26d7ad77ff080620/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets#L1860

https://github.com/xamarin/xamarin-android/blob/3c94f6f9ae7902919620c1ca26d7ad77ff080620/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets#L2366

A couple older tasks use the name AndroidSdkPlatform, though.


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

public override bool RunTask ()
{
ManifestDocument manifest = new ManifestDocument (ManifestFile);

var compileSdk = MonoAndroidHelper.SupportedVersions.GetApiLevelFromFrameworkVersion (TargetFrameworkVersion);
var compileSdk = string.IsNullOrEmpty (TargetFrameworkVersion) ?
ApiLevel :
Copy link

@Nirmal4G Nirmal4G Apr 21, 2020

Choose a reason for hiding this comment

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

Suggested change
ApiLevel :
TargetApiLevel :

MonoAndroidHelper.SupportedVersions.GetApiLevelFromFrameworkVersion (TargetFrameworkVersion);

if (!int.TryParse (manifest.GetMinimumSdk (), out int minSdk)) {
minSdk = 1;
Expand Down
1 change: 1 addition & 0 deletions src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/XASdkTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public void DotNetPublish (string runtimeIdentifier, bool isRelease)

var dotnet = CreateDotNetBuilder (proj);
Assert.IsTrue (dotnet.Publish (), "`dotnet publish` should succeed");
Assert.IsTrue (StringAssertEx.ContainsText (dotnet.LastBuildOutput, " 0 Warning(s)"), "Should have no MSBuild warnings.");

var apk = Path.Combine (Root, dotnet.ProjectDirectory, proj.OutputPath,
runtimeIdentifier, "UnnamedProject.UnnamedProject.apk");
Expand Down
6 changes: 2 additions & 4 deletions src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/Builder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,9 @@ public class Builder : IDisposable
public IEnumerable<string> LastBuildOutput {
get {
if (!string.IsNullOrEmpty (buildLogFullPath) && File.Exists (buildLogFullPath)) {
foreach (var line in File.ReadLines (buildLogFullPath, Encoding.UTF8)) {
yield return line;
}
return File.ReadLines (buildLogFullPath, Encoding.UTF8);
}
yield return String.Empty;
return Enumerable.Empty<string> ();
}
}
public TimeSpan LastBuildTime { get; protected set; }
Expand Down
10 changes: 10 additions & 0 deletions src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/DotNetCLI.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Text;

namespace Xamarin.ProjectTools
Expand Down Expand Up @@ -84,6 +85,15 @@ public bool Publish (string target = null)
return Execute (arguments.ToArray ());
}

public IEnumerable<string> LastBuildOutput {
get {
if (!string.IsNullOrEmpty (BuildLogFile) && File.Exists (BuildLogFile)) {
return File.ReadLines (BuildLogFile, Encoding.UTF8);
}
return Enumerable.Empty<string> ();
}
}

List<string> GetDefaultCommandLineArgs (string verb, string target = null)
{
string testDir = Path.GetDirectoryName (projectOrSolution);
Expand Down
9 changes: 0 additions & 9 deletions src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved.
<UsingTask TaskName="Xamarin.Android.Tasks.CalculateAdditionalResourceCacheDirectories" AssemblyFile="Xamarin.Android.Build.Tasks.dll" />
<UsingTask TaskName="Xamarin.Android.Tasks.CalculateLayoutCodeBehind" AssemblyFile="Xamarin.Android.Build.Tasks.dll" />
<UsingTask TaskName="Xamarin.Android.Tasks.CalculateProjectDependencies" AssemblyFile="Xamarin.Android.Build.Tasks.dll" />
<UsingTask TaskName="Xamarin.Android.Tasks.CheckGoogleSdkRequirements" AssemblyFile="Xamarin.Android.Build.Tasks.dll" />
<UsingTask TaskName="Xamarin.Android.Tasks.CheckForRemovedItems" AssemblyFile="Xamarin.Android.Build.Tasks.dll" />
<UsingTask TaskName="Xamarin.Android.Tasks.CheckForInvalidResourceFileNames" AssemblyFile="Xamarin.Android.Build.Tasks.dll" />
<UsingTask TaskName="Xamarin.Android.Tasks.CompileToDalvik" AssemblyFile="Xamarin.Android.Build.Tasks.dll" />
Expand Down Expand Up @@ -550,14 +549,6 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved.
/>
</Target>

<Target Name="_CheckGoogleSdkRequirements"
Condition="Exists('$(IntermediateOutputPath)android\AndroidManifest.xml') And '$(AndroidEnableGooglePlayStoreChecks)' == 'true' ">
<CheckGoogleSdkRequirements
TargetFrameworkVersion="$(TargetFrameworkVersion)"
ManifestFile="$(IntermediateOutputPath)android\AndroidManifest.xml"
/>
</Target>

<!--
*******************************************
Application Build
Expand Down
11 changes: 10 additions & 1 deletion src/Xamarin.Android.Build.Tasks/Xamarin.Android.Legacy.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ projects. .NET 5 projects will not import this file.
</CleanDependsOn>
</PropertyGroup>

<UsingTask TaskName="Xamarin.Android.Tasks.Legacy.ValidateJavaVersion" AssemblyFile="Xamarin.Android.Build.Tasks.dll" />
<UsingTask TaskName="Xamarin.Android.Tasks.CheckGoogleSdkRequirements" AssemblyFile="Xamarin.Android.Build.Tasks.dll" />
<UsingTask TaskName="Xamarin.Android.Tasks.Legacy.ValidateJavaVersion" AssemblyFile="Xamarin.Android.Build.Tasks.dll" />
<UsingTask TaskName="Xamarin.Android.Tasks.Legacy.ResolveAndroidTooling" AssemblyFile="Xamarin.Android.Build.Tasks.dll" />

<Target Name="_GetReferenceAssemblyPaths">
Expand Down Expand Up @@ -245,4 +246,12 @@ projects. .NET 5 projects will not import this file.
</ItemGroup>
</Target>

<Target Name="_CheckGoogleSdkRequirements"
Condition="Exists('$(IntermediateOutputPath)android\AndroidManifest.xml') And '$(AndroidEnableGooglePlayStoreChecks)' == 'true' ">
<CheckGoogleSdkRequirements
TargetFrameworkVersion="$(TargetFrameworkVersion)"
ManifestFile="$(IntermediateOutputPath)android\AndroidManifest.xml"
/>
</Target>

</Project>