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.

Sign up for GitHub

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

Jump to bottom

Generate AppImage #2082

Merged
merged 4 commits into from
Feb 21, 2017
Merged

Generate AppImage #2082

merged 4 commits into from
Feb 21, 2017

Conversation

andyleejordan
Copy link
Member

Replaces #2027 as it had merge conflicts due to macOS -> OS X rename. Rebased to fix failing Pester test run.

@probonopd and @TravisEz13 please review that I fixed conflicts correctly.

Push build is here.

@msftclas
Copy link

Hi @andschwa, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (Andy Schwartzmeyer). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

@andyleejordan
Copy link
Member Author

Push build succeeded. Pester tests ran and AppImage was created successfully.

@probonopd
Copy link
Contributor

Thanks, looks good to me.

@andyleejordan
Copy link
Member Author

@TravisEz13 can you try out the AppImage?

@TravisEz13
Copy link
Member

@andschwa I see the images generated here, but you don't have artifacts enabled so I cannot download them. https://travis-ci.org/andschwa/PowerShell/builds/155445453

@andyleejordan
Copy link
Member Author

@TravisEz13 ah, of course. Here's an upstream build which should upload the artifacts when its complete.

@probonopd probonopd mentioned this pull request Aug 28, 2016
Closed
@probonopd
Copy link
Contributor

Upload succeeded, download_url: https://s3.amazonaws.com/powershell-artifacts/PowerShell/PowerShell/258/258.1/powershell-6.0.0-alpha.9-x86_64.AppImage

Will test now.

@probonopd
Copy link
Contributor

Tested on Ubuntu 16.04, runs.

@andyleejordan
Copy link
Member Author

andyleejordan commented Aug 28, 2016 •
edited
Loading

LGTM. @TravisEz13?

Edit for clarity: this is on indefinite hold, waiting on @KrisBash.

@andyleejordan
Copy link
Member Author

@probonopd I have a hold that came internally, we can't merge this yet.

@KrisBash can you please explain?

@andyleejordan
Copy link
Member Author

@KrisBash nevermind, I spoke with @probonopd.

lzybkr
lzybkr reviewed Sep 12, 2016
View reviewed changes
tools/appimage.sh Outdated

cd ./$APP/

wget -q https://github.com/probonopd/AppImages/raw/master/functions.sh -O ./functions.sh
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't be fetching scripts from potentially untrusted sources as part of building our package - we should have copies of everything we need in our repo.

Copy link
Contributor

@probonopd probonopd Sep 12, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

functions.sh is part of the probonopd/AppImages project which is under the MIT License so you are free to mirror it but it is neither part of the PowerShell project nor subject to the Microsoft CLA.

The binaries it downloads in the generate_appimage function are built from the sources in the the probonopd/AppImageKit project which is also under the MIT License but is also neither part of the PowerShell project nor subject to the Microsoft CLA. You are free to rebuild them from your own mirror, too, if you like.

@andyleejordan andyleejordan removed their assignment Sep 12, 2016
@andyleejordan
Copy link
Member Author

Hm, I can't assign @KrisBash to the issue.

@lzybkr should we add him as a collaborator?

@probonopd probonopd mentioned this pull request Oct 2, 2016
Closed
@zaxebo1
Copy link

zaxebo1 commented Oct 2, 2016 •
edited
Loading

I am using powershell AppImage on linux , available at https://bintray.com/probono/AppImages/PowerShell . It helps in single portable binary executable file getting executed on various linux distributions.

I will humbly request and very strongly recommend that each powershell release by the powershell development team, should also officially happen in AppImage format , and should be available officially at https://github.com/PowerShell/PowerShell/releases

Please merge the pull request as soon as possible and release it in Powershell 6.0

@andyleejordan
Copy link
Member Author

Hello, and thank you for your input @zaxebo1. I am following up internally on why this is still blocked; we should have a better answer here soon. Thanks again 😄

@SteveL-MSFT
Copy link
Member

Working on getting resolution on this, won't happen til next week earliest

@andyleejordan
Copy link
Member Author

Thanks for following up @SteveL-MSFT 😄

@PowerShellTeam PowerShellTeam added the Review - Needed The PR is being reviewed label Nov 22, 2016
@andyleejordan
Copy link
Member Author

@vors this is ready now as far as I know.

@andyleejordan
Copy link
Member Author

andyleejordan commented Feb 15, 2017 •
edited
Loading

Steve can you send the attributions requirements again?

Found and included them.

andyleejordan
andyleejordan commented Feb 15, 2017
View reviewed changes
README.md Outdated
@@ -39,13 +40,15 @@ You can download and install a PowerShell package for any of the following platf
[rl-ubuntu16]: https://github.com/PowerShell/PowerShell/releases/download/v6.0.0-alpha.15/powershell_6.0.0-alpha.15-1ubuntu1.16.04.1_amd64.deb
[rl-ubuntu14]: https://github.com/PowerShell/PowerShell/releases/download/v6.0.0-alpha.15/powershell_6.0.0-alpha.15-1ubuntu1.14.04.1_amd64.deb
[rl-centos]: https://github.com/PowerShell/PowerShell/releases/download/v6.0.0-alpha.15/powershell-6.0.0_alpha.15-1.el7.centos.x86_64.rpm
[rl-ai]: TODO: Post link when availabl
Copy link
Member Author

Choose a reason for hiding this comment

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

Oops a typo.

@andyleejordan
Copy link
Member Author

  • Rebased and merge conflicts resolved
  • Pinned external code download to specific commit hash
  • Included additional third party licenses from legal
  • Successful build of AppImage binary
  • Anything else?

@PowerShell/powershell-maintainers you'll want to include the AppImage binary in the next release on GitHub, and fix the link in the readme.

@andyleejordan
Copy link
Member Author

@SteveL-MSFT handing over to you for final review and merge 😄

@joeyaiello
Copy link
Contributor

Anything else?

@andschwa just another rebase ;)

@andyleejordan
Copy link
Member Author

andyleejordan commented Feb 17, 2017 •
edited
Loading

@joeyaiello your turn!

All right, taken care of. But if the tests fail, it's not my fault!

probonopd and others added 4 commits February 17, 2017 11:07
@probonopd @andyleejordan
Add AppImage tooling
0e9d850 
* Use icon from local repository
  PowerShell#2027 (comment)

* Use the deb that has been generated in this build

* Copyright and license
  PowerShell#2027 (comment)

* MIT License for appimage.sh
  PowerShell#2027 (comment)

* Full text of the MIT License is in license_thirdparty_proprietary.txt

* Clarify license and clean up unused code
  PowerShell#2027 (comment)

* Mark appimage.sh as executable
@probonopd @andyleejordan
Add AppImage documentation
0c1b117
@andyleejordan
Add AppImage to CI
8185fd9
@andyleejordan
Add Third Party Notices for AppImage
db93f41 
This license specifically covers the additional libraries that
are bundled inside the AppImage archive.
@probonopd
Copy link
Contributor

Never ending story ;-) @andschwa

SteveL-MSFT
SteveL-MSFT approved these changes Feb 21, 2017
View reviewed changes
Copy link
Member

@SteveL-MSFT SteveL-MSFT left a comment

Choose a reason for hiding this comment

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

LGTM

@SteveL-MSFT SteveL-MSFT merged commit 152de40 into PowerShell:master Feb 21, 2017
@probonopd
Copy link
Contributor

Thanks for merging. Minor cosmetic glitch, in README.md it now says [.AppImage][rl-ai] which doesn't look right.

@SteveL-MSFT
Copy link
Member

@probonopd that's because that link doesn't exist yet and points to a TODO

@SteveL-MSFT
Copy link
Member

@raghushantha do you need to update your publishing pipeline to generate the appimage? once we have an official appimage, can you update the link to point to it?

@andyleejordan andyleejordan deleted the appimage branch February 21, 2017 19:59
@andyleejordan
Copy link
Member Author

Woo hoo!

@probonopd
Copy link
Contributor

Let's celebrate @andschwa 🥇

@SteveL-MSFT SteveL-MSFT mentioned this pull request Feb 21, 2017
Closed
@raghushantha
Copy link
Member

@SteveL-MSFT I have integrated AppImage generation as part of the Release pipeline.
Got a couple of concerns which I will followup offline

@daxian-dbw daxian-dbw mentioned this pull request Mar 3, 2017
Closed
@iSazonov iSazonov removed the Review - Needed The PR is being reviewed label Mar 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SteveL-MSFT SteveL-MSFT SteveL-MSFT approved these changes

Assignees

@SteveL-MSFT SteveL-MSFT

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet