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

Add cmake build system #71

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from

Conversation

Bjoe
Copy link

@Bjoe Bjoe commented Oct 30, 2017

First POC commit for issue #53

Some files are copied from Poco project.

The interesting part are in macchina.io/platform/OSP. I add for following bundles a CMakeLists.txt:

  • Core
  • Net
  • Web
  • ServiceListener sample

At the moment its only building some bundles. Launcher/Server part is missing yet.

@CLAassistant
Copy link

CLAassistant commented Oct 30, 2017

CLA assistant check
All committers have signed the CLA.

@lilianmoraru
Copy link

I did a fast skim through and besides a few nitpicks, this looks Really Good 😃

#############################################################
# Uninstall stuff see: http://www.vtk.org/Wiki/CMake_FAQ
configure_file(
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/cmake_uninstall.cmake.in"

Choose a reason for hiding this comment

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

Just want to point out that this file(cmake_uninstall.cmake.in) does not exist

Copy link
Author

Choose a reason for hiding this comment

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

Yep, sorry. Forgot to commit :-(. It's fixed.

@obiltschnig
Copy link
Member

Hi @Bjoe, could you please sign the CLA and then re-submit the pull request for the feature/cmake branch? We'll then merge to develop when its done.

Thanks!

@Bjoe
Copy link
Author

Bjoe commented Nov 4, 2017

@obiltschnig Ok, I sign the CLA.

Copy link

@lilianmoraru lilianmoraru left a comment

Choose a reason for hiding this comment

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

cmake/ is a duplicate of platform/cmake/.

My guess would be that it would be better for maintainability to use one of these and the other to be removed.

CMakeLists.txt Outdated
@@ -0,0 +1,88 @@
cmake_minimum_required(VERSION 3.6)

Choose a reason for hiding this comment

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

Are you sure this needs CMake 3.6?
I tried to search for some of the CMake 3.6 specific features and did not find any(of course, it might be related to behavior, which I cannot catch with a simple search).

Usually CMake 3.1 is enough for most use-cases(CMAKE_CXX_STANDARD does something only starting with CMake 3.1).

Ubuntu 14.04 LTS has CMake 3.2 and Ubuntu 16.04 LTS has CMake 3.5.
Travis CI of course uses the oldest LTS available(library compat reasons - example: glibc) and that means CMake 3.2.
So, most Travis CI users have CMake 3.2.

Could we get this to at least 3.5(preferably 3.2 of course)?

Choose a reason for hiding this comment

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

Well, I looked again and this file would work with CMake 2.8(just set, string, option, older CMake variables, etc...), but since the platform sets cmake_minimum_required to 3.2.0, this is the actual minimum.

Could you please change the minimum to 3.2(Travis CI compat)?

Copy link
Author

Choose a reason for hiding this comment

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

I downgraded to 3.2 but it should not be a problem at all to use a new version. Download and extract the newest Linux binary somewhere from cmake.org (It doesn't matter if you have another cmake version installed) and use it... it works out of the box on every Linux/Unix platform (even on Windows and Mac I think). That's a big feature of cmake ... no dependency and platform requirements.

@lilianmoraru
Copy link

A general point not specifically to be addressed by this issue but still relevant:

If the Poco code is not modified(compared to upstream), than switching to CMake, the copy of the code could be removed and let CMake make sure that the appropriate version is pulled in or updated.

Possible solutions:

  1. There is the ExternalProject feature.
  2. And my personal favorite is to add the dependency as a submodule and check at configure step for "submodule-name/.git", if it does not exist than I do: git submodule update --init(the builds of the submodule need to be out-of-source, so the update is seamless in case the reference was updated to a different version of the dependency).
    This makes sure that everything works, even if the user didn't clone with --recursive.

Both solutions with their advantages and disadvantages of course(ex: reliability vs perf)...

@Bjoe
Copy link
Author

Bjoe commented Jan 8, 2018

ExternalProject or git Submodule ... that's exactly what I think at the beginning, but unfortunately I did not come up with a correct solution.

From the build process perspective, Poco is a third party library for macchina, because Poco has its own cmake build.
We should not mixed both project cmake builds togehther because they can influence each other. This means, ExternalProject is the right solution.
The biggest disadvantage is, find_package(PocoXXX ) in macchina project will not proper work anymore!
There are some solutions for this. I will figure out the solutions and try to create a test for you so that we can decide what we will use.

Btw. You can launch now macchina. After the build, execute, under Linux, bin/Linux/x86_64/macchina .... At the moment, all "binaries" are installed in the current build directory. If it is important that macchina is installed under the "server" folder, like the make based build, its also possible.

@Bjoe
Copy link
Author

Bjoe commented Sep 26, 2018

Hi @ALL. Here a new proposal/POC version of macchina.io. In the last year, I try to solve the "Poco third-party" problem. I tried with conan etc. but a few months ago I found Hunter. I try Hunter and I get some new ideas. It should be possible to build macchina/OSGi bundles without to "checkout" macchina project. Like a macchina SDK.
First, I solved the problem with the poco third-party dependency. It is now very easy to update to newest poco version. Secound, I create a "macchina SDK" to build bundles indepentely. See here for example in my github repo: Bjoe/macchina-example-bundle
To build the bundle:

$ git clone https://github.com/Bjoe/macchina-example-bundle.git
$ cmake -Hmacchina-example-bundle -Bbuild-macchina-bundle
$ cmake --build build-macchina-bundle

This will build a build-macchina-bundle/bundles/com.appinf.osp.samples.servicelistener_1.0.1.bndl bundle and it can be installed into macchina.io.

The first time, it take a while, because the third-party libraries (Poco and macchina) will be build with Hunter. After this libraries are build, it will not build again. You can change something in the bundle, and it will build only the bundle stuff, quickly.

@obiltschnig
Copy link
Member

Thanks! I have also done some CMake work for OSP with a customer, so we'll probably have to merge the two approaches. Will look at it hopefully next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants