-
Notifications
You must be signed in to change notification settings - Fork 283
feat: add summary to Action #2469
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
Conversation
afe6ae0
to
4a4812f
Compare
8951944
to
27ce67b
Compare
6ae8343
to
6316b3e
Compare
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
b525035
to
dc25a28
Compare
This is pretty cool. I couldn't help myself - I had a play around with formatting for this, what do you think?
🎡 cibuildwheelBuild optionsplatform: macos
architectures: arm64, universal2, x86_64
build_selector:
build_config: *
skip_config:
requires_python: None
enable: ['cpython-experimental-riscv64', 'cpython-freethreading', 'cpython-prerelease']
output_dir: /Users/runner/work/cibuildwheel/cibuildwheel/wheelhouse
package_dir: /Users/runner/work/cibuildwheel/cibuildwheel/sample_proj
build_frontend: build[uv] 17 wheels created
|
Wheel | Size | Build identifier | Time |
---|---|---|---|
spam-0.1.0-cp38-cp38-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl | 8.2 kB | cp38-manylinux_x86_64 | 2s |
spam-0.1.0-cp39-cp39-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl | 7.7 kB | cp39-manylinux_x86_64 | 1s |
spam-0.1.0-cp310-cp310-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl | 7.8 kB | cp310-manylinux_x86_64 | 1s |
spam-0.1.0-cp311-cp311-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl | 7.8 kB | cp311-manylinux_x86_64 | 1s |
spam-0.1.0-cp312-cp312-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl | 8.0 kB | cp312-manylinux_x86_64 | 1s |
spam-0.1.0-cp313-cp313-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl | 8.0 kB | cp313-manylinux_x86_64 | 1s |
spam-0.1.0-cp313-cp313t-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl | 8.1 kB | cp313t-manylinux_x86_64 | 1s |
spam-0.1.0-cp314-cp314-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl | 8.1 kB | cp314-manylinux_x86_64 | 1s |
spam-0.1.0-cp314-cp314t-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl | 8.1 kB | cp314t-manylinux_x86_64 | 1s |
spam-0.1.0-cp38-cp38-musllinux_1_2_x86_64.whl | 7.9 kB | cp38-musllinux_x86_64 | 2s |
spam-0.1.0-cp39-cp39-musllinux_1_2_x86_64.whl | 7.8 kB | cp39-musllinux_x86_64 | 1s |
spam-0.1.0-cp310-cp310-musllinux_1_2_x86_64.whl | 8.0 kB | cp310-musllinux_x86_64 | 2s |
spam-0.1.0-cp311-cp311-musllinux_1_2_x86_64.whl | 8.0 kB | cp311-musllinux_x86_64 | 1s |
spam-0.1.0-cp312-cp312-musllinux_1_2_x86_64.whl | 8.1 kB | cp312-musllinux_x86_64 | 1s |
spam-0.1.0-cp313-cp313-musllinux_1_2_x86_64.whl | 8.2 kB | cp313-musllinux_x86_64 | 2s |
spam-0.1.0-cp313-cp313t-musllinux_1_2_x86_64.whl | 8.3 kB | cp313t-musllinux_x86_64 | 1s |
spam-0.1.0-cp314-cp314-musllinux_1_2_x86_64.whl | 8.2 kB | cp314-musllinux_x86_64 | 2s |
spam-0.1.0-cp314-cp314t-musllinux_1_2_x86_64.whl | 8.3 kB | cp314t-musllinux_x86_64 | 2s |
I didn't implement it yet, just playing around with HTML/markdown.
Those improvements sound good! |
Would you like me to have a crack at implementing that new format? I might get some time towards the end of this week. |
Sure, I'm heading to SciPy then PyHEP.dev in Seattle. Likely won't have much time for a while. |
Cool! Enjoy! |
Here's where I landed- 🎡 cibuildwheelBuild optionsoutput_dir: /private/var/folders/ld/k24nt7054698bctspqwrjq1r0000gn/T/tmpxz77ycv4/wheelhouse
package_dir: /private/var/folders/ld/k24nt7054698bctspqwrjq1r0000gn/T/tmpxz77ycv4
6 wheels created in 26 seconds
|
) | ||
|
||
out.write( | ||
textwrap.dedent("""\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use inspect.cleandoc
instead of textwrap.dedent
then you don't have to worry about leading/trailing whitespace. Up to you which you prefer.
textwrap.dedent("""\ | |
inspect.cleandoc(""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No complaints from the iOS side of things - and the rest of the implementation makes sense to me as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good for Pyodide-side changes as well. The summary looks quite decent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks nice, thanks, @henryiii and @joerick!
I had one comment about the summaries when being added to Markdown: https://github.com/pypa/cibuildwheel/actions/runs/16222307793/attempts/1#summary-45805767785
As you can see when expanding the "Build options" dropdown, there are remnants of ANSI colour codes in the output at the end of each option, like this: �[0m
. I think these are coming from the fact that GitHub step summaries convert ANSI into raw UTF-8, where these codes become invisible bytes and show up in this weird manner. I suggest that we turn off ANSI colouring when writing Markdown output to the step summary, and I think there should be two ways to handle this:
- either we add another argument such as
options.summary(..., colors_enabled=True)
and then doc = Colors(enabled=colors_enabled)
instead ofc = log.colors
; - or we add another
options.summary_markdown(...)
variant which disables colours and call that one in_github_step_summary()
What do you think? I'm also happy to do so myself in a separate PR, not blocking this PR, or push a few commits here itself :D
Also, would it make sense to add a SHA-256 checksum column to the table in the future? It would be really nice to grab the hashes for the wheels directly from cibuildwheel's output and sanity check (manually) if they match, say, artifacts for a GitHub release or those uploaded to PyPI. |
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! My concerns and questions were resolved, and it looks much better. Thank you!
Signed-off-by: Henry Schreiner <[email protected]>
This prints out a summary in the GitHub action step.
Some logging commands were missing for Pyodide.
You can see the general idea, but I plan to flesh it out soon.