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

Remove rainbow #1321

Merged
merged 2 commits into from
Nov 28, 2022
Merged

Remove rainbow #1321

merged 2 commits into from
Nov 28, 2022

Conversation

xvzcf
Copy link
Contributor

@xvzcf xvzcf commented Nov 24, 2022

Removes the rainbow signature scheme as per #1317. I'll submit a PR per algorithm since removing each one is not totally mechanical and straightforward. Once all the algorithms are removed I'll move on to the downstream projects.

  • Does this PR change the input/output behaviour of a cryptographic algorithm (i.e., does it change known answer test values)? (If so, a version bump will be required from x.y.z to x.(y+1).0.)
  • Does this PR change the the list of algorithms available -- either adding, removing, or renaming? Does this PR otherwise change an API? (If so, PRs in oqs-provider, OQS-OpenSSL, OQS-BoringSSL, and OQS-OpenSSH will also need to be ready for review and merge by the time this is merged.)

Copy link
Member

@baentsch baentsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Piecemeal removal is OK -- but is the assumption that you merge each PR before moving on to the next? If so, I'd strongly suggest you work "backwards", i.e., first remove the algs from downstream use. Otherwise, this approach will generate many failed CI runs. If not, i.e., you intend to merge only when all algs are done and then move on "downstream", the failing downstream CI runs will not be so numerous ... maybe I'll install an oqs-github-mailstorm-filter on my Inbox :)

@xvzcf
Copy link
Contributor Author

xvzcf commented Nov 24, 2022

Piecemeal removal is OK -- but is the assumption that you merge each PR before moving on to the next? If so, I'd strongly suggest you work "backwards", i.e., first remove the algs from downstream use. Otherwise, this approach will generate many failed CI runs. If not, i.e., you intend to merge only when all algs are done and then move on "downstream", the failing downstream CI runs will not be so numerous ... maybe I'll install an oqs-github-mailstorm-filter on my Inbox :)

If we let the downstream CIs fail for a few days, once all the algorithms are removed in liboqs it'll just be one PR to each downstream as opposed to the 4 PRs per downstream if we update each time we remove an algorithm here. Granted, it would be ideal if each PR in liboqs gets reviewed and merged quickly to minimize the number of emails sent.

@baentsch
Copy link
Member

If we let the downstream CIs fail for a few days, once all the algorithms are removed in liboqs it'll just be one PR to each downstream as opposed to the 4 PRs per downstream if we update each time we remove an algorithm here.

The above does not argue for (4) downstream PR's per algorithm removal, but for first removing all algs from the downstream projects in one go: The downstream projects can build OK with the algorithms-to-be-removed-in-liboqs eliminated/not referenced any more -- but not vice versa. Therefore PRs for the downstreams have just been created: If those would be OK for you (and get merged), removal of the algs in liboqs should be possible to proceed piecemeal without any CI breakages.

@dstebila
Copy link
Member

If we let the downstream CIs fail for a few days, once all the algorithms are removed in liboqs it'll just be one PR to each downstream as opposed to the 4 PRs per downstream if we update each time we remove an algorithm here.

The above does not argue for (4) downstream PR's per algorithm removal, but for first removing all algs from the downstream projects in one go: The downstream projects can build OK with the algorithms-to-be-removed-in-liboqs eliminated/not referenced any more -- but not vice versa. Therefore PRs for the downstreams have just been created: If those would be OK for you (and get merged), removal of the algs in liboqs should be possible to proceed piecemeal without any CI breakages.

Ah, very clever!

@xvzcf xvzcf merged commit 203c9c2 into main Nov 28, 2022
@xvzcf xvzcf deleted the remove-rainbow branch November 28, 2022 16:35
@baentsch
Copy link
Member

That was thought-transfer: You merged while I was typing whether anything is missing to merge and move to the next alg :-) BTW, did you see #1314 (comment)?

@xvzcf
Copy link
Contributor Author

xvzcf commented Nov 28, 2022

I did, perhaps by the time the other algorithms are removed this will have been merged in.

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.

3 participants