Skip to content

Add new functionality to build_tfm.py #80

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

Merged
merged 6 commits into from
Feb 22, 2021

Conversation

jainvikas8
Copy link
Contributor

@jainvikas8 jainvikas8 commented Feb 18, 2021

This enables the building of TF-M latest config which was set up by check_rebase.py.

build_external_tfm.py builds ARM_MUSCA_S1 target with the external TF-M + patches required by Mbed OS. It relies on check_rebase.py to set up the required dependencies.

@jainvikas8 jainvikas8 changed the title Add new functionality to build_tfm.py [WIP] Add new functionality to build_tfm.py Feb 18, 2021
Copy link
Contributor

@LDong-Arm LDong-Arm left a comment

Choose a reason for hiding this comment

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

The change looks good. The list of files to copy seems tricky as it has changed already...

@jainvikas8
Copy link
Contributor Author

The PR requires a merge of #78

@jainvikas8
Copy link
Contributor Author

Forced pushed to address review comments

@jainvikas8 jainvikas8 changed the title [WIP] Add new functionality to build_tfm.py Add new functionality to build_tfm.py Feb 18, 2021
Copy link
Contributor

@LDong-Arm LDong-Arm left a comment

Choose a reason for hiding this comment

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

LGTM. The discussion on VERSION.txt is out of the scope of the PR itself, so it's okay for now.

Copy link
Contributor

@LDong-Arm LDong-Arm left a comment

Choose a reason for hiding this comment

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

The scripts look good to me, will give them a run

Copy link
Contributor

@LDong-Arm LDong-Arm left a comment

Choose a reason for hiding this comment

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

Tested the scripts locally

@jainvikas8
Copy link
Contributor Author

jainvikas8 commented Feb 18, 2021

Forced pushed to address review comments and rebased

@jainvikas8 jainvikas8 force-pushed the rebase-build-step branch 4 times, most recently from 3413cb5 to 82fa7b2 Compare February 18, 2021 19:45
Use `mbed-tfm-rebase-check` branches of respective remote and
repository (dependencies) instead of `mbed-tfm`.

This helps to resolve any rebase issues on the Mbed OS patches required
for TF-M latest implementation.

Additionally, clone `psa-arch-tests` repository which is used by
TF-M + Mbed OS.
@jainvikas8
Copy link
Contributor Author

Forced pushed to address review comments.

`build_tfm.py`:
This feature deletes the cloned dependencies directory enabling user to
start in a clean state.

`test_psa_target.py`:
Since this script calls the `build_tfm.py` therefore this
functionality is extended.

Note: This would be an expensive operation as every time it would
delete the cloned dependencies directory, forcing it to set up the
dependencies again.
`build_tfm.py`:
This feature skips the cloning/checkout of TF-M dependencies enabling
user to work with their setup.

Note: This feature will be overridden if `--clean` option is used.

`test_psa_target`:
Since this script calls the `build_tfm.py` therefore this
functionality is extended.
This feature skips the copying of TF-M dependencies files and folders
to Mbed OS.
The purpose of this script is to use cloned TF-M dependencies and build
TF-M regression and PSA compliance test suites.

This is to support internal CI run.
@jainvikas8
Copy link
Contributor Author

Forced pushed to split VERSION.txt in a new commit.

Copy link
Contributor

@LDong-Arm LDong-Arm left a comment

Choose a reason for hiding this comment

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

LGTM. I've triggered a CI run.

@LDong-Arm
Copy link
Contributor

CI passed!

@evedon
Copy link

evedon commented Feb 22, 2021

build_external_tfm.py builds ARM_MUSCA_B1 target with the external TF-M + patches required by Mbed OS. It relies on check_rebase.py to set up the required dependencies.

The script builds ARM_MUSCA_S1.
Other than that I am fine with the PR

@jainvikas8
Copy link
Contributor Author

build_external_tfm.py builds ARM_MUSCA_B1 target with the external TF-M + patches required by Mbed OS. It relies on check_rebase.py to set up the required dependencies.

The script builds ARM_MUSCA_S1.
Other than that I am fine with the PR

Fixed, typo.

At the moment on ARM_MUSCA_S1 is only built, due to the target name change of ARM_MUSCA_B1 to sse_200/musca_b1 in TF-M. This requires changes on - https://github.com/ARMmbed/mbed-os/blob/feature-tf-m-1.2-integration/targets/targets.json#L4729

@evedon evedon merged commit af7b5ce into ARMmbed:tf-m-v1.2-integration Feb 22, 2021
jainvikas8 added a commit to jainvikas8/mbed-os-tf-m-regression-tests that referenced this pull request Feb 23, 2021
Fixes ARMmbed#24

The issue on Windows (Host OS) is that it clones dependencies in
read-only mode, which creates an issue when deleting them if
`test_psa_target.py` or `build_tfm.py` is executed.

Therefore handle the permission issue by setting them in read-write
mode.

This fix was missed when PR ARMmbed#80 was merged after ARMmbed#83

To be squashed with 03fb1c0
jainvikas8 added a commit to jainvikas8/mbed-os-tf-m-regression-tests that referenced this pull request Feb 23, 2021
Fixes ARMmbed#24

The issue is with the permission model on Windows (Host OS),
which creates an issue when deleting them if `test_psa_target.py`
or `build_tfm.py` is executed.

Therefore handle the permission issue by setting them in read-write
mode.

This fix was missed when PR ARMmbed#80 was merged after ARMmbed#83

To be squashed with 03fb1c0
jainvikas8 added a commit to jainvikas8/mbed-os-tf-m-regression-tests that referenced this pull request Feb 23, 2021
Fixes ARMmbed#24

The permission model on Windows (Host OS) creates an issue when
deleting files, if `test_psa_target.py` or `build_tfm.py` is executed.

Therefore handle it by setting them in read-write mode.

This fix was missed when PR ARMmbed#80 was merged after ARMmbed#83

To be squashed with 03fb1c0
evedon pushed a commit that referenced this pull request Feb 23, 2021
Fixes #24

The permission model on Windows (Host OS) creates an issue when
deleting files, if `test_psa_target.py` or `build_tfm.py` is executed.

Therefore handle it by setting them in read-write mode.

This fix was missed when PR #80 was merged after #83

To be squashed with 03fb1c0
evedon pushed a commit that referenced this pull request Mar 3, 2021
Fixes #24

The permission model on Windows (Host OS) creates an issue when
deleting files, if `test_psa_target.py` or `build_tfm.py` is executed.

Therefore handle it by setting them in read-write mode.

This fix was missed when PR #80 was merged after #83

To be squashed with 03fb1c0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants