Support creating tarball package for Linux and macOS #5085
Conversation
daxian-dbw
requested review from
SteveL-MSFT,
TravisEz13 and
adityapatwardhan
| { | ||
| Copy-Item -Path $appImageFile.FullName -Destination $destination -force | ||
| } | ||
| "AppImage" { $extraPackages += @(Get-ChildItem -Path $location -Filter '*.AppImage') } |
There was a problem hiding this comment.
Instead of get and then copy, can't we just copy?
Copy-Item -Path "$location/*.AppImage", "$location/*.tar.gz" -Destination $destination -ForceThere was a problem hiding this comment.
Or even this:
Copy-Item -Path "$location/powershell*/*.deb","$location/powershell*/*.rpm","$location/*.AppImage", "$location/*.tar.gz" -Destination $destination -Force
There was a problem hiding this comment.
Please make sure the code logs all copies. Remember this is copying between machines and we need to be able to diagnose where the failure occurred if a file doesn't show up somewhere.
There was a problem hiding this comment.
@TravisEz13 Is Write-Verbose -verbose sufficient for the logging purpose?
There was a problem hiding this comment.
Thanks @adityapatwardhan, your comment is addressed.
@TravisEz13 I added Write-Verbose -Verbose for each file to be copied.
|
|
||
| if (Test-Path -Path $packagePath) { | ||
| if ($Force -or $PSCmdlet.ShouldProcess("Overwrite existing package file")) { | ||
| Write-Verbose "Overwrite existing package file at $packagePath" -Verbose |
There was a problem hiding this comment.
I guess we can remove -Verbose
There was a problem hiding this comment.
Here I put -Verbose explicitly to notify that the existing package is going to be removed.
| } | ||
|
|
||
| if($AppImage.IsPresent) | ||
| $linuxPackages = Get-ChildItem "$location/powershell*" -Include *.deb,*.rpm,*.AppImage,*.tar.gz |
There was a problem hiding this comment.
In the previous commit, *.deb and .rpm packages are found under $location/powershell while *.AppImage and *.tar.gz are found under $location. With this change *.AppImage and *.tar.gz package won't be found.
There was a problem hiding this comment.
It's not under $location/powershell, but $location/powershell* (there is a wildcard * here). All packages start with powershell.
|
@adityapatwardhan Your comments are all addressed/replied. Can you please take another look? |
3 participants
Related to #5027
build.jsonto build and publish tarball package.deb-armtype inStart-PSPackagefor now because the underlyingNew-UnixPackagedoesn't support it.Documentation needs to be added about the dependencies of PowerShell Core, so the tarball package can be useful to users. This task is tracked in #3961