Rename/Update PowerShell ETW manifest to remove the Windows PowerShell dependency.#5360
Uh oh!
There was an error while loading. Please reload this page.
Conversation
dantraMSFT requested review from TravisEz13, adityapatwardhan and daxian-dbw as code ownersbuild.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.
daxian-dbw commented Nov 7, 2017
The manifest has some windows powershell specific resources. Should this PR include the cleanup work of those resources? |
SteveL-MSFT commented Nov 7, 2017
@daxian-dbw I think we can do the cleanup of the manifest in a separate PR |
dantraMSFT commented Nov 7, 2017
@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. |
dantraMSFT commented Nov 7, 2017
@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) |
daxian-dbw commented Nov 7, 2017
@TravisEz13@adityapatwardhan Can you please also review this PR? |
…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-powershellremotinghere.
In this way, we don't need to add a .gitignore file in the powershell-win-core folder to ignore the .man file there.
dantraMSFT commented Nov 8, 2017
@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.
dantraMSFT commented Nov 8, 2017
@daxian-dbw This should be good to go. |
daxian-dbw commented Nov 9, 2017
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.