Relocated libraries in features/frameworks #13430

LDong-Arm · 2020-08-14T11:01:17Z

Summary of changes

As per the directory restructure plan, we get rid of features/frameworks by relocating libraries inside.

This PR covers the following:

mbed-trace -> platform/mbed-trace and refactored
mbed-client-randlib -> platform/randlib (shortened) and refactored.
Note: The include path is still include/mbed-client-randlib/ because it's mirrored as a standalone repo and we want to avoid breaking changes. Is that still in use? @0xc0170

Greentea frameworks libraries will be covered separately.

Impact of changes

None

Migration actions required

None

Documentation

None

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

@ARMmbed/mbed-os-core @andypowers @jamesbeyond @rajkan01

ciarmcom · 2020-08-14T11:30:35Z

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

LDong-Arm · 2020-08-14T13:32:49Z

@0xc0170 Spell checks failed for mbed-trace and mbed-client-randlib. Should we fix them here and port them to their own repos? Or exclude them from the check?
Also their restructure (e.g. adding include/) need to be ported to their standalone repos.

rajkan01

LGTM

mergify · 2020-08-21T12:52:57Z

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

0xc0170 · 2020-08-21T13:50:05Z

I've merged couple of Prs, they caused a conflict here.

LDong-Arm · 2020-08-21T14:42:48Z

Rebased

LDong-Arm · 2020-08-21T16:53:49Z

Now Travis spell check passes

LDong-Arm · 2020-08-21T16:56:55Z

Travis Pytest failed with

$ arm-none-eabi-gcc --version
The program 'arm-none-eabi-gcc' is currently not installed. To run 'arm-none-eabi-gcc' please ask your administrator to install the package 'gcc-arm-none-eabi'
The command "arm-none-eabi-gcc --version" failed and exited with 127 during .

evedon · 2020-08-24T08:51:29Z

Travis Pytest failed with

$ arm-none-eabi-gcc --version
The program 'arm-none-eabi-gcc' is currently not installed. To run 'arm-none-eabi-gcc' please ask your administrator to install the package 'gcc-arm-none-eabi'
The command "arm-none-eabi-gcc --version" failed and exited with 127 during .

@0xc0170 Any idea what is the issue here? Re-start CI?

0Grit · 2020-08-24T09:11:34Z

Why is the cli client deprecated and removed?

If goal is to migrate away from the monolithic codebase I am okay with the removal BUT only if there is an useable NPM like package lookup and import tool in the works.

evedon · 2020-08-24T10:03:20Z

Why is the cli client deprecated and removed?

If goal is to migrate away from the monolithic codebase I am okay with the removal BUT only if there is an useable NPM like package lookup and import tool in the works.

I agree that the deprecation and removal of mbed-client-cli should be done in a different PR and discuss there.

mergify · 2020-08-25T07:47:55Z

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

evedon · 2020-08-25T16:27:53Z

PR is ready for CI

adbridge · 2020-08-25T16:32:12Z

CI started

mbed-ci · 2020-08-25T18:20:58Z

Jenkins CI Test : ❌ FAILED

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-ARM	❌
jenkins-ci/mbed-os-ci_build-GCC_ARM	❌

