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.

Sign up for GitHub

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

Jump to bottom

Modify check generated files script to work with TF PSA Crypto too #8523

Merged
merged 34 commits into from
Dec 11, 2023
Merged

Modify check generated files script to work with TF PSA Crypto too #8523

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
b4f1ee0
Remove superfluous leading whitespace
tom-daubney-arm Nov 13, 2023
c9f8386
Modify check-generated-files.sh to work in both repos
tom-daubney-arm Nov 13, 2023
b10cc7a
Modify generate_driver_wrappers.py to work in both repos
tom-daubney-arm Nov 14, 2023
d3f8443
Further modify check-generated-files.sh
tom-daubney-arm Nov 14, 2023
0bb761c
Remove further extraneous whitespace in lcov script
tom-daubney-arm Nov 14, 2023
c1750bb
Apply correct license to generate_driver_wrappers.py
tom-daubney-arm Nov 14, 2023
e58128e
Refactor repository detection
tom-daubney-arm Nov 14, 2023
d289b8b
Stylise TF-PSA-Crypto correctly
tom-daubney-arm Nov 14, 2023
0eb2dc1
Call the right function
tom-daubney-arm Nov 14, 2023
4291bc2
Remove trailing whitespace
tom-daubney-arm Nov 14, 2023
5556f90
Rename variables in script
tom-daubney-arm Nov 15, 2023
13ecb69
Introduce function to return library/core directory
tom-daubney-arm Nov 16, 2023
79cae20
Remove useless line
tom-daubney-arm Nov 22, 2023
b42c50b
Make use of new crypto_core_directory function
tom-daubney-arm Nov 22, 2023
772056c
Replace repo_root with project_root
tom-daubney-arm Nov 22, 2023
d35b94b
Improve implementation of crypto_core_directory
tom-daubney-arm Nov 22, 2023
755d321
Rename guess_mbedtls_root to guess_project_root
tom-daubney-arm Nov 22, 2023
d0c3076
Make use of crypto_core_directory function in script
tom-daubney-arm Nov 23, 2023
8932404
Introduce project_crypto_name in build_tree.py
tom-daubney-arm Nov 23, 2023
beec452
Use os.path.join in crypto_core_directory
tom-daubney-arm Nov 24, 2023
cdbf2fd
Add documentation for new public functions
tom-daubney-arm Nov 24, 2023
fc60e9b
Make function calls consistent
tom-daubney-arm Nov 24, 2023
6130a61
Remove unused variable
tom-daubney-arm Nov 24, 2023
e8f3789
Revert change that removed in_tf_psa_crypto_repo variable
tom-daubney-arm Nov 24, 2023
08c6dc4
Rename project_crypto_name
tom-daubney-arm Nov 30, 2023
46588de
Improve documentation of crypto_core_directory
tom-daubney-arm Nov 30, 2023
56bee03
Rename variable for better clarity
tom-daubney-arm Nov 30, 2023
d1f2934
Introduce guess_mbedtls_root
tom-daubney-arm Nov 30, 2023
db80b23
Introduce guess_tf_psa_crypto_root
tom-daubney-arm Nov 30, 2023
99030e2
Remove trailing whitespace
tom-daubney-arm Dec 1, 2023
04c446c
Modify crypto_core_directory to also return a relative path
tom-daubney-arm Dec 1, 2023
3a06906
Use guess_mbedtls_root in Mbed-TLS-only script
tom-daubney-arm Dec 1, 2023
10769bc
Fix bad whitespace in keyword argument assignment
tom-daubney-arm Dec 1, 2023
f05b768
Use existing variable containing full path
tom-daubney-arm Dec 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scripts/lcov.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ set -eu
# Repository detection
in_mbedtls_build_dir () {
test -d library
}
}

