Rename/Update PowerShell ETW manifest to remove the Windows PowerShell dependency. #5360
Conversation
dantraMSFT
requested review from
TravisEz13,
adityapatwardhan and
daxian-dbw
as code owners
build.psm1
Outdated
|
|
||
| # Place the remoting configuration script in the same directory | ||
| # as the binary so it will get published. | ||
| Copy-Item .\Install-PowerShellRemoting.ps1 $dstPath |
There was a problem hiding this comment.
Bin placing Install-PowerShellRemoting.ps1 has been removed in #5330
build.psm1
Outdated
| Start-NativeExecution { Invoke-Expression -Command:$command } | ||
|
|
||
| # Copy the binaries and manifest to the packaging directory | ||
| $dstPath = "$PSScriptRoot\src\powershell-win-core" |
There was a problem hiding this comment.
No, but I don't want to assume the previous build step sets it as expected.
|
The manifest has some windows powershell specific resources. Should this PR include the cleanup work of those resources? |
|
@daxian-dbw I think we can do the cleanup of the manifest in a separate PR |
|
@daxian-dbw I did do an initial scrub by removing all events that are not referenced in code (i.e, scanning for all PSEvents.* code references). Going further would require removing Windows-specific code which I thought would only muddle the PR. |
|
@daxian-dbw: I made you the assignee; let me know if it should be someone else. I need to create a nuget package once this is merged. |
| [string] $command = $null | ||
| if ($Unregister) | ||
| { | ||
| $command = [string]::Format("wevtutil um {0}", $manifest.FullName) |
There was a problem hiding this comment.
Please use "wevtutil um {0}" -f $manifest.FullName for the formatting.
| } | ||
| else | ||
| { | ||
| $command = [string]::Format("wevtutil.exe im {0} /rf:{1} /mf:{1}", $manifest.FullName, $binary.FullName) |
|
@TravisEz13 @adityapatwardhan Can you please also review this PR? |
…the ETW resources.
…arnings/errors. Replace [string]::Format with value -f
build.psm1
Outdated
| $FilesToCopy = @( | ||
| [IO.Path]::Combine($location, $Configuration, 'PowerShell.Core.Instrumentation.dll'), | ||
| [IO.Path]::Combine($location, 'PowerShell.Core.Instrumentation.man') | ||
| [IO.Path]::Combine($location, 'RegisterManifest.ps1') |
There was a problem hiding this comment.
Do not bin-place the .ps1 and .man file. Instead, we can copy them to publish folder directly by powershell-win-core.csproj file. See how we bin-place install-powershellremoting here.
In this way, we don't need to add a .gitignore file in the powershell-win-core folder to ignore the .man file there.
|
@TravisEz13 @adityapatwardhan Do you have any feedback? |
|
|
||
| #define VER_FILETYPE VFT_DLL | ||
| #define VER_FILESUBTYPE VFT2_UNKNOWN | ||
| #define VER_FILEDESCRIPTION_STR "DSC" |
There was a problem hiding this comment.
Should this be PowerShellCore?
There was a problem hiding this comment.
@dantraMSFT please push your fix so that @adityapatwardhan can sign off.
|
@daxian-dbw This should be good to go. |
|
Test failure in AppVeyor is a known issue, but Travis CI hasn't finished yet. |
4 participants
This is the first part of the fix for #4939 and include the following changes:
1: Rename the manifest file (PowerShell.Core.Instrumentation.man)
2:
Change the ETW provider GUID and name and move to outside of the Microsoft/Windows tree in the event log. The provider name is now simply PowerShellCore.[edited by @daxian-dbw: provider GUID and provider names are not changed in this PR]3: Create a registration script for registering. (RegisterManifest.ps1)
4: Produce a binary resource-only DLL to contain the ETW manifest and associated string resources. (PowerShell.Core.Instrumentation.dll)
The second part will involve creating a nuget package that will be pulled in at build time, similar to psl-native. for Linux Additionally, tests to verify the provider registration and event raising will be included as part of the nuget package work.