Add cmake build system #290

Merged
bvbfan merged 6 commits from cmake_build into master 2019-07-19 20:22:00 +02:00
bvbfan commented 2019-07-02 14:26:14 +02:00 (Migrated from github.com)

Signed-off-by: Anthony Fieroni bvbfan@abv.bg

Signed-off-by: Anthony Fieroni <bvbfan@abv.bg>
bvbfan commented 2019-07-02 14:39:11 +02:00 (Migrated from github.com)

We can use cmake as reproducible build system, as well as rolling one. By default it's done to perform a monolith (static) build of all depends, on other hand it can be used to perform dynamic build against system libraries. The flexibility is that when depend is not found in system libs it's compiled as static one, so we have exactly what you need. On the other hand i have an idea to manage a branch that includes diffs by file against concrete bitcoin version so you can reproduce exactly the version you want. Since that is not very interesting in previous version, it'll be pretty much to newer one, since you can easy try to apply all patches on top of newer bitcoin version to generate a lbrycrd one. When can tag changes in diffs like tags in bitcoin which will make process easier, despite that menage and review diffs is not that easy task, it should be done by manually testing but furthermore i think this will make future rebases painless.

We can use cmake as reproducible build system, as well as rolling one. By default it's done to perform a monolith (static) build of all depends, on other hand it can be used to perform dynamic build against system libraries. The flexibility is that when depend is not found in system libs it's compiled as static one, so we have exactly what you need. On the other hand i have an idea to manage a branch that includes diffs by file against concrete bitcoin version so you can reproduce exactly the version you want. Since that is not very interesting in previous version, it'll be pretty much to newer one, since you can easy try to apply all patches on top of newer bitcoin version to generate a lbrycrd one. When can tag changes in diffs like tags in bitcoin which will make process easier, despite that menage and review diffs is not that easy task, it should be done by manually testing but furthermore i think this will make future rebases painless.
BrannonKing (Migrated from github.com) requested changes 2019-07-09 18:59:42 +02:00
BrannonKing (Migrated from github.com) left a comment

I did some testing on this today. Here are issues that I see with it:

The boost build doesn't correctly locate the ICU libs. This is a must-have. "Performing configuration checks" must report that it found the ICU. I played with this for a little while in trying to make it more like the boost.mk that we use presently. However, I was unable to make it work.
Do we really need a separate b2 call for headers?
The Boost and ICU and other builds don't account for the RELEASE vs DEBUG build flags in CMake.
Use Boost 1.69 and ICU 63.2.
It was unable to locate my local boost libraries.
If use a local boost library, we don't need to use ICU.
We don't need program_options boost component, and the test component is named "test".
I think lbrycrd's --enable-cxx is deprecated.
It would be nice to use cmake vars instead of using "x86_64", etc. directly.

I did some testing on this today. Here are issues that I see with it: The boost build doesn't correctly locate the ICU libs. This is a must-have. "Performing configuration checks" must report that it found the ICU. I played with this for a little while in trying to make it more like the boost.mk that we use presently. However, I was unable to make it work. Do we really need a separate b2 call for headers? The Boost and ICU and other builds don't account for the RELEASE vs DEBUG build flags in CMake. Use Boost 1.69 and ICU 63.2. It was unable to locate my local boost libraries. If use a local boost library, we don't need to use ICU. We don't need program_options boost component, and the test component is named "test". I think lbrycrd's --enable-cxx is deprecated. It would be nice to use cmake vars instead of using "x86_64", etc. directly.
bvbfan commented 2019-07-09 19:24:23 +02:00 (Migrated from github.com)

Do we really need a separate b2 call for headers?

Yes.

If use a local boost library, we don't need to use ICU.

I don't get it. We need ICU when we build boost locally, we don't need if we use system wide one (because it's prebuild with icu in).

and the test component is named "test"

Component is called unit_test_framework, i used in local build too.

It would be nice to use cmake vars instead of using "x86_64", etc. directly.

Good catch, i'll correct it.

Can you show the output, i've test it in all variants, boost locate icu in my test cases.

> Do we really need a separate b2 call for headers? Yes. > If use a local boost library, we don't need to use ICU. I don't get it. We need ICU when we build boost locally, we don't need if we use system wide one (because it's prebuild with icu in). > and the test component is named "test" Component is called unit_test_framework, i used in local build too. > It would be nice to use cmake vars instead of using "x86_64", etc. directly. Good catch, i'll correct it. Can you show the output, i've test it in all variants, boost locate icu in my test cases.
BrannonKing commented 2019-07-16 20:20:35 +02:00 (Migrated from github.com)

I tested this with Clion today. It doesn't work. However, I have found a new and better way to make Clion work with the autotools stuff. Essentially, you use compiledb to build the compile_commands.json file from the make file. This works better than that older CMakeLists.txt file I've been using. With that in mind, I'm thinking to get rid of the CMakeLists.txt file. This newer CMake approach allows a mix of local and downloaded dependencies. That is an improvement over the existing build system, where it's either all or nothing on local dependencies (or it's some custom build work). At the same time, I don't want to confuse our users by having multiple ways to build the project. Another advantage of CMake is that it keeps the compiled files separated from the sources. However, the present approach to using Externals doesn't quite achieve this goal.

I tested this with Clion today. It doesn't work. However, I have found a new and better way to make Clion work with the autotools stuff. Essentially, you use compiledb to build the compile_commands.json file from the make file. This works better than that older CMakeLists.txt file I've been using. With that in mind, I'm thinking to get rid of the CMakeLists.txt file. This newer CMake approach allows a mix of local and downloaded dependencies. That is an improvement over the existing build system, where it's either all or nothing on local dependencies (or it's some custom build work). At the same time, I don't want to confuse our users by having multiple ways to build the project. Another advantage of CMake is that it keeps the compiled files separated from the sources. However, the present approach to using Externals doesn't quite achieve this goal.
BrannonKing (Migrated from github.com) reviewed 2019-07-16 20:21:57 +02:00
BrannonKing (Migrated from github.com) commented 2019-07-16 20:21:57 +02:00

Would it be better to default this to ON? I kind of like the idea of using local dependencies by default.

Would it be better to default this to ON? I kind of like the idea of using local dependencies by default.
bvbfan commented 2019-07-16 20:22:07 +02:00 (Migrated from github.com)

What exactly fails?

What exactly fails?
BrannonKing commented 2019-07-16 21:58:36 +02:00 (Migrated from github.com)

Clion must be using the files passed to the add_executable/library Cmake commands. Those aren't called as part of the ExternalProject stuff. Like I said, though, this other approach with compiledb actually works much better than what I had.

Clion must be using the files passed to the add_executable/library Cmake commands. Those aren't called as part of the ExternalProject stuff. Like I said, though, this other approach with compiledb actually works much better than what I had.
BrannonKing (Migrated from github.com) reviewed 2019-07-17 00:08:35 +02:00
BrannonKing (Migrated from github.com) commented 2019-07-17 00:08:35 +02:00

The "depends" system uses sed instead of patch to achieve this. It doesn't need the extra file. The sed approach seems superior to me.

The "depends" system uses `sed` instead of `patch` to achieve this. It doesn't need the extra file. The `sed` approach seems superior to me.
bvbfan commented 2019-07-17 08:01:25 +02:00 (Migrated from github.com)

Clion, or any other IDE, should call cmake 'dir' then make, it shouldn't care it's external project or not. IMHO you can add custom command at end to satisfy Clion needs.

Clion, or any other IDE, should call cmake 'dir' then make, it shouldn't care it's external project or not. IMHO you can add custom command at end to satisfy Clion needs.
bvbfan commented 2019-07-17 09:53:57 +02:00 (Migrated from github.com)

I just test with KDevelop, it works, open project, import CMakeFiles.txt and then run build.

I just test with KDevelop, it works, open project, import CMakeFiles.txt and then run build.
bvbfan (Migrated from github.com) reviewed 2019-07-19 19:48:09 +02:00
bvbfan (Migrated from github.com) left a comment

Boost version should be increase as well, not tag only.

Boost version should be increase as well, not tag only.
BrannonKing commented 2019-07-19 20:07:38 +02:00 (Migrated from github.com)

No, I did the version number very specifically: I want to download the 69 if I'm going to download one. If I'm not going to download one, then version 64+ will be fine.

No, I did the version number very specifically: I want to download the 69 if I'm going to download one. If I'm not going to download one, then version 64+ will be fine.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lbrycrd#290
No description provided.