rnpgp / rnp

RNP: high performance C++ OpenPGP library used by Mozilla Thunderbird
https://www.rnpgp.org
Other
198 stars 55 forks source link

Support linking a static botan library to a static rnp library #888

Closed kaie closed 4 years ago

kaie commented 5 years ago

I'd like to link RNP statically.

It seems, the usual approach to build statically with cmake is to simply omit the -DBUILD_SHARED_LIBS=on flag when executing cmake.

To produce a static library, the "add_library" call needs to add a STATIC keyword after the library name.

I tried that on Linux, but I get failures, unresolved externals from the pthreads library. I suggest to add the threading library as a dependency.

I suggest that the build system either builds a shared library or a static library (not both at the same time), controlled by the presence or the absence of the -DBUILD_SHARED_LIBS=on flag.

I'll submit a pull request, which builds for me on Linux and on Windows (cross compiled, using the the code from https://github.com/riboseinc/rnp/pull/883 )

dewyatt commented 5 years ago

To produce a static library, the "add_library" call needs to add a STATIC keyword after the library name.

This should already be handled automatically based on BUILD_SHARED_LIBS.

I tried that on Linux, but I get failures, unresolved externals from the pthreads library. I suggest to add the threading library as a dependency.

I haven't looked at this.

I suggest that the build system either builds a shared library or a static library (not both at the same time), controlled by the presence or the absence of the -DBUILD_SHARED_LIBS=on flag.

This should already be the case, unless I'm misunderstanding what you mean.

dewyatt commented 5 years ago

I tried that on Linux, but I get failures, unresolved externals from the pthreads library. I suggest to add the threading library as a dependency.

I tried and didn't have any issues building a static lib:

cd rnp
rm -rf build && mkdir build && cd build
cmake ..
make -j8
ls src/lib/librnp-0.a

Can you provide more information (sequence of commands)?

kaie commented 5 years ago

Ok, after some more experiments, you're partially right, it indeed works already, if the botan library offers a shared library.

However, I want to link the botan library statically. I built botan with --disable-shared-library

kaie commented 5 years ago

The static botan library is the cause of the issue.

dewyatt commented 5 years ago

Generally it's not ideal to link the core cryptographic library statically.

Anyways, I would have to revisit this later but it should probably be handled in the FindBotan2 cmake module.

kaie commented 5 years ago

Ok, this isn't a high priority right now. We can postpone to a later time. Maybe having botan dynamic and rnp static will work for me.

kaie commented 5 years ago

Generally it's not ideal to link the core cryptographic library statically.

Are there specific reasons? Note I'd use a botan library that uses the minimum necessary for rnp. I've configure botan with this module list:

--disable-shared-library --minimized-build --enable-modules="bigint,ffi,pgp_s2k,aead,aes,blowfish,camellia,cast128,des,idea,sm4,twofish,cbc,cfb,auto_rng,hmac,hmac_drbg,crc24,hash,md5,sha1,sha2_32,sha2_64,sha3,sm3,dl_group,ec_group,pubkey,dsa,ecdh,ecdsa,ed25519,elgamal,rsa,sm2,eme_pkcs1,emsa_pkcs1,emsa_raw,kdf,rfc3394,sp800_56a,eax,ocb,rmd160,curve25519"

dewyatt commented 5 years ago

Generally it's not ideal to link the core cryptographic library statically.

Are there specific reasons? Note I'd use a botan library that uses the minimum necessary for rnp. I've configure botan with this module list:

I just meant from the standpoint that using a shared object typically makes it easier to update that particular code (without rebuilding). As in, if YOUR_EXE uses a system-installed libbotan-2.so, then an apt upgrade/yum update/pacman -Syu etc will take care of security updates.

It does kind of depend on your specific situation though.

kaie commented 4 years ago

I don't expect to need this any more. I'll use dynamic linking. Feel free to close.

ni4 commented 4 years ago

Ok, closing this. May be related to #996