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

Run TF-PSA-Crypto components without cloning Mbed TLS #190

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Run TF-PSA-Crypto components without cloning Mbed TLS #190

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e1f8283
Rename all_all_sh_components
bensze01 Dec 20, 2024
31dd5b6
Collect information about tf-psa-crypto components
bensze01 Dec 20, 2024
84f8435
Break out submodule checkout into separate methods
bensze01 Dec 20, 2024
5de7135
Test tf-psa-crypto components without Mbed TLS
bensze01 Dec 20, 2024
4c2a1ac
Run tf-psa-crypto all.sh in PR jobs for that repo
bensze01 Jan 13, 2025
5054935
Update wording of log message
bensze01 Jan 13, 2025
cb1450f
Remove left-over checkout of TLS repo
bensze01 Jan 13, 2025
fcce535
Pass correct repo value for tf-psa-crypto tests
bensze01 Jan 13, 2025
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
22 changes: 19 additions & 3 deletions vars/gen_jobs.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ fi
}
}

def gen_all_sh_jobs(BranchInfo info, platform, component, label_prefix='') {
def gen_all_sh_jobs(BranchInfo info, String platform, String component, String label_prefix='', String repo='tls') {
def shorthands = [
"arm-compilers-amd64": "armcc",
"ubuntu-16.04-amd64": "u16",
Expand All @@ -198,7 +198,7 @@ def gen_all_sh_jobs(BranchInfo info, platform, component, label_prefix='') {
]
/* Default to the full platform hame is a shorthand is not found */
def shortplat = shorthands.getOrDefault(platform, platform)
def job_name = "${label_prefix}all_${shortplat}-${component}"
def job_name = "${label_prefix}${repo == 'tls' ? '' : "${repo}_"}all_${shortplat}-${component}"
def outcome_file = "${job_name.replace((char) '/', (char) '_')}-outcome.csv"
def use_docker = platform_has_docker(platform)
def extra_setup_code = ''
Expand Down Expand Up @@ -245,7 +245,7 @@ echo >&2 'Note: "clang" will run /usr/bin/clang -Wno-error=c11-extensions'
'''
}

if (info.has_min_requirements) {
if (repo == 'tls' && info.has_min_requirements) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I understand why we need the repo == 'tls' part: currently tf-psa-crypto doesn't have scripts/min_requirements.py but I'd guess info.has_min_requirements should reflect that?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, it seems to me we'll need to get scripts/min_requirements.py (and supporting files) in tf-psa-crypto (or the framework) before moving python scripts that have requirements beyond what happens to be already installed by apt. @ronald-cron-arm not sure if that dependency was already on your radar?

Copy link
Contributor

Choose a reason for hiding this comment

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

No that was not on my radar. I can see that @valeriosetti has started working on Mbed-TLS/mbedtls-framework#86. Does this issue fully cover the dependency?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should.

Copy link

Choose a reason for hiding this comment

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

I can see that @valeriosetti has started working on Mbed-TLS/mbedtls-framework#86.

Yes, PRs Mbed-TLS/mbedtls-framework#105 and related ones (Mbed-TLS/mbedtls#9863 Mbed-TLS/mbedtls#9864) are ready for review. There's only the usual framework confict on development and mbedtls-3.6, but that can easily be fixed once Mbed-TLS/mbedtls-framework#105 is merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mpg Currently the code only checks if min_requirements.py exists in the MbedTLS branch we are testing, and this is what's represented by has_min_requirements. Since we're planning to add such a script, I'll add a separate variable to track this for the framework.

Copy link

Choose a reason for hiding this comment

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

For the records, I created Mbed-TLS/TF-PSA-Crypto#148 to add min_requirements.py to 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.

Since we're planning to add such a script, I'll add a separate variable to track this for the framework.

Not sure if this was a typo, but just to be sure: I was talking about tf-psa-crypto, not the framework. Currently the plan is for scripts/min_requirements.py to exist only in the two "top-level" repos (mbedtls, tf-psa-crypto).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yes sorry, that was a typo - I meant tf-psa-crypto.

extra_setup_code += """
scripts/min_requirements.py --user ${info.python_requirements_override_file}
"""
Expand All @@ -258,6 +258,16 @@ scripts/min_requirements.py --user ${info.python_requirements_override_file}
common.get_docker_image(platform)
}
dir('src') {
switch(repo) {
case 'tls':
checkout_repo.checkout_tls_repo(info)
break
case 'tf-psa-crypto':
checkout_repo.checkout_tf_psa_crypto_repo()
break
default:
error("Invalid repo: $repo")
}
checkout_repo.checkout_tls_repo(info)
mpg marked this conversation as resolved.
Show resolved Hide resolved
writeFile file: 'steps.sh', text: """\
#!/bin/sh
Expand Down Expand Up @@ -668,6 +678,12 @@ def gen_release_jobs(BranchInfo info, String label_prefix='', boolean run_exampl
}
}

if (env.RUN_TF_PSA_CRYPTO_ALL_SH == "true") {
info.tf_psa_crypto_all_sh_components.each({ component, platform ->
jobs = jobs + gen_all_sh_jobs(info, platform, component, label_prefix)
mpg marked this conversation as resolved.
Show resolved Hide resolved
})
}

if (env.RUN_WINDOWS_TEST == "true") {
jobs = jobs + gen_windows_jobs(info, label_prefix)
}
Expand Down