Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade googletest to 1.8.0 #1437

Closed
wants to merge 1 commit into from
Closed

Conversation

str4d
Copy link
Contributor

@str4d str4d commented Sep 23, 2016

Since 1.8.0, the googletest and googlemock repositories have merged.

Closes #1432

@str4d str4d added A-testing Area: Tests and testing infrastructure A-build Area: Build system A-dependencies Area: Dependencies labels Sep 23, 2016
@str4d
Copy link
Contributor Author

str4d commented Sep 23, 2016

Currently fails with a linker error.

This is not security-critical, so if no one has a quick fix then we can kick it forward.

@bitcartel
Copy link
Contributor

Maybe this? google/googletest#50

@bitcartel
Copy link
Contributor

Not enough time to fix for rc1, moving to 1.0.0, could be moved to 1.0.1 too since low priority.

@daira daira modified the milestones: 1.0.1 stabilization, 1.0.0 Launch - First Sprout release Oct 15, 2016
@zkbot
Copy link
Contributor

zkbot commented Feb 9, 2017

☔ The latest upstream changes (presumably #2050) made this pull request unmergeable. Please resolve the merge conflicts.

@str4d
Copy link
Contributor Author

str4d commented Feb 20, 2017

Rebased on master and tweaked to better follow the upstream build process. Still fails with linker errors:

libtool: link: /home/str4d/dev/zcash/zcash/depends/x86_64-unknown-linux-gnu/share/../native/bin/ccache g++ -m64 -std=c++11 -pipe -O1 -fwrapv -fno-strict-aliasing -Werror -g -Wformat -Wformat-security -Wstack-protector -fstack-protector-all -fPIE -pthread -Wl,-z -Wl,relro -Wl,-z -Wl,now -pie -o zcash-gtest gtest/zcash_gtest-main.o gtest/zcash_gtest-utils.o gtest/zcash_gtest-test_checktransaction.o gtest/zcash_gtest-json_test_vectors.o gtest/zcash_gtest-test_foundersreward.o wallet/gtest/zcash_gtest-test_wallet_zkeys.o gtest/zcash_gtest-test_tautology.o gtest/zcash_gtest-test_equihash.o gtest/zcash_gtest-test_joinsplit.o gtest/zcash_gtest-test_keystore.o gtest/zcash_gtest-test_noteencryption.o gtest/zcash_gtest-test_merkletree.o gtest/zcash_gtest-test_metrics.o gtest/zcash_gtest-test_miner.o gtest/zcash_gtest-test_pow.o gtest/zcash_gtest-test_random.o gtest/zcash_gtest-test_rpc.o gtest/zcash_gtest-test_transaction.o gtest/zcash_gtest-test_circuit.o gtest/zcash_gtest-test_txid.o gtest/zcash_gtest-test_libzcash_utils.o gtest/zcash_gtest-test_proofs.o gtest/zcash_gtest-test_checkblock.o wallet/gtest/zcash_gtest-test_wallet.o -fopenmp  -L/home/str4d/dev/zcash/zcash/depends/x86_64-unknown-linux-gnu/share/../lib -lgmock libbitcoin_server.a libbitcoin_cli.a libbitcoin_common.a libbitcoin_util.a crypto/libbitcoin_crypto.a univalue/.libs/libunivalue.a ./leveldb/libleveldb.a ./leveldb/libmemenv.a -lboost_filesystem-mt -lboost_program_options-mt -lboost_thread-mt -lboost_chrono-mt -lboost_unit_test_framework-mt secp256k1/.libs/libsecp256k1.a libbitcoin_zmq.a -L/home/str4d/dev/zcash/zcash/depends/x86_64-unknown-linux-gnu/lib /home/str4d/dev/zcash/zcash/depends/x86_64-unknown-linux-gnu/lib/libzmq.a -lrt -lpthread -lstdc++ libbitcoin_wallet.a ./.libs/libzcashconsensus.a -ldb_cxx-6.2 -lssl -ldl -lminiupnpc libzcash.a -lsnark /home/str4d/dev/zcash/zcash/depends/x86_64-unknown-linux-gnu/lib/libgmpxx.a /home/str4d/dev/zcash/zcash/depends/x86_64-unknown-linux-gnu/lib/libgmp.a -lboost_system-mt -lcrypto /home/str4d/dev/zcash/zcash/depends/x86_64-unknown-linux-gnu/lib/libsodium.a -lanl -fopenmp -pthread
gtest/zcash_gtest-test_foundersreward.o: In function `__static_initialization_and_destruction_0':
/home/str4d/dev/zcash/zcash/src/gtest/test_foundersreward.cpp:89: undefined reference to `testing::internal::MakeAndRegisterTestInfo(char const*, char const*, char const*, char const*, void const*, void (*)(), void (*)(), testing::internal::TestFactoryBase*)'
/home/str4d/dev/zcash/zcash/src/gtest/test_foundersreward.cpp:113: undefined reference to `testing::internal::MakeAndRegisterTestInfo(char const*, char const*, char const*, char const*, void const*, void (*)(), void (*)(), testing::internal::TestFactoryBase*)'
/home/str4d/dev/zcash/zcash/src/gtest/test_foundersreward.cpp:121: undefined reference to `testing::internal::MakeAndRegisterTestInfo(char const*, char const*, char const*, char const*, void const*, void (*)(), void (*)(), testing::internal::TestFactoryBase*)'
/home/str4d/dev/zcash/zcash/src/gtest/test_foundersreward.cpp:129: undefined reference to `testing::internal::MakeAndRegisterTestInfo(char const*, char const*, char const*, char const*, void const*, void (*)(), void (*)(), testing::internal::TestFactoryBase*)'
/home/str4d/dev/zcash/zcash/src/gtest/test_foundersreward.cpp:138: undefined reference to `testing::internal::MakeAndRegisterTestInfo(char const*, char const*, char const*, char const*, void const*, void (*)(), void (*)(), testing::internal::TestFactoryBase*)'
gtest/zcash_gtest-test_foundersreward.o:/home/str4d/dev/zcash/zcash/src/gtest/test_foundersreward.cpp:171: more undefined references to `testing::internal::MakeAndRegisterTestInfo(char const*, char const*, char const*, char const*, void const*, void (*)(), void (*)(), testing::internal::TestFactoryBase*)' follow
collect2: error: ld returned 1 exit status
Makefile:3160: recipe for target 'zcash-gtest' failed
make[2]: *** [zcash-gtest] Error 1

@zkbot
Copy link
Contributor

zkbot commented Mar 25, 2017

☔ The latest upstream changes (presumably #2176) made this pull request unmergeable. Please resolve the merge conflicts.

@nathan-at-least nathan-at-least changed the title [WIP] Upgrade googletest to 1.8.0 Upgrade googletest to 1.8.0 Apr 5, 2017
@daira
Copy link
Contributor

daira commented Apr 9, 2017

The linker command that fails has -lgmock but not -lgtest. (It should be near the end.)

Since 1.8.0, the googletest and googlemock repositories have merged.

Author: Jack Grigg <str4d@z.cash>
Signed-off-by: Daira Hopwood <daira@jacaranda.org>
@daira
Copy link
Contributor

daira commented Nov 15, 2017

@zkbot try

@zkbot
Copy link
Contributor

zkbot commented Nov 15, 2017

⌛ Trying commit 561025d with merge d07d922...

zkbot added a commit that referenced this pull request Nov 15, 2017
Upgrade googletest to 1.8.0

Since 1.8.0, the googletest and googlemock repositories have merged.

Closes #1432
@daira daira requested a review from bitcartel November 15, 2017 11:09
Copy link
Contributor

@daira daira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK and rebased.

@zkbot
Copy link
Contributor

zkbot commented Nov 15, 2017

💔 Test failed - pr-try

@daira
Copy link
Contributor

daira commented Nov 15, 2017

I'm hitting this when running ./qa/zcash/full_test_suite.py libsnark locally:

g++ -o /home/daira/zecc/str4d/depends/x86_64-unknown-linux-gnu/lib/gtest-all.o   -I /usr/src/gtest -c -isystem /usr/src/gtest/include /usr/src/gtest/src/gtest-all.cc -fPIC -DBINARY_OUTPUT -DNO_PT_COMPRESSION=1 -std=c++11 -Wall -Wextra -Wno-unused-parameter -Wno-comment -Wfatal-errors -O2 -march=x86-64 -DMONTGOMERY_OUTPUT -DCURVE_ALT_BN128 -I/home/daira/zecc/str4d/depends/x86_64-unknown-linux-gnu/include -Isrc -DNO_PROCPS -static -DSTATIC -DMULTICORE -fopenmp
In file included from /usr/src/gtest/src/gtest-all.cc:42:0:
/usr/src/gtest/src/gtest.cc: In destructor ‘virtual testing::Test::~Test()’:
/usr/src/gtest/src/gtest.cc:1897:10: error: type ‘const class testing::internal::scoped_ptr<testing::internal::GTestFlagSaver>’ argument given to ‘delete’, expected pointer
   delete gtest_flag_saver_;
          ^
compilation terminated due to -Wfatal-errors.
Makefile:196: recipe for target '/home/daira/zecc/str4d/depends/x86_64-unknown-linux-gnu/lib/libgtest.a' failed
make[1]: *** [/home/daira/zecc/str4d/depends/x86_64-unknown-linux-gnu/lib/libgtest.a] Error 1
make[1]: Leaving directory '/home/daira/zecc/str4d/src/snark'

Looks to be similar to https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=844896 .

@daira
Copy link
Contributor

daira commented Nov 15, 2017

Maybe related to this commit: google/googletest@0928adb

@daira
Copy link
Contributor

daira commented Nov 15, 2017

Oh!

In file included from /usr/src/gtest/src/gtest-all.cc:42:0:

It's picking up my system gtest (or a mixture of different source files/headers), not the one built by the depends system :-(

@daira
Copy link
Contributor

daira commented Nov 15, 2017

sudo apt-get install --upgrade googletest worked around my problem, although obviously this is not fixing the underlying problem. (The version installed by apt-get was libgtest-dev_1.8.0-6_amd64.)

@daira
Copy link
Contributor

daira commented Nov 15, 2017

I think I see what's going on. In the libsnark testing build step, libgtest is either not being built or not being found by the linker. On my VM, gtest is installed globally so that version is picked up by the build (causing the gtest_flag_saver_ problem until I upgraded). On the CI workers that fail, gtest is not installed globally so the link fails.

@str4d str4d added this to the 1.0.14 milestone Nov 21, 2017
@daira
Copy link
Contributor

daira commented Nov 25, 2017

Superceded by #2751.

@daira daira closed this Nov 25, 2017
@str4d str4d deleted the 1432-upgrade-gtest branch November 27, 2017 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build Area: Build system A-dependencies Area: Dependencies A-testing Area: Tests and testing infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants