Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support creating tarball package for Linux and macOS #5085

Merged
merged 2 commits into from
Oct 12, 2017

Conversation

daxian-dbw
Copy link
Member

Related to #5027

  • Create tarball package to allow advanced deployment on Linux and macOS.
  • Update build.json to build and publish tarball package.
  • Disable deb-arm type in Start-PSPackage for now because the underlying New-UnixPackage doesn'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

{
Copy-Item -Path $appImageFile.FullName -Destination $destination -force
}
"AppImage" { $extraPackages += @(Get-ChildItem -Path $location -Filter '*.AppImage') }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of get and then copy, can't we just copy?

Copy-Item -Path "$location/*.AppImage", "$location/*.tar.gz" -Destination $destination -Force

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or even this:

Copy-Item -Path "$location/powershell*/*.deb","$location/powershell*/*.rpm","$location/*.AppImage", "$location/*.tar.gz" -Destination $destination -Force

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TravisEz13 Is Write-Verbose -verbose sufficient for the logging purpose?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we can remove -Verbose

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not under $location/powershell, but $location/powershell* (there is a wildcard * here). All packages start with powershell.

@daxian-dbw
Copy link
Member Author

@adityapatwardhan Your comments are all addressed/replied. Can you please take another look?

Copy link
Member

@adityapatwardhan adityapatwardhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@adityapatwardhan adityapatwardhan merged commit a391998 into PowerShell:master Oct 12, 2017
@daxian-dbw daxian-dbw deleted the tarball branch October 15, 2017 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants