CMake: Add post build operation support #14235

hugueskamba · 2021-02-04T00:26:52Z

A CMake custom target, mbed-post-build, is added as a dependency of the
application CMake target if a Mbed target adds a CMake custom target
named mbed-post-build-bin. mbed-post-build-bin is added as a dependency
of mbed-post-build. mbed-post-build-bin depends on the application binary.
This is done so a CMake custom command that executes post-build can be added.

The Python scripts that implement the operations have been modified to add
CLI entry points so they can be called from CMake. Dependency on the old
tool has been removed on those scripts by passing them exactly what they
require instead of passing old tool Python objects. A consequence of that
was to slightly amend how the old tool calls some of those Python modules.

Support has only been added for Mbed targets that currently have a requirement
for post build operations. This includes: LPC1114, LPC1768, ARCH_PRO, LPC54114,
LPC546XX, FF_LPC546XX, CY8CKIT064B0S2_4343W, CYTFM_064B0S2_4343W, CYSBSYSKIT_01.

The following Mbed boards do not have post build operations support as TFM
is not yet supported:
ARM_MUSCA_B1, ARM_MUSCA_B1_NS, ARM_MUSCA_S1, ARM_MUSCA_S1_NS.

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

ciarmcom · 2021-02-04T00:30:18Z

@hugueskamba, thank you for your changes.
@ARMmbed/team-cypress @ARMmbed/team-nxp @ARMmbed/mbed-os-tools @ARMmbed/team-arm-oss-platform @ARMmbed/mbed-os-maintainers please review.

0xc0170 · 2021-02-04T16:00:44Z

The Python scripts that implement the operations have been modified to add
CLI entry points so they can be called from CMake. Dependency on the old
tool has been removed on those scripts by passing them exactly what they
require instead of passing old tool Python objects. A consequence of that
was to slightly amend how the old tool calls some of those Python modules.

Backward compatible then, where we start tracking what we actually use in CMake so we do not remove these dependencies. We already have one memmap.

0xc0170

Let's add to our Readme these dependencies. We can add a section: to migrate from old tools and have a list of files we updated and reusing.

