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

PSA_WANT_xxx is only meaningful when PSA crypto is enabled #58

Conversation

gilles-peskine-arm
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm commented Oct 10, 2024

Fix a missing dependency in configuration symbol test enumeration: PSA_WANT_xxx is only meaningful when PSA crypto is enabled. This isn't needed in TF-PSA-Crypto where all crypto goes through PSA anyway, but it is relevant in 3.6 and harmless (as long as we keep MBEDTLS_PSA_CRYPTO_CLIENT at least as an internal symbol) in TF-PSA-Crypto.

@gilles-peskine-arm gilles-peskine-arm added needs-ci Needs to pass CI tests priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most) labels Oct 10, 2024
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm gilles-peskine-arm force-pushed the dev/gilles-peskine-arm/generate_config_tests-require_psa branch from ef7cd7b to 239c0d8 Compare October 10, 2024 19:39
@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review and removed needs-ci Needs to pass CI tests labels Oct 11, 2024
Copy link
Contributor

@yanesca yanesca left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -80,7 +80,7 @@ def dependencies_of_setting(cfg: config_common.Config,
be negated by prefixing them with '!'. This is the same syntax as a
depends_on directive in test data.
"""
#pylint: disable=too-many-return-statements
#pylint: disable=too-many-branches,too-many-return-statements
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: instead of silencing the warning, we could move the TLS setting processing to a separate function.

Copy link
Member

@paul-elliott-arm paul-elliott-arm left a comment

Choose a reason for hiding this comment

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

LGTM

@gilles-peskine-arm gilles-peskine-arm added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Oct 16, 2024
@gilles-peskine-arm gilles-peskine-arm merged commit 3eafac1 into main Oct 16, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most)
Projects
Development

Successfully merging this pull request may close these issues.

3 participants