-
Notifications
You must be signed in to change notification settings - Fork 241
Get project target framework from msbuild #3433
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
base: main
Are you sure you want to change the base?
Conversation
| var frameworks = msbuildOutput.Properties.TargetFrameworks | ||
| .Split(';') | ||
| .Where(x => x.StartsWith("net") || x.StartsWith("netstandard")) | ||
| .OrderBy(ParseFrameworkVersion) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the parse fails, it returns 0.0, which will come before any valid version. Is that intentional?
| { | ||
| var frameworks = msbuildOutput.Properties.TargetFrameworks | ||
| .Split(';') | ||
| .Where(x => x.StartsWith("net") || x.StartsWith("netstandard")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parsing the TargetFramework is not robust. For example, these are all valid values:
net9.0net90(would be considered version 90.0)net9000(would be considered version 9000.0)net9.0.0.0(does the version class here support four parts)NET9.0(would be filtered out by case-sensitive checks earlier)
In this case, it only impacts projects that multi-target.
To do this correctly involves more steps. We need to take each of the values from TargetFrameworks, then call MSBuild again for each. The properties that should be used here are TargetFrameworkIdentifier and TargetFrameworkVersion. These are the "real" values that define what framework the project uses. The TargetFramework is just an alias and can be anything.
For example, given project:
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>net9.0;netstandard2.1</TargetFrameworks>
</PropertyGroup>
</Project>The following command will give you a reliable version:
$ dotnet msbuild -p:TargetFramework="netstandard2.1" -getProperty:"TargetFrameworkIdentifier;TargetFrameworkVersion" MyProject.csproj
{
"Properties": {
"TargetFrameworkIdentifier": ".NETStandard",
"TargetFrameworkVersion": "v2.1"
}
}
$ dotnet msbuild -p:TargetFramework="net9.0" -getProperty:"TargetFrameworkIdentifier;TargetFrameworkVersion" MyProject.csproj
{
"Properties": {
"TargetFrameworkIdentifier": ".NETCoreApp",
"TargetFrameworkVersion": "v9.0"
}
}
So, I'd recommend keeping the first call as:
$ dotnet msbuild -getProperty:"TargetFramework;TargetFrameworks" MyProject.csproj
{
"Properties": {
"TargetFramework": "",
"TargetFrameworks": "net9.0;netstandard2.1"
}
}
Then you have the following decisions:
- If
TargetFrameworkhas a value, the project only has one target and you can return it directly. - If
TargetFrameworkshas a single value (which can happen) you can return it directly. - Otherwise, split
TargetFrameworksand for each, invoke the command passing-p:TargetFramework="<value>"and-getProperty:"TargetFrameworkIdentifier;TargetFrameworkVersion". This will give you pairs of identifier/version values to process. If you want to keep the same logic, you can filter out anything that's not.NETStandardor.NETCoreApp, then order by the parsed versions (removing the leadingv).
|
In the case that the version is not supported, I think it'd be nice to not show the user a stack trace. Stack traces are usually shown when something internal goes wrong. In this case, the problem is the user input data (their project), and the tooling should handle all kinds of invalid input in as graceful and helpful way as possible. |
Ok! I will update to a log and exit instead of a throw here. |
|
updated so log instead of throw when there is a unsupported target framework in the project. See the description for the updated image of what this will look like |
| else | ||
| { | ||
| throw new NotSupportedException($"Target framework '{targetFramework}' is not supported."); | ||
| logger?.LogError("Target framework '{TargetFramework}' is not supported. Installing recent release of '{PackageName}'. Consider upgrading your target framework to install a compatible package version.", targetFramework, package.Name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies if I've asked this before, but does this need to be localized?
What does "Installing recent release of ..." mean in this context? I'm not 100% clear on this error message.
first commit:
dotnet msbuildto get targetframework(s)second commit:
third coimmit:
when project has target framework not supported (supports net 8, 9, 10):
