WIP: Add 1:1 CMake File

DesWurstes commented at 8:18 am on August 12, 2018: contributor

Currently lacks of:

Module ECDH
JNI but is very stable.

DesWurstes force-pushed on Aug 12, 2018

Add CMake File e07d46cdec

DesWurstes force-pushed on Aug 12, 2018

afk11 commented at 5:47 pm on August 12, 2018: contributor

#315 aimed to add this before, and some points were raised about testing, and keeping the configuration in sync with new features. Perhaps you could address these points so others can assess how viable this is?

Lots of improvements c766e02707

DesWurstes force-pushed on Sep 5, 2018

Fix more bugs e4e889be85

DesWurstes force-pushed on Sep 16, 2018

More improvements 59deef2092

More improvements 718c7fbef6

DesWurstes force-pushed on Sep 19, 2018

Add openssl tests 3d3dffcf48

Silence some warnings 11a4372754

squashme 40da97a99d

remove linker flags ab6d443621

Typo 27598f072c

Add bench f2abbc5952

zone117x cross-referenced this on Oct 20, 2018 from issue Get this pushed directly to sec256 repo by dangershony

Add linker flags 5e6d4c4a05

Stabilize 4ddd7659c5

gmaxwell commented at 11:49 am on February 20, 2019: contributor

I’m really so super not enthused by having yet another build system. (and last I checked cmake still didn’t support a lot of systems, or cross compiling… though I know its well liked for windows support)

DesWurstes cross-referenced this on Feb 25, 2019 from issue Respect LDFLAGS and #undef STATIC_PRECOMPUTATION if using basic config by DesWurstes

gmaxwell referenced this in commit 912680ed86 on May 23, 2019

gmaxwell commented at 12:05 pm on May 29, 2019: contributor

Unfortunately, we lack the resources or interest to adequately maintain another parallel build system on an ongoing basis. To the extent that build-system concerns would be a good use of development time, we believe that they’d better be spent making it easier to build the library without any build system and documenting how that’s done. I believe we’re likely to close any further cmake or vcproject pull requests unless something changes– nothing wrong with your efforts and thank you for the attempt, they just don’t reflect this libraries focus at this time.

gmaxwell closed this on May 29, 2019

DesWurstes commented at 12:55 pm on May 29, 2019: contributor

Thanks anyway.

real-or-random cross-referenced this on Jun 25, 2020 from issue Add cmake support by xDimon

xloem commented at 1:35 pm on April 10, 2021: none

Just a comment that cmake appears valuable enough to me to find some way to facilitate collaboration among the people making pull requests.

I see that libsecp256k1 gets a pull request adding cmake about every year or so, so if there were some community-maintained buildfile in a ‘contrib’ or ‘unsupported’ folder or somesuch, it might be maintainable simply by those who use it.

I’m working with code that embeds libsecp256k1 using cmake, and when doing so it becomes clear how fragmented maintenance effort around that is.

Maintaining documentation for building libsecp256k1 without autotools would of course also meet a significant portion of this concern.

real-or-random commented at 2:38 pm on April 10, 2021: contributor

I see that libsecp256k1 gets a pull request adding cmake about every year or so, so if there were some community-maintained buildfile in a ‘contrib’ or ‘unsupported’ folder or somesuch, it might be maintainable simply by those who use it.

Hm I’d be skeptical to accept this, because I feel that would still create a maintenance burden for us. Would it be possible to create an external “overlay” that adds CMake support? I admit that’s probably not exactly elegant.

Maintaining documentation for building libsecp256k1 without autotools would of course also meet a significant portion of this concern.

Yes, we have #622 for this.

zone117x commented at 5:39 pm on February 7, 2022: none

Would be nice to have an open issue for discussing cmake support. I maintain a C# wrapper for this lib and cmake is (for me) the easiest way to build cross-platform binaries. I’ve recently looked at adding linux-arm64 and Apple Silicon support, and cmake support would have been helpful here as well.

real-or-random commented at 7:31 am on February 8, 2022: contributor

Would be nice to have an open issue for discussing cmake support. I maintain a C# wrapper for this lib and cmake is (for me) the easiest way to build cross-platform binaries. I’ve recently looked at adding linux-arm64 and Apple Silicon support, and cmake support would have been helpful here as well.

autotools have good support for cross-compilation. For example, use ./configure --host=aarch64-linux-gnu for ARM64.

https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.71/html_node/Specifying-Target-Triplets.html#Specifying-Names

zone117x commented at 9:22 am on February 8, 2022: none

I was able to update my cmake setup to build universal binaries for macos (x86 + arm64) with CMAKE_OSX_ARCHITECTURES=arm64;x86_64.

Another useful feature is export symbols in the Windows dll binary with set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON), and it builds on Windows systems with regular cmake+msvc tooling which has some benefits compared to cross-compiling from Linux to Windows.

I’m sure those are both possible using autotools as well but it looks more painful. Cmake seems to have emerged as the de facto cross platform build system.

One thing I had trouble with was trying to rebase my cmake config onto the latest master branch due to the changes around USE_ECMULT_STATIC_PRECOMPUTATION. So the library is still building off a fork from 2018. If this repo had a cmakelists.txt I’m betting it would have been maintained. Maybe the next PR to this repo adding cmake support will figure it out for me :)

real-or-random commented at 10:37 am on February 8, 2022: contributor

Unfortunately, we lack the resources or interest to adequately maintain another parallel build system on an ongoing basis.

I think this is still true. Of course, we could switch to CMake entirely, and I believe it’s probably the better build system. But even if we have consensus, I’m not sure if it’s worth the effort given that autotools work pretty well.

One thing I had trouble with was trying to rebase my cmake config onto the latest master branch due to the changes around USE_ECMULT_STATIC_PRECOMPUTATION. So the library is still building off a fork from 2018. If this repo had a cmakelists.txt I’m betting it would have been maintained. Maybe the next PR to this repo adding cmake support will figure it out for me :)

Any specific questions?

zone117x commented at 6:59 pm on February 8, 2022: none

Unfortunately, we lack the resources or interest to adequately maintain another parallel build system on an ongoing basis.

I think this is still true. Of course, we could switch to CMake entirely, and I believe it’s probably the better build system. But even if we have consensus, I’m not sure if it’s worth the effort given that autotools work pretty well.

Yep fair enough – makes sense. I do think there’s room for including a basic cmakelists.txt that produces a typical output (e.g. only the non-experimental modules, using the pre-generated context), without replicating the various tests and other functionality in autotools. It could use the “experimental” label/docs so folks understand it could in theory build a broken output and not get tested.

Here’s the one I’m using https://github.com/MeadowSuite/secp256k1/blob/master/CMakeLists.txt, which seems like it would be even smaller if adjusted to work the latest codebase because the pre-generated context changes, and some of those directives being automatic or removed.

Any specific questions?

It seems like this should work:

 0cmake_minimum_required(VERSION 3.4)
 1project(secp256k1 LANGUAGES C)
 2
 3set(COMMON_COMPILE_FLAGS, ENABLE_MODULE_ECDH, ENABLE_MODULE_RECOVERY, USE_BASIC_CONFIG)
 4set(COMPILE_OPTIONS -O3 -W -std=c89 -pedantic -Wall -Wextra -Wcast-align -Wnested-externs -Wshadow -Wstrict-prototypes -Wno-unused-function -Wno-long-long -Wno-overlength-strings)
 5
 6add_library(secp256k1 SHARED src/secp256k1.c)
 7
 8target_compile_definitions(secp256k1 PRIVATE ${COMMON_COMPILE_FLAGS} ${COMPILE_FLAGS})
 9target_include_directories(secp256k1 PRIVATE ${CMAKE_SOURCE_DIR} ${CMAKE_SOURCE_DIR}/src)
10target_compile_options(secp256k1 PRIVATE ${COMPILE_OPTIONS})

But results in errors from ECMULT_WINDOW_SIZE and ECMULT_GEN_PREC_BITS being undeclared – shouldn’t the USE_BASIC_CONFIG option specify those?

0[ 50%] Building C object CMakeFiles/secp256k1.dir/src/secp256k1.c.o
1In file included from /Users/matt/Projects/secp256k1/src/secp256k1.c:17:
2In file included from /Users/matt/Projects/secp256k1/src/ecmult_impl.h:16:
3/Users/matt/Projects/secp256k1/src/ecmult.h:26:4: error: Set ECMULT_WINDOW_SIZE to an integer in range [2..24].
4#  error Set ECMULT_WINDOW_SIZE to an integer in range [2..24].

Then if I specify those myself like this:

 0cmake_minimum_required(VERSION 3.4)
 1project(secp256k1 LANGUAGES C)
 2
 3set(COMMON_COMPILE_FLAGS, ENABLE_MODULE_ECDH, ENABLE_MODULE_RECOVERY, USE_BASIC_CONFIG)
 4set(COMPILE_OPTIONS -O3 -W -std=c89 -pedantic -Wall -Wextra -Wcast-align -Wnested-externs -Wshadow -Wstrict-prototypes -Wno-unused-function -Wno-long-long -Wno-overlength-strings)
 5
 6add_library(secp256k1 SHARED src/secp256k1.c)
 7
 8target_compile_definitions(secp256k1 PRIVATE ECMULT_GEN_PREC_BITS=4)
 9target_compile_definitions(secp256k1 PRIVATE ECMULT_WINDOW_SIZE=15)
10
11target_compile_definitions(secp256k1 PRIVATE ${COMMON_COMPILE_FLAGS} ${COMPILE_FLAGS})
12target_include_directories(secp256k1 PRIVATE ${CMAKE_SOURCE_DIR} ${CMAKE_SOURCE_DIR}/src)
13target_compile_options(secp256k1 PRIVATE ${COMPILE_OPTIONS})