tools/memmap.py
tools/targets/*
etc

To maintain this deps somewhere and once we do clean up we do not lose what is important.

My initial though we start from scratch and port what we can reuse but without old system deps, not bringing what we do not need. Let's see what others think.

0xc0170 · 2021-02-05T12:40:49Z

tools/targets/__init__.py

@@ -1,6 +1,6 @@
 """
 mbed SDK
-Copyright (c) 2011-2020 ARM Limited
+Copyright (c) 2011-2021 ARM Limited


why do we need this file (changing it here) ? I would expect to not to use this one at all. It's one common that we should not need. If we need something from here, we should move it to targets scripts rather. This file contains too many information for the old build system.

This file is not used for the new build system. It was modified because it uses one of the API that was modified. However, I can add a wrapper API function so this stays as it was.

Done. Changes to this file have been reverted.

CMakeLists.txt

Pull request has been modified.

hugueskamba · 2021-02-08T11:41:57Z

This force-push adds to the CMake tools README a listing of the Python modules needed to run post-build hooks.

0xc0170

I am still voting for moving out scripts we use to separate tools/cmake directory (generic ones), and targets to targets/..../scripts.
tools/targets/ should not be invoked from CMake. We are just postponing the clean up to later day for what benefit?

I can't tell from this PR what we actually still using from old tools in some scripts - PSOC6.py is questionable.

Let's move targets script to their CMake parts and have all in one place cleaned up. Even if we duplicate, we have clean slate and use CMake to provide information. Any deps from the old tools should be removed.

tools/targets/PSOC6.py

+def merge_action(args):
+    """Entry point for the "merge" CLI command."""
+    complete_func(
+        print, args.elf, args.m4hex, args.m0hex


0xc0170 · 2021-02-09T10:33:18Z

tools/targets/ARM_MUSCA_S1.py

+
+# Ensure the psa module is visible to this script
+sys.path.insert(0, TFM_SCRIPTS)
+
 from tools.psa.tfm.bin_utils.assemble import Assembly


I am scared of this to see tools. there but that will be cleaned up hopefully via TFM support PR.

tools/targets/PSOC6.py

rwalton-arm

I think we're still too coupled to the old tools. Let's just factor out the business logic of these post build hooks and duplicate the core functionality it in a set of super simple scripts we can run from CMake.

After we've done that we can see what we actually need from the old tools/scripts and possibly factor that code into a python package later, but only if it makes sense to do so.

I think this approach of trying to merge our new code with the old tools spaghetti is going to leave us with more problems to solve later when we deprecate the old tools.

Pull request has been modified.

hugueskamba · 2021-02-09T12:01:44Z

I think we're still too coupled to the old tools. Let's just factor out the business logic of these post build hooks and duplicate the core functionality it in a set of super simple scripts we can run from CMake.

After we've done that we can see what we actually need from the old tools/scripts and possibly factor that code into a python package later, but only if it makes sense to do so.

I think this approach of trying to merge our new code with the old tools spaghetti is going to leave us with more problems to solve later when we deprecate the old tools.

The current approach is to re-use what has been working this is the scope of this task. Only the necessary changes to be called from the CLI had to be made. It still leaves the opportunity to call new Python scripts once we schedule the re-factoring of post-build operations.

Duplicating the functionality and refactoring the code could introduce some bugs. My experience with the old tools is that they usually do more than what is immediately apparent. It will therefore require more time than we currently have allocated to refactor and add tests to make sure it works exactly as the old scripts.

I will add a task in the backlog to refactor the post build operations scripts.

mbed-ci · 2021-02-10T20:16:53Z

Jenkins CI Test : ❌ FAILED

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_cmake-example-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM	❌
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM	❌
jenkins-ci/mbed-os-ci_build-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-example-ARM	✔️

0xc0170 · 2021-02-10T20:25:05Z

Github hiccups, CI restarted

The file with the requested change is not part of this PR.

mbed-ci · 2021-02-11T01:08:30Z

Jenkins CI Test : ❌ FAILED

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM	✔️
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-test	✔️
jenkins-ci/mbed-os-ci_greentea-test	❌

mergify · 2021-02-11T11:50:14Z

This PR cannot be merged due to conflicts. Please rebase to resolve them.

0xc0170 · 2021-02-11T14:50:38Z

@hugueskamba Please rebase, we will restart testing asap

A CMake custom target, mbed-post-build, is added as a dependency of the application CMake target if a Mbed target adds a CMake custom target named mbed-post-build-bin. mbed-post-build-bin is added as a dependency of mbed-post-build. mbed-post-build-bin depends on the application binary. This is done so a CMake custom command that executes post-build can be added. The Python scripts that implement the operations have been modified to add CLI entry points so they can be called from CMake. Dependency on the old tool has been removed on those scripts by passing them exactly what they require instead of passing old tool Python objects. A consequence of that was to slightly amend how the old tool calls some of those Python modules. Support has only been added for Mbed targets that currently have a requirement for post build operations. This includes: LPC1114, LPC1768, ARCH_PRO, LPC54114, LPC546XX, FF_LPC546XX, CY8CKIT064B0S2_4343W, CYTFM_064B0S2_4343W, CYSBSYSKIT_01 The following targets are not supported as TFM support is not yet included: ARM_MUSCA_B1, ARM_MUSCA_B1_NS, ARM_MUSCA_S1, ARM_MUSCA_S1_NS.

Pull request has been modified.

hugueskamba · 2021-02-11T17:08:55Z

@hugueskamba Please rebase, we will restart testing asap

I have now rebased from master and squashed the commits.

0xc0170 · 2021-02-12T16:56:42Z

CI started

mbed-ci · 2021-02-12T19:39:09Z

Jenkins CI Test : ✔️ SUCCESS

Build Number: 3 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-test	✔️
jenkins-ci/mbed-os-ci_greentea-test	✔️

hugueskamba requested review from Patater, 0xc0170, evedon, jainvikas8, rwalton-arm and rajkan01 February 4, 2021 00:26

ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Feb 4, 2021

ciarmcom requested review from a team February 4, 2021 00:30

ciarmcom added the needs: review label Feb 4, 2021

mergify bot added needs: work and removed needs: review labels Feb 4, 2021

0xc0170 previously requested changes Feb 5, 2021

View reviewed changes

rajkan01 reviewed Feb 5, 2021

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

hugueskamba force-pushed the hk_post_build_op branch from 75f8e6a to 40f1111 Compare February 5, 2021 16:49

hugueskamba force-pushed the hk_post_build_op branch from 40f1111 to 161376a Compare February 8, 2021 11:24

0xc0170 previously requested changes Feb 9, 2021

View reviewed changes

rwalton-arm previously requested changes Feb 9, 2021

View reviewed changes

hugueskamba requested a review from rwalton-arm February 10, 2021 14:13

hugueskamba force-pushed the hk_post_build_op branch from 7e4000c to bd37c6e Compare February 10, 2021 16:14

mergify bot added needs: CI and removed needs: work labels Feb 10, 2021

mergify bot added needs: work and removed needs: CI labels Feb 11, 2021

hugueskamba force-pushed the hk_post_build_op branch from bd37c6e to deeaa79 Compare February 11, 2021 17:06

0xc0170 added needs: review and removed needs: work labels Feb 12, 2021

0xc0170 approved these changes Feb 12, 2021

View reviewed changes

mergify bot added needs: CI and removed needs: review labels Feb 12, 2021

mergify bot added ready for merge and removed needs: CI labels Feb 12, 2021

0xc0170 merged commit 8321943 into ARMmbed:master Feb 14, 2021

mergify bot removed the ready for merge label Feb 14, 2021

hugueskamba deleted the hk_post_build_op branch February 15, 2021 09:44

mbedmain added release-version: 6.8.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Feb 16, 2021

CMake: Add post build operation support #14235

CMake: Add post build operation support #14235

Uh oh!

Conversation

hugueskamba commented Feb 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

Uh oh!

ciarmcom commented Feb 4, 2021

Uh oh!

0xc0170 commented Feb 4, 2021

Uh oh!

0xc0170 left a comment • edited by hugueskamba Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 Feb 5, 2021

Choose a reason for hiding this comment

Uh oh!

hugueskamba Feb 5, 2021

Choose a reason for hiding this comment

Uh oh!

hugueskamba Feb 5, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hugueskamba commented Feb 8, 2021

Uh oh!

0xc0170 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

0xc0170 Feb 9, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rwalton-arm left a comment

Choose a reason for hiding this comment

Uh oh!

hugueskamba commented Feb 9, 2021

Uh oh!

mbed-ci commented Feb 10, 2021

Jenkins CI Test : ❌ FAILED

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

Uh oh!

0xc0170 commented Feb 10, 2021

Uh oh!

mbed-ci commented Feb 11, 2021

Jenkins CI Test : ❌ FAILED

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

Uh oh!

mergify bot commented Feb 11, 2021

Uh oh!

0xc0170 commented Feb 11, 2021

Uh oh!

hugueskamba commented Feb 11, 2021

Uh oh!

0xc0170 commented Feb 12, 2021

Uh oh!

mbed-ci commented Feb 12, 2021

Jenkins CI Test : ✔️ SUCCESS

Build Number: 3 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

Uh oh!

Uh oh!

hugueskamba commented Feb 4, 2021 •

edited

Loading

0xc0170 left a comment •

edited by hugueskamba

Loading

0xc0170 left a comment •

edited

Loading