NuGet / Home

Repo for NuGet Client issues
Other
1.5k stars 252 forks source link

Feature request: Override selection of "nearest" framework #7416

Open MV10 opened 6 years ago

MV10 commented 6 years ago

The Problem

Now that the .NET ecosystem has overlapping feature-sets between .NET Framework, .NET Standard, and .NET Core, it is no longer correct to blindly assume the "nearest" framework target is what the developer requires. It is impossible for NuGet to always make the correct decision. At best, it is difficult and perhaps sometimes impossible for a package author to work around this forced selection to allow package consumers to target desired feature-sets.

Real-World Example

Don't get too hung up on the specifics of the following example. I've run across this problem in other scenarios in the past, but this illustrates the problem very clearly -- and happens to be a real-world example that I (as a package author) am facing today that is causing real problems for some of my 250,000+ package users -- including myself as a package user!

Consider a multi-targeted library that supports two approaches to configuration: the older-style System.Configuration classes (e.g. XML-based app.config or web.config, originally only in .NET Framework, but then added to .NET Core 2.0, yet not part of any .NET Standard definition); as well as the newer .NET Standard Microsoft.Extensions.Configuration packages (aka M.E.C).

Everyone knows the best-practices approach for authoring libraries is to target the lowest possible versions of any dependencies. Because there are only two configuration scenarios, the project behind the package should only require two sets of TFMs and the corresponding code variations. The minimum TMFs supporting System.Configuration would be net452 and netcoreapp2.0, and the minimum TFM supporting M.E.C would be netstandard2.0.

In today's world, there is no ambiguity if you reference that package from a net452 library or app, or a netstandard2.0 library. But if you reference it from a netcoreapp2.0 app or library, you're locked into the System.Configuration variant, there is no way to use the netstandard2.0 variant. Similarly, if you reference it from a net461 or newer library or app, you're locked into the M.E.C variant, there's no way to choose the System.Configuration variant.

Inadequate Work-Arounds

In that specific example, the two configuration methods are technically not incompatible. I could hack together some third TFM group for net461 and netcoreapp2.0 that supported both, although the real-world fact is that this is extremely sloppy. System.Configuration works transparently, whereas M.E.C is an external package, so configuration by that approach requires the package consumer to obtain and provide a reference to a configuration object. There are many other package-specific complications not worth getting into here. Suffice to say it's ugly and obviously a hack, at best.

I haven't done exhaustive research of the tens of thousands of APIs available, but I assume there are also scenarios where the TFM variations are not compatible, which leaves package authors with no possibility of a work-around, no matter how sloppy or ugly they're willing to make their interfaces.

I don't mean to editorialize, but I did label this section "inadequate work-arounds"... Lately it seems as if the default Microsoft response to many NuGet questions and issues is to just hack up the package locally and effectively abandon the whole reason we use packaging in the first place. I find that suggestion as appalling as it is unrealistic, and in professional / corporate scenarios it's almost guaranteed to break the rules or raise serious red flags, especially if you're subject to a complex integration pipeline, locked-down package sources like MyGet, and so on. It's just bad advice no matter how you look at it.

Feature Request

Fortunately, this seems like an easy problem to fix.

In the VS NuGet GUI, simply adding radio buttons alongside the list of build targets shown under "Dependencies" ought to be sufficient.

Behind the scenes, this would obviously require a change to configuration syntax, and I assume some build support and new CLI switches (I'm old enough to remember when CLIs were hated and a good GUI was preferred, and I still avoid CLIs whenever possible, so I don't have any specific proposals there).

In terms of configuration support, it seems it could be as simple as accepting an optional TFM as part of the PackageReference node. Resolving the example discussed earlier would look like this:

<PropertyGroup>
  <TargetFramework>netcoreapp2.0</TargetFramework>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="ConfigurableLibrary" Version="1.0.0" TargetFramework="netstandard2.0" />
</ItemGroup>

I'm also unclear about possible impacts for other dependencies (RIDs? platforms like x86 vs x64?). I don't have much need for those so I'd have to research it -- but I assume the NuGet teammembers would have a good idea of any related impacts. I'd even consider PRing this one myself except that I don't clear picture of the probably-many repos involved in terms of build support, csproj support, and so on. I imagine that's the nature of NuGet changes in general, even though this appears to be very straightforward as a concept.

This ran a bit long, and I appreciate the time anyone invests in giving it consideration.

mattleibow commented 5 years ago

