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

Remove nuget frameworks dependency #2544

Closed
wants to merge 3 commits into from

Conversation

nohwnd
Copy link
Member

@nohwnd nohwnd commented Aug 27, 2020

ℹ️ Replaced by #4693

Description

Translate short and long framework names ourselves instead of depending on Nuget.Frameworks

@nohwnd
Copy link
Member Author

nohwnd commented Aug 27, 2020

@vritant24 this is what I need to do to replace Nuget.Frameworks. Are you able to validate that this would fix the LUT issue?

Maybe we should add additional logic that will create a netcoreapp like Framework object from any net* string as a fallback? that way when net5.1 or net6.0 are finally added we don't have to update the class as long as they are in the form of net. I think something similar is happening in the SDK because setting the TFM to net7.99.123 will produce: The current .NET SDK does not support targeting .NET Core 7.99.123. Either target .NET Core 5.0 or lower, or use a version of the .NET SDK that supports .NET Core 7.99.123.

Version = "4.6.2.0",
ShortName = "net462",
},
[".NETFramework,Version=v4.6.3"] = new Framework
Copy link
Member Author

Choose a reason for hiding this comment

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

Huh, missing .NETFramework 47*?

@nohwnd nohwnd marked this pull request as draft August 28, 2020 10:07
@vritant24
Copy link
Member

vritant24 commented Aug 28, 2020

I'm not sure of the entire information that the testplatform has access to, but it looks like currently it is operating on TargetFramework. There is also TargetFrameworkMoniker that is much more consistent across the different runtimes.

This project system doc elaborates on that reasoning. For instance in the .NET 5 upgrade, the TargetFramework for is net5.0 but the TargetFrameworkMoniker is .NetCoreApp,Version=5.0 and didn't change from .NetCore 3.1. This suggestion is also in the .NET 5 spec

@nohwnd
Copy link
Member Author

nohwnd commented Aug 31, 2020

@vritant24 thanks for the tip, but imho that is relevant only to msbuild and project properties, we don't use any of those. We look at TFM attribute in the assembly, because vstest console handles only built dlls.

@MarcoRossignoli
Copy link
Contributor

Looks like too stale to revive, I'm going to close(lot of conflicts and we bumped a lot of versions in these time frame).

@nohwnd nohwnd reopened this Jun 16, 2023
@nohwnd
Copy link
Member Author

nohwnd commented Jun 16, 2023

@MarcoRossignoli that merge does not look that bad, might have a look on it when I am back.

@nohwnd nohwnd self-assigned this Jun 16, 2023
@MarcoRossignoli
Copy link
Contributor

MarcoRossignoli commented Jun 19, 2023

@MarcoRossignoli that merge does not look that bad, might have a look on it when I am back.

Yep doesn't look so bad and we should catch issue with new tfm really soon in the dogfooding/insertion process.

cc: @sharwell

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.

None yet

5 participants