Add dependency nuget instructions for PowerShell.Core.Instrumentation resource binary by dantraMSFT · Pull Request #5396 · PowerShell/PowerShell

Dan Travison (dantraMSFT) · 2017-11-09T20:21:17Z

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.

…umentation ETW resource binary.

Dan Travison (dantraMSFT) · 2017-11-09T21:15:04Z

NOTE: Merging is dependent on the PowerShell.Core.Instrumentation nuget package being published. I'll update the thread when it is ready.

Dongbo Wang (daxian-dbw) · 2017-11-09T23:13:54Z

+
+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.


PowerShell.Core.Instrumentation.nuspec should be put in the code block ...

Dongbo Wang (daxian-dbw) · 2017-11-09T23:15:33Z

+    │       └── 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.


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.

Dongbo Wang (daxian-dbw) · 2017-11-09T23:18:17Z

    <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*" />


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?

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.

Thanks for the clarification. #Close

Dongbo Wang (daxian-dbw) · 2017-11-09T23:19:14Z

+
+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`.


run nuget pack from teh root of the repo

Type teh.

How about changing it to "Lastly, run the following command from the root of repo."?

Dongbo Wang (daxian-dbw) · 2017-11-09T23:22:22Z

+
+```powershell
+nuget pack .\src\PowerShell.Core.Instrumentation\PowerShell.Core.Instrumentation.nuspec -BasePath c:\mypackage -OutputDirectory .\src\powershell-win-
+core


'core' should not be in a new line.

Dongbo Wang (daxian-dbw) · 2017-11-09T23:24:18Z

+
+### PowerShell.Core.Instrumentation
+
+To successfully decode PowerShell Core ETW events, The manifest and resource binary need to be registered on the system.


events, The manifest

typo.

Dongbo Wang (daxian-dbw) · 2017-11-09T23:25:20Z

+<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.


packagestructure

typo

Ilya (iSazonov) · 2017-11-10T04:13:28Z

+    <authors>Microsoft</authors>
+    <owners>Microsoft</owners>
+    <requireLicenseAcceptance>false</requireLicenseAcceptance>
+    <description>PowerShell Core ETW resource binary</description>


Should we remove "Core"? It seems we did this previously.

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.

Ilya (iSazonov) · 2017-11-10T04:13:42Z

+    <owners>Microsoft</owners>
+    <requireLicenseAcceptance>false</requireLicenseAcceptance>
+    <description>PowerShell Core ETW resource binary</description>
+    <copyright>Copyright 2017 Microsoft</copyright>


We use the string (c) Microsoft Corporation. All rights reserved.

Also should we start all XML tags with capitalization?

I did a survey of all nuspec files in my package cache, and they all use lowercase for the xml elements.

Fixed the copyright text

Dongbo Wang (daxian-dbw) · 2017-11-10T22:29:24Z

