cmake: design doc update #13321

0xc0170 · 2020-07-21T08:02:54Z

Summary of changes

Add requirements
Fix cmake folder updates
Fix label function

Plus other small edits

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

[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

rwalton-arm

Looks good. I have a couple of suggestions around grammar etc.

docs/design-documents/tools/cmake.md

0xc0170 · 2020-07-21T09:41:35Z

The license check is on master as well, creating a new blocker and will fix.

docs/design-documents/tools/cmake.md

Pull request has been modified.

mergify · 2020-07-21T12:43:47Z

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

0xc0170 · 2020-07-21T12:45:50Z

Rebased to get the travis fix in

docs/design-documents/tools/cmake.md

hugueskamba · 2020-07-21T11:40:51Z

docs/design-documents/tools/cmake.md

-All what can be defined should be in CMake files and configuration (app/mbed .json files). Our build system would parse the configuration, create rules and generate top level CMake file that would include others (application CMake files plus also Mbed OS core CMake).
+Everything that can be defined should be in CMake files and configuration (mbed_app/mbed_lib.json) files. Our build system would parse the configuration and generate a CMake config file.

 To stay backward compatible, we create common rules as follows.


Suggested change

To stay backward compatible, we create common rules as follows.

For backward compatibility, the following rules must be respected:

I'd change that to "The following rules must be respected" to avoid duplicating "follow..." in the same sentence.

docs/design-documents/tools/cmake.md

hugueskamba · 2020-07-21T12:02:15Z

docs/design-documents/tools/cmake.md

+An example, to add `TARGET_STM` in the a folder `targets`:

 ```
-./TARGET_STM/qspi/driver_stm_qspi.cpp
-./TARGET_STM/peripheral_specific_to_stm/stm_specific_driver.cpp
-```
-
-As result, the top level application CMake:
-
-```
-add_subdirectory_if_target(STM TARGET_STM/qspi)
-add_subdirectory_if_target(STM TARGET_STM/peripheral_specific_to_stm)
-
+mbed_add_cmake_directory_if_labels("TARGET")
 ```

-There would be static CMakes in the `./TARGET_STM/qspi/` and `./TARGET_STM/peripheral_specific_to_stm` that would define what files should be included or specific settins for these modules.
+The same could be applied to other labels like features or components.


Suggested change

An example, to add `TARGET_STM` in the a folder `targets`:

```

./TARGET_STM/qspi/driver_stm_qspi.cpp

./TARGET_STM/peripheral_specific_to_stm/stm_specific_driver.cpp

```

As result, the top level application CMake:

```

add_subdirectory_if_target(STM TARGET_STM/qspi)

add_subdirectory_if_target(STM TARGET_STM/peripheral_specific_to_stm)

mbed_add_cmake_directory_if_labels("TARGET")

```

There would be static CMakes in the `./TARGET_STM/qspi/` and `./TARGET_STM/peripheral_specific_to_stm` that would define what files should be included or specific settins for these modules.

The same could be applied to other labels like features or components.

I.e As the `DISCO_L475VG_IOT01A` target inherits the `STM` label and has the `BLE` feature, the `TARGET_STM` directory in `mbed-os/targets/` and the `FEATURE_BLE` directory in `mbed-os/features/` are processed because their parent directories contain `CMakeLists.txt` files that have each respectively `mbed_add_cmake_directory_if_labels("TARGET")` and `mbed_add_cmake_directory_if_labels("FEATURE")`.

docs/design-documents/tools/cmake.md

hugueskamba · 2020-07-21T13:03:59Z

docs/design-documents/tools/cmake.md

+This file statically defines the structure of the component within Mbed OS. It contains conditional statements that are based on the config. We use regular CMake expressions, plus extend it with our own macros, to conditionally include/exclude directories. The rule of thumb: do not expose headers that are internal. We would like to avoid having everything in the include paths as we do now.
+
+
+`add_subdirectory` always adds the directory to the main CMake.


Is it necessary to explain a CMake function here?

I'll remove

hugueskamba · 2020-07-21T13:05:00Z

docs/design-documents/tools/cmake.md

 A user create own CMake file to configure an application, also with `mbed_app.json` configuration file. The building of an app would look like:

 1. Parse the arguments provided to build command
 2. Parse the application configuration


Do you want to carry on using 1 for numbered bullet points so you do not need to remember what the next number is?

hugueskamba · 2020-07-21T13:05:33Z

docs/design-documents/tools/cmake.md

-
 ## Application CMake

 We should provide application CMake functionality with  our own configuration. There are couple of approaches we could take. Statically defined CMake but then this disconnectes config and CMake - as CMake contains configuration for a project (like includes, sources, etc). Our build tool would need to parse CMake to get all paths used in the project or Mbed OS to find out where to look for configuration file. Therefore the build system has a knowledge as it is currently. We use `requires` to include/exclude modules.


There are extra spaces in this paragraph.

mergify · 2020-07-21T16:59:59Z

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

0xc0170 · 2020-07-22T08:08:00Z

I'll rebase and fix all the reviews comments 👍

0xc0170 · 2020-07-23T08:18:08Z

Rebased, history fixed as well.

0xc0170 · 2020-07-23T08:21:58Z

Ah, wait , lost some changes, fixing that :)

