Add dependency nuget instructions for PowerShell.Core.Instrumentation resource binary #5396

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.

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.

daxian-dbw · 2017-11-09T23:13:54Z

docs/building/internals.md

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

daxian-dbw · 2017-11-09T23:15:33Z

docs/building/internals.md

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

daxian-dbw · 2017-11-09T23:18:17Z

src/System.Management.Automation/System.Management.Automation.csproj

@@ -16,6 +16,7 @@
    <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

daxian-dbw · 2017-11-09T23:19:14Z

docs/building/internals.md

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

daxian-dbw · 2017-11-09T23:22:22Z

docs/building/internals.md

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

daxian-dbw · 2017-11-09T23:24:18Z

docs/building/internals.md

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

daxian-dbw · 2017-11-09T23:25:20Z

docs/building/internals.md

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

iSazonov · 2017-11-10T04:13:28Z

src/PowerShell.Core.Instrumentation/PowerShell.Core.Instrumentation.nuspec

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

iSazonov · 2017-11-10T04:13:42Z

src/PowerShell.Core.Instrumentation/PowerShell.Core.Instrumentation.nuspec

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

daxian-dbw · 2017-11-10T22:29:24Z

docs/building/internals.md

+
+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"?

daxian-dbw

LGTM

dantraMSFT · 2017-11-11T01:43:50Z

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

iSazonov · 2017-11-11T09:45:47Z

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

SteveL-MSFT · 2017-11-11T17:41:22Z

@iSazonov @dantraMSFT misspoke, next release is RC

daxian-dbw · 2017-11-11T18:08:03Z

@iSazonov @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.

daxian-dbw · 2017-11-11T18:18:37Z

@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).

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.

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.

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

daxian-dbw · 2017-11-12T05:19:39Z

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.

dantraMSFT · 2017-11-12T20:59:29Z

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

adityapatwardhan · 2017-11-13T18:17:48Z

@dantraMSFT Just to confirm, the nuget package has been published right?

dantraMSFT · 2017-11-13T18:36:19Z

@adityapatwardhan: Yes, the nuget package has been published.

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

c90eaf5

…umentation ETW resource binary.

dantraMSFT requested review from daxian-dbw and SteveL-MSFT November 9, 2017 20:21

dantraMSFT requested review from adityapatwardhan and TravisEz13 as code owners November 9, 2017 20:21

Fix markdown MD031 Fenced code blocks errors.

74bdbb3

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

daxian-dbw reviewed Nov 9, 2017

View reviewed changes

PR Feedback.

ed2c484

iSazonov reviewed Nov 10, 2017

View reviewed changes

daxian-dbw reviewed Nov 10, 2017

View reviewed changes

daxian-dbw assigned adityapatwardhan Nov 10, 2017

dantraMSFT added 2 commits November 10, 2017 15:12

Fix grammar

f60a430

Update copyrigth value

8b150a5

daxian-dbw approved these changes Nov 10, 2017

View reviewed changes

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

adityapatwardhan approved these changes Nov 13, 2017

View reviewed changes

adityapatwardhan merged commit d5a974f into PowerShell:master Nov 13, 2017

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

Add dependency nuget instructions for PowerShell.Core.Instrumentation resource binary #5396

Add dependency nuget instructions for PowerShell.Core.Instrumentation resource binary #5396

Conversation

dantraMSFT commented Nov 9, 2017

dantraMSFT commented Nov 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daxian-dbw left a comment

Choose a reason for hiding this comment

dantraMSFT commented Nov 11, 2017

iSazonov commented Nov 11, 2017

SteveL-MSFT commented Nov 11, 2017

daxian-dbw commented Nov 11, 2017 • edited

daxian-dbw commented Nov 11, 2017

iSazonov commented Nov 11, 2017

daxian-dbw commented Nov 11, 2017

dantraMSFT commented Nov 12, 2017 • edited

daxian-dbw commented Nov 12, 2017

dantraMSFT commented Nov 12, 2017

adityapatwardhan commented Nov 13, 2017

dantraMSFT commented Nov 13, 2017

daxian-dbw commented Nov 11, 2017 •

edited

dantraMSFT commented Nov 12, 2017 •

edited