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

Add meson option to disable explicit EC keys tests #492

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Mulling
Copy link

@Mulling Mulling commented Dec 26, 2024

Description

This PR adds a meson option to disable explicit EC keys tests.

Checklist

  • Code modified for feature
  • Test suite updated with functionality tests
  • Test suite updated with negative tests
  • Documentation updated

Reviewer's checklist:

  • Any issues marked for closing are addressed
  • There is a test suite reasonably covering new functionality or modifications
  • This feature/change has adequate documentation added
  • Code conform to coding style that today cannot yet be enforced via the check style test
  • Commits have short titles and sensible commit messages
  • Coverity Scan has run if needed (code PR) and no new defects were found

Signed-off-by: Lucas Mulling <[email protected]>
@Mulling Mulling changed the title Add option to disable explicit EC keys tests Add meson option to disable explicit EC keys tests Dec 26, 2024
@Jakuje
Copy link
Contributor

Jakuje commented Dec 26, 2024

Should we make it the other way round and disable the explicit curves by default and enable them only if some configuration option is provided instead?

@Mulling
Copy link
Author

Mulling commented Dec 26, 2024

Would be better, I think debian is the only one that allows it.

@Mulling
Copy link
Author

Mulling commented Dec 27, 2024

@Jakuje Not sure how you want to handle the workflow config, I just copied what was already done for preload_libasan.

Jakuje
Jakuje previously approved these changes Dec 28, 2024
Copy link
Contributor

@Jakuje Jakuje left a comment

Choose a reason for hiding this comment

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

just one nit, otherwise looks good!

tests/tecxc Outdated Show resolved Hide resolved
Signed-off-by: Lucas Mulling <[email protected]>
Copy link
Contributor

@Jakuje Jakuje left a comment

Choose a reason for hiding this comment

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

lgtm. Thank you for your contribution!

@@ -2,3 +2,8 @@ option('preload_libasan',
type: 'string',
value: 'no',
description: 'Path to libasan.so to preload')

option('enable_explicit_EC_test',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if this needs to be generic, but meson provides a feature option type for enabled/disabled kind of options.

Copy link
Author

Choose a reason for hiding this comment

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

Not sure either, I'm guessing it would be more useful if we had a dependency in this case, but since this is just a on-and-off switch I don't see the need for it.

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