0xc0170 · 2020-07-23T09:19:07Z

Rebased and recovered old data from github 🙄 magic here and there. I'll squash once reviewed and approved.

docs/design-documents/tools/cmake.md

Patater · 2020-07-23T10:54:13Z

docs/design-documents/tools/cmake.md

+
+`mbed-tools` has consolidated all of the required modules to build Mbed OS, along with the command line interface, into a single Python package which can be installed using standard python packaging tools.

 A user create own CMake file to configure an application, also with `mbed_app.json` configuration file. The building of an app would look like:


I don't think this is accurate anymore. mbed-tools creates the Mbed configuration CMake file (.mbedbuild/mbed_config.cmake) and we don't expect users to have to edit it manually.

this was about CMakelists.txt in the application, like blinky has in the root directory.

Do we want the tools to create a template top-level CMakeLists.txt when the user runs mbedtools init (or whatever the command to create a new workspace ends up being called, it's init currently)?

that would be ideal, if cmakelists does not exists and they run init, yotta had similar init flow as I recall.

I guess we want to avoid scanning folders , so we let user to fill in the template.

docs/design-documents/tools/cmake.md

Pull request has been modified.

docs/design-documents/tools/cmake.md

Pull request has been modified.

0xc0170 · 2020-07-23T12:28:45Z

I'll rebase for the final CI run

docs/design-documents/tools/cmake.md

Pull request has been modified.

0xc0170 · 2020-07-23T13:03:36Z

Updated

docs/design-documents/tools/cmake.md

rwalton-arm · 2020-07-23T13:52:45Z

docs/design-documents/tools/cmake.md

+The same could be applied to other labels like features or components.

-As result, the top level application CMake:
+To migrate to the new build system, we can provide auto scanning of the module and generate CMake based on what we find or use the way as described above. In both cases, we could stay backward compatible.


I think we need to fix the grammar here, but I'm not sure what we mean by "auto scanning of the module"; do we mean "auto scanning of Mbed OS module and component directories"?

File globbing is what I read

I'll remove it, the migration path will come in separate update

Patater

LGTM other than the commit history

Pull request has been modified.

Refactor Mbed OS CMake scripts section - details about mbed-os/cmake scripts Add mbed-tools - how mbed-tools built an application with CMake

0xc0170 · 2020-07-23T14:07:40Z

History fixed, should be ready now

0xc0170 requested review from a team, hugueskamba and ithinuel July 21, 2020 08:03

mergify bot added release-version: feature branch needs: work labels Jul 21, 2020

0xc0170 mentioned this pull request Jul 21, 2020

CMake: remove version number from ARM toolchain #13316

Merged

rwalton-arm reviewed Jul 21, 2020

View reviewed changes

0xc0170 force-pushed the feature-cmake-design-update branch from 3f9b678 to 32681f4 Compare July 21, 2020 08:34

0xc0170 mentioned this pull request Jul 21, 2020

Travis: license check is failing (scancode error) #13322

Closed

0xc0170 force-pushed the feature-cmake-design-update branch from c05625f to 435a50b Compare July 21, 2020 10:37

0xc0170 requested a review from rwalton-arm July 21, 2020 10:38

0xc0170 added needs: review and removed needs: work labels Jul 21, 2020

mergify bot added needs: work and removed needs: review labels Jul 21, 2020

rwalton-arm reviewed Jul 21, 2020

View reviewed changes

docs/design-documents/tools/cmake.md Outdated Show resolved Hide resolved

rwalton-arm previously approved these changes Jul 21, 2020

View reviewed changes

0xc0170 force-pushed the feature-cmake branch from f51723c to ade9959 Compare July 21, 2020 12:43

0xc0170 force-pushed the feature-cmake-design-update branch from f7bf625 to bd80d4d Compare July 21, 2020 12:45

hugueskamba reviewed Jul 21, 2020

View reviewed changes

hugueskamba force-pushed the feature-cmake branch from ade9959 to 03449e9 Compare July 21, 2020 16:59

0xc0170 force-pushed the feature-cmake-design-update branch 3 times, most recently from 4b43249 to 4c60dde Compare July 22, 2020 11:53

0xc0170 force-pushed the feature-cmake-design-update branch from c814ea8 to 44a89ad Compare July 23, 2020 09:17

Patater previously requested changes Jul 23, 2020

View reviewed changes

mergify bot added needs: work and removed needs: review labels Jul 23, 2020

0xc0170 force-pushed the feature-cmake-design-update branch from ae99878 to 4d4f4d3 Compare July 23, 2020 11:30

Patater previously requested changes Jul 23, 2020

View reviewed changes

docs/design-documents/tools/cmake.md Outdated Show resolved Hide resolved

Patater previously requested changes Jul 23, 2020

View reviewed changes

docs/design-documents/tools/cmake.md Outdated Show resolved Hide resolved

rwalton-arm previously requested changes Jul 23, 2020

View reviewed changes

Patater previously approved these changes Jul 23, 2020

View reviewed changes

CMake design doc: update to the latest mbed-tools updates

1f639f7

Refactor Mbed OS CMake scripts section - details about mbed-os/cmake scripts Add mbed-tools - how mbed-tools built an application with CMake

0xc0170 force-pushed the feature-cmake-design-update branch from 28bfd86 to 1f639f7 Compare July 23, 2020 14:07

Patater approved these changes Jul 23, 2020

View reviewed changes

rwalton-arm approved these changes Jul 23, 2020

View reviewed changes

0xc0170 added needs: CI ready for merge and removed needs: work needs: CI labels Jul 23, 2020

0xc0170 merged commit 157ebfb into ARMmbed:feature-cmake Jul 23, 2020

mergify bot removed the ready for merge label Jul 23, 2020

	To stay backward compatible, we create common rules as follows.
	For backward compatibility, the following rules must be respected:

		This file statically defines the structure of the component within Mbed OS. It contains conditional statements that are based on the config. We use regular CMake expressions, plus extend it with our own macros, to conditionally include/exclude directories. The rule of thumb: do not expose headers that are internal. We would like to avoid having everything in the include paths as we do now.


		`add_subdirectory` always adds the directory to the main CMake.


		## Application CMake

		We should provide application CMake functionality with our own configuration. There are couple of approaches we could take. Statically defined CMake but then this disconnectes config and CMake - as CMake contains configuration for a project (like includes, sources, etc). Our build tool would need to parse CMake to get all paths used in the project or Mbed OS to find out where to look for configuration file. Therefore the build system has a knowledge as it is currently. We use `requires` to include/exclude modules.


		`mbed-tools` has consolidated all of the required modules to build Mbed OS, along with the command line interface, into a single Python package which can be installed using standard python packaging tools.

		A user create own CMake file to configure an application, also with `mbed_app.json` configuration file. The building of an app would look like:

cmake: design doc update #13321

cmake: design doc update #13321

Uh oh!

Conversation

0xc0170 commented Jul 21, 2020

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

Uh oh!

rwalton-arm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

0xc0170 commented Jul 21, 2020

Uh oh!

Uh oh!

mergify bot commented Jul 21, 2020

Uh oh!

0xc0170 commented Jul 21, 2020

Uh oh!

Uh oh!

hugueskamba Jul 21, 2020 • 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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Jul 21, 2020

Uh oh!

0xc0170 commented Jul 22, 2020

Uh oh!

0xc0170 commented Jul 23, 2020

Uh oh!

0xc0170 commented Jul 23, 2020

Uh oh!

0xc0170 commented Jul 23, 2020

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rwalton-arm Jul 23, 2020 • 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!

0xc0170 Jul 23, 2020 • 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!

Uh oh!

hugueskamba Jul 21, 2020 •

edited

Loading

rwalton-arm Jul 23, 2020 •

edited

Loading

0xc0170 Jul 23, 2020 •

edited

Loading