mbed_create_distro() reborn: a function to make adding multiple targets easy #15126

multiplemonomials · 2021-10-03T20:05:26Z

Summary of changes

This PR adds mbed_create_distro(), a function that lets you compile multiple apps in an Mbed OS project without waiting for Mbed OS to build multiple times.

You can use it like this:

mbed_create_distro(mbed_for_my_app mbed-os mbed-storage-kvstore mbed-storage-filesystem)

add_executable(myapp1 MyApp1.cpp)
target_link_libraries(myapp1 PRIVATE mbed_for_my_app)
mbed_set_post_build(myapp1)

add_executable(myapp2 MyApp2.cpp)
target_link_libraries(myapp2 PRIVATE mbed_for_my_app)
mbed_set_post_build(myapp2)

Both myapp1 and myapp2 will act like they were linked to mbed-os, mbed-storage-kvstore, and mbed-storage-filesystem. Note that if you actually did target_link_libraries(myapp1 PRIVATE mbed-os mbed-storage-kvstore mbed-storage-filesystem), it would compile a new version of the Mbed OS source files for each target. However, using mbed_create_distro(), Mbed OS will only be compiled once.

Note that I originally submitted this like 6 months ago, this is an update of the original PR (#14274). At the time I closed that PR because it seemed like Mbed OS was going to switch to using object libraries for all targets, which would also fix the building-multiple-times problem. However, it's been close to 6 months and that change still hasn't happened, and Mbed CLI still isn't usable for RPL until this issue is fixed. So, I'd like to resubmit this PR as at least a stopgap measure until that change happens.

If merged, this will provide a satisfactory fix for #13981.

Impact of changes

mbed_create_distro() is now offered by the build system. This function replaces mbed_configure_app_target() and creates a separate "distribution" OBJECT library from the given Mbed OS libraries that only needs to be compiled once.

Migration actions required

None

Documentation

Docs provided in code comments.

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

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

Reviewers

ciarmcom · 2021-10-03T20:30:14Z

@multiplemonomials, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

ladislas · 2021-10-03T20:41:39Z

Thanks @multiplemonomials! I'll try it asap :)

0xc0170

Thanks for resubmitting this fix, let's get this in.

I only found some debug messages left, otherwise looks fine to me.

0xc0170 · 2021-10-04T08:25:12Z

tools/cmake/create_distro.cmake

+			endif()
+		endforeach()
+
+		#message("REMAINING_MODULES: ${REMAINING_MODULES}")


shall we remove this debug messages (line 63 and 51 as well) ?

Pull request has been modified.

0xc0170 · 2021-10-05T08:16:46Z

tools/cmake/create_distro.cmake

+	get_property(PROP_IS_DEFINED TARGET ${SOURCE} PROPERTY ${PROPERTY} SET)
+	if(PROP_IS_DEFINED)
+		get_property(PROP_VALUE TARGET ${SOURCE} PROPERTY ${PROPERTY})
+		#message("Copying ${PROPERTY} from ${SOURCE} -> ${DESTINATION}: ${PROP_VALUE} ")


we have missed one more

ladislas · 2021-10-05T15:36:22Z

I've tried it with my template project and I can confirm this works!

ladislas/mbed-cmake-template#10

The good news is that it's not mandatory to do it, it won't break people's projects if they don't use the mbed_create_distro() function.

I can't try it now with our big project (https://github.com/leka/LekaOS) as it will need to change a lot of things but I think it's a good start. And we can start building from this PR.

0xc0170 · 2021-10-06T08:51:21Z

CI started, however waiting for one more approval from the core team

mbed-ci · 2021-10-06T09:26:01Z

Jenkins CI Test : ✔️ SUCCESS

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

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-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_cmake-example-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_greentea-test	✔️

ladislas · 2021-10-08T08:18:18Z

ping :)

ladislas · 2021-10-19T09:11:10Z

More and more people on the forum are moving to mbed cli2 and are experiencing the issue this PR is resolving in a elegant, easy and opt-in manner.

It would be great if it could be merged so people could start using it and improve if needed.

ladislas · 2021-10-19T12:28:58Z

