8000 Add CMakeLists.txt for Mbed CLI v2 by Oxore · Pull Request #35 · ARMmbed/mbed-mqtt · GitHub
[go: up one dir, main page]

Skip to content

Add CMakeLists.txt for Mbed CLI v2 #35

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension
8000
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add license header
  • Loading branch information
Oxore committed May 18, 2021
commit 0758348675c8cf945161bdc3b0858b284bfb5843
3 changes: 3 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# Copyright (c) 2021 ARM Limited. All rights reserved.
# SPDX-License-Identifier: Apache-2.0

add_library(mbed-mqtt INTERFACE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a license header

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be an interface? I suspect STATIC would work for this library (no weak linking symbols) ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this license header be sufficient? No names, just copyright, and SPDX identifier.

# Copyright (c) 2021 ARM Limited. All rights reserved.
# SPDX-License-Identifier: Apache-2.0

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be an interface? I suspect STATIC would work for this library (no weak linking symbols) ?

I think you are right. Give me a couple more days and I will figure it out how to make it compile being STATIC. I'm not really competent neither with cmake nor with mbed-os' CMakeLists.txt files organization.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed my mind. Here is the reason why I think this library should be declared as INTERFACE:

  1. When it is declared as STATIC, it anyways turns out to be built alongside with mbed-os in any project. Therefore it uses all the same macros passed to the compiler on invocation. Consider an example when I have mbedtls-config-changes.h file in the root of my project and I pass it as "MBEDTLS_USER_CONFIG_FILE=\"mbedtls-config-changes.h\"" in mbed_app.json. This MBEDTLS_USER_CONFIG_FILE macro is passed to mbed-mqtt sources too when it is being built and this config header file cannot be found, because mbed-mqtt is being built separately as a static library and it is not aware of my project's root directory where the file is located. To mitigate this error I have to declare something like this in my project's root CMakeLists.txt: target_include_directories(mbed-mqtt PRIVATE .). When mbed-mqtt declared as INTERFACE there is no such problems.

  2. When it is declared as STATIC, all mbed-mqtt dependencies are built separately from the project and number of files to be compiled becomes almost twice as many than if it was declared as INTERFACE - 868 files when INTERFACE vs 1717 when STATIC in my case. Basically the whole OS is being built twice. This is inconvenient.

  3. I have no evidence of any library for mbed-os declared as STATIC - they all are declared as INTERFACE. Why mbed-mqtt should not be declared as INTERFACE?


target_include_directories(mbed-mqtt
Expand Down
0