Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

src/common/pqclean_shims/compat.h: Fix polyfill version check sense #1449

Closed

Conversation

distorted-mdw
Copy link
Contributor

@distorted-mdw distorted-mdw commented May 2, 2023

We need to define the types if GCC is too /old/.

  • 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 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.)

We need to define the types if GCC is too /old/.

Signed-off-by: Mark Wooding <mark.wooding@trustonic.com>
@distorted-mdw distorted-mdw marked this pull request as ready for review May 3, 2023 01:40
@distorted-mdw distorted-mdw requested a review from dstebila as a code owner May 3, 2023 01:40
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.

Thanks for the contribution. Is (possibly not correctly) supporting compilers this old really an issue, though? General question: Does liboqs really want to support compilers this old or rather add documentation and compiler version checks (for a certain minimum version) to minimize our efforts on outdated software? We also check for minimum cmake version, for example... Thoughts @dstebila @xvzcf @christianpaquin ?

Add: One of the CI failures in the Sphincs+ update are also related to a gcc > 7.1 version check (failing: It seems McElice code will not compile for gcc<7.1).

@dstebila
Copy link
Member

dstebila commented May 3, 2023

I don't have a particular minimum gcc target in mind, but am agreeable to raising it unless we hear a strong demand and commitment to support.

@baentsch
Copy link
Member

baentsch commented May 4, 2023

As agreed in our call, we rather prefer documenting and enforcing via CMakeLists.txt a minimum compiler version of 7.1, thus making this PR moot.

@distorted-mdw Please let us know if you know of a use case (for liboqs) where an older compiler would need to be supported.

@baentsch
Copy link
Member

baentsch commented May 8, 2023

As discussed above, this PR is becoming superfluous when #1451 is merged and will be closed then.

@baentsch baentsch closed this May 9, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants