-
Notifications
You must be signed in to change notification settings - Fork 8
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
Build Ubuntu CI containers on GitHub Actions #85
Conversation
Signed-off-by: Spencer Wilson <[email protected]>
Signed-off-by: Spencer Wilson <[email protected]>
Signed-off-by: Spencer Wilson <[email protected]>
Signed-off-by: Spencer Wilson <[email protected]>
57dac1d
to
ec2caa4
Compare
@SWilson4 looks like I need to make a dockerhub token? anything else? |
- x86_64 | ||
distro: | ||
- focal | ||
- jammy |
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.
Why Jammy? It's not listed in https://github.com/open-quantum-safe/liboqs/blob/main/PLATFORMS.md
Focal is listed, but shouldn't be (if this lands) -- so remove this once PLATFORMS gets updated?
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.
My intent was to update PLATFORMS.md to list support for Focal, Jammy, and Noble (i.e., every Ubuntu LTS version still receiving standard security maintenance) on x86_64 and arm64. Do we really want to support only the latest LTS? Focal will be receiving security updates for another year and Jammy for another three.
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.
Makes some sense -- although I would limit this to the last two LTS' to also minimize our support obligation (if we see it and act on it as such, i.e., to respond/fix issues found on any Tier 1 platform listed).
But is it really necessary & worth while running all platforms at every push/PR? What about running only latest on push/PR and latest-1 only in the weekly regression and release runs?
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.
That seems reasonable to me.
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.
Logically LGTM; some nits and questions on efficiency.
It would be nice but not necessary to have the dockerhub username (oqsbot) saved in a non-secret variable. Other than that nothing. |
@SWilson4 I've added |
What does it take to also add it to oqs-demos, @ryjones ? Would allow implementing open-quantum-safe/oqs-demos#294 |
Disregard. It already works there -- just wonder why now :-) |
Signed-off-by: Spencer Wilson <[email protected]>
3898af2
to
cab34a7
Compare
I'm going to merge now so that I can try out these images in liboqs CI. I may need to add additional dependencies in a follow-up PR. |
This failed at merge due to an incorrect Docker Hub login. :( Could you please take a look @ryjones? |
@SWilson4 fixed :) |
This PR attempts to accomplish the following:
liboqs
,Fixes #81, fixes #74.
Related issues: open-quantum-safe/liboqs#1780.
I'm leaving this as draft until the necessary environment variables/secrets are added (cc @ryjones). And, until it's not a Friday, since this could break all of our CI, and I'd rather not deal with that on the weekend. :)
I may need to further update these Dockerfiles (e.g., to add a dependency) once I have a chance to use them in liboqs CI while working on open-quantum-safe/liboqs#1780.