I totally second this - and I just cam across this very issue when trying to make use of Xaml islands. I tried to install the Xamarin.Forms package into the WPF project, but I had no way to tell NuGet to install the uap libraries instead of the netstandard. As a result, I did not get what I wanted.

This will become increasingly harder as the frameworks overlap - WPF/WinForms/net4x can pull in net4x AND uap assets (Xaml islands), netcoreapp can pull in netstandard AND net4x assets (WPF on .NET Core).

kzu commented 5 years ago

I don't think this is reasonable. As a package author, you have to make decisions for your users.

The minimum TMFs supporting System.Configuration would be net452 and netcoreapp2.0, and the minimum TFM supporting M.E.C would be netstandard2.0.

You have just made the decision that netcoreapp2.0 must use System.Configuration. If you want users to use the M.E.C. version, just provide a different package with that "provider". That's how users make their choice today. I think that's sufficient.

@mattleibow I'd say the Xaml islands part of XF should be available as a separate package too, maybe Xamarin.Forms.XamlIslands? This would allow users to opt-in explicitly.

It might be a useful feature still, showing in the NuGet UI that for your current project TargetFramework, there is more than one package lib you can choose, but how will users know which one to pick? You'd need far more than just a dropdown, since you will need to make an informed decision: you'll need a readme/description at least, maybe more. At that point, it's just more intuitive for your users to install different packages which can have all this info too.

akoeplinger commented 3 years ago

Copying from https://github.com/NuGet/Home/issues/10937 as requested:

With the transition of Xamarin from Mono (== .NET Framework) to .NET Core based class libraries and TFMs we're hitting cases where a netcoreapp asset is selected instead of e.g. a netstandard one like before due to the new precedence rules.

This causes problems since some packages assumed that netcoreapp meant ".NET on desktop platforms" and did things that won't work on mobile like checking the existence of some files etc while the netstandard asset would work just fine.

It'd be good to have a way to somehow be able to override which asset is selected for a given package to have an escape hatch until the ecosystem gets updated.

zkat commented 3 years ago

@akoeplinger do you have any data on how many customers would need to use this as an escape hatch, specifically? I think we need more information on this pain point, because the potential side-effects of having something like this available might be significant on other compatibility consideration.

It's unlikely that we would be able to do anything about this for net6.0 GA, or even 6.0.200.

akoeplinger commented 3 years ago

I don't think we have data on this specific problem since it'd only manifest itself at runtime and most users aren't running the .NET6 previews. Adding @Redth in case he has more info.

ghost commented 3 years ago

This issue has been automatically marked as stale because it has been marked as requiring customer feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within another 14 days of this comment.

huoyaoyuan commented 3 years ago

Requiring this to use Rx/RxUI with WinUI in 5.0. When targeting 5.0-windows, Rx will unconditionally bring WinForms, which is incompatible with WinUI/arm64.

dotMorten commented 3 years ago

Here's the issue I'm currently facing: I have a package that supports netcoreapp3.1 for Windows, xamarinios for ios, and monoandroid10 for android. When referencing that package from .NET Maui, the net6.0-ios and net6.0-android will incorrectly pick the netcoreapp3.1 assembly instead of the ios and android assemblies. Being able to control which one is picked in the package reference would be very helpful.

Repro:

<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
      <TargetFrameworks>net6.0-ios;net6.0-android</TargetFrameworks>
   </PropertyGroup>
  <ItemGroup Condition="'$(TargetFramework)'=='net6.0-ios'">
    <PackageReference Include="Esri.ArcGISRuntime" Version="100.11.2" TargetFramework="xamarinios10" />
  </ItemGroup>
  <ItemGroup Condition="'$(TargetFramework)'=='net6.0-android'">
    <PackageReference Include="Esri.ArcGISRuntime" Version="100.11.2" TargetFramework="monoandroid60" />
  </ItemGroup>
</Project>

Error:

Error   NETSDK1136  The target platform must be set to Windows (usually by including '-windows' in the TargetFramework property) when using Windows Forms or WPF, or referencing projects or packages that do so.   MauiApp4    C:\Program Files\dotnet\sdk\6.0.100-preview.6.21355.2\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.DefaultItems.Shared.targets  250 
adirh3 commented 3 years ago

I believe this is a most as well, we are seeing many official NuGet packages pushing <UseWinForms> in case the TFM is net5-windowsX, this may cause compatibility issues as described above and huge increase in output -

