toddams / RazorLight

Template engine based on Microsoft's Razor parsing engine for .NET Core
Apache License 2.0
1.51k stars 261 forks source link

2.0.0-rc.1 Targeting .NETStandard 2.0 doesn't allow upgrading of v5 of framework libraries #390

Closed Pete-PlaytimeSolutions closed 3 years ago

Pete-PlaytimeSolutions commented 3 years ago

.NETStandard 2.0 dependencies do not allow upgrading beyond V 3.0.0

e.g. Microsoft.Extensions.Caching.Abstractions 5.0.0 is disallowed, even though this version of the library should be available for .NETStandard 2.0

Expected behavior Should be allowed to upgrade dependent packages like Microsoft.Extensions.Caching.Abstractions to version 5.0.0

jzabroski commented 3 years ago

Hi, If I understand your point, https://github.com/toddams/RazorLight/blob/master/src/RazorLight/RazorLight.csproj#L21-L33

disallows the following project structure:

[net5.0 TFM csproj] (executable)
 |
 |-- [netstandard2.0 TFM csproj] (library)
    |-- package references
       |-- RazorLight-2.0.0-rc.1
       |-- Microsoft.Extensions.Caching.Abstractions 5.0.0

as well as

[net5.0 TFM csproj] (executable)
 |
 |-- [netstandard2.0 TFM csproj] (library)
    |-- package references
       |-- RazorLight-2.0.0-rc.1
 |-- package references
    |-- Microsoft.Extensions.Caching.Abstractions 5.0.0

because Nuget will detect a version conflict in that RazorLight caps the version number on dependencies at at UP TO 3.0

To which I would ask - can you switch the netstandard2.0 csproj to use net5.0?

Pete-PlaytimeSolutions commented 3 years ago

Ideally, I need my csproj to be able to target both netstandard2.0 and net5.0, otherwise I cannot upgrade any of my other 50 odd libraries to use v 5.0.0.0 of the Microsoft.Extensions.* libraries, or have an incomplete set...

jzabroski commented 3 years ago

Why can't you use <TargetFrameworks>netstandard2.0;net5.0</TargetFrameworks> and then add conditions downstream?

Can you explain why you need to target netstandard2.0? I just don't know every combination people deploy RazorLight with and its these unexpected combinations that are extremely hard to regression test against.


BELOW MIGHT BE AN INCORRECT ANALYSIS ON MY PART. IT IS MY ATTEMPT TO TRY TO OUTLINE BEST PRACTICES FOR MANAGING DEPENDENCIES USING THE NEW CSPROJ SDK FILE FORMAT

To elaborate a bit, using netstandard2.0 at all may have been a mistake with good intentions. The original theory behind netstandard2.0 was to be able to write your code once against a common API and then have common compatibility moving forward with all "in box" .NET APIs as well as a few "out of box" APIs. However, the problem with this is that RazorLight doesn't depend just on a set of APIs but on a FrameworkReference and those FrameworkReference dependencies have their own minimum and maximum support capabilities baked into the IL instructions (they expect certain function calls to be there, etc.).

The good intentions was to give someone the opportunity to target netstandard2.0 if they had minimal dependencies. But in practice, everyone is including things that are transitively included by the FrameworkReference, so you end up in a situation where you get conflicts.

jzabroski commented 3 years ago

I think I understand your problem.

You have this:

[netstandard2.0 TFM csproj] (library)
    |-- package references
       |-- RazorLight-2.0.0-rc.1
       |-- 50 other nuget packages TRANSITIVELY targeting netstandard2.0 5.0.0 libraries like Microsoft.* whatever
       |-- Microsoft.Extensions.Caching.Abstractions 5.0.0

And you have interdependencies between RazorLight and those other packages (e.g., tag helpers, whatever). So it's non trivial to segregate RazorLight from everything else and put it into a net5.0 library so that the late binding of dependencies agrees on/unifies to a common nuget package version.

Is that correct?

jzabroski commented 3 years ago

Can you share the csproj. I really just need the TargetFramework(s) and PackageReferences.

Pete-PlaytimeSolutions commented 3 years ago

Targeting both is what I am doing, across a suite of about 50 different projects, that roll out as a set. By being restricted to UP TO V 3.0 by including RazorLight-2.0.0-rc.1, it prevents me being able to include the single Communications library, as part of the rest of my suite of packages.

If I am understanding correctly, the RazorLight package actually requires a FrameworkReference. If that is the case, that's fine, I can work around that.

I'll just change my communications package to target that various frameworks, rather than the more generic netstandard2.0... I was just hoping to be able to have the flexibility to chop and change a bit more.. ;)

PS I think its a great library!

jzabroski commented 3 years ago

