Enhancements

* Replace call with check_call to throw exception on failure
* Check if binaries actually been changes before calling git commit
* Docstrings for all functions
* Small refactor

tools/psa/release.py

@@ -21,53 +21,95 @@ import sys
 import shutil
 from argparse import ArgumentParser
 
-ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir, os.pardir))
+ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__),
+                                    os.pardir, os.pardir))
 sys.path.insert(0, ROOT)
 from tools.targets import Target, TARGET_MAP, TARGET_NAMES
 
 MAKE_PY_LOCATTION = os.path.join(ROOT, 'tools', 'make.py')
 TEST_PY_LOCATTION = os.path.join(ROOT, 'tools', 'test.py')
-MBED_PSA_TESTS = '*psa-spm*,*psa-crypto_access_control'
 TFM_MBED_APP = os.path.join(ROOT, 'tools', 'psa', 'tfm', 'mbed_app.json')
+MBED_PSA_TESTS = '*psa-spm*,*psa-crypto_access_control'
 TFM_TESTS = {
     '*psa-spm_smoke': ['USE_PSA_TEST_PARTITIONS', 'USE_SMOKE_TESTS_PART1'],
     '*psa-spm_client': ['USE_PSA_TEST_PARTITIONS', 'USE_CLIENT_TESTS_PART1'],
-    '*psa-spm_server': ['USE_PSA_TEST_PARTITIONS', 'USE_SERVER_TESTS_PART1', 'USE_SERVER_TESTS_PART2'],
+    '*psa-spm_server': ['USE_PSA_TEST_PARTITIONS', 'USE_SERVER_TESTS_PART1',
-    '*psa-crypto_access_control': ['USE_PSA_TEST_PARTITIONS', 'USE_CRYPTO_ACL_TEST']
+                        'USE_SERVER_TESTS_PART2'],
+    '*psa-crypto_access_control': ['USE_PSA_TEST_PARTITIONS',
+                                   'USE_CRYPTO_ACL_TEST']
 }
 
 
-def _psa_backend(target_name):
+def _psa_backend(target):
-    return 'TFM' if 'TFM' in Target.get_target(target_name).labels else 'MBED_SPM'
+    """
+    Returns a target PSA backend.
+
+    :param target: Target name as in targets.json
+    :return:  PSA backend as string (TFM/MBED_SPM)
+    """
+    return 'TFM' if 'TFM' in Target.get_target(target).labels else 'MBED_SPM'
 
 
-def get_mbed_official_psa_release():
+def _get_target_info(target):
+    """
+    Creates a PSA target tuple with default toolchain and
+    artifact delivery directory.
+
+    :param target: Target name.
+    :return: tuple (target, toolchain, delivery directory).
+    """
+    delivery_dir = os.path.join(ROOT, 'targets',
+                                TARGET_MAP[target].delivery_dir)
+
+    if not os.path.exists(delivery_dir):
+        raise Exception("{} does not have delivery_dir".format(target))
+
+    return tuple([TARGET_MAP[target].name,
+                  TARGET_MAP[target].default_toolchain,
+                  delivery_dir])
+
+
+def get_mbed_official_psa_release(target=None):
+    """
+    Creates a list of PSA targets with default toolchain and
+    artifact delivery directory.
+
+    :param target: Ask for specific target, None for all targets.
+    :return: List of tuples (target, toolchain, delivery directory).
+    """
     psa_targets_release_list = []
-    psa_secure_targets = [t for t in TARGET_NAMES if Target.get_target(t).is_PSA_secure_target]
+    psa_secure_targets = [t for t in TARGET_NAMES if
-    for t in psa_secure_targets:
+                          Target.get_target(t).is_PSA_secure_target]
-        delivery_dir = os.path.join(ROOT, 'targets', TARGET_MAP[t].delivery_dir)
+    if target is not None:
-        if not os.path.exists(delivery_dir):
+        if target not in psa_secure_targets:
-            raise Exception("{} does not have delivery_dir".format(TARGET_MAP[t].name))
+            raise Exception("{} is not a PSA secure target".format(target))
-        psa_targets_release_list.append(
+        psa_targets_release_list.append(_get_target_info(target))
-            tuple(
+    else:
-                [
+        for t in psa_secure_targets:
-                    TARGET_MAP[t].name,
+            psa_targets_release_list.append(_get_target_info(target))
-                    TARGET_MAP[t].default_toolchain,
-                    delivery_dir,
-                ]
-            )
-        )
 
     return psa_targets_release_list
 
 
 def create_mbed_ignore(build_dir):