amazing! thank you!

multiplemonomials added 2 commits October 3, 2021 12:36

Add mbed-create-distro

f5fd336

Whoops, add license header

22a7ebc

multiplemonomials changed the title ~~Create distro reborn~~ mbed_create_distro() reborn: a function to make adding multiple targets easy Oct 3, 2021

ciarmcom added the release-type: feature label Oct 3, 2021

ciarmcom requested a review from a team October 3, 2021 20:30

ciarmcom added needs: review component: tools labels Oct 3, 2021

0xc0170 requested a review from a team October 4, 2021 07:58

0xc0170 previously requested changes Oct 4, 2021

View reviewed changes

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

0xc0170 requested a review from a team October 4, 2021 13:20

Remove debug messages

7663be1

0xc0170 reviewed Oct 5, 2021

View reviewed changes

0xc0170 mentioned this pull request Oct 5, 2021

CMake: From USCRPL/mbed-cmake to Mbed OS first-party CMake #13981

Closed

One more debug message

503b9b8

ladislas approved these changes Oct 5, 2021

View reviewed changes

0xc0170 approved these changes Oct 6, 2021

View reviewed changes

mergify bot added needs: CI and removed needs: work labels Oct 6, 2021

mergify bot added ready for merge and removed needs: CI labels Oct 6, 2021

0xc0170 requested a review from a team October 6, 2021 13:13

0xc0170 removed the ready for merge label Oct 6, 2021

0xc0170 added the needs: review label Oct 6, 2021

LDong-Arm approved these changes Oct 19, 2021

View reviewed changes

0xc0170 added ready for merge and removed needs: review labels Oct 19, 2021

0xc0170 merged commit f69af59 into ARMmbed:master Oct 19, 2021

mergify bot removed the ready for merge label Oct 19, 2021

0xc0170 added release-type: patch Indentifies a PR as containing just a patch and removed release-type: feature labels Nov 22, 2021

mbedmain added release-version: 6.15.1 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Nov 22, 2021

wmmc88 mentioned this pull request Jan 7, 2022

Update cmake to use mbed_create_distro and latest mbed release uwrobotics/MarsRoverFirmware#404

Open

multiplemonomials deleted the create-distro-reborn branch May 17, 2023 06:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mbed_create_distro() reborn: a function to make adding multiple targets easy #15126

mbed_create_distro() reborn: a function to make adding multiple targets easy #15126

multiplemonomials commented Oct 3, 2021 •

edited by 0xc0170

Loading

ciarmcom commented Oct 3, 2021

ladislas commented Oct 3, 2021

0xc0170 left a comment

0xc0170 Oct 4, 2021

multiplemonomials Oct 5, 2021

0xc0170 Oct 5, 2021

multiplemonomials Oct 5, 2021

ladislas commented Oct 5, 2021

0xc0170 commented Oct 6, 2021

mbed-ci commented Oct 6, 2021

ladislas commented Oct 8, 2021

ladislas commented Oct 19, 2021

ladislas commented Oct 19, 2021

mbed_create_distro() reborn: a function to make adding multiple targets easy #15126

mbed_create_distro() reborn: a function to make adding multiple targets easy #15126

Conversation

multiplemonomials commented Oct 3, 2021 • edited by 0xc0170 Loading

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

ciarmcom commented Oct 3, 2021

ladislas commented Oct 3, 2021

0xc0170 left a comment

Choose a reason for hiding this comment

0xc0170 Oct 4, 2021

Choose a reason for hiding this comment

multiplemonomials Oct 5, 2021

Choose a reason for hiding this comment

0xc0170 Oct 5, 2021

Choose a reason for hiding this comment

multiplemonomials Oct 5, 2021

Choose a reason for hiding this comment

ladislas commented Oct 5, 2021

0xc0170 commented Oct 6, 2021

mbed-ci commented Oct 6, 2021

Jenkins CI Test : ✔️ SUCCESS

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

ladislas commented Oct 8, 2021

ladislas commented Oct 19, 2021

ladislas commented Oct 19, 2021

multiplemonomials commented Oct 3, 2021 •

edited by 0xc0170

Loading