@Pete-PlaytimeSolutions I tried to figure out how to make it as generic as possible, but it's quite complicated. I ran into a bunch of unexpected things, and have been trying to keep track of a list of potholes to keep my head straight . My above description of what I think is happening I believe to be correct, but I also don't have the precise info as to why readily available (I can guess at why, but would need to build precise repros to demonstrate counterexamples where a configuration of dependencies you'd expect to work, does not). RazorLight is easily the most complicated dependency I have in all my libraries. That's also why I want to add CefSharp to build out some integration tests to verify some complex dependencies scenarios.

See: https://github.com/jzabroski/Home/issues/5 for my hobby horse of trying to better understand how to support dependencies in .NET better. Feedback welcome.

Pete-PlaytimeSolutions commented 3 years ago

Maybe just a looser version range for the generic netstandard2.0 target, (i.e. without specifying the max version) could provide a more flexible setup... The max version restriction, is probably only relevant when targeting a specific FrameworkReferences...

e.g. `

<PackageReference Include="Microsoft.AspNetCore.Hosting.Abstractions" Version="2.1.0" />
<PackageReference Include="Microsoft.AspNetCore.Html.Abstractions" Version="2.1.0" />
<PackageReference Include="Microsoft.AspNetCore.Mvc.Razor.Extensions" Version="2.1.0" />
<PackageReference Include="Microsoft.AspNetCore.Razor.Language" Version="2.1.0" />
<PackageReference Include="Microsoft.AspNetCore.Razor.Runtime" Version="2.1.0" />
<PackageReference Include="Microsoft.Extensions.Caching.Abstractions" Version="2.1.0" />
<PackageReference Include="Microsoft.Extensions.Caching.Memory" Version="2.1.0" />
<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="2.1.0" />
<PackageReference Include="Microsoft.Extensions.DependencyModel" Version="2.1.0" />
<PackageReference Include="Microsoft.Extensions.FileProviders.Physical" Version="2.1.0" />
<PackageReference Include="Microsoft.Extensions.Primitives" Version="2.1.0" />
<PackageReference Include="System.Buffers" Version="4.5.0" />

`

That way any class libraries using RazorLight, really only care about the minimum version requirement. Only once the class library is then included in a hosting project (that specifies the framework version being targeted), is the version range enforced....

Vincentvwal commented 3 years ago

We are currently also running into this issue, When running the older beta versions it works, however we also use a personal shared nuget package which includes Microsoft.Extensions.Caching.Memory 3.1.1

So including this into our .net standard project currently this does not work. Personally i would also like for .net standard to only use the minimal version but not the maximum.

jzabroski commented 3 years ago

We can give it a shot and see whether it surfaces any issues

jzabroski commented 3 years ago

@vincentvwal for my knowledge, what target framework(s) consume your netstandard2.0 csproj

jzabroski commented 3 years ago

That way any class libraries using RazorLight, really only care about the minimum version requirement. Only once the class library is then included in a hosting project (that specifies the framework version being targeted), is the version range enforced....

I think it depends on how and whether a FrameworkReference places an implicit upper boundary on major versions. I am curious and will use dotPeek to see what the output is

We will see.

Vincentvwal commented 3 years ago

We mainly use .net Core 3.1 as consumers, however our project also consists or a public package (.net standard 2.0) which gets included in .net Framework 4.6.1. this public package does not contain the RazorLight package.

Our full references in out templating project are currently (not working due to packages from the rc.1) `

`

Our Utilities.Public contains the following

`

`

And for us Microsoft.Extensions.Caching.Memory is causing the issue.

jzabroski commented 3 years ago

Thanks for that info. Replying by email because something in your formatting is causing FastHub mobile app to crash.

On Wed, Nov 18, 2020, 8:33 AM Vincentvwal notifications@github.com wrote:

We mainly use .net Core 3.1 as consumers, however our project also consists or a public package (.net standard 2.0) which gets included in .net Framework 4.6.1. this public package does not contain the RazorLight package.

Our full references in out templating project are currently (not working due to packages from the rc.1)

Our Utilities.Public contains the following

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/toddams/RazorLight/issues/390#issuecomment-729679639, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADNH7MIRXHTNZKA4G6TBXDSQPELBANCNFSM4TYXAL2Q .

jzabroski commented 3 years ago

Pushed 2.0.0-rc.2

jzabroski commented 3 years ago

Some final thoughts:

  1. FrameworkReference is fairly confusing, since they don't show up on nuget.org (I just realized this now).
  2. When you're in a netcoreapp3.1 context, netstandard2.0 should bind to Microsoft.Extensions.* packages in the [3.1,4) version range, because RollForward is Minor and you're referencing a FrameworkReference to Microsoft.AspNetCore.App which will resolve to 3.1 right now (I believe this is correct). This would also be why the current release broke, because it broke this flow of dependencies.
  3. When you're in a net5.0 context, netstandard2.0 should bind to Microsoft.Extensions.* packages in the [5.0,6) version range, because RollForward default is Minor and you're referencing a FrameworkReference to Microsoft.AspNetCore.App which will resolve to 5.0 right now.

However, the problem remains that you can run into runtime late binding issues if netstandard2.0 libraries deprecate some logic due to the fact we have pre-processor pragmas to handle higher framework versions. So, extra care has to be taken when defining these pragmas, as its the "Error of omission" that likely causes most run-time late binding errors.

Pete-PlaytimeSolutions commented 3 years ago

Thanks John, 2.0.0-rc.2 is working a treat!