search

dotnet / sdk

Core functionality needed to create .NET Core projects, that is shared between Visual Studio and CLI
https://dot.net/core
MIT License
2.67k stars 1.06k forks source link

Windows TFM still with 3 parts, not 4, even when properly specified #14155

Open azchohfi opened 3 years ago

azchohfi commented 3 years ago

SDK: 5.0.100-rc.2.20479.15 MSBuild: 16.8.0-preview-20507-02+c2c2722df (v16.8.0-pre.4.0) csproj:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>net5.0-windows10.0.18362.0</TargetFramework>
  </PropertyGroup>
</Project>

msbuild /t:pack generated nuget: image

Its missing the .0 in the end.

azchohfi commented 3 years ago

This is causing issues if you manually add contents to the package, using Pack and PackagePath with $(TargetFramework). It creates both folders inside the Nuget, and when a project tries to use it, it wrongfully tries to copy from the wrong path: 1>C:\Program Files (x86)\Microsoft Visual Studio\2019\Preview\MSBuild\Current\Bin\Microsoft.Common.CurrentVersion.targets(4815,5): error MSB3030: Could not copy the file "C:\Users\alzollin\.nuget\packages\microsoft.toolkit.graph.controls\8.0.0-build.15.g19c10f9260\lib\net5.0-windows10.0.19041\Microsoft.Toolkit.Graph.Controls\Assets\person.png" because it was not found. That file is inside the package, but inside the .0 folder.

dsplaisted commented 3 years ago

@zkat @nkolev92 Is this expected?

nkolev92 commented 3 years ago

Do we know where the 0 is lost? This goes through MSBuild -> TargetPlatformMoniker -> NuGet which then uses it at restore time & later at pack time.

At this point NuGet normalizes the 3rd and 4th part when dealing with target framework/target platform versions.

Example:

Input Normalized name
net5.0.0 net5.0
net5.0-windows7.0 net5.0-windows7.0
net5.0-windows10.0.18362.0 net5.0-windows10.0.18362

https://nugettoolsdev.azurewebsites.net/5.8.0-preview.3.6823/parse-framework?framework=net5.0-windows10.0.18362.0 normalizes to net5.0-windows10.0.18362

Related issue here: https://github.com/NuGet/Home/issues/10063.

I do think the versions in general should get normalized (either by the SDK and/or by NuGet), but I think the gap that https://github.com/NuGet/Home/issues/10063 is important to fix.

nkolev92 commented 3 years ago

cc @zivkan

azchohfi commented 3 years ago

Any updates on this?

zivkan commented 3 years ago

If you're using a new enough .NET SDK, you should be able to use their new task to get the short folder name that NuGet generates for automatically added assets: https://github.com/dotnet/sdk/pull/13924/files#diff-a0bae50a43d919d6112f9c2de679520c7fdcb3060fe92081395bb315d4313193R78-R84. It probably makes more sense for NuGet to provide this task in our NuGet.Build.Tasks.Pack.targets, but the SDK already merged this change.

Otherwise, as Nikolche wrote, we need to work on NuGet/Home#10063, but it might just be documentation with examples on how to use the new GetNuGetShortFolderName MSBuild task provided by the .NET SDK. Although it would be a better customer experience if NuGet set an MSBuild property (NuGetShortFolderName?) before running TargetsForTfmSpecificContentInPackage. But that's not implemented yet, so doesn't help solve any problems today.

As for why the last zero is being dropped, NuGetFramework.GetShortFolderName calls IFrameworkNameProvider.GetVersionString, which only uses the greater than zero components.

I don't know whether or not it's desirable to normalize version numbers in NuGetFramework when generating the short folder name. But given the second comment in this issue, it appears that NuGet does, so there's risk of breaking other customers if we change this. Even if we stop normalizing the versions, we still have to ensure that files added via PackagePath="..." match the auto-added files, and when customers use net5.0-windows without specifying which windows version, they need a way to find the correct short folder name for whatever default version that the .NET SDK uses.

dasMulli commented 3 years ago

From a usage standpoint: Does it make sense to wire this up as an MSBuild intrinsic function so it can be used in the static part of a project and not from a target? And: do we expect more people to use this? There are already intrinsic functions in MSBuild that call into NuGet - https://github.com/dotnet/msbuild/blob/40beb5a025fb18c82a159f407b5ce2383531a8eb/src/Build/Evaluation/IntrinsicFunctions.cs#L492-L515

zivkan commented 3 years ago

It's a great suggestion. I posted some ideas in NuGet's issue: https://github.com/NuGet/Home/issues/10063#issuecomment-713083004