Add dependency nuget instructions for PowerShell.Core.Instrumentation resource binary#5396
Add dependency nuget instructions for PowerShell.Core.Instrumentation resource binary#5396adityapatwardhan merged 9 commits intoPowerShell:masterfrom dantraMSFT:dantra/issue4939
Conversation
…umentation ETW resource binary.
|
NOTE: Merging is dependent on the PowerShell.Core.Instrumentation nuget package being published. I'll update the thread when it is ready. |
docs/building/internals.md
Outdated
|
|
||
| To successfully decode PowerShell Core ETW events, The manifest and resource binary need to be registered on the system. | ||
|
|
||
| To create a new NuGet package for `PowerShell.Core.Instrumentation.dll`, you will need the PowerShell.Core.Instrumentation.nuspec found in the repo under src\PowerShell.Core.Instrumentation. |
There was a problem hiding this comment.
PowerShell.Core.Instrumentation.nuspec should be put in the code block ...
docs/building/internals.md
Outdated
| │ └── PowerShell.Core.Instrumentation.dll | ||
| ``` | ||
|
|
||
| NOTE: Since these are native binaries used on Windows, they need to be AuthenticodeDual signed, certificate code: 402 before creating the nuget package. |
There was a problem hiding this comment.
certificate code: 402
per my discussion with Travis, this code should not be exposed in public. I think only mentioning "need to be AuthenticodeDual signed" should be good enough.
| <PackageReference Include="System.Security.Permissions" Version="4.4.0" /> | ||
| <PackageReference Include="System.Text.Encoding.CodePages" Version="4.4.0" /> | ||
| <PackageReference Include="Microsoft.Management.Infrastructure" Version="1.0.0-alpha*" /> | ||
| <PackageReference Include="PowerShell.Core.Instrumentation" Version="6.0.0-beta*" /> |
There was a problem hiding this comment.
Is it depended by System.Management.Automation.dll or powershell as a whole? Put it another way, if an application is hosting System.Management.Automation.dll only, does it need the resource dll to work properly?
There was a problem hiding this comment.
None of the code has a direct dependency on the binary. I chose SMA to pull the nuget package since it contains the event raising code. If the dll is not present or the manifest isn't registered, PowerShell will continue to work without issue but the event log and custom consumers won't be able to decode the events.
There was a problem hiding this comment.
Thanks for the clarification. #Close
docs/building/internals.md
Outdated
|
|
||
| NOTE: Since these are native binaries used on Windows, they need to be AuthenticodeDual signed, certificate code: 402 before creating the nuget package. | ||
|
|
||
| Lastly, run `nuget pack` from teh root of the repo. The following command creates the nuget package from the c:\mypackage directory and places the nuget package in .\src\powershell-win-core. Note that you may need the latest `nuget.exe`. |
There was a problem hiding this comment.
run
nuget packfrom teh root of the repo
Type teh.
How about changing it to "Lastly, run the following command from the root of repo."?
docs/building/internals.md
Outdated
|
|
||
| ```powershell | ||
| nuget pack .\src\PowerShell.Core.Instrumentation\PowerShell.Core.Instrumentation.nuspec -BasePath c:\mypackage -OutputDirectory .\src\powershell-win- | ||
| core |
There was a problem hiding this comment.
'core' should not be in a new line.
docs/building/internals.md
Outdated
|
|
||
| ### PowerShell.Core.Instrumentation | ||
|
|
||
| To successfully decode PowerShell Core ETW events, The manifest and resource binary need to be registered on the system. |
docs/building/internals.md
Outdated
| <version>6.0.0-RC</version> | ||
| ``` | ||
|
|
||
| Next, create the directory structure needed for the contents of the nuget packagestructure. The final directory and file layout is listed below. |
| <authors>Microsoft</authors> | ||
| <owners>Microsoft</owners> | ||
| <requireLicenseAcceptance>false</requireLicenseAcceptance> | ||
| <description>PowerShell Core ETW resource binary</description> |
There was a problem hiding this comment.
Should we remove "Core"? It seems we did this previously.
There was a problem hiding this comment.
Hmm, we use PowerShell Core all across our .MD files in the repo. (547 times). I figured I'd follow that pattern as well as ensure that there is no confusion between this and Windows PowerShell. That's also the reason I selected the assembly name.
| <owners>Microsoft</owners> | ||
| <requireLicenseAcceptance>false</requireLicenseAcceptance> | ||
| <description>PowerShell Core ETW resource binary</description> | ||
| <copyright>Copyright 2017 Microsoft</copyright> |
There was a problem hiding this comment.
We use the string (c) Microsoft Corporation. All rights reserved.
Also should we start all XML tags with capitalization?
There was a problem hiding this comment.
I did a survey of all nuspec files in my package cache, and they all use lowercase for the xml elements.
There was a problem hiding this comment.
Fixed the copyright text
docs/building/internals.md
Outdated
|
|
||
| NOTE: Since these are native binaries used on Windows, they need to be Authenticode Dual signed before creating the nuget package. | ||
|
|
||
| Lastly, run the following command from the root of the repo to create the nuget package. The nuget package is placed `.\src\powershell-win-core`. Note that you may need the latest `nuget.exe`. |
There was a problem hiding this comment.
The nuget package is placed
.\src\powershell-win-core
Maybe "is placed at"?
|
FYI: I updated the nuspec to refer to beta.10 |
|
What is next release? Beta.10 or RC.1 ? |
|
@iSazonov @dantraMSFT misspoke, next release is RC |
|
@iSazonov @dantraMSFT was referring to the version of the nuget package After publishing the beta.9 |
|
@dantraMSFT There are spelling errors in the .md file. Please see the failed Linux CI. |
|
It's confusing. Shouldn't we be using the same version for all packages? It seems .Net Core do so. |
|
I agree. That's something we should consider to do. We currently use inconsistent versions for nuget packages that PowerShell depends on, such as pspr.windows, psrp, libmi, MMI and powershell.core.instrument. It would be good if we can unify them before GA. |
|
libmi is not owned by PowerShell, it is part if omi and it's version is tied to it. Psrp has a build dependency on both libmi and omi so I tied the version to it. It has no dependency on PowerShell and will limely be reved independent of it so I see no reason to version it with PowerShell |
|
Good point. So maybe just those nuget packages that are owned by PowerShell itself. @dantraMSFT Please fix the spelling errors in Travis-CI build. See my comment #5396 (comment) for more info. We cannot merge the PR with the failed CI. |
|
@daxian-dbw, I fixed spelling issues; updated .spelling with new entries (authenticode and PowerShell.Core.Instrumentation) |
|
@dantraMSFT Just to confirm, the nuget package has been published right? |
|
@adityapatwardhan: Yes, the nuget package has been published. |
Fix: #4939
This PR adds a dependency on PowerShell.Core.Instrumentation.dll to System.Management.Automation.csproj.
This is a native, resource-only binary containing ETW resource manifest and associated strings.
internals.md documents the steps for creating the nuget package and PowerShell.Core.Instrumentation.nuspec contains the template nuget spec.