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.

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

CMake: Replace MBED_TEST_LINK_LIBRARIES with MBED_TEST_BAREMETAL #14874

Merged
merged 2 commits into from
Jul 6, 2021

Conversation

LDong-Arm
Copy link
Contributor

Summary of changes

Fixes #14871

Currently we have MBED_TEST_LINK_LIBRARIES for specifying

  • Whether to link mbed-os or mbed-baremetal
  • Any additional libraries we want tests to link

It's not fit for purpose anymore, because

  • No flavor of Mbed OS is selected by default, but we should've really defaulted to mbed-os, the full RTOS version. Build doesn't work unless -DMBED_TEST_LINK_LIBRARIES=<...> is passed, which is redundant.
  • A test should never need additional libraries passed via command line - its CMakeLists.txt should specify what it link.

This PR replaces MBED_TEST_LINK_LIBRARIES with a new option MBED_TEST_BAREMETAL to build a test with either RTOS (default) or without it (by passing -DMBED_TEST_BAREMETAL=ON).

Impact of changes

Now MBED_TEST_LINK_LIBRARIES is not needed/does not work anymore. MBED_TEST_BAREMETAL selects the bare metal profile for a Greentea test.

Migration actions required

No change is needed for existing tests. Pass the new CMake option when needed, as instructed by the updated tools/cmake/README.md.

Documentation

How to build a greentea test in tools/cmake/README.md has been updated.


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)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[x] Tests / results supplied as part of this PR

CMake support for Greentea tests is not covered in CI yet. Manual testing:

  • platform/tests/TESTS/mbed_platform/system_reset: full profile and bare metal profile
  • rtos/tests/TESTS/mbed_rtos/basic/: as expected, full profile builds and bare metal profile (-DMBED_TEST_BAREMETAL=ON) doesn't

Reviewers

@ARMmbed/mbed-os-core


@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Jul 5, 2021
@ciarmcom ciarmcom requested a review from a team July 5, 2021 17:00
@ciarmcom
Copy link
Member

ciarmcom commented Jul 5, 2021

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

Patater
Patater previously approved these changes Jul 6, 2021
@mergify
Copy link

mergify bot commented Jul 6, 2021

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

0xc0170
0xc0170 previously approved these changes Jul 6, 2021
@0xc0170
Copy link
Contributor

0xc0170 commented Jul 6, 2021

I've merged MBED_PATH removal PR, rebase required and CI will be the next.

The coding standards of Mbed OS require indentation of four spaces.
@LDong-Arm LDong-Arm force-pushed the optional_MBED_TEST_LINK_LIBRARIES branch from ed23c1d to b8dc717 Compare July 6, 2021 12:40
@LDong-Arm
Copy link
Contributor Author

@Patater @0xc0170 Rebased, ready for CI?

Currently we have `MBED_TEST_LINK_LIBRARIES` for specifying
* Whether to link `mbed-os` or `mbed-baremetal`
* Any additional libraries we want tests to link

It's not fit for purpose anymore, because
* No flavor of Mbed OS is selected by default, but we should've
really defaulted to `mbed-os`, the full RTOS version. Build doesn't
work unless `-DMBED_TEST_LINK_LIBRARIES=<...>` is passed, which
is redundant.
* A test should never need additional libraries passed via command
line - its `CMakeLists.txt` should specify what it links.

This commit replaces `MBED_TEST_LINK_LIBRARIES` with a new option
`MBED_TEST_BAREMETAL` to build a test with either RTOS (default)
or without it (by passing `-DMBED_TEST_BAREMETAL=ON`).
@mergify mergify bot dismissed stale reviews from Patater and 0xc0170 July 6, 2021 12:43

Pull request has been modified.

@LDong-Arm LDong-Arm force-pushed the optional_MBED_TEST_LINK_LIBRARIES branch from b8dc717 to f4d551e Compare July 6, 2021 12:43
@LDong-Arm LDong-Arm requested review from Patater and 0xc0170 July 6, 2021 12:43
@0xc0170
Copy link
Contributor

0xc0170 commented Jul 6, 2021

CI started

@mbed-ci
Copy link

mbed-ci commented Jul 6, 2021

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_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-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 ✔️

@Patater Patater merged commit 9ec92f1 into ARMmbed:master Jul 6, 2021
@mbedmain mbedmain added release-version: 6.13.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch labels Jul 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace MBED_TEST_LINK_LIBRARIES
6 participants