Update packaging to only package PowerShell binaries when packaging symbols #5145
Conversation
…ting symbols zip. disallow all other package types for symbols
TravisEz13
requested review from
adityapatwardhan and
daxian-dbw
as code owners
|
https://github.com/PowerShell/PowerShell/pull/5145/files?w=1 make this a lot easier to review. |
| elseif(-not $IncludeSymbols.IsPresent -and $Script:Options.CrossGen) { | ||
| $crossGenCorrect = $true | ||
| } | ||
| elseif ($IncludeSymbols.IsPresent) { |
There was a problem hiding this comment.
So now it's OK to specify -IncludeSymbols while build with -Crossgen?
There was a problem hiding this comment.
Yes, the non-publish builds always keep the symbols, unaltered.
tools/packaging/packaging.psm1
Outdated
| log 'setting IncludeSymbols' | ||
| $IncludeSymbols = $PSBoundParameters['IncludeSymbols'] | ||
| } | ||
| log "$($IncludeSymbols.IsPresent):$IncludeSymbols" |
There was a problem hiding this comment.
When -IncludeSymbols is not specified, this log will print just a colon.
There was a problem hiding this comment.
I'll remove... this was troubleshooting...
| { | ||
| $tempPath = [System.IO.Path]::GetTempPath() | ||
|
|
||
| $tempFolder = Join-Path -Path $tempPath -ChildPath ([System.IO.Path]::GetRandomFileName()) |
There was a problem hiding this comment.
How about replace these 2 lines with [System.IO.Path]::GetTempFileName()?
There was a problem hiding this comment.
This API creates the file. I want a folder.
There was a problem hiding this comment.
I know, but you can use that name to create a folder. The two lines of script shown here are doing the same thing.
There was a problem hiding this comment.
I would have to delete the file before I can create a folder with the name.
There was a problem hiding this comment.
I see your point. Yeah, please ignore this comment.
tools/packaging/packaging.psm1
Outdated
| } | ||
| if($IncludeSymbols.IsPresent) | ||
| { | ||
| Remove-Item -Path $Source -Recurse -Force |
There was a problem hiding this comment.
Maybe -ErrorAction SilentlyContinue?
|
@daxian-dbw can you update your review? |
| { | ||
| $tempPath = [System.IO.Path]::GetTempPath() | ||
|
|
||
| $tempFolder = Join-Path -Path $tempPath -ChildPath ([System.IO.Path]::GetRandomFileName()) |
There was a problem hiding this comment.
I know, but you can use that name to create a folder. The two lines of script shown here are doing the same thing.
| } | ||
| if($IncludeSymbols.IsPresent) | ||
| { | ||
| Remove-Item -Path $Source -Recurse -Force -ErrorAction SilentlyContinue |
There was a problem hiding this comment.
It would be great if you can put a comment here saying that "$Source is point to a temporary folder when -IncludeSymbols is present".
3 participants
disallow all other package types for symbols, as this package is no longer intended to be installed.
I run compliance tools on all binaries in this packages and we are getting more issues from the dependencies than from our product binaries.