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

Extend CMake build #181

Merged
merged 46 commits into from
Feb 8, 2022
Merged

Extend CMake build #181

merged 46 commits into from
Feb 8, 2022

Conversation

Oxymoron79
Copy link
Collaborator

@Oxymoron79 Oxymoron79 commented Feb 2, 2022

Extend / refactor the CMake build to imitate the Makefile build:

  • Build dynamic and static libraries for client, server and client-server:
    • Create Object libraries to assemble the dynamic and static libraries from.
    • The sources from the port/* folders need to be built separately for client and server object libraries, due to a OC_CLIENT compilation switch in the source code.
    • Build patched mbedtls from source (as the Makefile does) instead of using the mbedtls CMake project. This made the creation of static and dynamic libraries easier.
  • Extend build configuration variables (CACHED)
  • Generate pkg-config files using the template files port/linux/*.pc.in
  • Generate CMake package containing all library variants {client,server,client-server}-{static,shared} as components in the iotivity-lite namespace. This provides a modern use of the library in CMake, e.g:
    find_package(iotivity-lite REQUIRED)
    if(USE_STATIC_LIB)
        target_link_libraries(${PROJECT_NAME} PRIVATE iotivity-lite::client-server-static)
    else(USE_STATIC_LIB)
        target_link_libraries(${PROJECT_NAME} PRIVATE iotivity-lite::client-server-shared)
    
  • Add installation steps with COMPONENTS to allow future packaging with CPack.
  • Build unit tests using the CTest module.
  • Build binaries from the tests directory with a custom check target that executes the tools/check.py script.
  • Build most example applications (can be disabled by setting the build variable BUILD_EXAMPLE_APPLICATIONS to OFF). The example applications are currently not installed, because the use a relative path for the storage directory that requires them to be executed in a specific directory, which a installation might change.
  • Build the python client library only as dynamic library (as in Makefile). It is also not installed (as in Makefile).

I have manually tested the CMake build on Ubuntu 18.04 with CMake 3.10.2.

TODO

  • The unittests apitest, platformtest and messagingtest fail if OC_IPV4 is not set. Is this also the case with the Makefile build?
    --> No, with the Makefile build the unittests pass. There was a compile definition missing for mbedtls, Fixed in 633c82a
  • The test client_get_linux_test hangs during execution. Is this also the case with the Makefile build?
    --> Yes, with the Makefile build the test hangs as well.
  • @Danielius1922 : Could the PR CMake: add support for clang-tidy static analysis #178 be integrated here?
  • Windows builds with MSVC and MinGW should be possible, but I haven't tested it yet. Help with that would be welcome!

* BUILD_SHARED_LIBS is a global cmake variable. If it is set to ON,
  add_library generates shared libraries if not explicitly set otherwise.

* If BUILD_SHARED_LIBS is ON, mbedtls needs to be built as shared library
  as well. This can be done by setting the USE_STATIC_MBEDTLS_LIBRARY
  and USE_SHARED_MBEDTLS_LIBRARY CMake option from mbedtls as cache variable.

* The iotivity-api interface library has to be linked pubicly so that
  its symbols are exported.
Setting the version causes a CMake Policy warning CMP0048 because
mbedtls does not set  the project version.
This makes its linking to iotivity-lite independent of the
BUILD_SHARED_LIBS variable (handles the PIC compiler flag for shared
libs build).
The library is statically linked into the dynamic library.
This allows to link mbedtls only when security is enabled.
These properties are propagated to the user of the library.
These BUILD_INTERFACE properties help with the compilation of the
onboarding tool and the sample apps.
deps/mbedtls.cmake Outdated Show resolved Hide resolved
@WAvdBeek
Copy link
Contributor

WAvdBeek commented Feb 3, 2022

can we do these changes in stages?
e.g. add first the tests
then add obt & java builds etc.
it is hard to follow all the changes in one go.

@Danielius1922
Copy link
Member

Good job, nice work!

@Oxymoron79
Copy link
Collaborator Author

can we do these changes in stages? e.g. add first the tests then add obt & java builds etc. it is hard to follow all the changes in one go.

Yes, the diff on the main CMakeLists.txt file is quite extensive. This is because it is basically a complete rewrite needed to compile static and dynamic libraries at the same time.

I tried to have a clean commit log to make it easier to understand.

  • Up to commit bc17c61 deals only with the compilation of the libraries and unit tests. IMO that would be the first stage for a pull request. But even here the diff on the main CMakeLists.txt file is already quite extensive. From here on, the commits are quite self-contained.
  • The commit 953ea7b adds the onboarding tool.
  • The commit 31b1474 extends the example applications.
  • The commit 1a9bcd1 adds the test applications
  • The commit 41157ed adds the python client dynamic library

The SWIG/Java build is not in this PR.

I can create separate PRs from these commits if you prefer, but I'm not sure that it reduces the diff on the main CMakeLists.txt file.

@Oxymoron79
Copy link
Collaborator Author

Good job, nice work!

Thanks, I'm glad you like it!

@Oxymoron79
Copy link
Collaborator Author

Oxymoron79 commented Feb 4, 2022

During testing of the library built with MSVC I came across a issue when trying to link against the DLL: Since the library doesn't export any symbols, it is missing the import library that is required by the MSVC toolchain to link against a DLL. It's the same issue as reported in issue #182.

It works fine though when the library is built with the MinGW Toolchain.

So until the symbols of the library are exported, I would suggest to disable the build of a DLL with the MSVC toolchain. Would you be OK with that for now?

@Danielius1922
Copy link
Member

Danielius1922 commented Feb 4, 2022

It works fine though when the library is built with the MinGW Toolchain.

That's probably down to the fact that it behaves as GCC or clang on Unix - by default all symbols have "default" visibility, which means that all are exported. However, usually exporting all symbols in shared lib is not something you want, so the general praxis on Unix is to change the default visibility to "hidden" and only mark the public API functions with a macro to be exported. I think this could be done when solving issue #182, so both Unix and Windows export similar set of functions.

@Oxymoron79 Oxymoron79 marked this pull request as ready for review February 7, 2022 08:39
@WAvdBeek WAvdBeek merged commit 8bbbdcc into master Feb 8, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Feb 8, 2022
@Oxymoron79 Oxymoron79 deleted the Oxymoron79/feature/extend-cmake branch February 8, 2022 10:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants