-
Notifications
You must be signed in to change notification settings - Fork 479
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
Move from CircleCI to GitHub Actions #1849
Conversation
--user ${BUILD_TRIGGER_TOKEN}: \ | ||
--request POST \ | ||
--header "Content-Type: application/json" \ | ||
--data '{ "branch": "OQS-OpenSSL_1_1_1-stable", "parameters": { "run_downstream_tests": true } }' \ |
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.
Doesn't look necessary to keep this one
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.
Same comment as below re liboqs-rust applies here.
--user ${BUILD_TRIGGER_TOKEN}: \ | ||
--request POST \ | ||
--header "Content-Type: application/json" \ | ||
--data '{ "branch": "OQS-v8", "parameters": { "run_downstream_tests": true } }' \ |
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.
When to add/change this to v9?
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.
The v9 branch doesn't yet work; the current PR from @geedo0 gets the basic test suite running but still doesn't have full PQ or hybrid support. I think we can merge this PR with v8, and then when v9 is ready, we can come back and update the liboqs CI.
--request POST \ | ||
--header "Content-Type: application/json" \ | ||
--data '{ "branch": "master" }' \ | ||
https://circleci.com/api/v2/project/gh/open-quantum-safe/liboqs-java/pipeline | tee curl_out \ |
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.
Doesn't look as necessary as triggering liboqs-rust, does it?
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.
Or liboqs-go... My intent with this PR was just to duplicate existing CCI functionality as closely as possible to make sure it worked on GH and make any improvements in a follow-up PR. But I could do both in one shot, if that would be preferable from a review standpoint.
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.
Signed-off-by: Spencer Wilson <spencer.wilson@uwaterloo.ca>
Signed-off-by: Spencer Wilson <spencer.wilson@uwaterloo.ca>
Signed-off-by: Spencer Wilson <spencer.wilson@uwaterloo.ca>
Signed-off-by: Spencer Wilson <spencer.wilson@uwaterloo.ca>
Signed-off-by: Spencer Wilson <spencer.wilson@uwaterloo.ca>
Signed-off-by: Spencer Wilson <spencer.wilson@uwaterloo.ca>
Signed-off-by: Spencer Wilson <spencer.wilson@uwaterloo.ca>
Signed-off-by: Spencer Wilson <spencer.wilson@uwaterloo.ca>
Signed-off-by: Spencer Wilson <spencer.wilson@uwaterloo.ca>
77f380c
to
9e99faa
Compare
The downstream trigger failed after the merge. I can trigger downstream pipelines locally with the same API token that should be in CI, so I think this is a GitHub variables config error. Will follow up with @ryjones later today. |
Duplicate jobs from the CircleCI workflow as closely as possible in GitHub Actions. Remove Ubuntu Bionic / i386 support in CI. --------- Signed-off-by: Spencer Wilson <spencer.wilson@uwaterloo.ca> Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com>
This PR moves our remaining CircleCI workflows over to GitHub Actions. I've left it in draft for now as it's based on the branch for #1848.
I have tried to duplicate the testing coverage from CircleCI without additions or removals. This PR isn't intended to overhaul or evaluate our test coverage, it's just meant to move it to a different platform.
Three items of note:
(1) The CCI
bionic-i386
could not be ported over, as the GitHubcheckout
action requires a newer version ofnode
and can't run in the older container (at least not without some finicky work). Bionic is no longer supported anyhow, so it's probably for the best that we're no longer using it for testing. However, the multiarch images that we'd been using don't have options for i386 past Bionic, so there is no easy way to continue testing on i386. We don't list Linux / x86 as a supported platform, so I've opted to simply remove CI support for it.(2) The CCI
focal-clang-15
job had-DOQS_OPT_TARGET=skylake
set. It didn't configure-DOQS_DIST_BUILD=OFF
, however, so the opt target value was ignored. I have omitted the opt target variable in GitHub CI to reflect what was actually being tested.(3) Finally, @ryjones helped me add a CircleCI API token to the liboqs environment so that GitHub can trigger CircleCI for downstream projects. These triggers are contained in the new
commit-to-main.yml
file. We may want to revise them for completeness, but for now I have left them as they were on CCI.Fixes #1795.