-
Notifications
You must be signed in to change notification settings - Fork 746
Add NU1703 warning for packages using deprecated MonoAndroid framework #7229
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.
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
base: dev
Are you sure you want to change the base?
Changes from 6 commits
5d9755a
e4cb623
2995ad0
f1ae34f
a7d4667
e2c1ce2
a553bef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| // Copyright (c) .NET Foundation. All rights reserved. | ||
| // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
|
||
| using System; | ||
| using NuGet.Frameworks; | ||
| using NuGet.ProjectModel; | ||
|
|
||
| namespace NuGet.Commands | ||
| { | ||
| /// <summary> | ||
| /// Detects when a package uses the deprecated MonoAndroid framework instead of net6.0-android or later. | ||
| /// This warning is gated on .NET 11 SDK (SdkAnalysisLevel >= 11.0.100) and targeting net11.0-android or later. | ||
| /// </summary> | ||
| internal static class MonoAndroidDeprecation | ||
| { | ||
| /// <summary> | ||
| /// Determines whether the MonoAndroid deprecation check should be performed for the given project and target framework. | ||
| /// </summary> | ||
| /// <param name="project">The package spec containing restore metadata.</param> | ||
| /// <param name="framework">The target framework of the current graph.</param> | ||
| /// <returns>True if the deprecation check should be performed.</returns> | ||
| internal static bool ShouldCheck(PackageSpec project, NuGetFramework framework) | ||
| { | ||
| if (project.RestoreMetadata == null) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| // Gate on SDK analysis level >= 11.0.100 | ||
| if (!SdkAnalysisLevelMinimums.IsEnabled( | ||
| project.RestoreMetadata.SdkAnalysisLevel, | ||
| project.RestoreMetadata.UsingMicrosoftNETSdk, | ||
| SdkAnalysisLevelMinimums.V11_0_100)) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| // Only check for .NETCoreApp frameworks targeting android with version >= 11.0 | ||
| return StringComparer.OrdinalIgnoreCase.Equals(framework.Framework, FrameworkConstants.FrameworkIdentifiers.NetCoreApp) | ||
| && framework.Version.Major >= 11 | ||
| && framework.HasPlatform | ||
| && framework.Platform.Equals("android", StringComparison.OrdinalIgnoreCase); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if NuGet is the right place to check for this. Can this logic be moved to the Android workload instead? It's probably easiest to do in NuGet, but this is code that will run for every restore, even when the Android workload isn't installed. When I take a binlog of a normal build, the .NET SDK reads NuGet's assets file in the It's not as precise as checking the Another design idea is to change the assets file to emit a "framework" metadata for every asset, and change the SDK's Given restore almost never breaks backwards compatibility, I just want to make sure that NuGet isn't stuck with this "forever" when it's fundamentally a non-NuGet concern. So, trying to do due diligence in checking if the owning component can feasibly implement the check, rather than NuGet.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not familiar enough with the design of the workloads to know whether it would make sense to handle it there. @jonathanpeppers what do you think? My instinct is that this should warn during restore, not during build, which is why I thought it made the most sense to handle it in NuGet. Plus there was precedent in the (now removed) Xamarin.iOS warning. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The design says to emit this warning here in NuGet (but note the code was NU1701 back then): I'm not aware of us running any MSBuild logic during restore from a workload. If we had to do it from a workload, it would be easiest to do during a build -- which is maybe not what we'd want. |
||
| } | ||
|
|
||
| /// <summary> | ||
| /// Checks whether the given framework is a MonoAndroid framework. | ||
| /// </summary> | ||
| /// <param name="framework">The framework to check, or null.</param> | ||
| /// <returns>True if the framework uses the MonoAndroid framework identifier.</returns> | ||
| internal static bool IsMonoAndroidFramework(NuGetFramework framework) | ||
| { | ||
| return framework != null | ||
| && StringComparer.OrdinalIgnoreCase.Equals( | ||
| framework.Framework, | ||
| FrameworkConstants.FrameworkIdentifiers.MonoAndroid); | ||
| } | ||
| } | ||
| } | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.