build: Link `bench` binary statically

hebasto commented at 7:49 am on July 5, 2022: member

While testing bitcoin-core/secp256k1#1022 I’ve noticed a linker error when cross compiling with --host=x86_64-w64-mingw32 in particular circumstances.

I cannot recall my building environment then, but currently, on Ubuntu 22.04 with gcc 11.2, this error raises unconditionally:

 0$ ./autogen.sh && ./configure --host=x86_64-w64-mingw32 && make clean
 1$ make
 2make  all-am
 3make[1]: Entering directory '/home/hebasto/git/secp256k1'
 4  CC       src/bench.o
 5  CC       src/libsecp256k1_la-secp256k1.lo
 6  CC       src/libsecp256k1_precomputed_la-precomputed_ecmult.lo
 7  CC       src/libsecp256k1_precomputed_la-precomputed_ecmult_gen.lo
 8  CCLD     libsecp256k1_precomputed.la
 9  CCLD     libsecp256k1.la
10  CCLD     bench.exe
11libtool:   error: Could not determine the host path corresponding to
12libtool:   error:   '/home/hebasto/git/secp256k1/.libs'
13libtool:   error: Continuing, but uninstalled executables may not work.
14libtool:   error: Could not determine the host path corresponding to
15libtool:   error:   '/home/hebasto/git/secp256k1/.libs:/usr/local/lib:/usr/local/bin'
16libtool:   error: Continuing, but uninstalled executables may not work.
17  CC       src/bench_internal-bench_internal.o
18  CCLD     bench_internal.exe
19  CC       src/bench_ecmult-bench_ecmult.o
20  CCLD     bench_ecmult.exe
21  CC       src/tests-tests.o
22  CCLD     tests.exe
23  CC       src/exhaustive_tests-tests_exhaustive.o
24  CCLD     exhaustive_tests.exe
25make[1]: Leaving directory '/home/hebasto/git/secp256k1'

This PR prevents linker errors when cross compiling with --host=x86_64-w64-mingw32.

Please note that the same linker flag is used for all of the tests and examples. Moreover, it was used in the initial Autotools commit:https://github.com/bitcoin-core/secp256k1/blob/78cd96b15153e209cf4829a511f9efdfdcf7e4d0/Makefile.am#L45

build: Link `bench` binary statically

This change prevents linker errors when cross compiling with
`--host=x86_64-w64-mingw32`.

51cfc49c64

real-or-random commented at 10:38 am on July 5, 2022: contributor

For what it’s worth, these messages are entirely harmless.

For shared libraries, libtool has some magic which creates wrapper executables (scripts or binaries) that let you run your programs without installing the shared library or fiddling around with environment settings such as LD_LIBRARY_PATH. In this case, this means libtool creates a ./bench.exe, which is no more than a wrapper binary that sets up an environment with the correct settings and then runs the proper ./libs/bench.exe, which in turn then knows where to find the shared library in ./libs/libsecp256k1-0.dll. So you could run ./bench.exe if you happen to have wine installed.

Now the crux is that even creating the wrappers needs a filename conversion between Unix and Windows, and this requires the winepath program from wine. Because you don’t have wine installed, this conversion fails. But this is not a problem because you anyway couldn’t run ./bench.exe because you don’t have wine installed…

The problem with -static is that it avoids the entire process of creating wrapper executables. Yes, this makes the problem disappear. But if you install wine and run ./configure --host=x86_64-w64-mingw32 --disable-static to disable static libraries entirely. Then ./bench.exe won’t work, it just can’t find the dll. (You may wonder why the build worked at all… This is just libtool being too clever and stripping -static when you pass --disable-static…) Same happens with the examples.

So I believe we should do the opposite: Drop -static for all binaries that should be linked against the library (as a user would do). This naturally includes all the examples but also the normal benchmark (because the bench binary benchmarks the public API only), but not the tests. In other words, there was a reason why -static was missing for the benchmarks. @elichai Is there a specific reason why you set -static for the examples? Or was this just “copy/paste and it worked”?

hebasto commented at 11:55 am on July 5, 2022: member

For what it’s worth, these messages are entirely harmless.

Yes, they are. In terms of the resulted artifacts. From the user’s point of view, the “error” word in the log means something wrong. It should be avoided or documented clearly.

So I believe we should do the opposite: Drop -static for all binaries that should be linked against the library (as a user would do). This naturally includes all the examples but also the normal benchmark (because the bench binary benchmarks the public API only), but not the tests. In other words, there was a reason why -static was missing for the benchmarks.

Maybe make behavior dependent on the fact whether Wine has been installed?

real-or-random commented at 12:24 pm on July 5, 2022: contributor

Maybe make behavior dependent on the fact whether Wine has been installed?

Yeah, I’d be somewhat reluctant to maintain a workaround for this if it’s not actually only a problem in the error message. I agree an “error” sounds scary but even the message then says “Continuing, but uninstalled executables may not work.”. What do you think of reporting that to libtool and suggesting they change it to a warning? The manual even says it’s a warning:

Failure to convert paths (see File Name Conversion Failure) will cause a warning to be issued,

libtool was dead for a long time but now they have a new maintainer and they recently tagged a release after 7 years.

elichai commented at 6:42 pm on July 6, 2022: contributor

@elichai Is there a specific reason why you set -static for the examples? Or was this just “copy/paste and it worked”?

From what I remember it was that I saw that everything was using -static so I also added it, I also seem to remember that removing it complicates running Valgrind a bit because it runs it on the wrapper script, but I’m not sure about that.

hebasto commented at 1:05 pm on July 12, 2022: member

But if you install wine and run ./configure --host=x86_64-w64-mingw32 --disable-static to disable static libraries entirely. Then ./bench.exe won’t work…

IIUC, we do not use such a configuration in CI. Why we should bother about it at all?

https://gnusha.org/secp256k1/2022-07-12.log

hebasto closed this on Jan 18, 2023

hebasto deleted the branch on Jan 19, 2023

build: Link bench binary statically #1119

build: Link `bench` binary statically #1119