Skip to content

Conversation

@CyberTailor
Copy link
Contributor

@CyberTailor CyberTailor commented Apr 7, 2023

I'd like to introduce CMake as a build system for Gridcoin. It's more modern and more fast. Also it'll allow to clean up lots of legacy autotools build system code (so, also more correct).

It's ready for Linux only, other systems are untested, so CMake support is WIP at the moment.

System leveldb and berkdb are used because they are packaged everywhere and local versions don't deviate from upstream. I think this decision is fine.

TODO for follow-up PRs:

  • Integrate NSIS installer in CMake
  • Integrate Info.plist.in in CMake
  • Integrate extract_strings_qt.py in CMake

@CyberTailor
Copy link
Contributor Author

Changes:

  • Remove -DNDEBUG with string(REPLACE) instead of just overriding

@CyberTailor CyberTailor force-pushed the cmake branch 3 times, most recently from 1c52ffb to dc7db19 Compare April 7, 2023 14:28
@jamescowens jamescowens changed the title Initial CMake support build: Initial CMake support Apr 7, 2023
@jamescowens jamescowens requested a review from div72 April 7, 2023 15:43
@jamescowens
Copy link
Member

jamescowens commented Apr 7, 2023

This is why I want to include bdb 5.3. OpenSUSE does NOT officially support bdb 5.3. You have to get libdb 5.3 from here. https://software.opensuse.org//download.html?project=devel%3Alibraries%3Ac_c%2B%2B&package=libdb-5_3

OpenSUSE/ SLE is very common and important.

I would rather control the exact source we are using rather than require people to install unofficial libraries compiled by unknown third parties.

@jamescowens
Copy link
Member

jco@jco-monster-ng:~/builds/Gridcoin-Research/build> cmake ..
-- The ASM compiler identification is GNU
-- Found assembler: /usr/bin/cc
-- The C compiler identification is GNU 12.2.1
-- The CXX compiler identification is GNU 12.2.1
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/gcc-12 - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/g++-12 - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Performing Test HAS_SSE41
-- Performing Test HAS_SSE41 - Success
-- Performing Test HAS_AVX2
-- Performing Test HAS_AVX2 - Failed
-- Performing Test HAS_X86_SHANI
-- Performing Test HAS_X86_SHANI - Failed
-- Performing Test HAS_ARM_SHANI
-- Performing Test HAS_ARM_SHANI - Failed
-- Looking for strerror_r
-- Looking for strerror_r - found
-- Performing Test STRERROR_R_CHAR_P
-- Performing Test STRERROR_R_CHAR_P - Success
-- Found Git: /usr/bin/git (found version "2.35.3")
-- Performing Test ATOMICS_LOCK_FREE_INSTRUCTIONS
-- Performing Test ATOMICS_LOCK_FREE_INSTRUCTIONS - Success
CMake Warning (dev) at build-aux/cmake/FindBerkeleyDB.cmake:85 (try_run):
try_compile given unknown argument "BerkeleyDB_VERSION_LIST".
Call Stack (most recent call first):
CMakeLists.txt:89 (find_package)
This warning is for project developers. Use -Wno-dev to suppress it.

CMake Error: The source directory "db-version.c" does not exist.
Specify --help for usage, or press the help button on the CMake GUI.
CMake Error at build-aux/cmake/FindBerkeleyDB.cmake:85 (try_run):
Failed to configure test project build system.
Call Stack (most recent call first):
CMakeLists.txt:89 (find_package)

CMake Error: TRY_COMPILE attempt to remove -rf directory that does not contain CMakeTmp:SOURCE_FROM_CONTENT
-- Configuring incomplete, errors occurred!
See also "/home/jco/builds/Gridcoin-Research/build/CMakeFiles/CMakeOutput.log".
See also "/home/jco/builds/Gridcoin-Research/build/CMakeFiles/CMakeError.log".

@jamescowens
Copy link
Member

@jamescowens
Copy link
Member

jamescowens commented Apr 7, 2023

Operating system is OpenSUSE Leap 15.4 with bdb 5.3 installed. Autotools full compile using dev branch head is successful. Autotools compile using this PR fails with missing rules after configuration.

@jamescowens
Copy link
Member

This PR cannot break the traditional autotools build, so we have to ensure there are no regressions on the autotools side.

@barton2526
Copy link
Member

It's ready for Linux only, other systems are untested, so this PR is WIP at the moment.

