Move mbed-stubs-platform to the platform directory #14773
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| # Copyright (c) 2021 ARM Limited. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| add_subdirectory(doubles) | ||
| add_subdirectory(ATCmdParser) | ||
| add_subdirectory(CircularBuffer) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # Copyright (c) 2021 ARM Limited. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| add_library(mbed-headers-platform INTERFACE) | ||
| target_include_directories(mbed-headers-platform | ||
| INTERFACE | ||
| ${mbed-os_SOURCE_DIR}/platform/include | ||
| ${mbed-os_SOURCE_DIR}/platform/include/platform | ||
| ${mbed-os_SOURCE_DIR}/platform/randlib/include/mbed-client-randlib/ | ||
| ${mbed-os_SOURCE_DIR}/platform/randlib/include/ | ||
| ${mbed-os_SOURCE_DIR}/platform/mbed-trace/include | ||
| ) | ||
|
|
||
| add_library(mbed-stubs-platform) | ||
|
There was a problem hiding this comment. stubs or doubles (the folder is named doubles but we talk or used stubs). Was there a reason to name the folder differently? There was a problem hiding this comment. I thought doubles was more "correct" as some of these are stubs, some are a bit more like fakes. I can revert the folder name back to stubs if we think that's better/less confusing. |
||
| target_sources(mbed-stubs-platform | ||
| PRIVATE | ||
| mbed_critical_stub.c | ||
| mbed_atomic_stub.c | ||
| mbed_error.c | ||
| mbed_poll_stub.cpp | ||
| mbed_assert_stub.cpp | ||
| mbed_wait_api_stub.cpp | ||
| mbed_retarget_stub.cpp | ||
| FileHandle_stub.cpp | ||
| nvic_wrapper_stub.c | ||
| randLIB_stub.c | ||
| randLIB_stub.cpp | ||
| ) | ||
| target_include_directories(mbed-stubs-platform | ||
| PUBLIC | ||
|
There was a problem hiding this comment. Adding all the platform stub headers into mbed-stubs-platform with PUBLIC again, this will be a problem if library unittest (example ATCmdParser unittest) don't want to include stub platform headers but target link aginst mbed-stubs-platform brought those the platform stub headers. Initially, that was one of the reasons we added a mbed-stubs-headers library, it is better to add a new library mbed-stubs-platform-headers INTERFACE library with all platform stub headers refer our discussion #14285 (comment) There was a problem hiding this comment. I'm not sure I'm seeing the issue with the example use case you've given. Why wouldn't you want the headers to be available if you specifically linked to the stub library? I could see the inverse of this being a potential problem: you want the "stub header" but want to provide another implementation yourself. However, if a stub has some "public headers" (every header in this Maybe I'm missing something, so if you could show me an example of where this is actually an issue it would help me understand why we need to do this. It doesn't seem to cause a problem with the There was a problem hiding this comment. In the original discussion @rajkan01 linked, @paul-szczepanek-arm said
My interpretation of the concern is, if stub libraries A and B both have PUBLIC headers, and B depends on A, and a test T directly depends on B, then A will be "inadvertently" be exposed to T, is this the concern? But in order for compilation to succeed (because of how preprocessors work), this exposure is necessary IMO - otherwise how can T Another thing is, the stub headers are for unit tests only which work within mbed-os, not intended for general use by other repositories, so flexibility isn't a concern and we should prioritise simplicity in my opinion. We are not messing with any "actual" Mbed OS libraries. (Sorry if I completely misunderstood the original concern!) |
||
| . | ||
| ) | ||
| target_link_libraries(mbed-stubs-platform | ||
| PUBLIC | ||
|
There was a problem hiding this comment. This PR approach is good and minimal-impact but Initially, we have a discussion to use PRIVATE instead of stubs libraries target link libraries with PUBLIC refer #14285 (comment) & #14285 (comment), please share your thoughts |
||
| mbed-headers-base | ||
| mbed-headers-hal | ||
| mbed-headers-platform | ||
| ) | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Referring to the lines below)
Shall we remove mbed-stubs-platform from mbed-stubs, and let tests explicitly link mbed-stubs-platform? Similar to what's been done to the headers - only link what is used.
Once all component stubs have been refactors, it'd be nice to have no global mbed-stubs but only component stubs eventually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recall some build failures when I tried that initially. I think we can only remove this when everything else depending on platform has been refactored. I'll take another look and see how tricky it is to untangle at this stage, now that we've merged some other stubs PRs.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if we take it out from mbed-stubs, lots of tests need to have mbed-stubs-platform explicitly added - this would be tedious. I imagine platform is the trickiest given everything depends on it, and it might be true for hal too.
Personally I'm okay with leaving it here for now.