ci: Add `release` job

hebasto commented at 7:40 pm on August 28, 2023: member

This PR introduces a new “Release” job that conducts sanity checks as defined in doc/release-process.md.

hebasto force-pushed on Aug 28, 2023

hebasto renamed this:
~~ci: Add `release` workflow~~
gha: Add `release` workflow
on Aug 28, 2023

in .github/workflows/release.yml:1 in 94dd59a525 outdated

0@@ -0,0 +1,42 @@
1+# See: https://github.com/bitcoin-core/secp256k1/blob/master/doc/release-process.md.

real-or-random commented at 9:07 am on August 29, 2023:

0# See: https://github.com/bitcoin-core/secp256k1/blob/master/doc/release-process.md

nit: link is broken otherwise

hebasto commented at 9:43 am on August 29, 2023:

Thanks! Fixed.

real-or-random commented at 9:12 am on August 29, 2023: contributor

This is neat!

We could also print the file listings for both installation methods (something like cd ${{ env.CI_INSTALL }} && find) and/or create tarball artifacts of the installation directories.

real-or-random added the label assurance on Aug 29, 2023

real-or-random added the label ci on Aug 29, 2023

real-or-random added the label build on Aug 29, 2023

hebasto commented at 9:16 am on August 29, 2023: member

We could also print the file listings for both installation methods (something like cd ${{ env.CI_INSTALL }} && find)

Instead of the current ls -l ${{ env.CI_INSTALL }}/include ${{ env.CI_INSTALL }}/lib, right?

… and/or create tarball artifacts of the installation directories.

What for?

hebasto force-pushed on Aug 29, 2023

real-or-random commented at 9:35 am on August 29, 2023: contributor

We could also print the file listings for both installation methods (something like cd ${{ env.CI_INSTALL }} && find)

Instead of the current ls -l ${{ env.CI_INSTALL }}/include ${{ env.CI_INSTALL }}/lib, right?

Yep, it’s good to see if too many files are installed.

… and/or create tarball artifacts of the installation directories.

What for?

This would allow for an even closer inspection of the installation, including file sizes etc. But yeah, it’s probably overkill. If people want to see this, they should check locally. We could also use ls -RlAh instead of using find, and this prints sizes and has also a nice format.

hebasto force-pushed on Aug 29, 2023

hebasto commented at 9:43 am on August 29, 2023: member

Updated 94dd59a525e22221190a7a18e5d96f509666daf8 -> f9ba3f200a5e67eb1ba454ee478c2bc023d2c2a2 (pr1413.03 -> pr1413.05, diff).

The following @real-or-random’s comments have been addressed:

hebasto force-pushed on Aug 29, 2023

hebasto commented at 10:07 am on August 29, 2023: member

For testing purposes, I set this pull request branch as the default one in my fork and manually triggered the new workflow: https://github.com/hebasto/secp256k1/actions/runs/6010823279.

real-or-random approved

real-or-random commented at 12:14 pm on August 29, 2023: contributor

ACK 0ae58afcedfe24babd7d58b69b3b69f5f43aa79b

real-or-random added this to the milestone 0.3.3 (or 0.4.0) on Aug 29, 2023

sipa commented at 1:36 pm on September 4, 2023: contributor

Is there a reason these checks couldn’t just run always as a normal CI job?

hebasto commented at 1:42 pm on September 4, 2023: member

Is there a reason these checks couldn’t just run always as a normal CI job?

If your question is about a separated workflow, the point is to allow the maintainer to run it at their convenience (on.workflow_dispatch).

If your concerns are about running the workflow conditionally only on release PRs, than that condition might be dropped from the script.

real-or-random commented at 1:48 pm on September 4, 2023: contributor

If your concerns are about running the workflow conditionally only on release PRs, than that condition might be dropped from the script.

I think that was his concern. I agree, we could just run these checks every time. They should always succeed.

sipa commented at 1:49 pm on September 4, 2023: contributor

It - in theory - makes sense to me that we have a workflow that can be run by maintainers at release time, to perform certain tests that we want only then.

But all of the tests currently included in this workflow appear to me to be things we want always. So why not add them to normal CI instead?

hebasto force-pushed on Sep 4, 2023

hebasto renamed this:
~~gha: Add `release` workflow~~
ci: Add `release` job
on Sep 4, 2023

ci: Update `actions/checkout` version f9b38894ba

hebasto force-pushed on Sep 4, 2023

hebasto commented at 2:00 pm on September 4, 2023: member

But all of the tests currently included in this workflow appear to me to be things we want always. So why not add them to normal CI instead?

Done.

ci: Add `release` job

The new job runs checks outlined in the `doc/release-process.md`.

8659a01714

in .github/workflows/ci.yml:787 in cf58df7d86 outdated

783           sage prove_group_implementations.sage
784+
785+  release:
786+    runs-on: ubuntu-latest
787+    # The `startsWith` and `contains` functions are not case sensitive.
788+    if: >

sipa commented at 2:01 pm on September 4, 2023:

This conditional also needs to go away I think, or it won’t do anything.

hebasto commented at 2:05 pm on September 4, 2023:

Thanks! Fixed.

hebasto force-pushed on Sep 4, 2023

hebasto commented at 2:06 pm on September 4, 2023: member

The PR description has been updated.

sipa commented at 2:24 pm on September 4, 2023: contributor

ACK 8659a01714c1b4fcd349ee1a7d733f6934c5d184

real-or-random approved

real-or-random commented at 2:26 pm on September 4, 2023: contributor

ACK 8659a01714c1b4fcd349ee1a7d733f6934c5d184

real-or-random merged this on Sep 4, 2023

real-or-random closed this on Sep 4, 2023

hebasto deleted the branch on Sep 4, 2023

ci: Add release job #1413

ci: Add `release` job #1413