mbedtls
mbedtls copied to clipboard
Separate `all.sh` from its components
Separate the components of all.sh
from the main script. This makes it more convenient to change the components used.
Fixes Mbed-TLS/psa-crypto#31
PR checklist
Please tick as appropriate and edit the reasons (e.g.: "backport: not needed because this is a new feature")
- [x] changelog not required - testing change only, not user facing
- [ ] backport done, or not required
- [x] tests not required - changes to test framework itself
Internal CI failures appear spurious.
I would like to request a high-level design review from @ronald-cron-arm and @gilles-peskine-arm please. The PR is not in it's final state but I would like to know if what I have done thus far looks like I am on the right track or not. I am aware there are conflicts but I will sort these out later. The git history is also a little messy which I will also take care of before this gets reviewed in full.
So far I have split things out inline with what I think has been requested. No doubt I have categorised some components incorrectly but hopefully things are broadly correct.
I plan to do the following things next:
- Add more documentation to the components-xxx.sh files inline with what is written in
components.sh
. - Delete
components.sh
- Split the new
components-xxx.sh
files up to three ways such that we potentially will havecomponents_xxx_psa.sh
,components_xxx_mbedtls.sh
and then keep the currentcomponents_xxx.sh
file where there are components that are common to both repos. - General tidy up etc before marking ready for review.
I would welcome any comments on what I have done and what I plan to do please.
I think this is now in a state where it is worthy of review, so am marking it as such.
Fully recreated the commit from fresh base, replaying the choices made in the previous for-review revision https://github.com/Mbed-TLS/mbedtls/commit/2a2c4cbd5d6553f1e33d98cb6d1598a8a08b3024
Notes to reviewers
- I have structured the commits per components, making each commit easy to review using
git diff --color-moved=dimmed-zebra
- The commits up to https://github.com/Mbed-TLS/mbedtls/pull/8226/commits/781c823255a7bd09e0812addbad5c57029f21c14 are replicating the split of the previous version.
- Commits after that are moving newly added components have been address based on estimate. If the choice is wrong I can easily amend the specific commit.
-
component_test_memory_buffer_allocator_backtrace
in https://github.com/Mbed-TLS/mbedtls/commit/2a2c4cbd5d6553f1e33d98cb6d1598a8a08b3024 was placed in two files in components-configuration-platform and components-configuration-tls. I assumed that was accidental and did not replicate it. - I have added @davidhorstmann-arm and @tom-daubney-arm as having signed-off in the comit range since that was meant to replicate the original PR in an easy to review chain. Resolutions of newly introduced components are single sign-off.
- (Optional) The python regex to track the components is
r'(?m)^\S+ ?\(\) {', data)
I have finally compared the components as sets between the two revisions and printed the symmetric difference bellow:
{'components-build-system.sh': ['support_build_cmake_programs_no_testing', # Newly-Added
'component_build_cmake_programs_no_testing'], # Newly-Added
'components-configuration-crypto.sh': ['component_test_no_rsa_key_pair_generation', # Newly-Added
'component_build_dhm_alt', # Removed
'component_test_full_no_cipher_no_psa_crypto', # Removed
'common_check_mbedtls_missing_symbols', # Removed
'component_build_aes_via_padlock', # Removed
'common_test_full_no_cipher_with_psa_crypto', # Removed
'support_build_aes_via_padlock_only', # Removed
'component_test_full_no_cipher_with_psa_crypto', # Removed
'component_test_full_no_bignum', # Removed
'component_test_psa_crypto_rsa_no_genprime', # Removed
'component_build_full_psa_crypto_client_without_crypto_provider', # Removed
'component_test_default_psa_crypto_client_without_crypto_provider', # Removed
'component_test_full_no_cipher_with_psa_crypto_config', # Removed
'component_test_full_no_cipher'], # Newly-Added
'components-configuration-platform.sh': ['component_test_memory_buffer_allocator_backtrace'], # Duplicated
'components-configuration-tls.sh': ['component_test_memory_buffer_allocator_backtrace', # Duplicated
'component_test_tls1_2_default_cbc_legacy_cbc_etm_cipher_only_use_psa', # Removed
'component_test_tls1_2_deafult_cbc_legacy_cipher_only_use_psa', # Removed
'component_test_tls1_2_default_stream_cipher_only_use_psa'], # Removed
'components-psasim.sh': ['helper_psasim_server', # Newly-Added
'component_test_psasim', # Newly-Added
'component_test_suite_with_psasim']} # Newly-Added
Everything that is marked as Removed
has been tested against git history to no longer exit in latest development. Newly-Added
files have been placed in estimated componenets. The duplication has not been reproduced.
As discussed, setting priority-very-high because of the potential for very painful conflicts in all.sh
.
Looks good to me at bf47cf7, but please remove 3d1bf49 where I realize I made a mistake (sorry about that).
Thats ok, that is precicely they were placed in separate commits. I have removed https://github.com/Mbed-TLS/mbedtls/commit/3d1bf4977fdb184a0df4c9a98e805b5930269c57 in both here and the backport.