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

Out of source build development #121

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

Harry-Ramsey
Copy link
Contributor

Description

Closes #9755

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.

  • changelog not required because: testing API changes.
  • development PR provided.
  • framework PR provided #77
  • 3.6 PR not required because: for repo split.
  • tests not required.

Notes for the submitter

Please refer to the contributing guidelines, especially the
checklist for PR contributors.

Help make review efficient:

  • Multiple simple commits
    • please structure your PR into a series of small commits, each of which does one thing
  • Avoid force-push
    • please do not force-push to update your PR - just add new commit(s)
  • See our Guidelines for Contributors for more details about the review process.

@Harry-Ramsey Harry-Ramsey self-assigned this Dec 17, 2024
@Harry-Ramsey Harry-Ramsey added the size-s Estimated task size: small (~2d) label Dec 17, 2024
@Harry-Ramsey Harry-Ramsey force-pushed the out-of-source-build-development branch 2 times, most recently from 35bc548 to d93d138 Compare December 20, 2024 15:18
}

component_test_tf_psa_crypto_cmake_as_subdirectory () {
msg "build: cmake 'as-subdirectory' build"
cd programs/test/cmake_subproject
cd "$TF_PSA_CRYPTO_ROOT_DIR/programs/test/cmake_subproject"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
cd "$TF_PSA_CRYPTO_ROOT_DIR/programs/test/cmake_subproject"
cd programs/test/cmake_subproject

@@ -34,7 +30,7 @@ component_test_tf_psa_crypto_cmake_as_subdirectory () {

component_test_tf_psa_crypto_cmake_as_package_install () {
msg "build: cmake 'as-installed-package' build"
cd programs/test/cmake_package_install
cd $TF_PSA_CRYPTO_ROOT_DIR/programs/test/cmake_package_install
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
cd $TF_PSA_CRYPTO_ROOT_DIR/programs/test/cmake_package_install
cd programs/test/cmake_package_install

@ronald-cron-arm
Copy link
Contributor

ronald-cron-arm commented Dec 20, 2024

In the CI, the three TF-PSA-Crypto components fail. I think this is because of:

if [ -d ../include/mbedtls -a -d ../framework ]; then                           
    # Running inside an mbedtls checkout: get the framework from mbedtls.       
    FRAMEWORK="$PWD/../framework"                                               
else                                                                            
    # Running standalone: use our own framework.                                
    FRAMEWORK="$PWD/framework"                                                  
fi 

in tf-psa-crypto/tests/scripts/all.sh
It means I think that while running the CI, the framework used by tf-psa-crypto/tests/scripts/all.sh is the mbedtls one that does not set TF_PSA_CRYPTO_ROOT_DIR or create automatically the directory for out-of-tree build. @mpg could probably help to resolve that properly.

@mpg
Copy link
Contributor

mpg commented Dec 23, 2024

@mpg could probably help to resolve that properly.

I don't think this is complicated to resolve: this block of code was written based on the assumption that we wanted tf-psa-crypto to use the framework from Mbed TLS - but that's not the direction we're taking now. (Also, something had to be done because the framework subdir of tf-psa-crypto did not exist when that code was written.)

So, I'd suggest simply replacing the quoted if else block with FRAMEWORK="$PWD/framework" and I think we should be OK.

@Harry-Ramsey Harry-Ramsey force-pushed the out-of-source-build-development branch 2 times, most recently from bc2f4e9 to 5793984 Compare December 23, 2024 17:01
This commit removes duplicate code to build TF-PSA-Crypto out of source
which has now been moved to mbedtls-framework.

Signed-off-by: Harry Ramsey <[email protected]>
@Harry-Ramsey Harry-Ramsey force-pushed the out-of-source-build-development branch 2 times, most recently from f6f62f8 to ebfcd14 Compare December 24, 2024 09:36
This commit updates the framework submodule to include commits relevant
to running TF-PSA-Crypto using CMake build system.

Signed-off-by: Harry Ramsey <[email protected]>
@Harry-Ramsey Harry-Ramsey force-pushed the out-of-source-build-development branch from ebfcd14 to 6fe6d7a Compare December 24, 2024 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size-s Estimated task size: small (~2d)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tf-psa-crypto all.sh: make the core manage the build directory
3 participants