Skip to content

Finalize split preparation-2 #9828

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

Conversation

ronald-cron-arm
Copy link
Contributor

@ronald-cron-arm ronald-cron-arm commented Dec 9, 2024

Description

Progresses #9624

PR checklist

@ronald-cron-arm ronald-cron-arm added enhancement needs-ci Needs to pass CI tests priority-high High priority - will be reviewed soon labels Dec 9, 2024
minosgalanakis
minosgalanakis previously approved these changes Dec 9, 2024
Copy link
Contributor

@minosgalanakis minosgalanakis left a comment

Choose a reason for hiding this comment

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

Looks good overall, minor tibits/comments.

@@ -2167,6 +2158,8 @@ component_test_ccm_aes_sha256 () {

# Setting a blank config disables everyhing in the library side.
echo '#define MBEDTLS_CONFIG_H ' >"$CONFIG_H"
cp configs/crypto-config-ccm-aes-sha256.h "$CRYPTO_CONFIG_H"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch, thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I've compared the outcomes.csv from before and after #9567 and it was quite clear there was an issue with test_ccm_aes_sha256.

gabor-mezei-arm
gabor-mezei-arm previously approved these changes Dec 9, 2024
Copy link
Contributor

@gabor-mezei-arm gabor-mezei-arm left a comment

Choose a reason for hiding this comment

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

The CI is not happy with the generated files. Otherwise looks good to me.

@ronald-cron-arm ronald-cron-arm force-pushed the finalize-split-preparation-2 branch 2 times, most recently from 4bb70cf to 1109a09 Compare December 10, 2024 08:59
Just to ease the eventual migration
of check-generated-files.sh to
the framework.

Signed-off-by: Ronald Cron <[email protected]>
Move test_keys.h to tests/include/test
instead of tests/src as it is used
outside of tests/src namely by
test_suite_pk.

Signed-off-by: Ronald Cron <[email protected]>
This reverts commit 939ce9d.

Build mbedtls_test library of objects to link
with TLS and x509 test suites and programs
with mbedtls framework not TF-PSA-Crypto
one (when it will be there).

Signed-off-by: Ronald Cron <[email protected]>
Signed-off-by: Ronald Cron <[email protected]>
Signed-off-by: Ronald Cron <[email protected]>
Signed-off-by: Ronald Cron <[email protected]>
Signed-off-by: Ronald Cron <[email protected]>
Signed-off-by: Ronald Cron <[email protected]>
Move the docuumentation files that after
the split will fit better in TF-PSA-Crypto
than Mbed TLS. No comment update.

Signed-off-by: Ronald Cron <[email protected]>
Add missing dependency of visualc
file generation on programs and
tests generated files.

Signed-off-by: Ronald Cron <[email protected]>
@ronald-cron-arm ronald-cron-arm force-pushed the finalize-split-preparation-2 branch from 1109a09 to 8a09a41 Compare December 10, 2024 21:17
@ronald-cron-arm ronald-cron-arm added needs-review Every commit must be reviewed by at least two team members, and removed needs-ci Needs to pass CI tests labels Dec 11, 2024
@ronald-cron-arm
Copy link
Contributor Author

@minosgalanakis @gabor-mezei-arm CI is green now, please have another look, thanks.

Copy link
Contributor

@gabor-mezei-arm gabor-mezei-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

minosgalanakis
minosgalanakis previously approved these changes Dec 11, 2024
Copy link
Contributor

@minosgalanakis minosgalanakis left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@gabor-mezei-arm gabor-mezei-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

Copy link
Contributor

@minosgalanakis minosgalanakis left a comment

Choose a reason for hiding this comment

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

LGTM

@ronald-cron-arm ronald-cron-arm added this pull request to the merge queue Dec 11, 2024
Merged via the queue into Mbed-TLS:development with commit f3720c7 Dec 11, 2024
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement needs-review Every commit must be reviewed by at least two team members, priority-high High priority - will be reviewed soon
Development

Successfully merging this pull request may close these issues.

3 participants