Allow packaging from a zip package to allow for signing#5418
Allow packaging from a zip package to allow for signing#5418daxian-dbw merged 8 commits intoPowerShell:masterfrom
Conversation
TravisEz13
commented
Nov 11, 2017
- Include a serialized version of PSOptions in an includesymbols zip
- Add a function which will create a zip package from the expanded includesymbols zip and a folder of signed files
- add a function to restore an includesymbols zip as a build and populated PSOptions with the options
- update related files
add ability to set the PSOptions
04ab9c7 to
94ca4c3
Compare
|
rebased due to merge conflict |
build.psm1
Outdated
There was a problem hiding this comment.
So the argument passed in could be a PSObject, right?
tools/packaging/packaging.psm1
Outdated
There was a problem hiding this comment.
Be careful about the entries PSModuleRestore and CrossGen when convert to/from json. Currnetly the values of these 2 are SwitchParameter values, which works fined with the comparison to bool. However, after convert from json, you will get a synthetic property named 'IsPresent' with a bool value, and the comparison to bool would behave differently. See the example:
> $p = New-PSOptions
> $s= $p | ConvertTo-Json | ConvertFrom-Json
> if (-not $p.PSModuleRestore) { echo "ff" }
ff
> if (-not $s.PSModuleRestore) { echo "ff" } ## print nothing.
I suggest to not use the SwitchParameter value in the first place. Instead, use $CrossGen.IsPresent and $PSModuleRestore.IsPresent when setting the initial $options
There was a problem hiding this comment.
updated to suggested implementation
tools/packaging/packaging.psm1
Outdated
There was a problem hiding this comment.
Can you please explain again why we need to restore modules again here? After restoring the modules, they only exist in the publish folder. We do signing and other compliance check using the files from the parent folder of publish so those restored modules won't affect any of that ...
There was a problem hiding this comment.
This is the first time modules are being restored. Restoring them during the initial build would add size to the files we are carrying around. It also ensures 100% that we do not sign 3rd party code, which not signing 3rd party code is a requirement.
There was a problem hiding this comment.
Restoring them during the initial build would add size to the files we are carrying around.
This makes sense. My worry is that the package management cmdlet may fail when running Restore-PSModuleToBuild when working with myget. I see this failure often enough in our CI runs. Before this change, the build can fail fast if this happens, but after this change, the signing effort would be in vain in case this happens.
But this is not a blocking comment. Let's do this and see how it works.
tools/packaging/packaging.psm1
Outdated
There was a problem hiding this comment.
Why checking for Linux? We don't sign individual files for Linux so this function should never be used in a Linux build, right?
There was a problem hiding this comment.
Minor: I guess we should only accept win7-x64 and win7-x86 now.
tools/releaseBuild/build.json
Outdated
There was a problem hiding this comment.
Just curious, why do we want to limit the memory for the container?
There was a problem hiding this comment.
This is increasing the memory in the container on many machines. I cannot add comments in JSON. but the packaging script verifies that we have increased the memory to at least 2GB.
There was a problem hiding this comment.
I'd say a minimum of this value if I could but they don't have this option.
tools/releaseBuild/vstsbuild.ps1
Outdated
There was a problem hiding this comment.
There are no common parameters that can be used for the default set. It seems to me we should use Build as the default parameter set.
tools/releaseBuild/vstsbuild.ps1
Outdated
There was a problem hiding this comment.
Previously, you pass in $PSBoundParameters, so if -ReleaseTag is not specified, it won't be passed to Invoke-Build.
Now you always pass in ReleaseTag, and the value will be $null in case -ReleaseTag is not specified when calling this script. Will this cause any problems?
There was a problem hiding this comment.
The new hashtable I have has ReleaseTag
There was a problem hiding this comment.
No, this won't cause any issues. I've verified this previously.
|
started a build at: https://mscodehub.visualstudio.com/PowerShellCore/_build/index?buildId=5708&_a=summary to verify |