CMake cleanup & MinGW support #121

crypto-ape · 2019-04-08T10:20:45Z

Hey Monkeys!

I have cleaned up the CMake build a little bit:

removed unused and irrelevant code
- editline, old and not supported Boost library handling, OpenSSL config search, ...
the custom FindBoost.cmake is evil
- I have put it into a separate directory and it is loaded only on Apple ecosystem
MinGW support ( supports building Windows binaries on Linux )
renamed _mm_crc32_u64 to _mm_crc32_u64_impl
- caused Internal compiler errors with MSVC because the symbol is already defined as a compiler intrinsic

Hopefully this helps.

Related: bitshares/bitshares-core#1706

Ape out!

CMakeLists.txt

jmjatlanta · 2019-04-08T13:08:46Z

* the custom `FindBoost.cmake` is evil
  
  * I have put it into a separate directory and it is loaded only on Apple ecosystem

Do you know why the FindBoost.cmake is there? It would be nice to get rid of it totally. I do not know the reason for its existence, and therefore am hesitant to remove it.

Another note: Your comment says you moved it to another directory, but it seems as if it is gone completely. Perhaps you need to include that directory in a commit?

CMakeLists.txt

crypto-ape · 2019-04-09T08:05:34Z

* the custom `FindBoost.cmake` is evil
  
  * I have put it into a separate directory and it is loaded only on Apple ecosystem
Do you know why the FindBoost.cmake is there? It would be nice to get rid of it totally. I do not know the reason for its existence, and therefore am hesitant to remove it.

Another note: Your comment says you moved it to another directory, but it seems as if it is gone completely. Perhaps you need to include that directory in a commit?

I would also like to remove the custom FindBoost.cmake, which overrides the find_package(BOOST...) routines.

What is does, is that it overrides the functionality that is bundled with your current version of CMake, effectively using Boost searching routines from an older CMake version regardless of your current version.

It brings only trouble when used on Windows and with newer Boost versions.

Why is it there ?
My guess is, that somebody wanted to fix a problem in the search routine, copied the most up-to-date version of FindBoost.cmake, patched it and included it in the project to override the default behaviour.

I have looked into it, and probably found the FindBoost.cmake that was the original file.

For convinience a comparison between the original file from CMake and our file in bitshares/fc is here:
crypto-ape/find-boost-compare@4075201

The interesting changes are:

extending the list of default Boost search paths
adding pthread as a library in some cases

So,

This looks like a correct usage of BOOST_ROOT and/or building an up-to-date Boost from source would fix.
This is more interesting.
It might be related to the error that broke the Travis CI task ( compilation failed with linker undefined reference to symbol 'pthread_setname_np@@GLIBC_2.12').
Importantly, the whole project was built successfully, making me think that the pthread
dependency is defined in some other way in the main project.

Yes @jmjatlanta, it seems like I have forgotten to include the commit with the FindBoost.cmake moved to Legacy folder.

Additionally, I will rework the CMake script to include the standalone FindPython scripts only for relevant CMake versions ( < 3.10 ).

pmconrad · 2019-04-17T14:00:19Z

It seems the travis build needs libpthread...

pmconrad · 2019-04-23T14:00:44Z

We've discussed this within the Core Team and agreed to bump CMake minimum to 3.2.

CMakeModules/Boost/BoostConfig.cmake

pmconrad · 2019-04-23T14:21:24Z

Can we get rid of FindBoost completely? If it's only required on APPLE and can be worked around by setting BOOST_LIBRARYDIR we should remove it and update the documentation. @jmjatlanta what do you think?

crypto-ape · 2019-04-23T15:10:11Z

Thanks @pmconrad for the review.

I have just compiled the whole project under Windows.

I do not have a Mac which I can experiment on, but you can see from the diff crypto-ape/find-boost-compare@4075201 that the custom FindBoost is not doing anything special compared to the default FindBoost.

Just to be sure, someone with a Mac should verify the compilation with default system libraries and BOOST_LIBRARYDIR set, before removing the legacy script.

jmjatlanta · 2019-04-23T21:29:43Z

I have a mac. I will test to figure out what is needed for getting rid of FindBoost.cmake.

I believe we always need pthread on platforms that have it. Is there a reason to not simply add it to our CMakeLists.txt file?

crypto-ape · 2019-04-24T07:27:11Z

I have a mac. I will test to figure out what is needed for getting rid of FindBoost.cmake.

Please try compile it with the custom FindBoost.cmake file deleted, so CMake would use the bundled version.
If it won't compile automatically, try setting the BOOST_LIBRARYDIR.

I believe we always need pthread on platforms that have it. Is there a reason to not simply add it to our CMakeLists.txt file?

I would not force this. If Boost requires pthread, let him ask for it.

pmconrad · 2019-04-24T13:24:10Z

Please rebase on master to get rid of the conflict

crypto-ape · 2019-04-25T14:21:10Z

Rebased & tested.

Compiles with MinGW on Linux and Windows hosts.

CMakeModules/Boost/BoostConfig.cmake

CMakeLists.txt

pmconrad · 2019-05-18T08:00:15Z

CMakeModules/Legacy/CMakeLists.txt

+#  - this is here to fix some older CMake + Boost configurations
+set(Boost_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../Boost" PARENT_SCOPE)
+
+if (APPLE OR USE_LEGACY_FIND_BOOST)


Is this still needed on apple or can we remote the old FindBoost? @jmjatlanta

pmconrad · 2019-06-06T16:56:38Z

CMakeLists.txt

@@ -22,6 +22,8 @@ INCLUDE(GetGitRevisionDescription)
 INCLUDE(CheckLibraryExists)
 INCLUDE(CheckLibcxxAtomic)


I'm experimenting with cross-compiling core using gitian. It seems that the CheckLibcxxAtomic include must be moved further down, after FIND_PACKAGE(Boost....

Have you tried compiling it recently, with current codebase ?

I successfully compiled the the main project's develop branch with fc being this branch, using Gitian through Docker, without the need to move CheckLibcxxAtomic.

Hm, yes, seems to be working now.
I wonder why - CheckLibcxxAtomic compiles a test program that uses boost::lockfree - how can that work before boost has been detected?

Looks like you were right.

I have moved CheckLibcxxAtomic after find_package(Boost...), because in some cases it did not work, specifically when working with MinGW compiler.

Still, I can't explain the behaviour. I thought that CMake might do some multi-pass magic, but it has probably something to do with default compiler paths and also with the fact that the main project runs find_package(Boost...) before CheckLibcxxAtomic.

pmconrad · 2019-06-08T15:20:57Z

@crypto-ape please check https://github.com/pmconrad/bitshares-gitian/pull/6 and the cross-build instructions contained therein.

pmconrad · 2019-06-08T16:06:16Z

More precisely, I'm unsure which of these hacks are generally applicable and which ones are only due to the way I'm building this and the 3rd party libs.
https://github.com/pmconrad/bitshares-gitian/blob/d9e7f2fac9d6bc9da25abcd5f25d55f909435df0/descriptors/bitshares-core-win.yml#L113-L117

crypto-ape · 2019-06-11T13:37:50Z

@pmconrad, I looked into this

pmconrad · 2019-07-24T17:05:58Z

Checked both my native Linux build and Gitian cross-build for windows with latest develop plus merging this.
Gitian build produces errors that are unrelated to this PR (apparently the new hardening compiler options are not checked properly).
Linux build works but seems to be more sensitive to the cmake-boost version interdependency.
I think we should merge this and hopefully get rid of the included FindBoost later.

Thanks @crypto-ape !

crypto-ape mentioned this pull request Apr 8, 2019

CMake cleanup & MinGW support bitshares/bitshares-core#1706

Merged

pmconrad reviewed Apr 8, 2019

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

pmconrad reviewed Apr 8, 2019

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

jmjatlanta reviewed Apr 8, 2019

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

pmconrad mentioned this pull request Apr 9, 2019

1584 simplifications #120

Merged

crypto-ape force-pushed the master_mingw branch from f7d1b73 to ebc9340 Compare April 17, 2019 12:15

pmconrad reviewed Apr 23, 2019

View reviewed changes

CMakeModules/Boost/BoostConfig.cmake Outdated Show resolved Hide resolved

crypto-ape mentioned this pull request Apr 24, 2019

fatal error: sys/ioctl.h: No such file or directory when building on Mingw64 bitshares/bitshares-core#1724

Closed

crypto-ape force-pushed the master_mingw branch from 24ea060 to f375d7f Compare April 25, 2019 14:11

pmconrad mentioned this pull request May 5, 2019

Add appveyor CI support #128

Closed

crypto-ape force-pushed the master_mingw branch from 0e11f66 to 5c0d07e Compare May 16, 2019 12:43

pmconrad reviewed May 18, 2019

View reviewed changes

CMakeModules/Boost/BoostConfig.cmake Outdated Show resolved Hide resolved

pmconrad reviewed May 18, 2019

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

pmconrad reviewed May 18, 2019

View reviewed changes

pmconrad reviewed Jun 6, 2019

View reviewed changes

pmconrad mentioned this pull request Jun 8, 2019

Added preliminary windows cross-build bitshares/bitshares-gitian#6

Merged

SSE intrinsic rename

d5fb99b

crypto-ape force-pushed the master_mingw branch from 56cae63 to ec0ddf8 Compare June 20, 2019 11:47

crypto-ape added 11 commits July 8, 2019 10:43

Update build system for MinGW

e0dd942

Updated Boost searching

c9b3d57

Fixed pthread dependency

8d33bb8

CMake version bump

c453db8

BoostConfig EOF newline

b8a87bd

Updated CMake for MinGW

a2303d3

Legacy/CMakeLists.txt EOF newline

241f1f2

Bumped Boost version

86b0250

Removed unneeded definitions

ab61720

Updated Boost Lockfree compilation test

dfc0a35

CheckLibcxxAtomic after Boost

b2c2a47

crypto-ape force-pushed the master_mingw branch from ec0ddf8 to b2c2a47 Compare July 8, 2019 12:16

pmconrad approved these changes Jul 24, 2019

View reviewed changes

abitmore approved these changes Jul 25, 2019

View reviewed changes

abitmore merged commit 966e9ea into bitshares:master Jul 25, 2019

pmconrad mentioned this pull request Aug 7, 2019

Fix simplifications #140

Merged

MichelSantos mentioned this pull request Aug 9, 2019

Release Notes: BitShares Core 3.3.0 bitshares/bitshares-core#1892

Closed

abitmore added this to the core release 3.3.0 milestone Aug 21, 2019

abitmore mentioned this pull request Aug 21, 2019

Editline fix bitshares/bitshares-core#1949

Merged

MichelSantos mentioned this pull request Oct 18, 2019

Release Notes: BitShares Core 4.0.0 bitshares/bitshares-core#2026

Closed

pmconrad mentioned this pull request Nov 18, 2019

Code Clean Up (Ape) bitshares/bitshares-core#2055

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CMake cleanup & MinGW support #121

CMake cleanup & MinGW support #121

crypto-ape commented Apr 8, 2019 •

edited

Loading

jmjatlanta commented Apr 8, 2019 •

edited

Loading

crypto-ape commented Apr 9, 2019

pmconrad commented Apr 17, 2019

pmconrad commented Apr 23, 2019

pmconrad commented Apr 23, 2019

crypto-ape commented Apr 23, 2019

jmjatlanta commented Apr 23, 2019 •

edited

Loading

crypto-ape commented Apr 24, 2019

pmconrad commented Apr 24, 2019

crypto-ape commented Apr 25, 2019

pmconrad May 18, 2019

pmconrad Jun 6, 2019

crypto-ape Jun 27, 2019

pmconrad Jun 27, 2019

crypto-ape Jul 8, 2019

pmconrad commented Jun 8, 2019

pmconrad commented Jun 8, 2019

crypto-ape commented Jun 11, 2019 •

edited

Loading

pmconrad commented Jul 24, 2019

		@@ -22,6 +22,8 @@ INCLUDE(GetGitRevisionDescription)
		INCLUDE(CheckLibraryExists)
		INCLUDE(CheckLibcxxAtomic)

CMake cleanup & MinGW support #121

CMake cleanup & MinGW support #121

Conversation

crypto-ape commented Apr 8, 2019 • edited Loading

jmjatlanta commented Apr 8, 2019 • edited Loading

crypto-ape commented Apr 9, 2019

pmconrad commented Apr 17, 2019

pmconrad commented Apr 23, 2019

pmconrad commented Apr 23, 2019

crypto-ape commented Apr 23, 2019

jmjatlanta commented Apr 23, 2019 • edited Loading

crypto-ape commented Apr 24, 2019

pmconrad commented Apr 24, 2019

crypto-ape commented Apr 25, 2019

pmconrad May 18, 2019

Choose a reason for hiding this comment

pmconrad Jun 6, 2019

Choose a reason for hiding this comment

crypto-ape Jun 27, 2019

Choose a reason for hiding this comment

pmconrad Jun 27, 2019

Choose a reason for hiding this comment

crypto-ape Jul 8, 2019

Choose a reason for hiding this comment

pmconrad commented Jun 8, 2019

pmconrad commented Jun 8, 2019

crypto-ape commented Jun 11, 2019 • edited Loading

pmconrad commented Jul 24, 2019

crypto-ape commented Apr 8, 2019 •

edited

Loading

jmjatlanta commented Apr 8, 2019 •

edited

Loading

jmjatlanta commented Apr 23, 2019 •

edited

Loading

crypto-ape commented Jun 11, 2019 •

edited

Loading