A switch to disable logging in optimized builds

[rrrooommmaaa]

Closes #1103

[codecov]

Codecov Report

Merging #1125 into master will increase coverage by 0.01%.
The diff coverage is 97.77%.

@@            Coverage Diff             @@
##           master    #1125      +/-   ##
==========================================
+ Coverage   83.47%   83.48%   +0.01%     
==========================================
  Files          81       82       +1     
  Lines       29755    29800      +45     
==========================================
+ Hits        24838    24879      +41     
- Misses       4917     4921       +4     

Impacted Files	Coverage Δ
src/lib/utils.h	50.00% <ø> (ø)
src/tests/log-switch.cpp	97.36% <97.36%> (ø)
src/lib/misc.cpp	76.79% <100.00%> (+0.93%)	⬆️
src/librepgp/stream-parse.cpp	77.75% <0.00%> (-0.34%)	⬇️
src/librepgp/stream-write.cpp	78.93% <0.00%> (+0.10%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eeaad5e...af99f84. Read the comment docs.

[ni4]

@rrrooommmaaa Looks good once tests are passing (Seems this breaks CLI Keystore test).

[ronaldtse]

Same as @ni4 -- @rrrooommmaaa can we fix the tests and get this merged? Thanks.

[rrrooommmaaa]

@ni4 This update builds ok on most systems, but triggers this error on Cyrrus CI:
Undefined symbol "_rnp_log_switch" referenced from COPY relocation in /home/rnpuser/local-builds/rnp-build/src/tests/rnp_tests
The idea was to have _rnp_log_switch available in a macro, so it is defined as extern in a header file.
So it passes in most configurations, but some trick should be used for Cyrrus?

[ni4]

@rrrooommmaaa My guess is that it is related to EFL dynamic symbols export and could be fixed with -rdynamic building flag, or adding global accessor/setter function.

[rrrooommmaaa]

@dewyatt @ni4 @ronaldtse @joke325 @antonsviridenko I would appreciate some help/opinion here.
I tried to expose the global variable _rnp_log_switch from librnp with the sole purpose of removing necessity to call a function from RNP_LOG macro for performance reasons. Is it even worthwhile?
The issue is that it fails to build on Cirrus CI (FreeBSD). -rdynamic flag or extern "C" doesn't help.
The error is when trying to run rnp_tests:

cd /home/rnpuser/local-builds/rnp-build/src/tests && /usr/local/bin/cmake -E cmake_link_script CMakeFiles/rnp_tests.dir/link.txt --verbose=1
/usr/bin/c++  -O2 -g -DNDEBUG   <list of .o files> C  -o rnp_tests  -Wl,-rpath,/home/rnpuser/local-builds/rnp-build/src/lib:/home/rnpuser/local-installs/jsonc-install/lib:/home/rnpuser/local-builds/rnp-build/lib -rdynamic -std=c++11 ../lib/librnp-0.so.0.0.0 /home/rnpuser/local-installs/jsonc-install/lib/libjson-c.so ../../lib/libgtest_main.so ../../lib/libgtest.so -lpthread 
cd /home/rnpuser/local-builds/rnp-build/src/tests && /usr/local/bin/cmake -D TEST_TARGET=rnp_tests -D TEST_EXECUTABLE=/home/rnpuser/local-builds/rnp-build/src/tests/rnp_tests -D TEST_EXECUTOR= -D TEST_WORKING_DIR=/home/rnpuser/local-builds/rnp-build/src/tests -D TEST_EXTRA_ARGS= -D "TEST_PROPERTIES=FIXTURES_REQUIRED;testdata;TIMEOUT;3000;ENVIRONMENT;RNP_TEST_DATA=/tmp/cirrus-ci-build/src/tests/data" -D TEST_PREFIX= -D TEST_SUFFIX= -D NO_PRETTY_TYPES=FALSE -D NO_PRETTY_VALUES=FALSE -D TEST_LIST=rnp_tests_TESTS -D CTEST_FILE=/home/rnpuser/local-builds/rnp-build/src/tests/rnp_tests[1]_tests.cmake -D TEST_DISCOVERY_TIMEOUT=5 -P /usr/local/share/cmake/Modules/GoogleTestAddTests.cmake
ld-elf.so.1: Undefined symbol "_rnp_log_switch" referenced from COPY relocation in /home/rnpuser/local-builds/rnp-build/src/tests/rnp_tests

[ni4]

@rrrooommmaaa Did you try to use getter/setter function instead of accessing variable directly?

P.S. And, as a side note - could you please slightly rebase on master/squash commits since it looks crazy in GUI Git client.

[rrrooommmaaa]

Edited and rebased.
Ping @ni4, @dewyatt, @ronaldtse for review

[ni4]

LGTM!
Just got an idea that at some point later we can update this with FFI function rnp_set_log_fd(), so logging could be redirected to some file/pipe/whatever else (or to -1 to disable it).

[dewyatt]

I think it would be good to support RNP_LOG_CONSOLE=0 instead of just checking for the presence of the env var. Sometimes it's easier to override an env var than it is to unset it.

[ronaldtse]

Agree! Let's update this and merge!