Fixes failing test during Arch package build (due to attempt to write to
~/.bitmonero/...).
Prefix temp dir path with "monero-" because we are not putting it on the
system, so good to identify ourselves in case the dir gets left over due
to crash, etc.
This fixes linking when path to openssl
is defined manually:
cmake -DOPENSSL_ROOT_DIR='/usr/include/openssl-1.0;/usr/lib/openssl-1.0' ...
This is useful for building with OpenSSL v1.0
when default system installation is v1.1.
The linking error is undefined SSL_load_error_strings symbol.
This is due to -L /usr/lib/openssl-1.0 not making it onto
the linkline (so -lssl pulls in the default system openssl).
Fixes compile error when building with OpenSSL v1.1:
contrib/epee/include/net/net_helper.h: In member function ‘void epee::net_utils::blocked_mode_client::shutdown_ssl()’:
contrib/epee/include/net/net_helper.h:579:106: error: ‘SSL_R_SHORT_READ’ was not declared in this scope
if (ec.category() == boost::asio::error::get_ssl_category() && ec.value() != ERR_PACK(ERR_LIB_SSL, 0, SSL_R_SHORT_READ))
^
contrib/epee/include/net/net_helper.h:579:106: note: suggested alternative: ‘SSL_F_SSL_READ’
See boost/asio/ssl/error.hpp.
Boost handles differences between OpenSSL versions.
cmake: fail if Boost is too old for OpenSSL v1.1
Warning issued on older boost and/or OS:
In file included from /usr/include/boost/asio/detail/socket_types.hpp:61:0,
from /usr/include/boost/asio/detail/epoll_reactor.hpp:30,
from /usr/include/boost/asio/detail/reactor.hpp:21,
from /usr/include/boost/asio/detail/impl/task_io_service.ipp:24,
from /usr/include/boost/asio/detail/task_io_service.hpp:198,
from /usr/include/boost/asio/impl/io_service.hpp:71,
from /usr/include/boost/asio/io_service.hpp:767,
from /usr/include/boost/asio/basic_io_object.hpp:19,
from /usr/include/boost/asio/basic_socket.hpp:20,
from /usr/include/boost/asio/basic_datagram_socket.hpp:20,
from /usr/include/boost/asio.hpp:21,
from /home/vagrant/slave/monero-static-alpine-3_5-x86_64/build/src/common/download.cpp:32:
/usr/include/sys/poll.h:1:2: warning: #warning redirecting incorrect #include <sys/poll.h> to <poll.h> [-Wcpp]
#warning redirecting incorrect #include <sys/poll.h> to <poll.h>
Setting COMPILE_FLAGS (or COMPILE_OPTIONS) property directly does not
end up on the command line (even though it should because
add_compile_options does just that).
Also, set -Werror for tests as well, because no warnings now.
Not set for 'external' only because simply moving add_compile_options
above add_subdirectory(external) doesn't do it, and moving add_usbdirectory
down is too big of a change (it will pick up new flags).
-Werror set only for GCC on Linux, since warnings not yet
cleared for other compilers/systems.
Support building internal libraries as shared. This reduces
development time by eliminating the need to re-link all
binaries every time non-interface code in the library changes.
Instead, can hack on libxyz, then `make libxyz`, and re-run
monerod.
By default BUILD_SHARED_LIBS is OFF in release build type,
and ON in debug build type, but can be overriden with -D.
Keep the immediate direct deps at the library that depends on them,
declare deps as PUBLIC so that targets that link against that library
get the library's deps as transitive deps.
Break dep cycle between blockchain_db <-> crytonote_core.
No code refactoring, just hide cycle from cmake so that
it doesn't complain (cycles are allowed only between
static libs, not shared libs).
This is in preparation for supproting BUILD_SHARED_LIBS cmake
built-in option for building internal libs as shared.
It's only blank only if somebody running cmake in MSYS/MinGW (Windows)
manually forgets to add -D ARCH, but when it is blank, without quotes
those lines are invalid cmake syntax.
This was disabled earlier as part of diagnosing failing tests
on ARM, which turned out to be due to aliasing, fixed by
adding -fno-strict-aliasing. So, re-enabling it back.
32-bit build would fail to link with 'mdb_env_create undefined' (because
for 32-bit build, mdb_env_create_vl32 is defined instead).
This bug was introduced with the recent change to virtual object
libraries. The problem is that the COMPILE_DEFINITIONS property
was not propagated from dependee target (lmdb) to depedent
target's (blockchain_db) virtual object lib (obj_blockchain_db).
This patch makes that happen.
I chose to include INTERFACE_COMPILE_DEFINITIONS because there
should not be a need to propagate private defs, but it doesn't
make a difference in this case.
The split is to make this software more packageable. 'make install'
is used by the package building scripts, and should not be installing
vendored dependencies onto the system.
CMake issued a warming about policy CMP0026: access of LOCATION
target property at config time was disallowed. Offending code
was the code that merged static libraries to generate
libwallet_merged.a.
This patch does that same merge task in a much simpler way. And,
since it doesn't violate the policy, the warning went away.
Access to LOCATION property at config time was disallowed by this
policy. This patch updates the offending code to comply with the policy
and thereby gets rid of the warning printed by cmake.
The fix is to use generator expressions instead of that property.
NOTE: Had to remove support for multiconfig (only affects MSVC which is
not supported anyway; and mutliconf is an IDE thing in any case),
because could not see how to support it with generator expressions.
I think, in this context, dynamically linked builds make sense
only for native builds, not these builds that target arch
families to produce portable binaries.
* some re-org of sections to make things more concise
* dedicated section for installing from packages vs building from source
* for Windows, instruct to run the top-level makefile targets,
because I don't see any reason not to -- advanced users
will look into the makefile and find the cmake command there
if they need to.
* for Windows, instruct to install *-toolchain meta package
which includes expat
* added some context info here and there to aid -- I tried
to clarify things that confused me, e.g. Windows build is
cross-compilation to be precise; motivation for targets
that are meant for generating portable binaries as opposed
to binaries optimized to host CPU.
This reverts commit ecd0f2dde7.
These targets that are not native builds. They are for builds
portable within processors of a given family.
'make release' used to not work to build a native build on ARM, but that
has been fixed. These targets are unrelated to the native build.
This reverts commit 8623492150.
Let's restrict ARCH to values accepted by -march to keep things clear
and consistent. ARCH is -march, with only one exception: a value of
"default" indicates to not pass -march at all.
It is not correct to do so, because ARCH should only take values
supported by the -march argument, with the exception of 'default'
which denotes not passing -march at all.
ARCH defines the target architecture for builds that are intended to be
portable to other machines.
This gets rid of bitmonerod.exe's dependecy on libwindpthreads-1.dll in build
on Windows on x86_64 (via MSYS2 default toolchain). With this patch all DLL
dependencies are on DLLs in c:\windows\system32.
The previous logic that used a COMMON_*_FLAGS intermediate variable
and then re-assigned CMAKE_*_FLAGS before including each subdirectory
was confusing and ugly. This PR is the right way to do it.
This commit is purely refactoring: built binaries unchanged.
By default the flag is enabled whenever libunwind is found on the
system, with the exception of static build on OSX (for which we can't
install the throw hook #932 due to lack of support for --wrap in OSX
ld64 linker).
This is an attempt to fix build with STATIC=ON on OSX (#932):
[ 95%] Linking CXX executable ../../bin/bitmonerod Undefined symbols for
architecture x86_64: "___real___cxa_throw", referenced from:
___wrap___cxa_throw in libcommon.a(stack_trace.cpp.o) ld: symbol(s) not found
for architecture x86_64
This fixes build of tests with STATIC=ON, which failed with:
/tmp/cc8lNtqY.ltrans12.ltrans.o: In function
`boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<boost::thread_resource_error>
>::rethrow() const [clone .lto_priv.41]':
cc8lNtqY.ltrans12.o:(.text+0x4e): undefined reference to `__wrap___cxa_throw'
The hook is implemented in libcommon, which is not linked into some of the test
binaries. An alternative solution is to link all tests against libcommon,
but that seems worse because it introduces a false dependency (also,
I tried that and for some of the test binaries the linker still failed to
pick up the symol from libcommon, strangely.)
It is not clear why libunbound was added to this in the first place,
since it wasn't here before and #915 doesn't seem to introduce any
new dependency on it.
Tested build with STATIC=OFF (with and without libunbound-dev libunbound8
installed) and STATIC=ON, on Ubuntu Trusty, Debian Jessie, and Arch
Linux. For static builds, beware of #926 and #907.
If this hack was introduced to make it build on some other system
(Windows? OS X?), then it will have to be dealt with, but not this way.
The tests currently issue a warning that
"warning: -fassociative-math disabled; other options take precedence"
The associative math optimization is turned on indirectly by -Ofast.
Apparently, the optimization is forced to be disabled, while compiling
test harnesses generated by Google Test framework.
Unfortunately, there is no -Wno-error=* flag to disable this warning
(see gcc --help=warnings).
An alternative to this patch is to disable the optimization explicitly
with -fno-associative-math, but that seems worse.
Another alternative is to not pass -Ofast for tests build, but we
want the tests to be built with exact same optimization flags as
the code being tested, otherwise the value of the tests is diminished.
Another alternative is to remove -Werror from the entire build, but
it's good to include that flag to preclude people leaving warnings.
A note regarding implementation of not passing -Werror for tests:
I considered filtering out -Werror from CMAKE_{C,CXX}_FLAGS but
that seems to be worse because it's surprizing behavior, to those
reading the code that adds -Werror. It is better to add it for
when it is used and not added otherwise. I also considered relying
on order, adding -Werror after inluding 'tests' subdir, but before
including the other subdirs, but that also seems cryptic to the
reader. So, I settled with the current solution, of explicitly
setting CMAKE_{C,CXX}_FLAGS to different values before including the
respective subdir.
Testing done: compared compiler invocation for non-tests source files
using `make VERBOSE=1` with and without this commit: the only difference
is the position of -Werror. So, this commit doesn't change the binary.
otherwise clang build fails with
../cryptonote_core/libcryptonote_core.a(miner.cpp.o): In function
`std::__atomic_base<unsigned long long>::load(std::memory_order) const':
/usr/bin/../lib/gcc/i686-pc-linux-gnu/6.1.1/../../../../include/c++/6.1.1/bits/atomic_base.h:396:
undefined reference to `__atomic_load_8'
This has no effect on the gcc build.
The one strange thing is that test code like
std::atomic<int> x;
int main() { return x; }
compiles and links without errors with clang, without -latomic. This
alone would suggest that this patch is unnecessary, but that is not the
case. It's not clear exactly why, though. The bitmonero code is
including the same header, but it must be doing something more complex
than in this test code snippet that causes the failure at link time
pasted above. In any case, passing -latomic fixes the problem and
seems safe.
.
This does two things:
1. fixes clang build, which otherwise errors with undefined symbol
'dlsym'.
2. simplifies the cmake script, delegating to cmake to figure
out platform-specific flags for linking against the dl library.
Tested that it builds with:
gcc 6.1.1, STATIC=OFF,i686
gcc 6.1.1, STATIC=OFF,armv7h
clang 3.8, STATIC=OFF,i686
clang 3.8, STATIC=OFF,armv7h
gcc 6.1.1, STATIC=ON,i686
clang 3.8, STATIC=ON,i686
Also tested that stack trace is generated fine on exception on:
i686, gcc 6.1.1, STATIC=OFF
(didn't bother testing all the other platforms/configs)
This should fix the build problem on OSX (#871, #901), but
I don't have OSX, so I could only test Clang on Linux.
Tested on Linux (Arch) with clang 3.7 and 3.8 i686 and ARM:
if -stdlib=c++ is passed to clang, then the build errors
out with <string>,<iostrea>,etc. headers not found. Simply
not passing the arg fixes the problem.
**NOTE**: not tested on OSX.
Keep the working directory (and umask) inherited from
the parent. Otherwise, it's impossible to control
the working directory of the daemon (from systemd, for
example).
Furthermoer, bitmonerod attempts to create logging directories and files
*in current working directory*. This fails due to permission denied and
generates a (caught, nonfatal) exception. Below is the strace with this
patch applied (so, no `chdir("/")`), showing successful opens at `log/`
relative path. Without this patch they fail (sorry, didn't save the
trace).
```
28911 getcwd("/.../bitmonero", 128) = 25
28911 stat64("/var/lib/bitmonero/.bitmonero", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
28911 stat64("/etc/bitmonerod.conf", {st_mode=S_IFREG|0644, st_size=244, ...}) = 0
28911 open("/etc/bitmonerod.conf", O_RDONLY|O_LARGEFILE) = 3
28911 open("/var/log/bitmonero/bitmonero.log", O_WRONLY|O_CREAT|O_APPEND|O_LARGEFILE, 0666) = 3
28911 stat64("log", {st_mode=S_IFDIR|0700, st_size=4096, ...}) = 0
28911 stat64("log/dbg", {st_mode=S_IFDIR|0700, st_size=4096, ...}) = 0
28911 open("log/dbg/main.log", O_WRONLY|O_CREAT|O_TRUNC|O_LARGEFILE, 0666) = 4
```
The reasoning of chdir("/") in order to prevent the daemon from holding
a filesystem in busy state is not compelling at all: the choice of
working directory for the daemon is the user's business not the
daemon's.
Shorten the list of warnings that are reported, but
which are forced to NOT generate an error, via -Wno-error.
Unwhitelist these: strict-aliasing, sign-compare, type-limits
For example, ignoring strict-aliasing warning caused
lots of wasted time diagnosing Issue #847.
The noexcept specs were added to make GCC 6.1.1 happy (#846), but this
one was missing (because GCC did not complain about it on Linux, but
does complain on OSX).
We need ARCH, because it needs to be set for ARM7, ARM6 to be
initialized.
Strangely, on different machines (both ARMv7, Arch), ${ARCH}
var is either empty or 'native'. Handle both cases.
The destructors get a noexcept(true) spec by default, but these
destructors in fact throw exceptions. An alternative fix might be to not
throw (most if not all of these throws are non-essential
error-reporting/logging).
The destructors get a noexcept(true) spec by default, but these
destructors in fact throw exceptions. An alternative fix might be to not
throw (most if not all of these throws are non-essential
error-reporting/logging).