+    """
+    Creates a .mbedignore file in a given directory.
+
+    :param build_dir: Directory to create .mbedignore file.
+    """
     with open(os.path.join(build_dir, '.mbedignore'), 'w') as f:
         f.write('*\n')
 
 
 def build_mbed_spm_platform(target, toolchain, profile='release'):
-    subprocess.call([
+    """
+    Builds Secure images for MBED-SPM target.
+
+    :param target: target to be built.
+    :param toolchain: toolchain to be used.
+    :param profile: build profile.
+    """
+    subprocess.check_call([
         sys.executable, '-u', TEST_PY_LOCATTION,
         '--greentea',
         '--profile', profile,
@@ -75,12 +117,14 @@ def build_mbed_spm_platform(target, toolchain, profile='release'):
         '-m', target,
         '--source', ROOT,
         '--build', os.path.join(ROOT, 'BUILD', 'tests', target),
-        '--test-spec', os.path.join(ROOT, 'BUILD', 'tests', target, 'test_spec.json'),
+        '--test-spec', os.path.join(ROOT, 'BUILD', 'tests',
-        '--build-data', os.path.join(ROOT, 'BUILD', 'tests', target, 'build_data.json'),
+                                    target, 'test_spec.json'),
+        '--build-data', os.path.join(ROOT, 'BUILD', 'tests',
+                                     target, 'build_data.json'),
         '-n', MBED_PSA_TESTS
-    ])
+    ], stdout=subprocess.PIPE)
 
-    subprocess.call([
+    subprocess.check_call([
         sys.executable, '-u', MAKE_PY_LOCATTION,
         '-t', toolchain,
         '-m', target,
@@ -88,16 +132,29 @@ def build_mbed_spm_platform(target, toolchain, profile='release'):
         '--source', ROOT,
         '--build', os.path.join(ROOT, 'BUILD', target),
         '--artifact-name', 'psa_release_1.0'
-    ])
+    ], stdout=subprocess.PIPE)
 
 
 def _tfm_test_defines(test):
+    """
+    Creates a define list to enable test partitions on TF-M.
+
+    :param test: Test name.
+    :return: List of defines with a leading -D.
+    """
     return ['-D{}'.format(define) for define in TFM_TESTS[test]]
 
 
 def build_tfm_platform(target, toolchain, profile='release'):
+    """
+    Builds Secure images for TF-M target.
+
+    :param target: target to be built.
+    :param toolchain: toolchain to be used.
+    :param profile: build profile.
+    """
     for test in TFM_TESTS.keys():
-        subprocess.call([
+        subprocess.check_call([
             sys.executable, '-u', TEST_PY_LOCATTION,
             '--greentea',
             '--profile', profile,
@@ -105,11 +162,14 @@ def build_tfm_platform(target, toolchain, profile='release'):
             '-m', target,
             '--source', ROOT,
             '--build', os.path.join(ROOT, 'BUILD', 'tests', target),
-            '--test-spec', os.path.join(ROOT, 'BUILD', 'tests', target, 'test_spec.json'),
+            '--test-spec', os.path.join(ROOT, 'BUILD', 'tests',
-            '--build-data', os.path.join(ROOT, 'BUILD', 'tests', target, 'build_data.json'),
+                                        target, 'test_spec.json'),
-            '--app-config', TFM_MBED_APP, '-n', test] + _tfm_test_defines(test))
+            '--build-data', os.path.join(ROOT, 'BUILD', 'tests',
+                                         target, 'build_data.json'),
+            '--app-config', TFM_MBED_APP, '-n', test] + _tfm_test_defines(test),
+                              stdout=subprocess.PIPE)
 