I get:

 0[ 50%] Building C object CMakeFiles/secp256k1.dir/src/secp256k1.c.o
 1[100%] Linking C shared library libsecp256k1.dylib
 2Undefined symbols for architecture arm64:
 3  "_secp256k1_ecmult_gen_prec_table", referenced from:
 4      _secp256k1_ecmult_gen in secp256k1.c.o
 5  "_secp256k1_pre_g", referenced from:
 6      _secp256k1_ecmult in secp256k1.c.o
 7  "_secp256k1_pre_g_128", referenced from:
 8      _secp256k1_ecmult in secp256k1.c.o
 9ld: symbol(s) not found for architecture arm64
10clang: error: linker command failed with exit code 1 (use -v to see invocation)
11make[3]: *** [libsecp256k1.dylib] Error 1
12make[2]: *** [CMakeFiles/secp256k1.dir/all] Error 2
13make[1]: *** [CMakeFiles/secp256k1.dir/rule] Error 2
14make: *** [secp256k1] Error 2

My guess is that I need to provide specific header inclusions and/or there’s some order in which they need to be included.

This comment seems to be pointing me towards the right direction: https://github.com/bitcoin-core/secp256k1/blob/077528317de94861552ed98cba8d0e8a4914e7f0/src/precompute_ecmult.c#L10-L14 But I’m not quite familiar enough with C or these build systems to figure it out myself.

real-or-random commented at 9:45 pm on February 8, 2022: contributor

Try add_library(secp256k1 SHARED src/secp256k1.c src/precomputed_ecmult_gen.c src/precomputed_ecmult.c)

The USE_BASIC_CONFIG thing is indeed an issue. It’s confusing, sorry. The plan is to make sure that the library builds even without any config but we haven’t made progress (https://github.com/bitcoin-core/secp256k1/issues/929). So for now it’s best to set ECMULT_GEN_PREC_BITS and ECMULT_WINDOW_SIZE explicitly. USE_BASIC_CONFIG will probably be removed.

zone117x commented at 1:20 am on February 9, 2022: none

That did the trick, thanks! https://github.com/MeadowSuite/secp256k1/blob/8f24760236f7db30a2a3bd8340a92ce231947810/CMakeLists.txt#L1-L48

real-or-random commented at 10:18 am on February 9, 2022: contributor

Nice! One final note: We usually get better performance with -O2 than with -O3, you may give it a try.

zone117x commented at 2:35 pm on February 9, 2022: none

Nice! One final note: We usually get better performance with -O2 than with -O3, you may give it a try.

Will do, thanks!

Btw, does ~1.2MB sound about right for the size of the shared library (using precomputed with the default config)?

01.2M	./linux-x64/libsecp256k1.so
11.2M	./linux-x86/libsecp256k1.so
21.2M	./linux-arm64/libsecp256k1.so
31.2M	./osx-x64/libsecp256k1.dylib
41.2M	./osx-arm64/libsecp256k1.dylib
51.3M	./win-x64/secp256k1.dll
61.2M	./win-x86/secp256k1.dll
71.3M	./win-arm64/secp256k1.dll

The older 2018 output was substantially smaller at around 200KB. I’m assuming it’s just the precomputed source?

real-or-random commented at 3:29 pm on February 9, 2022: contributor

The older 2018 output was substantially smaller at around 200KB. I’m assuming it’s just the precomputed source?

Indeed, it’s the precomputed source. And 1.2 MB looks about right.

zone117x commented at 4:33 pm on February 9, 2022: none

@real-or-random how would you feel about a PR that added something like a /doc/building-with-cmake.md which started off with something like the existing experimental disclaimer in the readme, e.g. “Building with CMake is experimental and has not received enough scrutiny to satisfy the standard of quality of this library but docs are made available for testing and review by the community. "

The doc would contain some minimal cmakelists.txt snippet like the one in the above fork. I think that would very clearly make it apparent that it’s not explicitly supported and should not be depended on. While at the same time, a lot of folks like myself and the others who open a cmake PR every year or so would find it quite helpful, as well as help contribute towards ensuring it stays reasonably up-to-date.

Thoughts?

real-or-random commented at 9:17 pm on February 17, 2022: contributor

Maybe then it’s easier to create an external repo for this? Otherwise we’d still need to look at the PRs changing it, so it will still need our resources.

I personally think that we’re not married to autotools. If a CMake file (e.g., maintained somewhere else) evolves to a reasonable alternative, and it turns out that it works better, is easier to maintain, or has other advantages (without huge disadvantages), I can imagine we’d be open to switch. (But I can’t speak for the others here.) I just don’t think that any of the current contributors is willing to spend time on porting to CMake given that we have a working build system.

hebasto cross-referenced this on Jun 28, 2022 from issue build: Add CMake-based build system by hebasto

sipa referenced this in commit df323b5c14 on Mar 8, 2023

WIP: Add 1:1 CMake File #549