dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.
https://docs.microsoft.com/visualstudio/msbuild/msbuild
MIT License
5.22k stars 1.35k forks source link

[Bug]: Projects with long descriptions (>~16k) don't emit FileVersion in the produced DLLs #8720

Open remoba opened 1 year ago

remoba commented 1 year ago

Issue Description

When compiling a versioned csproj with a Description property that is bigger than 16k, the FileVersion (+ other attributes) of the DLL stop being emitted.

I've attached a repro where the ConsoleApp4 project has a description of length 17k while ConsoleApp5 has 16k. When compiling the two DLLs and comparing their file attributes:

image image

I am not sure why this is happening but if the Description property has a limit shouldn't it be enforced with an error during the build rather than silently passing with missing info in the built DLL?

My use case here is I want to bundle the CHANGELOG.md file when I publish packages, so it's reasonable to add a Substring, to the Description but should I have to do that?

Steps to Reproduce

Steps to reproduce:

  1. Run dotnet build on the root of the solution
  2. Look at the generated DLLs and the respective bin/Debug folders

BugRepro.zip

Expected Behavior

The File version / Product version should be populated in ConsoleApp4.dll

Actual Behavior

The File version / Product version is empty in ConsoleApp4.dll

Analysis

No response

Versions & Configurations

msbuild -version MSBuild version 17.5.1+f6fdcf537 for .NET Framework 17.5.1.16304

dotnet --version 7.0.203

KalleOlaviNiemitalo commented 1 year ago

Roslyn generates the Win32 version resource from assembly attributes, in src/Compilers/Core/Portable/CvtRes.cs. In the resource format, StringFileInfo includes WORD wLength, so the total size of all version strings for a single language cannot exceed 64k bytes. I'm surprised that you already hit a limit at 16k characters.

In your BugRepro.zip, the version resource of ConsoleApp4/bin/Debug/net7.0/ConsoleApp4.dll includes a String structure with szKey = L"Comments" and Value = a long string, starting at file offset 0x4d34. I suppose the "Properties" dialog box just refuses to display that.

The GetAssemblyAttributes target in Microsoft.NET.GenerateAssemblyInfo.targets generates AssemblyDescriptionAttribute from the Description property. I think you should set the PackageDescription property rather than Description, so that it applies only to the NuGet package and not to the assembly; or even include the change log as a file in the package and refer to it via PackageReadmeFile.

remoba commented 1 year ago

@KalleOlaviNiemitalo Thank you, I will try your suggested workarounds and update here.

Note that my main concern is that devs will be able to see the changelog in the NuGet Manager view in Visual Studio (This is why I am bundling the changelog file in the Description property and not just the PackageReleaseNotes), hopefully using PackageReadmeFile will work in this sense.

remoba commented 1 year ago

Can confirm using PackageDescription works just as well and doesn't have this limit so I will switch over to using it. Packaging with PackageReadmeFile doesn't seem to show up in the VS NuGet Manager view or in the Azure DevOps feed view so I don't think it fits my scenario.

I can close this since my problem is resolved but in general don't you think the build process should produce an error (or at least a warning) in cases where the mentioned limit is reached?

KalleOlaviNiemitalo commented 1 year ago

I'm not sure what the actual limit is, and whether it applies to the single string or perhaps to the total size of the version resource. The C# compiler was able to write the version resource, but Windows did not display it. Does Windows document a limit?

KalleOlaviNiemitalo commented 1 year ago

I mean, if Windows has a limit but does not document it, then it could change between versions of Windows, and it would seem wrong to hardcode that limit in .NET.

I suppose something in .NET SDK could warn if the description, or the whole version resource, is suspiciously large. The warning threshold could then be set much lower than 16k characters. For example, 500 characters would already be very inconvenient to scroll in the Properties dialog box.

KalleOlaviNiemitalo commented 1 year ago

Some developers might have to include lot of text in the LegalCopyright or LegalTrademarks strings of the version resource in order to satisfy lawyers, even if they know that users won't be easily able to read all the text from there. Those developers could just disable the new warning, though.