-    subprocess.call([
+    subprocess.check_call([
         sys.executable, '-u', MAKE_PY_LOCATTION,
         '-t', toolchain,
         '-m', target,
@@ -117,29 +177,51 @@ def build_tfm_platform(target, toolchain, profile='release'):
         '--source', ROOT,
         '--build', os.path.join(ROOT, 'BUILD', target),
         '--app-config', TFM_MBED_APP
-    ])
+    ], stdout=subprocess.PIPE)
 
 
-def commit_biannries(target, delivery_dir):
+def commit_binaries(target, delivery_dir):
-    cmd = [
+    """
+    Commits changes in secure binaries.
+
+    :param target: Target name.
+    :param delivery_dir: Secure images should be moved to this folder
+    by the build system.
+    """
+    changes_made = subprocess.call([
         'git',
         '-C', ROOT,
-        'add', os.path.relpath(delivery_dir, ROOT)
+        'diff', '--exit-code',
-    ]
+        delivery_dir
+    ], stdout=subprocess.PIPE)
 
-    subprocess.call(cmd)
+    if changes_made:
-    commit_message = 'Update secure binaries for {}'.format(target)
+        subprocess.check_call([
-    cmd = [
+            'git',
-        'git',
+            '-C', ROOT,
-        '-C', ROOT,
+            'add', os.path.relpath(delivery_dir, ROOT)
-        'commit',
+        ], stdout=subprocess.PIPE)
-        '-m', commit_message
-    ]
 
-    subprocess.call(cmd)
+        commit_message = '-m\"Update secure binaries for {}\"'.format(target)
+        subprocess.check_call([
+            'git',
+            '-C', ROOT,
+            'commit',
+            commit_message
+        ], stdout=subprocess.PIPE)
 
 
-def build_psa_platform(target, toolchain, delivery_dir, debug=False, git_commit=False):
+def build_psa_platform(target, toolchain, delivery_dir, debug=False,
+                       git_commit=False):
+    """
+    Calls the correct build function and commits if requested.
+
+    :param target: Target name.
+    :param toolchain: Toolchain to be used.
+    :param delivery_dir: Artifact directory, where images should be placed.
+    :param debug: Build with debug profile.
+    :param git_commit: Commit the changes.
+    """
     profile = 'debug' if debug else 'release'
     if _psa_backend(target) is 'TFM':
         build_tfm_platform(target, toolchain, profile)
@@ -147,14 +229,14 @@ def build_psa_platform(target, toolchain, delivery_dir, debug=False, git_commit=
         build_mbed_spm_platform(target, toolchain, profile)
 
     if git_commit:
-        commit_biannries(target, delivery_dir)
+        commit_binaries(target, delivery_dir)
 
 
 def get_parser():
     parser = ArgumentParser()
     parser.add_argument("-m", "--mcu",
                         help="build for the given MCU",
-                        default='*',
+                        default=None,
                         metavar="MCU")
 
     parser.add_argument("-d", "--debug",
@@ -170,30 +252,26 @@ def get_parser():
     return parser
 
 
-def filter_target(mcu):
+def prep_build_dir():
-    def filter_func(t):
+    """
-        return t[0] == mcu
+    Creates a clean BUILD directory
-
+    """
-    return filter_func
-
-
-def main():
-    parser = get_parser()
-    options = parser.parse_args()
     build_dir = os.path.join(ROOT, 'BUILD')
     if os.path.exists(build_dir):
         shutil.rmtree(build_dir)
 
     os.makedirs(build_dir)
     create_mbed_ignore(build_dir)
-    target_filter_function = None
-    psa_platforms_list = get_mbed_official_psa_release()
 
-    if options.mcu is not '*':
-        target_filter_function = filter_target(options.mcu)
 
-    for target, toolchain, delivery_dir in filter(target_filter_function, psa_platforms_list):
+def main():
-        build_psa_platform(target, toolchain, delivery_dir, options.debug, options.commit)
+    parser = get_parser()
+    options = parser.parse_args()
+    prep_build_dir()
+    psa_platforms_list = get_mbed_official_psa_release(options.mcu)
+
+    for target, tc, directory in psa_platforms_list:
+        build_psa_platform(target, tc, directory, options.debug, options.commit)
 
 
 if __name__ == '__main__':