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

Release signed binaries #871

Merged
merged 2 commits into from
Apr 11, 2024
Merged

Release signed binaries #871

merged 2 commits into from
Apr 11, 2024

Conversation

agateau-gg
Copy link
Collaborator

@agateau-gg agateau-gg commented Apr 10, 2024

Context

This PR extends our CI so that signed macOS binaries are built on each release.

What has been done

Since testing CI code which only triggers on tags is not really possible, I used the following workarounds:

  1. The existing sign.yml workflow has been turned into a more generic build_release_assets.yml workflow. This workflow is called by the tag.yml workflow. It can also be called manually.

  2. I added a faketag.yml workflow. This is a copy of tag.yml except it triggers on pull requests and does not publish anything: each publish step is replaced by a step listing the downloaded artifacts which would be published. This allow verifies the changes to tag.yml as much as possible. You can see a run of it here.
    This faketag.yml will be removed before merging the PR.

Review

First commit contains the real work. Second commit just adds faketag.yml and can mostly be ignored (apart from verifying changes made to tag.yml are also present in faketag.yml). I will revert that commit before merging.

@agateau-gg agateau-gg force-pushed the agateau/release-signed-binaries branch 3 times, most recently from 7b4503f to de20b69 Compare April 10, 2024 09:50
@agateau-gg agateau-gg force-pushed the agateau/release-signed-binaries branch from de20b69 to e0f2a90 Compare April 10, 2024 09:58
@codecov-commenter
Copy link

codecov-commenter commented Apr 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.65%. Comparing base (26cac3c) to head (de20b69).
Report is 16 commits behind head on main.

❗ Current head de20b69 differs from pull request most recent head e0f2a90. Consider uploading reports for the commit e0f2a90 to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #871      +/-   ##
==========================================
- Coverage   91.92%   91.65%   -0.27%     
==========================================
  Files         170      170              
  Lines        7058     7061       +3     
==========================================
- Hits         6488     6472      -16     
- Misses        570      589      +19     
Flag Coverage Δ
unittests 91.65% <ø> (-0.27%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

To make this work, the sign.yml workflow was turned into a more generic
build_release_assets.yml workflow, which contains all the steps to build
the assets currently built by tag .yml (sdist, wheel, deb, rpm)

The tag.yml workflow calls build_release_assets.yml while it's running,
but it is also possible to trigger build_release_assets.yml manually.
@agateau-gg agateau-gg force-pushed the agateau/release-signed-binaries branch from e0f2a90 to 3610508 Compare April 10, 2024 10:15
Copy link
Contributor

@fnareoh fnareoh 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 but I"m not sure I understand why you end up with two macos binaries here what is "macos-14" ?

@agateau-gg
Copy link
Collaborator Author

Looks good but I"m not sure I understand why you end up with two macos binaries here what is "macos-14" ?

GitHub terminology is confusing:

  • macos-14 is for ARM-based macOS, it uses macOS 14
  • macos-latest is for Intel-based macOS

At least that was my understanding until I went back to https://github.com/actions/runner-images, and noticed macos-latest is now the same as macos-14! That changed on April 2.

That's my punishment for not pinning runner versions I guess 😒. Going to pin this runner (and the Ubuntu one).

@salome-voltz
Copy link
Collaborator

Looks good to me, I'm just not sure to understand how to trigger the Build release assets workflow manually ?

@agateau-gg
Copy link
Collaborator Author

Looks good to me, I'm just not sure to understand how to trigger the Build release assets workflow manually ?

The place to trigger workflows is https://github.com/GitGuardian/ggshield/actions, but there is a trick: you can't trigger it right now because I renamed it, and the workflow must exist in the default branch to be able to trigger it 😞.

Right now you can try to trigger the sign workflow, the old name for build_release_assets, which is in default branch.

@agateau-gg agateau-gg force-pushed the agateau/release-signed-binaries branch from c82b6db to e3ec990 Compare April 11, 2024 07:44
@agateau-gg agateau-gg marked this pull request as ready for review April 11, 2024 07:44
macos-latest runner is currently an Intel-based macOS, but it's being
migrated to become macos-14, which is ARM-based. Replace it with macos-13,
which is Intel-based, according to [1].

Similarly, turn ubuntu-latest into ubuntu-22.04 to avoid potential
surprises when ubuntu-latest becomes ubuntu-24.04.

[1]: https://github.com/actions/runner-images/blob/a0af039ba1565df601a4c5630c264ab777daf66a/README.md
@agateau-gg agateau-gg force-pushed the agateau/release-signed-binaries branch from e3ec990 to 50a2a0c Compare April 11, 2024 08:30
@agateau-gg agateau-gg merged commit e1fa88e into main Apr 11, 2024
26 checks passed
@agateau-gg agateau-gg deleted the agateau/release-signed-binaries branch April 11, 2024 08:51
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.

4 participants