dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.95k stars 4.65k forks source link

System.Diagnostics.EventLog.Messages.dll doesn't have fileversion and other resources #99997

Open f-camera opened 5 months ago

f-camera commented 5 months ago

Description

The file System.Diagnostics.EventLog.Messages.dl as present in NuGet package (e.g.) https://www.nuget.org/packages/System.Diagnostics.EventLog/8.0.0 is missing every detail. Since we need to include this in our product setup, this is an issue.

PS C:\> (Get-Item .\System.Diagnostics.EventLog.Messages.dll).VersionInfo

ProductVersion   FileVersion      FileName
--------------   -----------      --------
                                  System.Diagnostics.EventLog.Messages.dll

by comparison, the other dll in the same package

PS C:\> (Get-Item .\System.Diagnostics.EventLog.dll).VersionInfo

ProductVersion   FileVersion      FileName
--------------   -----------      --------
8.0.0+5535e31... 8.0.23.53103     System.Diagnostics.EventLog.dll

Is it possible to have the dll marked with the corresponding resources? Alternatively, is there a way to "patch" the file without breaking it? Thanks.

Reproduction Steps

  1. Download the released package (e.g. https://www.nuget.org/packages/System.Diagnostics.EventLog/8.0.0 ),
  2. unpack it
  3. inspect dll details

Expected behavior

dlls have correct File Version

Actual behavior

System.Diagnostics.EventLog.Messages.dl doesn't have

Regression?

I sampled older releases (e.g. 7.0.0 and 6.0.0) and in those packages the dll doesn't have the File version. So I think it can't be claimed being a regression.

Known Workarounds

No response

Configuration

No response

Other information

No response

dotnet-policy-service[bot] commented 5 months ago

Tagging subscribers to this area: @dotnet/area-system-diagnostics-eventlog See info in area-owners.md if you want to be subscribed.

MichalStrehovsky commented 5 months ago

This DLL is built here: https://github.com/dotnet/runtime/tree/main/src/libraries/System.Diagnostics.EventLog/src/Messages

The build of it is convoluted due to the reasons explained there. Because the CSPROJ file specifies a Win32Resource element, the C# compiler doesn't generate the file version info. It's probably not entirely trivial to add the version info.

Alternatively, is there a way to "patch" the file without breaking it?

If you're okay with patching it (and invalidating the Authenticode signature on the file), you can use a tool such as ResHacker. In ResHacker, open the System.Diagnostics.EventLog.dll file and save the Version info resource to a RES file. Then open System.Diagnostics.EventLog.Messages.dll in ResHacker and add the version info resource from the RES file. Click save, done.

f-camera commented 5 months ago

Thanks @MichalStrehovsky ! I'm not much an expert, but let me ask one more question: would it be feasible to have the Messages dll have at least fixed version numbers instead of nothing at all?

I didn't try myself, but could the https://github.com/dotnet/runtime/blob/main/src/libraries/System.Diagnostics.EventLog/src/Messages/EventLogMessages.rc file have effective version blocks?

MichalStrehovsky commented 5 months ago

Thanks @MichalStrehovsky ! I'm not much an expert, but let me ask one more question: would it be feasible to have the Messages dll have at least fixed version numbers instead of nothing at all?

I didn't try myself, but could the https://github.com/dotnet/runtime/blob/main/src/libraries/System.Diagnostics.EventLog/src/Messages/EventLogMessages.rc file have effective version blocks?

I assume you have a requirement to include a version number in all files you ship for a reason. Would a dummy version number that doesn't match the AssemblyFileVersionAttribute inside the assembly (that has the actual version) be an improvement?

f-camera commented 5 months ago

Would a dummy version number that doesn't match the AssemblyFileVersionAttribute inside the assembly (that has the actual version) be an improvement?

It would improve a lot, thanks. The case here is a strict check from a setup framework: it is required to have FileVersion. We are adopting the files packaged in https://www.nuget.org/packages/System.Diagnostics.EventLog/8.0.0

MichalStrehovsky commented 5 months ago

Would a dummy version number that doesn't match the AssemblyFileVersionAttribute inside the assembly (that has the actual version) be an improvement?

It would improve a lot, thanks. The case here is a strict check from a setup framework: it is required to have FileVersion. We are adopting the files packaged in https://www.nuget.org/packages/System.Diagnostics.EventLog/8.0.0

Honest question: if a non-sensical version is okay with your setup framework policies, why is a version needed at all? If the setup framework sees two different binaries that have the same version, would it know to do the right thing? And if so, why it can't do the same thing if the version is not present at all?

ericstj commented 1 month ago

I think we could give it a hardcoded version if that's helpful. We'd just need to add the version resource to the RC and regenerate it. If we ever needed to regenerate it, we could include incrementing that version in our process. It would be "lying" about version in that rebuilds wouldn't have new versions, but it might be better than no version at all.

It's a bummer CSC can't merge resources. We have hacky ways of getting a version resource that updates - see https://github.com/dotnet/runtime/blob/57f870f909dbfad35142e5aaa6e681464de4f439/src/coreclr/.nuget/Microsoft.NET.Sdk.IL/Microsoft.NET.Sdk.IL.targets.template#L71-L111 But since we'd need to both have that version and have the native binary section we'd probably need to write our own tool to do that.

andrewimcclement commented 1 month ago

In a similar vein, I see that System.Diagnostics.EventLog.Messages.dll is missing copyright (mentioned as "additional resources" in the original post).