Github allows you to mark PRs as "Draft"

@jamescowens jamescowens marked this pull request as draft April 8, 2023 03:54
@jamescowens
Copy link
Member

@CyberTailor If you can get this to reliably work for both autotools and cmake full compiles, (i.e. daemon and gui), then I will merge for the Miss Piggy milestone. You will own the maintenance of it! :)

@CyberTailor
Copy link
Contributor Author

Changes:

  • Allow local version of bdb53
  • Fix installation of Boost packages in CI
  • Disable failing compilerbug_tests

@CyberTailor CyberTailor force-pushed the cmake branch 2 times, most recently from 85a8312 to f1def77 Compare April 8, 2023 08:43
@CyberTailor
Copy link
Contributor Author

Github allows you to mark PRs as "Draft"

It's ready to merge, just some features (in TODO) are missing and some platforms are untested.

@CyberTailor
Copy link
Contributor Author

Changes:

  • CI: Change CMake generator to ninja

@CyberTailor
Copy link
Contributor Author

CMake Warning (dev) at build-aux/cmake/FindBerkeleyDB.cmake:85 (try_run): try_compile given unknown argument "BerkeleyDB_VERSION_LIST". Call Stack (most recent call first): CMakeLists.txt:89 (find_package) This warning is for project developers. Use -Wno-dev to suppress it.

CMake Error: The source directory "db-version.c" does not exist. Specify --help for usage, or press the help button on the CMake GUI. CMake Error at build-aux/cmake/FindBerkeleyDB.cmake:85 (try_run): Failed to configure test project build system. Call Stack (most recent call first): CMakeLists.txt:89 (find_package)

CMake Error: TRY_COMPILE attempt to remove -rf directory that does not contain CMakeTmp:SOURCE_FROM_CONTENT

Oh, CMake 3.25+ is needed for the try_run function. I've bumped the requirement in CMakeLists.txt.

@jamescowens
Copy link
Member

jamescowens commented Apr 8, 2023

@CyberTailor We MUST use the in tree version of leveldb. This is NOT negotiable. Bitcoin had a consensus issue that occurred early on in their use of leveldb due to slight differences between different system library versions.

@CyberTailor
Copy link
Contributor Author

I can set up Windows and macOS virtual machines to test the build on them

@CyberTailor
Copy link
Contributor Author

Testing on macOS. Toolchain seems to enable -Werror unconditionally, so the build fails. Any ways to disable this?

TODO:
* Integrate NSIS installer in CMake
* Integrate Info.plist.in in CMake
* Integrate extract_strings_qt.py in CMake
Rationale: most distros don't ship univalue library, its upstream is
inactive for several years
Rationale: to avoid version conflicts in the system. Electrum requires
libsecp256k1<0.2.0 and we need libsecp256k1>=0.2.0
Rationale: OpenSUSE doesn't have bdb5.3 in its repos
Rationale: possible consensus issue with system leveldb
Other changes:
* Prevent known warnings from being treated as errors on macOS
* Fix berkdb configure on macOS
* Fix GUI build on macOS
@CyberTailor CyberTailor requested a review from jamescowens June 30, 2023 04:08
@CyberTailor
Copy link
Contributor Author

Added CMake CI for macOS

Copy link
Member

@jamescowens jamescowens left a comment

Choose a reason for hiding this comment

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

tACK. I would like the one comment I put regarding the clang #pragma to be addressed in a followup PR.

Comment on lines +735 to +738
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wthread-safety-analysis"
a.Serialize(os);
#pragma clang diagnostic pop
Copy link
Member

Choose a reason for hiding this comment

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

Can we condition these to only be put in if clang is the compiler? It is noisy on GCC.

@jamescowens jamescowens merged commit 251b3a3 into gridcoin-community:development Jul 4, 2023
jamescowens added a commit to jamescowens/Gridcoin-Research that referenced this pull request Mar 2, 2024
Added
 - contrib: add nix file for compilation environment gridcoin-community#2660 (@div72)
 - gui: Make main Gridcoin window geometry save unique to datadir location gridcoin-community#2661 (@jamescowens)
 - build: Initial CMake support gridcoin-community#2676 (@CyberTailor)
 - util: Add `-shutdownnotify` and `startupnotify` options from upstream gridcoin-community#2688 (@barton2526)
 - gui, staking: Implement facilities for mandatory sidestakes and sidestake GUI gridcoin-community#2704 (@jamescowens)
 - gui, voting: Implement poll result caching and poll stale indicator gridcoin-community#2709 (@jamescowens)
 - gui, projects: Implement greylist state for projects in GUI projects table gridcoin-community#2715 (@jamescowens)
 - gui, poll: Implement poll expiration reminders gridcoin-community#2716 (@jamescowens)
 - serialize: allow variants to be serialized gridcoin-community#2729 (@div72)
 - gui: Implement poll field length limiters in GUI forms gridcoin-community#2742 (@jamescowens)

