Skip to content
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

[TokenCache] Downgrade dependency version for Microsoft.Extensions on net472 #1663

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

stephenjust
Copy link

@stephenjust stephenjust commented Mar 21, 2022

Fixes #1662

Microsoft.Extensions.* 5.0 removed a component which breaks Kestrel when running on Net Framework.

I'm only adjusting the net472 target because net462 users typically wouldn't be using Kestrel due to some challenges with netstandard packages.

Microsoft.Extensions.* 5.0 removed a component which breaks Kestrel when running on Net Framework.

I'm only adjusting the net472 target because net462 users typically wouldn't be using Kestrel due to some challenges with netstandard packages.
@stephenjust stephenjust changed the title Downgrade dependency version for Microsoft.Extensions on net472 [TokenCache] Downgrade dependency version for Microsoft.Extensions on net472 Mar 21, 2022
<PackageReference Include="Microsoft.Extensions.Caching.Memory" Version="5.0.0" />
<PackageReference Include="Microsoft.Extensions.Logging" Version="5.0.0" />
<PackageReference Include="Microsoft.AspNetCore.DataProtection" Version="5.0.8" />
<PackageReference Include="System.Text.Encodings.Web" Version="5.0.1" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0' or '$(TargetFramework)' == 'net472'">
Copy link
Member

@bgavrilMS bgavrilMS Mar 22, 2022

Choose a reason for hiding this comment

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

@jennyf19 @jmprieur @pmaytak - this package has no conditional compilation at all, so the only reason we have all these frameworks it to be able to have different dependencies, i.e. different versions of dependencies.

In particular, net462 and net472 seem identical?

If we get net4x2 to have the same dependencies as netstandard, we might as well eliminate net4x2 completely.

Copy link
Member

@bgavrilMS bgavrilMS Mar 22, 2022

Choose a reason for hiding this comment

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

I don't remember the details of the investigation into the target framework, but I see that some packages try to simplify things into supporting only netstandard2.0.
https://www.nuget.org/packages/Microsoft.Extensions.Configuration.AzureKeyVault/

Copy link
Author

Choose a reason for hiding this comment

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

I will note that while I was trying to assemble a simple repro of this issue, a project on net462 used the netstandard dependency set for some reason, while net472 used the expected net472 dependencies. I didn't look into it too deeply though, my org's projects are all net472.

Copy link
Collaborator

@jmprieur jmprieur left a comment

Choose a reason for hiding this comment

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

LGTM
I agree that net472 brings netstandard20
Thanks @stephenjust

cc: @GeoK

@jayesh-a-shah
Copy link

Can you please make this change for net462 as well?

@jmprieur
Copy link
Collaborator

jmprieur commented Jun 26, 2024

Can you please make this change for net462 as well?

What would be the reason, @jayesh-a-shah ? We carefully chose the dependencies, and unless there is a blocker with repro steps, I'm not going to accept that they are changed.

@stephenjust @jayesh-a-shah out of curiosity, did you try to bump-up these extensions to version 8? this is what the .NET team recommends now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Dependencies in Net Framework TokenCache package break Kestrel on Net Framework
6 participants