Repro -

<Project Sdk="Microsoft.NET.Sdk">
    <PropertyGroup>
        <OutputType>Exe</OutputType>
        <TargetFramework>net5.0-windows10.0.19041</TargetFramework>
    </PropertyGroup>
    <ItemGroup>
        <PackageReference Include="System.Reactive" Version="5.0.0" />
    </ItemGroup>
</Project>

Then run dotnet publish -c release -r win10-x64 --self-contained, output size is 140MB (even when trying to trim) for an empty app that does not use WinForms, and only need to use WinRT APIs.

AraHaan commented 1 year ago

Any status on this?

aortiz-msft commented 1 year ago

@nkolev92 - Would you please help understand the comments and make a recommendation?

michael-hawker commented 1 year ago

We've just been bit by this as well. The Windows App SDK supports net6.0-windows10.0.17763.0. So, we have our library target that as well. However, our library also targets net7.0 for other Uno Platform targets like Web Assembly that don't have an OS-specific TFM.

A consuming app now targets net7.0-windows10.0.17763.0, we expected it to fallback to the appropriate OS related target of net6.0-windows10.0.17763.0 because that has all the Windows specific code. However, it does not and tries to grab the net7.0 base non-OS specific code first! 😢 This apparently was decided in the spec, but feel like based on the examples above has a lot of real-world consequences.

It seems like grabbing the next lowest OS specific TFM would be more beneficial in most cases, as a default. In either case, there needs to be a way to change this behavior.

It's not well documented about how this behavior works, I opened a doc bug for that here: https://github.com/dotnet/docs/issues/36845

A collegue of mine at least discovered these tools which are helpful, don't know how they found them though: https://nugettools.azurewebsites.net/

We wanted to try to multi-target to net6.0-windows10.0.17763.0;net7.0-windows10.0.17763.0 in our library to resolve this, but I don't think we can as net7.0-windows10.0.17763.0 isn't valid for the WindowsAppSDK package as it's only targeting net6.0-windows10.0.17763.0 at the moment.

davidxuang commented 10 months ago

We've just been bit by this as well. The Windows App SDK supports net6.0-windows10.0.17763.0. So, we have our library target that as well. However, our library also targets net7.0 for other Uno Platform targets like Web Assembly that don't have an OS-specific TFM.

A consuming app now targets net7.0-windows10.0.17763.0, we expected it to fallback to the appropriate OS related target of net6.0-windows10.0.17763.0 because that has all the Windows specific code. However, it does not and tries to grab the net7.0 base non-OS specific code first! 😢 This apparently was decided in the spec, but feel like based on the examples above has a lot of real-world consequences.

It seems like grabbing the next lowest OS specific TFM would be more beneficial in most cases, as a default. In either case, there needs to be a way to change this behavior.

It's not well documented about how this behavior works, I opened a doc bug for that here: dotnet/docs#36845

A collegue of mine at least discovered these tools which are helpful, don't know how they found them though: https://nugettools.azurewebsites.net/

We wanted to try to multi-target to net6.0-windows10.0.17763.0;net7.0-windows10.0.17763.0 in our library to resolve this, but I don't think we can as net7.0-windows10.0.17763.0 isn't valid for the WindowsAppSDK package as it's only targeting net6.0-windows10.0.17763.0 at the moment.

