Add out of source build functions for TF-PSA-Crypto #77

Harry-Ramsey · 2024-11-18T10:47:47Z

This commit adds helper functions to build TF-PSA-Crypto out of source using CMake.

This commit is used for: Mbed-TLS/mbedtls#9767

gilles-peskine-arm · 2024-12-05T13:56:25Z

scripts/all-core.sh

+    in_tf_psa_crypto_repo
+    running_tf_psa_crypto_test="$?"
+    if [ "$running_tf_psa_crypto_test" -eq 0 ]; then
+        pre_create_tf_psa_crypto_out_of_source_directory
+    fi


We run the script with set -e active. This means that if any command exit with a nonzero status, the script exits. Note that this has a lot of limitations and pitfalls, but those limitations aren't a concern here. Also, we run parts of the script with an EXIT trap which catches some here, but that too is not relevant at this particular point in the script.

When all.sh is running in the mbedtls repository, the command in_tf_psa_crypto_repo exits with the status 1. This causes the whole script to exit with 1.

Here the fix is to put the command directly in an if statement. Untested:

Suggested change

in_tf_psa_crypto_repo

running_tf_psa_crypto_test="$?"

if [ "$running_tf_psa_crypto_test" -eq 0 ]; then

pre_create_tf_psa_crypto_out_of_source_directory

fi

if in_tf_psa_crypto_repo; then

running_tf_psa_crypto_test=1

pre_create_tf_psa_crypto_out_of_source_directory

fi

Thank you for saving me that debugging headache, that makes a lot more sense to me now.

Harry-Ramsey · 2024-12-06T16:01:23Z

I've ran a few various test cases for MbedTLS 3.6 and cannot seem to understand why this error is occurring. I have tried one with all the changes commented out and no changes to the test cases there appeared.

ronald-cron-arm

One thing that can be removed now that all-core.sh does not cd automatically to the directory for the out of tree build, otherwise this looks good to me.

I think the component test_tf_psa_crypto_cmake_out_of_source of the CI against mbedtls:development fails because the component tries (as in version 9e4ac374e2 of TF-PSA-Crypto) to create the out of tree build directory while it already exists.
An update of the TF-PSA-Crypto submodule in mbedtls:development will probably be necessary.

The CI against mbedtls:mbedtls-3.6 fails completely but that does not seem to be related to the content of this PR. Error line 232 in framework/scripts/mbedtls_framework/config_common.py but I do not understand the root cause.

ronald-cron-arm · 2024-12-20T15:18:06Z

scripts/all-core.sh

+    # Reset working directory to TF_PSA_Crypto as it is build out of source..
+    if [ $running_tf_psa_crypto_test -eq 1 ]; then
+        cd "$TF_PSA_CRYPTO_ROOT_DIR"
+    fi
+


Probably not needed anymore.

I think this should be kept as components which build out of source or change directory can be reset to source directory without additional code required in those tests. Components which don't change directory will be unaffected by this line as they should already be in the source directory.

Please check with @mpg but I think components are run in a sub-shell thus if they change directory it does not matter to the calling shell and thus to all-core.sh.

I haven't read this PR and haven't investigated whether this particular cd call is still needed. But I can confirm that components do run in a subshell, so if a component calls cd (or sets environment variables, and a few other things), this has no impact on subsequent components.

mpg · 2024-12-24T09:15:34Z

scripts/all-core.sh

