Skip to content

fix: action still runs if setup-python is not called #542

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

Merged
merged 4 commits into from
Jan 20, 2021

Conversation

henryiii
Copy link
Contributor

@henryiii henryiii commented Jan 17, 2021

This fixes the action so that it runs regardless of whether setup-python has been called first, on all runners except ubuntu 16.04. on ubuntu-20.04 or macos-latest (it always worked on windows-latest). It does not fix ubuntu-18.04 (ubuntu-latest), but hopefully that will become 20.04 soon.

I'm not proposing we change the directions, just make it more likely to work if someone doesn't follow them.

@henryiii henryiii mentioned this pull request Jan 17, 2021
Copy link
Member

@YannickJadoul YannickJadoul left a comment

Choose a reason for hiding this comment

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

Nice! Azure Pipelines failure seems unrelated?

@henryiii
Copy link
Contributor Author

henryiii commented Jan 17, 2021

I have a small simplification from a response to a GHA issue, but when I was looking through their setup for Action's runners, I realize they promise something exciting - let me try something totally different, and I'll revert it if it doesn't work.

@henryiii henryiii force-pushed the fix/action3 branch 2 times, most recently from f0fed01 to 2f9108c Compare January 17, 2021 04:27
@henryiii
Copy link
Contributor Author

henryiii commented Jan 17, 2021

Wow, this is fantastic! I noticed that internally GitHub actions is using pipx to install apps in their scripts, so I checked, and pipx is one of the package systems GitHub promises for all their modern runners. This is fantastic, because pipx was designed for this!

The action no longer depends on or is affected by setup-python at all, we should remove this now from our action documentation. Okay, looks like I forgot to put in in the action documentation. 😳 We always get Python 3 through pipx (not sure if it defaults to the system Python 3; if it does, then we get 3.6.9 on ubuntu 18.04; it can be set if you have a command or path). There are no side effects to the action now; after it runs, cibuildwheel and its dependencies are not left over and installed somewhere. (Though pipx keeps a cache, so rerunning is instant, if you do run the action twice in a job). This also supports all GitHub runners now, including the older ubuntu, windows, etc. (Except 16.04, it seems that predated the pipx setup; so all reasonable images, anyway. windows-2016 for example is fine).

Should we update our docs to use pipx instead of pip for the non-action GHA examples (probably azure, too, since they are the same runners)? pipx is the correct tool for a Python application, it's just usually not worth installing pipx just to get isolation in CI where it's already isolated. But since GHA promises it (and in fact it's apps are built on top of it), we could switch to using it. The command would be pipx run cibuildwheel==1.7.4 <args>, no separate install step needed, no side effects, no setup-python required.

Example run (ignore the Windows 2.7 failure, broken for a different reason and not using this action anyway): https://github.com/scikit-hep/boost-histogram/actions/runs/491092272

@henryiii
Copy link
Contributor Author

henryiii commented Jan 17, 2021

Could we add the Python version being used to the logging output? Something like this:

     _ _       _ _   _       _           _
 ___|_| |_ _ _|_| |_| |_ _ _| |_ ___ ___| |
|  _| | . | | | | | . | | | |   | -_| -_| |
|___|_|___|___|_|_|___|_____|_|_|___|___|_|
 
cibuildwheel version 1.7.4 - hosted on Python 3.9.1

That would help me tell what it's using here.

@YannickJadoul
Copy link
Member

Should we update our docs to use pipx instead of pip for the non-action GHA examples (probably azure, too, since they are the same runners)? pipx is the correct tool for a Python application, it's just usually not worth installing pipx just to get isolation in CI where it's already isolated.

I'll leave that call to @joerick, but when picking one, I would leave a comment hinting at the other one, at least :-)

Could we add the Python version being used to the logging output?

That makes sense to me; that's also what e.g. pytest does. Again, @joerick's preamble (and ASCII art), though ;-)

@henryiii
Copy link
Contributor Author

but when picking one, I would leave a comment hinting at the other one, at least :-)

Agreed, though might be enough to see it listed other places, depending on how it was included. Let's leave that for later, so it doesn't hold up this PR. (And there is the tiny downside that ubuntu-16.04 doesn't have it, but people should not be using that now, and you can always pip install pipx or convert the pipx line back to pip.)

@henryiii
Copy link
Contributor Author

PS: GitHub said they would add a symlink for python3 pointing at python on Windows runner. This will make running python3 without a setup-python run more consistent in the future for composite actions. :) But in this case, pipx is the correct tool anyway.

@henryiii
Copy link
Contributor Author

I would recommend we keep the "main" readme example as general as possible (not that pipx is unique to GHA/Azure, but I think that's the only place it's preinstalled as part of the standard image) - the main example not really supposed to be tailored to GHA, it just has to use some CI, and GHA is the most popular CI on GitHub currently. Then, post 1.8, we could change the dedicated GitHub Actions example to use pipx, with a note. (Or, we could change it to use the action).

It is a bit tempting to make the main example as short and simple as possible, though. ;)

@YannickJadoul
Copy link
Member

YannickJadoul commented Jan 19, 2021

It is a bit tempting to make the main example as short and simple as possible, though. ;)

Compared to what it's doing/promising, it's already pretty short and simple, now? I don't think we need to be too worried to scare off users; I think the simplicity the thing now already offers for the huge packaging task (the thing that no one likes) is already quite attractive? :-D

@henryiii
Copy link
Contributor Author

Yes, I probably wouldn't propose changing the main example, though this is so beautiful :)

- uses: actions/checkout@v2
- run: pipx run cibuildwheel==1.7.2
- uses: actions/upload-artifact@v2
  with:
    path: wheelhouse/*

And there's nothing unique there to GHA except the fact that pipx comes pre-installed :) Of course, this is also beautiful, but unique to GHA:

- uses: actions/checkout@v2
- uses: joerick/[email protected]
- uses: actions/upload-artifact@v2
  with:
    path: wheelhouse/*

@henryiii
Copy link
Contributor Author

henryiii commented Jan 20, 2021

PS: This is also great for build to make an SDist, by the way:

- uses: actions/checkout@v2
- run: pipx run --spec build pyproject-build --sdist
- uses: actions/upload-artifact@v2
  with:
    path: dist/*

I've opened an issue with a proposal to get pipx to support mismatched executable scripts and package names via a pipx.run entry point; that would reduce this to pipx run build --sdist. :)

@YannickJadoul
Copy link
Member

YannickJadoul commented Jan 20, 2021

That's true, though, yes :-D

How does pipx know in which packages to look for these commands, btw? Oh, right, I see. It needs to match the package name :-/

@henryiii
Copy link
Contributor Author

Currently, it looks for a entrypoint or script matching the package name inside the package. So pipx run cibuildwheel will make a venv, download a package called cibuildwheel, and run an entrypoint/script called cibuildwheel from that env. You can split the package download and script name by using --spec <packagename> instead. My proposal was to add a pipx.run entrypoint which would allow a package to say "Here's my pipx run entrypoint" if the name of the application and package differ.

@YannickJadoul
Copy link
Member

My proposal was to add a pipx.run entrypoint which would allow a package to say "Here's my pipx run entrypoint" if the name of the application and package differ.

Yeah, just realized. I had confused the package name with the entry point, since I knew python -m build works ;-)

Copy link
Contributor

@joerick joerick left a comment

Choose a reason for hiding this comment

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

Looks good!

@henryiii henryiii merged commit 4a02410 into pypa:master Jan 20, 2021
@henryiii henryiii deleted the fix/action3 branch June 6, 2024 15:20
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.

3 participants