Is AssetTargetFallback a solution for this? (though it's a legacy feature)

dotMorten commented 10 months ago

net7.0-windows10.0.17763.0 isn't valid for the WindowsAppSDK package as it's only targeting net6.0-windows10.0.17763.0

That shouldn't be an issue. I have packages that does this just fine. I think based on the current behavior you shouldn't author a package with a net7 target and have a lower platform specific target too.

jzabroski commented 4 months ago

As mentioned in duplicate #13554, the inability to override selection of "nearest" framework causes issues when debugging ASP.NET Classic applications with JetBrains Resharper 2024.1.1. The symptom is the repeated Debug Output message, "The parameters to the Event method do not match the parameters to the WriteEvent method. This may cause the event to be displayed incorrectly.", which swamps the CPU and causes requests to take 1,000x longer (see screenshot at bottom). The root cause is that the System.Diagnostics.DiagnosticSource nuget package supports frameworks as low as net462, and the "nearest" framework generally chooses the net462 over netstandard2.0 when the executable assembly is net48, and the net462 version of the assembly does not have the same method signature as the netstandard2.0 version.

There are two, possibly three, workarounds for this performance issue.

  1. Uninstall JetBrains Resharper 2024.1.1 and insall JetBrains Resharper 2022.3.3.
  2. Use VS Code instead of Visual Studio 2022.
  3. @nkolev92 suggested (without detailed guidance) to "Consider https://learn.microsoft.com/en-us/nuget/consume-packages/package-references-in-project-files#generatepathproperty for immediate workaround." This suggestion is not mentioned anywhere in this issue, so copy-pasting it here - it would be great for @nkolev92 to expand upon how to use this as a workaround.

However, the ideal "workaround" would be for Microsoft to backpatch the net462 implementation of System.Diagnostics.DiagnosticSource to support the missing parameter to the Event method that exists in newer implementations of the same nuget package version.

I don't think this is reasonable. As a package author, you have to make decisions for your users.

-- @kzu

@kzu - My example with System.Diagnostics.DiagnosticSource messiness demonstrates why overriding the selection of "nearest" framework is reasonable. In any case, users are allowed to choose roll forward behavior and override Microsoft on various things, so there is plenty of precedence why this would be a "reasonable" feature. Honestly, I just want something like what Java has with OSGi and Eclipse pde/bnd/bndtools.

image

jzabroski commented 4 months ago

Is AssetTargetFallback a solution for this? (though it's a legacy feature)

-- @davidxuang

No, it is not. As discussed in my duplicate issue #13554, @zivkan notes the fundamental limitation of AssetTargetFallback - it only works if the infrastructure needs to fallback. You can not use AssetTargetFallback to force it to use a different, arbitrary TFM from an asset. Copy-pasting here:

I think the problem is that .NETCoreApp (.NET Core and .NET 5+) is not directly compatible with .NET Framework. The .NET SDK defines asset target fallback. However, since NuGet finds that netstandard2.1 asset, asset target fallback is not used, hence the .NET Framework runtime asset is NOT compatible. Your package needs netstandard2.1 runtime assets.

To see AssetTargetFallback in action, you need a package with only .NET Framework assets, then reference the package from a project targeting .NET Framework. NuGet will raise a warning that the package was used, but might not be fully compatible, because there are bunch of .NET Framework APIs that don't work on .NET (Core), and if the package uses any of those APIs, the app will crash at runtime, which is basically what different TFM versions and TFM compatibility is trying to avoid.

But to reiterate the point, in order to reenforce its understanding, Asset Target Fallback works in two passes. First, NuGet attempts to do asset selection without any asset target fallback. if any assets match, from any asset class (compile, runtime, content files, build) finds a compatible asset, then asset target fallback will NOT be used. Only when NONE of the asset classes find any matches, then NuGet will try asset target fallback.

Therefore, projects that are compatible with .NET Standard 2.1, they will not select .NET Framework assets in a different asset class. This is by design because .NET Framework and .NET Core App are not "guaranteed" to be compatible. In fact, there are known API and runtime differences.

Originally posted by @zivkan in https://github.com/NuGet/Home/issues/12972#issuecomment-1791260350

jzabroski commented 4 months ago

Soapbox:

Architecturally, it is worth noting what other, mature, highly successful ecosystems do to solve this problem. In Java, OSGi had two competing but complementary approaches: Eclipse PDE and bnd/bndtools. PDE is manifest-driven, whereas bndtools analyzes JVM bytecode to determine the actual API signatures required by dependencies, thus preventing runtime failures. PDE is what we have today in .NET land. What we are missing is bndtools for .NET. Then we would not need this artificial "nearest framework solution" as much. The criteria would simply be a classic linker-loader architecture:

  1. Linker: Find all packages that are ABI compatible in as far as the transitive references need to use those signatures.
  2. Loader: Generate an explicit manifest based on the package reference version range rules that also pass the input from the linker step.

Bonus would be if these two steps were self-describing, as that would enable hot plugins to be written with an AssemblyLoadContext framework.

The end-product would be that you would not end up in jams where your code simply doesn't work because somebody decided it wasn't worth the time to support a new feature in an older framework. The reason this is the ideal architecture is that it avoids problems like maven's shading plug-in, a variant of which was proposed for nuget in 2021 as an experimental feature, which requires resource transformers to support third party linker formats (link the mono linker's xml descriptor). In this solution, the resource transformer can be trivially bijective (one-to-one and onto). Having trivial resource transformations is such an obvious win.