Changed
 - consensus, contract, scraper, protocol, project, beacon, rpc: Replace remaining appcache sections with native structures gridcoin-community#2639 (@jamescowens)
 - build: update libsecp256k1 to v0.3.0 gridcoin-community#2655 (@div72)
 - build: Replace $(AT) with .SILENT gridcoin-community#2674 (@barton2526)
 - build: allow system bdb gridcoin-community#2675 (@div72)
 - Resize Header Column with Additional Text gridcoin-community#2683 (@PrestackI)
 - rpc: use RPCErrorCode everywhere gridcoin-community#2687 (@Pythonix)
 - wallet: SecureString to allow null characters gridcoin-community#2690 (@barton2526)
 - random: port some upstream changes gridcoin-community#2696 (@div72)
 - depends: Bump dependencies gridcoin-community#2692 (@barton2526)
 - doc: Update link to Discord server gridcoin-community#2693 (@adriaanjoubert)
 - rpc: Change capitalization, remove whitespace of rpc keys gridcoin-community#2711 (@Pythonix)
 - ci: bump MacOS version to 12 gridcoin-community#2713 (@div72)
 - depends: no-longer nuke libc++abi.so* in native_clang package gridcoin-community#2719 (@div72)
 - doc: update windows `-fstack-clash-protection` doc gridcoin-community#2720 (@div72)
 - Silence `-Wcast-function-type` warning gridcoin-community#2721 (@div72)
 - build: Use newest `config.{guess,sub}` available gridcoin-community#2722 (@div72)
 - refactor: use the try_lock result in TryEnter gridcoin-community#2723 (@div72)
 - Updates for file src/qt/locale/bitcoin_en.ts in pt_PT gridcoin-community#2726 (@gridcoin-community)
 - ci: do not silently fail gridcoin-community#2727 (@div72)
 - Properly include Boost Array header gridcoin-community#2730 (@theMarix)
 - build: Update depends zlib to 1.3.1 gridcoin-community#2734 (@jamescowens)
 - util: Enhance Fraction class overflow resistance gridcoin-community#2735 (@jamescowens)
 - refactor: Fix compilation warnings gridcoin-community#2737 (@jamescowens)
 - gui, util: Improve upgrade dialog gridcoin-community#2738 (@jamescowens)
 - util: Improve allocation class gridcoin-community#2740 (@jamescowens)
 - translations: translation updates for Miss Piggy release gridcoin-community#2745 (@jamescowens)

Removed
 - gui: Disable snapshot GUI action gridcoin-community#2700 (@jamescowens)
 - build, crypto, script: remove most of OpenSSL usage gridcoin-community#2705 (@div72)
 - util: remove WSL 1 workaround in fs gridcoin-community#2717 (@div72)

Fixed
 - diagnostics: fix missing arg in ETTS warning gridcoin-community#2684 (@div72)
 - misc: fix include guard in netaddress.h gridcoin-community#2695 (@div72)
 - gui: Fix expired pending beacon display gridcoin-community#2698 (@jamescowens)
 - consensus: Fix 20230904 testnet forking issue gridcoin-community#2703 (@jamescowens)
 - gui: Fix filter by type in Transaction View gridcoin-community#2708 (@jamescowens)
 - depends: make fontconfig build under clang-16 gridcoin-community#2718 (@div72)
 - diag: fix researcher mode check gridcoin-community#2725 (@div72)
 - gui: Add missing switch cases for ALREADY_IN_MEMPOOL gridcoin-community#2728 (@jamescowens)
 - beacon, registry: Fix beacon history stall gridcoin-community#2731 (@jamescowens)
 - build: Implement comp_double comparison function in certain tests gridcoin-community#2741 (@jamescowens)
 - ci: change Qt path in CMake CI gridcoin-community#2743 (@div72)
 - net: Correct -tor argument handling gridcoin-community#2744 (@jamescowens)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants