Make EventLog work without .NETFramework#45884
Conversation
|
Tagging subscribers to this area: @tommcdon, @krwq Issue DetailsFixes #27742 This adds a 772KB resource file to the package, however that file compresses down to 6KB in the package. I chose to keep this as a separate file (as it was in .NETFramework) rather than just embed the resources in S.D.EventLog itself because the consumer for these resources is Windows/EventViewer which is a different process than that which is loading S.D.EventLog. This avoids increasing the memory footprint of the .NET app, however the disk footprint increases. We could consider making this opt-in via targets (which automatically opt-in for ARM64) if folks are concerned with it. I also chose to keep the .NETFramework path as the default if the file is present, but would be open to hearing folks opinion on deleting this part of the code. The method I chose for building the dll is similar to what is done for .resources.dlls in .NETCore. A separate invocation of CSC with only attributes, resources, and no other code. I've tested this manually by mucking with my windows installation (takeown / icacls / move on .NETFramework copy) to simulate a SKU without .NETFramework. I still need to see if I can write some automated tests to cover this.
|
src/libraries/System.Diagnostics.EventLog/src/GenerateMessages.targets
Outdated
Show resolved
Hide resolved
src/libraries/System.Diagnostics.EventLog/src/GenerateMessages.targets
Outdated
Show resolved
Hide resolved
src/libraries/System.Diagnostics.EventLog/src/System/Diagnostics/EventLog.cs
Show resolved
Hide resolved
|
I plan to add tests for this.
|
src/libraries/System.Diagnostics.EventLog/tests/EventLogMessagesTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Diagnostics.EventLog/tests/EventLogMessagesTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Diagnostics.EventLog/tests/EventLogMessagesTests.cs
Show resolved
Hide resolved
src/libraries/System.Diagnostics.EventLog/src/System.Diagnostics.EventLog.csproj
Outdated
Show resolved
Hide resolved
606dab8 to
6f4bd5c
Compare
|
@ericstj should we merge this one ? |
|
I would like for others to weigh in on #45884 (comment). |
src/libraries/System.Diagnostics.EventLog/tests/EventLogMessagesTests.cs
Outdated
Show resolved
Hide resolved
danmoseley
left a comment
There was a problem hiding this comment.
LGTM. Did you run tests with the flag necessary to include XunitConstants.IgnoreForCI tests -- that CI won't run?
I ran these tests locally (including OuterLoop) on a Win10 machine. I double checked the results and it includes the |
Also fix one test which would fail on machine without .NETFramework.
|
Hello @ericstj! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
* Make EventLog work without .NETFramework * Use a project file instead of targets to create message DLL * Add EventLogMessagesTests * Address feedback * Exclude EventLogMessage tests on net48 * Apply code review feedback Also fix one test which would fail on machine without .NETFramework. * Fix HelpLink test
* Make EventLog work without .NETFramework (#45884) * Make EventLog work without .NETFramework * Use a project file instead of targets to create message DLL * Add EventLogMessagesTests * Address feedback * Exclude EventLogMessage tests on net48 * Apply code review feedback Also fix one test which would fail on machine without .NETFramework. * Fix HelpLink test * Build EventLog package in servicing Update assembly version of EventLog package and build it in servicing. Also update the package baseline to ensure that Windows.Compatibility package gets the new version. * Don't rev the platforms package unless we ship it * Ensure we restore EventLog.Messages 5.0 infra doesn't traverse runtime-specific TFMs during restore. Workaround by referencing Message project everywhere, and only including it in package on windows. Co-authored-by: Anirudh Agnihotry <anirudhagnihotry098@gmail.com>
Fixes #27742
This adds a 772KB resource file to the package, however that file compresses down to 6KB in the package.
I chose to keep this as a separate file (as it was in .NETFramework) rather than just embed the resources in S.D.EventLog itself because the consumer for these resources is Windows/EventViewer which is a different process than that which is loading S.D.EventLog. This avoids increasing the memory footprint of the .NET app, however the disk footprint increases. We could consider making this opt-in via targets (which automatically opt-in for ARM64) if folks are concerned with it.
I also chose to keep the .NETFramework path as the default if the file is present, but would be open to hearing folks opinion on deleting this part of the code.
The method I chose for building the dll is similar to what is done for .resources.dlls in .NETCore. A separate invocation of CSC with only attributes, resources, and no other code.
I've tested this manually by mucking with my windows installation (takeown / icacls / move on .NETFramework copy) to simulate a SKU without .NETFramework. I still need to see if I can write some automated tests to cover this.