+
+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`.


The nuget package is placed .\src\powershell-win-core

Maybe "is placed at"?

Dongbo Wang (daxian-dbw)

LGTM

Dan Travison (dantraMSFT) · 2017-11-11T01:43:50Z

FYI: I updated the nuspec to refer to beta.10

Ilya (iSazonov) · 2017-11-11T09:45:47Z

What is next release? Beta.10 or RC.1 ?

Steve Lee (SteveL-MSFT) · 2017-11-11T17:41:22Z

Ilya (@iSazonov) Dan Travison (@dantraMSFT) misspoke, next release is RC

Dongbo Wang (daxian-dbw) · 2017-11-11T18:08:03Z

Ilya (@iSazonov) Dan Travison (@dantraMSFT) was referring to the version of the nuget package PowerShell.Core.Instrumentation, not the next PowerShell release.

After publishing the beta.9 PowerShell.Core.Instrumentation we learnt that resource DLL also needs to be signed, and that's why the beta.10 package was published shortly after.

Dongbo Wang (daxian-dbw) · 2017-11-11T18:18:37Z

Dan Travison (@dantraMSFT) There are spelling errors in the .md file. Please see the failed Linux CI.
For file name and file path, you can put them in code block to avoid the spelling check.
For other spelling errors, see https://github.com/PowerShell/PowerShell/blob/master/.github/CONTRIBUTING.md#spellchecking-documentation about how to add them to the .spelling file. When adding a new word to .spelling, please make sure the section that contains the new word is sorted ascendingly (each existing section should have been sorted).

Ilya (iSazonov) · 2017-11-11T18:38:31Z

It's confusing. Shouldn't we be using the same version for all packages? It seems .Net Core do so.

Dongbo Wang (daxian-dbw) · 2017-11-11T18:51:37Z

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.

Dan Travison (dantraMSFT) · 2017-11-12T00:33:21Z

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

Dongbo Wang (daxian-dbw) · 2017-11-12T05:19:39Z

Good point. So maybe just those nuget packages that are owned by PowerShell itself.

Dan Travison (@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.

Dan Travison (dantraMSFT) · 2017-11-12T20:59:29Z

Dongbo Wang (@daxian-dbw), I fixed spelling issues; updated .spelling with new entries (authenticode and PowerShell.Core.Instrumentation)

Aditya Patwardhan (adityapatwardhan) · 2017-11-13T18:17:48Z

Dan Travison (@dantraMSFT) Just to confirm, the nuget package has been published right?

Dan Travison (dantraMSFT) · 2017-11-13T18:36:19Z

Aditya Patwardhan (@adityapatwardhan): Yes, the nuget package has been published.

… resource binary (PowerShell#5396) * Add dependency nuget packaging instructions for PowerShell.Core.Instrumentation ETW resource binary. * Fix markdown MD031 Fenced code blocks errors. * Fix MD022 Headers should be surrounded by blank lines * Add a note about Authenticode Dual-signing requirement. * PR Feedback. * Set version to Beta.10

Add dependency nuget packaging instructions for PowerShell.Core.Instr…

c90eaf5

…umentation ETW resource binary.

Dan Travison (dantraMSFT) requested review from Steve Lee (SteveL-MSFT) and Dongbo Wang (daxian-dbw) November 9, 2017 20:21

Dan Travison (dantraMSFT) requested review from Travis Plunk (TravisEz13) and Aditya Patwardhan (adityapatwardhan) as code owners November 9, 2017 20:21

Fix markdown MD031 Fenced code blocks errors.

74bdbb3

Dan Travison (dantraMSFT) added 2 commits November 9, 2017 13:17

Fix MD022 Headers should be surrounded by blank lines

8902527

Add a note about Authenticode Dual-signing requirement.

da71450

Dongbo Wang (daxian-dbw) reviewed Nov 9, 2017

View reviewed changes

PR Feedback.

ed2c484

Ilya (iSazonov) reviewed Nov 10, 2017

View reviewed changes

Dongbo Wang (daxian-dbw) reviewed Nov 10, 2017

View reviewed changes

Dongbo Wang (daxian-dbw) assigned Aditya Patwardhan (adityapatwardhan) Nov 10, 2017

Dan Travison (dantraMSFT) added 2 commits November 10, 2017 15:12

Fix grammar

f60a430

Update copyrigth value

8b150a5

Dongbo Wang (daxian-dbw) approved these changes Nov 10, 2017

View reviewed changes

Dongbo Wang (daxian-dbw) added this to the 6.0.0-RC milestone Nov 10, 2017

Set version to Beta.10

51f99f7

Fix spelling in the document and update .spelling file.

ed7fad9

Aditya Patwardhan (adityapatwardhan) approved these changes Nov 13, 2017

View reviewed changes

Aditya Patwardhan (adityapatwardhan) merged commit d5a974f into PowerShell:master Nov 13, 2017

Dan Travison (dantraMSFT) deleted the dantra/issue4939 branch November 13, 2017 20:19


		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.


		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`.


		### PowerShell.Core.Instrumentation

		To successfully decode PowerShell Core ETW events, The manifest and resource binary need to be registered on the system.


		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`.

Conversation

Dan Travison (dantraMSFT) commented Nov 9, 2017

Uh oh!

Dan Travison (dantraMSFT) commented Nov 9, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dongbo Wang (daxian-dbw) left a comment

Choose a reason for hiding this comment

Uh oh!

Dan Travison (dantraMSFT) commented Nov 11, 2017

Uh oh!

Ilya (iSazonov) commented Nov 11, 2017

Uh oh!

Steve Lee (SteveL-MSFT) commented Nov 11, 2017

Uh oh!

Dongbo Wang (daxian-dbw) commented Nov 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dongbo Wang (daxian-dbw) commented Nov 11, 2017

Uh oh!

Ilya (iSazonov) commented Nov 11, 2017

Uh oh!

Dongbo Wang (daxian-dbw) commented Nov 11, 2017

Uh oh!

Dan Travison (dantraMSFT) commented Nov 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dongbo Wang (daxian-dbw) commented Nov 12, 2017

Uh oh!

Dan Travison (dantraMSFT) commented Nov 12, 2017

Uh oh!

Aditya Patwardhan (adityapatwardhan) commented Nov 13, 2017

Uh oh!

Dan Travison (dantraMSFT) commented Nov 13, 2017

Uh oh!

Reviewers

Assignees

Dongbo Wang (daxian-dbw) commented Nov 11, 2017 •

edited

Loading

Dan Travison (dantraMSFT) commented Nov 12, 2017 •

edited

Loading