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

Recognize all supported versions of the .NET Framework 4.x #715

Merged
merged 3 commits into from
Dec 28, 2019

Conversation

CharliePoole
Copy link
Collaborator

Fixes #713

rprouse
rprouse previously approved these changes Dec 16, 2019
Copy link
Member

@rprouse rprouse left a comment

Choose a reason for hiding this comment

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

Looks good. I'd love to know who uses this feature though 😄

@CharliePoole
Copy link
Collaborator Author

I wondered about that... for a long time we only distinguished net-4.0 and net-4.5, with the latter actually meaning "whatever version of 4.x you happen to have installed." I remember back when emulating 4.0 behavior under 4.5 was important to some people, causing us to make that distinction, but nobody has ever actually asked for this. OTOH, if we start checking TargetFrameworkAttribute, as @ChrisMaddoc has discussed, we'll find things like ".NETFramework,Version=v4.7.2" and end up saying we can't run the code without this fix.

Copy link
Member

@ChrisMaddock ChrisMaddock left a comment

Choose a reason for hiding this comment

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

Thanks Charlie! Spotted a minor type, and got a couple of clarification questions - just keen to better understand what our current limitations are here. 🙂

this.FrameworkVersion = new Version(1, 0);
return Runtime == RuntimeType.Mono && clrVersion.Major == 1
? new Version(1, 0)
: new Version(clrVersion.Major, clrVersion.Minor);
Copy link
Member

Choose a reason for hiding this comment

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

Am I reading this right, that this is to detect Mono 1.0? This is the sort of complexity I'd love to strip out, in a future iteration! 😅

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

in principle, the CLR version doesn't necessarily have obvious relation to the framework version. You could have SomeFramework 1.0 with a CLR of 5.1234.5678. Mono was merely the first place where this showed up - Mono 1.0 is essentially equivalent to MS .NET 1.1.

Easiest way to simplify IMO is to figure out how to do without the CLR version entirely. In the meantime, I'm thinking of a change in my own engine where the individual Runtimes figure out the CLR version, so e.g. Mono.Runtime would have a method that allowed deducing the CLR version from the framework version. Otherwise, this code will get a lot more complicated for .NET Core and other runtimes we add.

…otNetFrameworkLocator.cs

Co-Authored-By: Chris Maddock <chrismaddock@live.co.uk>
Copy link
Member

@ChrisMaddock ChrisMaddock left a comment

Choose a reason for hiding this comment

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

Great, thanks Charlie

@ChrisMaddock ChrisMaddock merged commit 44101c9 into master Dec 28, 2019
@ChrisMaddock ChrisMaddock deleted the issue-713 branch December 28, 2019 17:46
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.

Engine will not recognize .NET Framework versions beyond 4.5
3 participants