# Collect stats and build a HTML report.
lcov_library_report () {
Expand Down
31 changes: 20 additions & 11 deletions tests/scripts/check-generated-files.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@ EOF
exit
fi

if [ -d library -a -d include -a -d tests ]; then :; else
echo "Must be run from Mbed TLS root" >&2
IN_MBEDTLS=0
if [ -d library -a -d include -a -d tests ]; then
IN_MBEDTLS=1
elif [ -d core -a -d include -a -d tests ]; then :;
Copy link
Contributor

Choose a reason for hiding this comment

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

Deciding between mbedtls and tf-psa-crypto from library vs core feels fragile. How about testing the presence of include/mbedtls vs include/tf_psa_crypto?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please do it like in all.sh in this PR to keep things consistent. include/tf_psa_crypto was introduced recently thus not used yet in the repo root detection checks. We could use it and in that case we would update all such checks in all *.sh files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, will do. I did think that however I thought the way I was doing it here look more consistent within this particular script. Happy to change it though

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we introduce a scripts/which_repo.sh? Or a scripts/repo_name.txt?

For Python code there should be a function in build_tree.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gilles-peskine-arm I am just wondering why doing it this way seems more fragile than using the include directory as you suggest? It's not clear to me why one approach is more robust than the other.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gilles-peskine-arm By "use a regular file" do you mean we should simply have a file named mbedtls in the root directory of Mbed TLS and then the same idea of TF-PSA-Crypto and then we test for it's existence with the proposed scripts/which-repo.sh script?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I thought each repository should have a file with the same name, and we'd use the content of that file to identify the repository.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool ok. In this PR I think it is probably best if I do it the way @ronald-cron-arm suggests in his first comment and then I will raise a subsequent PR that implements this file idea and modifies how repo detection is done across both the repositories. Does that sound alright to everyone here?

Copy link
Contributor

Choose a reason for hiding this comment

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

fine by me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The standardisation of repository detection is now being tracked with issue #8524 .

else
echo "Must be run from Mbed TLS root or TF-PSA-Crypto root" >&2
exit 1
fi

Expand Down Expand Up @@ -114,16 +118,21 @@ check()
# - **/CMakeLists.txt (to (re)build them with cmake)
# - scripts/make_generated_files.bat (to generate them under Windows)

check scripts/generate_errors.pl library/error.c
check scripts/generate_query_config.pl programs/test/query_config.c
check scripts/generate_driver_wrappers.py library/psa_crypto_driver_wrappers.h library/psa_crypto_driver_wrappers_no_static.c
check scripts/generate_features.pl library/version_features.c
check scripts/generate_ssl_debug_helpers.py library/ssl_debug_helpers_generated.c
# generate_visualc_files enumerates source files (library/*.c). It doesn't
# care about their content, but the files must exist. So it must run after
# the step that creates or updates these files.
check scripts/generate_visualc_files.pl visualc/VS2013
# These checks are common to Mbed TLS and TF PSA Crypto
ronald-cron-arm marked this conversation as resolved.
Show resolved Hide resolved
check scripts/generate_psa_constants.py programs/psa/psa_constant_names_generated.c
check tests/scripts/generate_bignum_tests.py $(tests/scripts/generate_bignum_tests.py --list)
check tests/scripts/generate_ecp_tests.py $(tests/scripts/generate_ecp_tests.py --list)
check tests/scripts/generate_psa_tests.py $(tests/scripts/generate_psa_tests.py --list)

# Additional checks for Mbed TLS only
if [ $IN_MBEDTLS -eq 1 ]; then
check scripts/generate_errors.pl library/error.c
check scripts/generate_query_config.pl programs/test/query_config.c
check scripts/generate_driver_wrappers.py library/psa_crypto_driver_wrappers.h library/psa_crypto_driver_wrappers_no_static.c
check scripts/generate_features.pl library/version_features.c
check scripts/generate_ssl_debug_helpers.py library/ssl_debug_helpers_generated.c
# generate_visualc_files enumerates source files (library/*.c). It doesn't
# care about their content, but the files must exist. So it must run after
# the step that creates or updates these files.
check scripts/generate_visualc_files.pl visualc/VS2013
fi