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

Secure binaries release script #9928

Merged
merged 2 commits into from Mar 6, 2019
Merged

Secure binaries release script #9928

merged 2 commits into from Mar 6, 2019

Conversation

orenc17
Copy link
Contributor

@orenc17 orenc17 commented Mar 4, 2019

Description

This script will find all the PSA targets and compile their secure binaries (Including test secure binaries)

Pull request type

[ ] Fix
[X] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@ARMmbed/mbed-os-tools @ARMmbed/mbed-os-maintainers

Release Notes

theotherjimmy
theotherjimmy reviewed Mar 4, 2019
View reviewed changes
tools/psa/release.py Outdated Show resolved Hide resolved
theotherjimmy
theotherjimmy reviewed Mar 4, 2019
View reviewed changes
tools/psa/release.py Outdated
def build_mbed_spm_platform(target, toolchain):
subprocess.call([
PYTHON_EXEC,
'-u',
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason you chose to call test.py instead of invoke the python directly? Both are within the same directory after all.

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 was clear here: I mean you can import the functions that test.py calls and call them as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

main() function?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking of build_project and build_tests and build_library.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and then i'll need to update this tool everytime the internals change ( true, that's rare to happen but better safe than sorry)

theotherjimmy
theotherjimmy reviewed Mar 4, 2019
View reviewed changes
tools/psa/release.py Outdated Show resolved Hide resolved
theotherjimmy
theotherjimmy suggested changes Mar 4, 2019
View reviewed changes
tools/psa/release.py Outdated Show resolved Hide resolved
@orenc17
Copy link
Contributor Author

orenc17 commented Mar 4, 2019

@theotherjimmy i've addressed all the issues please review again

@theotherjimmy
Copy link
Contributor

@orenc17 Sorry for the miscommunication. when I was recommending that you drop the if-expression for python executable, I meant to use sys.executable for all of them, not "python". That way when you do python3 tools/psa/release.py you invoke make.py with the same python, regardless of what the system default is.

theotherjimmy
theotherjimmy suggested changes Mar 4, 2019
View reviewed changes
Copy link
Contributor

@theotherjimmy theotherjimmy left a comment

Choose a reason for hiding this comment

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

This is what I was trying to convey. Sorry for the confusion.

tools/psa/release.py Outdated Show resolved Hide resolved
tools/psa/release.py Outdated Show resolved Hide resolved
tools/psa/release.py Outdated Show resolved Hide resolved
tools/psa/release.py Outdated Show resolved Hide resolved
Oren Cohen added 2 commits March 4, 2019 18:42
Secure binaries release script
7b44834 
This script will find all the PSA targets and compile their secure binaries
Including test secure binaries
Add option to run on a single target
cf8ab27
@orenc17
Copy link
Contributor Author

orenc17 commented Mar 5, 2019

@theotherjimmy fixed
@ARMmbed/mbed-os-maintainers can we run CI?

@NirSonnenschein NirSonnenschein requested review from a team March 5, 2019 13:54
0xc0170
0xc0170 reviewed Mar 5, 2019
View reviewed changes
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

Where's documentation for this script? docs PR?

@NirSonnenschein
Copy link
Contributor

got request to move this to 5.12Rc2 , added tag pending final review.
This PR is important for uniform generation of all secure side images upon changes to the secure side code.

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 6, 2019 •
edited

@NirSonnenschein where should documentation live for this? How uses it and how?

Will it be part of PSA docs?

@NirSonnenschein
Copy link
Contributor

NirSonnenschein commented Mar 6, 2019 •
edited

@0xc0170 this is a tool, mainly for internal use (e.g. maintainers, PSA development, etc..) but could also be used for silicon partners and potentially as part of the release process.
I think for such tools it is probably good to document inline, unless it becomes part of an official procedure ( e.g. release process ) in which case it should definitely be added to the docs

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 6, 2019

If that is case, inline docs should be present

0xc0170
0xc0170 previously requested changes Mar 6, 2019
View reviewed changes
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

Inline docs as mentioned above what is this script for and how to use it, LGTM otherwise

@ghost ghost added the PM_ACCEPTED label Mar 6, 2019
@orenc17 orenc17 mentioned this pull request Mar 6, 2019
Merged
@orenc17
Copy link
Contributor Author

orenc17 commented Mar 6, 2019

@NirSonnenschein @0xc0170
complete documentation is in #9963

@cmonr
Copy link
Contributor

cmonr commented Mar 6, 2019

CI started

@cmonr cmonr requested a review from 0xc0170 March 6, 2019 19:47
@mbed-ci
Copy link

mbed-ci commented Mar 6, 2019

Test run: SUCCESS

Summary: 13 of 13 test jobs passed
Build number : 1
Build artifacts

@cmonr cmonr dismissed 0xc0170’s stale review March 6, 2019 22:55

Docs will be added in separate PR

@cmonr cmonr merged commit 120e43e into ARMmbed:master Mar 6, 2019
@cmonr cmonr removed the needs: CI label Mar 6, 2019
@ccli8 ccli8 mentioned this pull request Mar 8, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmonr cmonr cmonr approved these changes

@theotherjimmy theotherjimmy theotherjimmy approved these changes

@0xc0170 0xc0170 Awaiting requested review from 0xc0170

Assignees
No one assigned
Labels
release-version: 5.12.0-rc2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@orenc17 @theotherjimmy @NirSonnenschein @0xc0170 @cmonr @mbed-ci