X Tutup
The Wayback Machine - https://web.archive.org/web/20201204225803/https://github.com/PythonTurtle/PythonTurtle/pull/152
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

Install wxPython and make the ELF executable #152

Open
wants to merge 3 commits into
base: master
from

Conversation

@cclauss
Copy link
Contributor

@cclauss cclauss commented Sep 25, 2020

This pull request pip installs wxPython before pyinstaller creates the Linux ELF file.
It also sets the execution permission on the ELF file so that it can be run.

Related to #107
@cclauss cclauss force-pushed the cclauss:patch-3 branch from 6c327d5 to 67512e3 Sep 25, 2020
@cclauss cclauss changed the title Make the ELF executable Install wxPython and make the ELF executable Sep 25, 2020
@cclauss cclauss marked this pull request as ready for review Sep 25, 2020
@cclauss cclauss mentioned this pull request Sep 25, 2020
@bittner
Copy link
Member

@bittner bittner commented Sep 25, 2020

Please, always provide a motivation for the PR! An explanatory comment, to make it easier to review (understand) the changes.

I'll do this once again on your behalf, if you don't mind.

@cclauss
Copy link
Contributor Author

@cclauss cclauss commented Sep 25, 2020

Done, above.

@@ -13,7 +13,11 @@ jobs:
- uses: actions/setup-python@v2
- run: sudo apt-get install libsdl2-2.0-0
- run: python -m pip install pyinstaller
- env:
URL: https://extras.wxpython.org/wxPython4/extras/linux/gtk3/ubuntu-18.04
run: python -m pip install --find-links ${URL} wxPython

This comment has been minimized.

@bittner

bittner Sep 25, 2020
Member

Nicely solved, congrats!

- run: python setup.py clean bundle
- run: chmod +x /home/runner/work/PythonTurtle/PythonTurtle/dist/PythonTurtle

This comment has been minimized.

@bittner

bittner Sep 25, 2020
Member

I took a look at the documentation. The permissions will be lost in any case. The only solution is to tar up the file instead.

All in all, I'm not happy with the zipp'ed binary. It should be possible to download the binary directly. Also, the Upload-Artifact is probably the wrong Action. There should be a Create-Release action that does what we need, instead. Do you want to give that a shot?

@bittner
Copy link
Member

@bittner bittner commented Sep 29, 2020

I'm happy you continue on this road to get the implementation into a final state.

Note that this can (should!) only be an intermediate solution. It's because where people would look for downloads is always the repository's Releases section. We need to publish new version there, in the long run.

I was afraid I had pointed you into the current direction from the beginning. But looking at our past conversations I see that I suggested using the Create-Release GitHub Action (over the Upload-Artifact) from the beginning. – I believe, uploading artifacts really has a different use case than publishing releases. It's more for intermediate results you want to use in later stages of automation, or asynchronous pipeline processes.

Co-authored-by: Peter Bittner <django@bittner.it>
@cclauss
Copy link
Contributor Author

@cclauss cclauss commented Sep 29, 2020

I suggested using the Create-Release GitHub Action (over the Upload-Artifact) from the beginning.

Yes. I agree but I have taken a couple of shots at using Create-Release without success. Perhaps if you or someone else could create a PR to demonstrate how Create-Release would be applied to this project, then I could plug my efforts into that. Sorry for being so dense.

@bittner
Copy link
Member

@bittner bittner commented Sep 29, 2020

Have you tried the code sample in their README? It should be pretty much copy+paste. Not even configuring a secret seems to be needed:

on:
  push:
    tags:
    - "*"

jobs:
  deploy:
    runs-on: ubuntu-latest
    steps:
    - name: Create Release
      id: create_release
      uses: actions/create-release@v1
      env:
        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
      with:
        tag_name: ${{ github.ref }}
        release_name: PythonTurtle-v${{ github.ref }}

(I omitted the body, draft and prerelease options, assuming they are empty or false by default.)

It's possible that the artifacts that you create with your implementation come in handy here. "Something" needs to be provided for the release to be created. That needs to be figured out.

P.S.: Interestingly, they don't seem to eat their own dog food... 😏 Would be good if we could find a similar project repository that uses that Action. For a working example.

@bittner
Copy link
Member

@bittner bittner commented Sep 29, 2020

It appears that creating the build artifacts and releasing are (meant to be) separate activities.

If that is true, the release workflow may look like this:

  1. Every time a change is merged to master we run the builds that create the artifacts (binaries for all target platforms)
  2. If a Git tag is pushed to master then the create release workflow uses the artifact of builds related to that very commit to craft a release.

Now, how do we access the artifacts of the ${{ github.ref }} and feed them into the Create-Release Action? There are some interesting examples in the issues of their repository, but I didn't yet understand how supplying the artifacts is done. 🤔

@cclauss
Copy link
Contributor Author

@cclauss cclauss commented Sep 29, 2020

One workflow needs other workflows?

@bittner
Copy link
Member

@bittner bittner commented Sep 29, 2020

Good point! I hope that doesn't tightly couple the two workflows. (Imagine you have to directly create a release, but you wanted to - manually - try out the binaries first. That would be possible with independent workflows.)

For the assets, it seems like one must first create a release and then upload assets using the upload_url value of the response of the create release API endpoint. This is also hinted at from their README, I figured. Looks like we're getting closer! 🤞

@bittner
Copy link
Member

@bittner bittner commented Sep 29, 2020

Issue actions/create-release#79 seems to describe a setup that comes close to what we probably need.

Or similar here, actions/create-release#14 (comment).

@cool-RR
Copy link
Member

@cool-RR cool-RR commented Sep 29, 2020

Me walking into this thread like:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.
X Tutup