Add temporary test component for SHA3 testing #10201
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gabor-mezei-arm
added
needs-review
github-project-automation
bot
moved this to In Development
in Roadmap pull requests (new board)
gabor-mezei-arm
added
the
size-xs
With the removal of MBEDTLS_SHA3_C the test cases with disabled SHA3 dependency are never executed. Adding a temporary `all.sh` component which disabling the `PSA_WANT_ALG_SHA3_*` macros to cover these test cases. Signed-off-by: Gabor Mezei <[email protected]>
gabor-mezei-arm
force-pushed
the
9144_SHA3_testing
branch
from
5bcb25b to
8e5ee47
Compare
4 tasks
gilles-peskine-arm
requested changes
Jun 2, 2025
Contributor
gilles-peskine-arm
left a comment
gilles-peskine-arm
left a comment
There was a problem hiding this comment.
Please link to the issue for the removal of the temporary thing. Other than that LGTM
| } | ||
|
|
||
| # Temporary component for SHA3 config option removal | ||
| # Must be removed when SHA3 removal is merged |
Contributor
There was a problem hiding this comment.
Can you please link to the issue? “Temporary” things tend to last when we reach the end of a quarter and stuff gets forgotten for the new priority.
| component_test_full_no_sha3 () { | ||
| msg "build: full config without SHA3" | ||
| scripts/config.py full | ||
| scripts/config.py unset-all PSA_WANT_ALG_SHA3_* |
Contributor
There was a problem hiding this comment.
To avoid breaking if someone has a weird file name:
Suggested change
| scripts/config.py unset-all PSA_WANT_ALG_SHA3_* | |
| scripts/config.py unset-all 'PSA_WANT_ALG_SHA3_*' |
Signed-off-by: Gabor Mezei <[email protected]>
gilles-peskine-arm
approved these changes
Jun 2, 2025
Contributor
gilles-peskine-arm
left a comment
gilles-peskine-arm
left a comment
There was a problem hiding this comment.
Thanks for the update, LGTM
bensze01
approved these changes
Jun 2, 2025
github-project-automation
bot
moved this from In Development
to Has Approval
in Roadmap pull requests (new board)
bensze01
added
approved
github-project-automation
bot
moved this from Has Approval
to Done
in Roadmap pull requests (new board)
Contributor
|
Question: why add this new component on the mbedtls side rather than on the crypto side? |
Contributor
|
@mpg Because as I understand it, a crypto component is not enough because |
Contributor
Author
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With the removal of MBEDTLS_SHA3_C the test cases with disabled SHA3 dependency are never executed. Adding a temporary
all.shcomponent which disabling thePSA_WANT_ALG_SHA3_*macros to cover these test cases.Prerequisite for Mbed-TLS/TF-PSA-Crypto#266
PR checklist
Please remove the segment/s on either side of the | symbol as appropriate, and add any relevant link/s to the end of the line.
If the provided content is part of the present PR remove the # symbol.