@@ -165,13 +165,19 @@ pre_initialize_variables () {
            PSA_CORE_PATH='tf-psa-crypto/core'
            BUILTIN_SRC_PATH='tf-psa-crypto/drivers/builtin/src'
            CONFIG_TEST_DRIVER_H='tf-psa-crypto/tests/configs/crypto_config_test_driver.h'
+            MBEDTLS_ROOT_DIR="$PWD"
+            TF_PSA_CRYPTO_ROOT_DIR="$PWD/tf-psa-crypto"
+            MBEDTLS_FRAMEWORK_ROOT_DIR="$PWD/framework"


Question: we already have a variable FRAMEWORK defined in the top-level file (all.sh), why do we need that one in addition?

I believe that I added this before it was finalised where FRAMEWORK was going to be set as a temporary workaround. I can remove that now.

This commit adds helper functions to build TF-PSA-Crypto out of source using CMake. Signed-off-by: Harry Ramsey <[email protected]>

This commit adds MBEDTLS_ROOT_DIR as a variable to the bash script. This will be used in later commits when migrating to an independent TF-PSA-Crypto testing suite. Signed-off-by: Harry Ramsey <[email protected]>

This commit fixes out of source directory builds, where in_tf_psa_crypto_repo would fail due to project_name.txt not existing. Signed-off-by: Harry Ramsey <[email protected]>

This commit adds a variable for locating Mbed TLS framework which is used to run multiple test scripts between Mbed TLS and TF-PSA-Crypto. Signed-off-by: Harry Ramsey <[email protected]>

This commit reverts changing the directory to the out of source directory, enabling tests which require source code to run. Tests that require building TF-PSA-Crypto will have to change directory to OUT_OF_SOURCE_DIR. Signed-off-by: Harry Ramsey <[email protected]>

This commit removes unnecessary directory changes as the commands for components run in subshells which do not affect the main scripts directory. Signed-off-by: Harry Ramsey <[email protected]>

This commit replaces the relative Mbed TLS out of source build path with a more generic named absolute out of source build path for both Mbed TLS and TF-PSA-Crypto. Signed-off-by: Harry Ramsey <[email protected]>

This commit removes a duplicate variable MBEDTLS_FRAMEWORK_ROOT_DIR as it is now previously defined as FRAMEWORK. Signed-off-by: Harry Ramsey <[email protected]>

Harry-Ramsey mentioned this pull request Nov 18, 2024

TF-PSA-Crypto out of source build Mbed-TLS/mbedtls#9767

Open

5 tasks

Harry-Ramsey self-assigned this Nov 20, 2024

Harry-Ramsey force-pushed the tf-psa-crypto-out-of-source-development branch from db35a71 to 6db9018 Compare November 21, 2024 14:06

Harry-Ramsey force-pushed the tf-psa-crypto-out-of-source-development branch 6 times, most recently from efa612e to 8902bd5 Compare December 5, 2024 12:54

gilles-peskine-arm reviewed Dec 5, 2024

View reviewed changes

Harry-Ramsey force-pushed the tf-psa-crypto-out-of-source-development branch 7 times, most recently from 26a98cb to 8879d15 Compare December 6, 2024 16:00

Harry-Ramsey force-pushed the tf-psa-crypto-out-of-source-development branch 3 times, most recently from a8ce45b to d09782c Compare December 12, 2024 17:23

Harry-Ramsey force-pushed the tf-psa-crypto-out-of-source-development branch from d09782c to 1797ef2 Compare December 17, 2024 10:22

Harry-Ramsey mentioned this pull request Dec 17, 2024

Out of source build development Mbed-TLS/TF-PSA-Crypto#121

Open

4 tasks

Harry-Ramsey force-pushed the tf-psa-crypto-out-of-source-development branch 3 times, most recently from 1797ef2 to 6638abf Compare December 20, 2024 09:54

ronald-cron-arm requested changes Dec 20, 2024

View reviewed changes

mpg reviewed Dec 24, 2024

View reviewed changes

Harry-Ramsey added 2 commits December 24, 2024 14:25

Add out of source build functions for TF-PSA-Crypto

974b413

This commit adds helper functions to build TF-PSA-Crypto out of source using CMake. Signed-off-by: Harry Ramsey <[email protected]>

Add MBEDTLS_ROOT_DIR variable

b435d12

This commit adds MBEDTLS_ROOT_DIR as a variable to the bash script. This will be used in later commits when migrating to an independent TF-PSA-Crypto testing suite. Signed-off-by: Harry Ramsey <[email protected]>

Harry-Ramsey added 6 commits December 24, 2024 14:25

Fix out of source directory checks

4c9361b

This commit fixes out of source directory builds, where in_tf_psa_crypto_repo would fail due to project_name.txt not existing. Signed-off-by: Harry Ramsey <[email protected]>

Add variable for Mbed TLS-Framework directory

9794a45

This commit adds a variable for locating Mbed TLS framework which is used to run multiple test scripts between Mbed TLS and TF-PSA-Crypto. Signed-off-by: Harry Ramsey <[email protected]>

Remove directory change as commands run in subshells

e11462c

This commit removes unnecessary directory changes as the commands for components run in subshells which do not affect the main scripts directory. Signed-off-by: Harry Ramsey <[email protected]>

Use absolute out of source build path

008445c

This commit replaces the relative Mbed TLS out of source build path with a more generic named absolute out of source build path for both Mbed TLS and TF-PSA-Crypto. Signed-off-by: Harry Ramsey <[email protected]>

Remove duplicate Mbed TLS Framework variable

f3044f8

This commit removes a duplicate variable MBEDTLS_FRAMEWORK_ROOT_DIR as it is now previously defined as FRAMEWORK. Signed-off-by: Harry Ramsey <[email protected]>

Harry-Ramsey force-pushed the tf-psa-crypto-out-of-source-development branch from 2ff9ae9 to f3044f8 Compare December 24, 2024 14:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add out of source build functions for TF-PSA-Crypto #77

Add out of source build functions for TF-PSA-Crypto #77

Harry-Ramsey commented Nov 18, 2024 •

edited

Loading

gilles-peskine-arm Dec 5, 2024 •

edited

Loading

Harry-Ramsey Dec 5, 2024

Harry-Ramsey commented Dec 6, 2024

ronald-cron-arm left a comment •

edited

Loading

ronald-cron-arm Dec 20, 2024

Harry-Ramsey Dec 20, 2024

ronald-cron-arm Dec 20, 2024 •

edited

Loading

gilles-peskine-arm Dec 20, 2024

mpg Dec 24, 2024

Harry-Ramsey Dec 24, 2024

Add out of source build functions for TF-PSA-Crypto #77

Are you sure you want to change the base?

Add out of source build functions for TF-PSA-Crypto #77

Conversation

Harry-Ramsey commented Nov 18, 2024 • edited Loading

gilles-peskine-arm Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

Harry-Ramsey Dec 5, 2024

Choose a reason for hiding this comment

Harry-Ramsey commented Dec 6, 2024

ronald-cron-arm left a comment • edited Loading

Choose a reason for hiding this comment

ronald-cron-arm Dec 20, 2024

Choose a reason for hiding this comment

Harry-Ramsey Dec 20, 2024

Choose a reason for hiding this comment

ronald-cron-arm Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

gilles-peskine-arm Dec 20, 2024

Choose a reason for hiding this comment

mpg Dec 24, 2024

Choose a reason for hiding this comment

Harry-Ramsey Dec 24, 2024

Choose a reason for hiding this comment

Harry-Ramsey commented Nov 18, 2024 •

edited

Loading

gilles-peskine-arm Dec 5, 2024 •

edited

Loading

ronald-cron-arm left a comment •

edited

Loading

ronald-cron-arm Dec 20, 2024 •

edited

Loading