* Move mbed-client-randlib/ headers into include/ (Note: we don't rename it to "randlib" because this library is mirrored to https://github.com/ARMmbed/mbed-client-randlib, and "mbed-client-randlib" may be reference by some projects) * Move the standalone local unit test into tests/unit

The library "drivers" is a core one available to bare metal. It should not depend on mbed-trace which is an optional library.

LDong-Arm · 2020-09-02T15:07:59Z

drivers/source/SFDP.cpp

 #if (DEVICE_SPI || DEVICE_QSPI)

-#include "features/frameworks/mbed-trace/mbed-trace/mbed_trace.h"
+#if MBED_CONF_MBED_TRACE_ENABLE
+#include "mbed-trace/mbed_trace.h"
 #define TRACE_GROUP "SFDP"
+#else // MBED_CONF_MBED_TRACE_ENABLE
+#define tr_info(...)
+#define tr_error(...)
+#define tr_debug(...)
+#endif // MBED_CONF_MBED_TRACE_ENABLE


As something in drivers (a "core" library available for bare metal), mbed-trace shouldn't be a strict requirement. I made this change to fix bare metal build. @evedon @rajkan01

evedon · 2020-09-03T15:05:05Z

@adbridge PR ready for re-run of CI

adbridge · 2020-09-04T09:59:32Z

CI started

mbed-ci · 2020-09-04T10:43:01Z

Jenkins CI Test : ✔️ SUCCESS

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

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_build-ARM	✔️
jenkins-ci/mbed-os-ci_build-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_dynamic-memory-usage	✔️
jenkins-ci/mbed-os-ci_greentea-test	✔️
jenkins-ci/mbed-os-ci_cloud-client-pytest	✔️

0xc0170 · 2020-09-04T11:52:51Z

mbed-client-randlib -> platform/randlib (shortened) and refactored.
Note: The include path is still include/mbed-client-randlib/ because it's mirrored as a standalone repo and we want to avoid breaking changes. Is that still in use? @0xc0170

@artokin can you review this part? where is this one used and how? Just wondering if it fits platform folder?

artokin

What is the purpose of renamed include directories? Why they need to be changed?

artokin · 2020-09-04T12:38:06Z

UNITTESTS/CMakeLists.txt

@@ -114,6 +114,7 @@ set(unittest-includes-base
  "${PROJECT_SOURCE_DIR}/../features"
  "${PROJECT_SOURCE_DIR}/../platform/include"
  "${PROJECT_SOURCE_DIR}/../platform/include/platform"
+  "${PROJECT_SOURCE_DIR}/../platform/mbed-trace/include"


Why the mbed-trace directory is renamed as include? (in mbed-trace and randlib)

We still have mbed-trace/mbed_trace.h inside include/. The change is to do with include path namespacing in the new build system being developed.

LDong-Arm · 2020-09-04T12:49:45Z

What is the purpose of renamed include directories? Why they need to be changed?

We are implementing a new build system where only include of each library is in the include paths - not its subdirectories, not source directories, etc. This gives better encapsulation and namespaces compared to the current build system. We have done this for most other libraries already.

For example include/mbed-trace/mbed_trace.h translates to #include "mbed-trace/mbed_trace.h" to follow the <library name>/<header>.h rule. The none-namespaced #include "mbed_trace.h" will not be supported anymore in the near future.

LDong-Arm · 2020-09-04T13:56:54Z

Discussed with @artokin offline: Yes there are projects using the upstream repos mbed-trace and mbed-client-randlib. But at this stage our directory structure changes are non-breaking, because the current build system doesn't distinguish include paths. So alignment with the upstream is unlike to be problematic (or even urgent).

Hopefully it's all good?

LDong-Arm mentioned this pull request Aug 14, 2020

Move Greentea test libraries into TESTS/COMMON #13431

Closed

ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Aug 14, 2020

ciarmcom requested review from andypowers, jamesbeyond, rajkan01 and a team August 14, 2020 11:30

ciarmcom added the needs: review label Aug 14, 2020

mergify bot added needs: work and removed needs: review labels Aug 14, 2020

rajkan01 approved these changes Aug 14, 2020

View reviewed changes

LDong-Arm mentioned this pull request Aug 20, 2020

Support ignore list for spell checks #13469

Merged

LDong-Arm force-pushed the refactor_frameworks branch 2 times, most recently from bf64c15 to e05f2b2 Compare August 21, 2020 14:42

evedon force-pushed the refactor_frameworks branch from e06298f to 95071e5 Compare August 24, 2020 14:52

evedon force-pushed the refactor_frameworks branch from 95071e5 to b6810af Compare August 25, 2020 11:09

adbridge removed the needs: work label Aug 25, 2020

LDong-Arm added 6 commits September 2, 2020 12:30

Move mbed-trace into platform

a88438c

Add include/ path for mbed-trace

af1950f

Move mbed-client-randlib -> platform/randlib

9b1e0a3

.codecheckignore: ignore imported libraries randlib and mbed-trace

71eecfc

drivers/SFDP should compile without mbed-trace

183ca77

The library "drivers" is a core one available to bare metal. It should not depend on mbed-trace which is an optional library.

LDong-Arm force-pushed the refactor_frameworks branch from b6810af to 183ca77 Compare September 2, 2020 14:52

LDong-Arm commented Sep 2, 2020

View reviewed changes

evedon approved these changes Sep 3, 2020

View reviewed changes

mergify bot added needs: CI and removed needs: work labels Sep 3, 2020

mergify bot added ready for merge and removed needs: CI labels Sep 4, 2020

artokin reviewed Sep 4, 2020

View reviewed changes

artokin approved these changes Sep 4, 2020

View reviewed changes

0xc0170 approved these changes Sep 7, 2020

View reviewed changes

0xc0170 merged commit 41f2ee5 into ARMmbed:master Sep 7, 2020

mergify bot removed the ready for merge label Sep 7, 2020

LDong-Arm deleted the refactor_frameworks branch September 7, 2020 08:57

mbedmain added release-version: 6.3.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Sep 14, 2020

Relocated libraries in features/frameworks #13430

Relocated libraries in features/frameworks #13430

Uh oh!

Conversation

LDong-Arm commented Aug 14, 2020 • edited by evedon 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 Aug 14, 2020

Uh oh!

LDong-Arm commented Aug 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rajkan01 left a comment

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Aug 21, 2020

Uh oh!

0xc0170 commented Aug 21, 2020

Uh oh!

LDong-Arm commented Aug 21, 2020

Uh oh!

LDong-Arm commented Aug 21, 2020

Uh oh!

LDong-Arm commented Aug 21, 2020

Uh oh!

evedon commented Aug 24, 2020

Uh oh!

0Grit commented Aug 24, 2020

Uh oh!

evedon commented Aug 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mergify bot commented Aug 25, 2020

Uh oh!

evedon commented Aug 25, 2020

Uh oh!

adbridge commented Aug 25, 2020

Uh oh!

mbed-ci commented Aug 25, 2020

Jenkins CI Test : ❌ FAILED

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

Uh oh!

LDong-Arm Sep 2, 2020

Choose a reason for hiding this comment

Uh oh!

evedon commented Sep 3, 2020

Uh oh!

adbridge commented Sep 4, 2020

Uh oh!

mbed-ci commented Sep 4, 2020

Jenkins CI Test : ✔️ SUCCESS

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

Uh oh!

0xc0170 commented Sep 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

artokin left a comment

Choose a reason for hiding this comment

Uh oh!

artokin Sep 4, 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!

LDong-Arm Sep 4, 2020

Choose a reason for hiding this comment

Uh oh!

LDong-Arm commented Sep 4, 2020

Uh oh!

LDong-Arm commented Sep 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

LDong-Arm commented Aug 14, 2020 •

edited by evedon

Loading

LDong-Arm commented Aug 14, 2020 •

edited

Loading

evedon commented Aug 24, 2020 •

edited

Loading

0xc0170 commented Sep 4, 2020 •

edited

Loading

artokin Sep 4, 2020 •

edited

Loading

LDong-Arm commented Sep 4, 2020 •

edited

Loading