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

Google test question #206

Closed
CaitlinA opened this issue Jul 4, 2018 · 11 comments
Closed

Google test question #206

CaitlinA opened this issue Jul 4, 2018 · 11 comments
Assignees
Labels
Milestone

Comments

@CaitlinA
Copy link
Member

CaitlinA commented Jul 4, 2018

Has anything changed about the way we connect to or use the google test submodule?

Previously, I have cloned and/or checked out any branch on SOILWAT2 and google test would work after the 'make test' command.

I just clean cloned SOILWAT2 and 'make test' is not working. I used the command 'git submodule init' and 'git submodule update --remote'. I received this error:

MacBook-Pro-4:SOILWAT2 candrews$ make test
c++ -Wall -Wextra -Werror -std=gnu++11 -isystem googletest/googletest/include -Igoogletest/googletest
-pthread -c googletest/googletest/src/gtest-all.cc
In file included from googletest/googletest/src/gtest-all.cc:42:
googletest/googletest/src/gtest.cc:393:12: error: missing field 'owner_' initializer [-Werror,-Wmissing-field-initializers]
GTEST_API_ GTEST_DEFINE_STATIC_MUTEX_(g_linked_ptr_mutex);
^
googletest/googletest/include/gtest/internal/gtest-port.h:2096:80: note: expanded from macro 'GTEST_DEFINE_STATIC_MUTEX_'
::testing::internal::MutexBase mutex = { PTHREAD_MUTEX_INITIALIZER, false }
^
1 error generated.
make: *** [libgtest.a] Error 1

Maybe I am forgetting something. I can't find anything in the repository documentation.

@dschlaep
Copy link
Member

dschlaep commented Jul 5, 2018

This is weird; I don't get this locally or see it on the CIs. We updated the submodule googletest a few days ago (commit c159ebe): it was updated to commit google/googletest@ba96d0b

Can you please check that you have the correct commit and a clean submodule by running

cd googletest/
git status

and making sure that you only get

HEAD detached at ba96d0b
nothing to commit, working tree clean

@dschlaep
Copy link
Member

dschlaep commented Jul 9, 2018

@CaitlinA What is the status on this issue?

@CaitlinA
Copy link
Member Author

CaitlinA commented Jul 9, 2018

@dschlaep

The googletest folder is pointing to the correct place

MacBook-Pro-4:SOILWAT2 candrews$ cd googletest/
MacBook-Pro-4:googletest candrews$ git status
HEAD detached at ba96d0b
nothing to commit, working tree clean

I am still unable to run units tests and get the same error as above.

dschlaep added a commit that referenced this issue Jul 9, 2018
…tain setups

- address issue #206:
```
In file included from googletest/googletest/src/gtest-all.cc:42:
googletest/googletest/src/gtest.cc:393:12: error: missing field 'owner_' initializer [-Werror,-Wmissing-field-initializers]
GTEST_API_ GTEST_DEFINE_STATIC_MUTEX_(g_linked_ptr_mutex);
```
- this appears to be a problem described here google/googletest#1521 which remains unsolved as of today

- another project on github addressed this issue temporarily (advancedtelematic/aktualizr#841) by checking out `googletest` one commit before this problem was introduced with commit google/googletest@87a4cdd, i.e, google/googletest@7888184
@dschlaep
Copy link
Member

dschlaep commented Jul 9, 2018

Hm, too bad. I cannot replicate and thus cannot debug very well. However, I found an issue description on googletest google/googletest#1521 which appears to be the same as yours.

Another project on github addresses this issue temporarily (advancedtelematic/aktualizr#841) by checking out one commit before this problem was introduced with commit google/googletest@87a4cdd, i.e, google/googletest@7888184 (see branch https://github.com/DrylandEcology/SOILWAT2/tree/bugfix_206 and commit 5baeec0). Does this work for you?

@CaitlinA
Copy link
Member Author

CaitlinA commented Jul 9, 2018

The problem persists with commit 5baeec0

Additionally, I manually applied the suggested edit in google/googletest#1521 and I now get a different error:

MacBook-Pro-4:SOILWAT2 candrews$ make test
c++ -Wall -Wextra -Werror -std=gnu++11 -isystem googletest/googletest/include -Igoogletest/googletest
-pthread -c googletest/googletest/src/gtest-all.cc
ar -r libgtest.a gtest-all.o
ar: creating archive libgtest.a
c++ -Wall -Wextra -Werror -g -O0 -DSWDEBUG -fstack-protector-all -std=gnu++11 -D_FORTIFY_SOURCE=2 -fsanitize=undefined -fsanitize=address -c SW_Main_lib.c SW_VegEstab.c SW_Control.c generic.c rands.c Times.c mymemory.c filefuncs.c SW_Files.c SW_Model.c SW_Site.c SW_SoilWater.c SW_Markov.c SW_Weather.c SW_Sky.c SW_Output_mock.c SW_VegProd.c SW_Flow_lib.c SW_Flow.c SW_Carbon.c
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
clang: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
make: *** [libSOILWAT2++.a] Error 1

@dschlaep
Copy link
Member

dschlaep commented Jul 9, 2018

  • I now figured out how to replicate that error; by using clang++ as the compiler, i.e.,
CXX=clang++ make cleaner test test_run

however, commit 5baeec0 resolved this error.

  • Weird that the commit 5baeec0 didn't resolve it for you, but when you manually applied the change then it solved it -- weird because the commit is supposed to do just that. Are you sure that checking out the commit brought along the correct commit of googletest in your local copy? But it confirms that this was the problem.

  • One recommendation to address the C++ mode error that you are now seeing is to separately compile C files with a c compiler and C++ files with a c++ compiler and then link them in a second step (e.g., https://stackoverflow.com/questions/19644816/linking-c-from-c-in-os-x-mavericks). You could have Nathan to try this out by re-writing the makefile locally; our makefile is almost set up like that except that we compile the SOILWAT2 library for the test binary with the c++ compiler instead the c compiler.

  • You could switch to the gnu c++ compiler, e.g,

CXX=g++ make cleaner test test_run
  • You could turn off converting the errors into warnings in your local clone, i.e., remove -Werror from the makefile (line 36).

@dschlaep dschlaep added this to the Code testing milestone Jul 10, 2018
dschlaep added a commit that referenced this issue Jul 10, 2018
- addressing #206: both clang and gcc work (at least locally)
* CXX=clang++ make clean test test_run
* CXX=g++ make clean test test_run

- don't overwrite standard flags such as CFLAGS, etc.
- ignore errors when cleaning up (prefix commands with a hyphen)
- ignore errors arising from deprecated warnings (as c code under clang++) --> remove once #208 is fixed

- target `clean` is now a synonym of `cleaner`
- first target is now `bin`, i.e., calling `make` will now compile the executable

- updated `SW_Output_mock.c`
@dschlaep
Copy link
Member

@CaitlinA Ok, I took the opportunity to clean up the makefile and with commit 87aea18 you should be able to compile without changing anything (as suggested here #206 (comment)). You will see the warnings, but they are no longer converted to errors (this is now a separate issue #208). I will merge this branch into master and close this issue once you confirm that this is resolved.

@CaitlinA
Copy link
Member Author

Hi Daniel,

Thanks for looking at this. I switched to commit 87aea18 and confirmed the googletest submodule is on the master branch (as indicated in the .gitmodules file. Is this correct?). When I run make test, I now receive a warning instead of an error for treating 'c' as 'c++', but now I am running into another error.

c++ -Wno-error=deprecated -g -O0 -DSWDEBUG -Wall -Wextra
-D_FORTIFY_SOURCE=2 -Wno-macro-redefined -fsanitize=undefined -fsanitize=address -std=gnu++11
-isystem googletest/googletest/include -pthread
test/*.cc -o sw_test -lgtest -lSOILWAT2_severe -lm -L.
Undefined symbols for architecture x86_64:
"___ubsan_handle_dynamic_type_cache_miss", referenced from:
RUN_ALL_TESTS() in sw_maintest-53839e.o
testing::internal::TestFactoryImpl<(anonymous namespace)::CarbonTest_Constructor_Test>::~TestFactoryImpl() in test_SW_Carbon-066d1f.o
testing::internal::TestFactoryImpl<(anonymous namespace)::CarbonTest_Constructor_Test>::~TestFactoryImpl() in test_SW_Carbon-066d1f.o
testing::internal::TestFactoryBase::~TestFactoryBase() in test_SW_Carbon-066d1f.o
testing::internal::TestFactoryBase::~TestFactoryBase() in test_SW_Carbon-066d1f.o
testing::internal::TestFactoryImpl<(anonymous namespace)::CarbonTest_Constructor_Test>::~TestFactoryImpl() in test_SW_Carbon-066d1f.o
(anonymous namespace)::CarbonTest_Constructor_Test::~CarbonTest_Constructor_Test() in test_SW_Carbon-066d1f.o
...
"___ubsan_vptr_type_cache", referenced from:
RUN_ALL_TESTS() in sw_maintest-53839e.o
testing::internal::TestFactoryImpl<(anonymous namespace)::CarbonTest_Constructor_Test>::~TestFactoryImpl() in test_SW_Carbon-066d1f.o
testing::internal::TestFactoryImpl<(anonymous namespace)::CarbonTest_Constructor_Test>::~TestFactoryImpl() in test_SW_Carbon-066d1f.o
testing::internal::TestFactoryBase::~TestFactoryBase() in test_SW_Carbon-066d1f.o
testing::internal::TestFactoryBase::~TestFactoryBase() in test_SW_Carbon-066d1f.o
testing::internal::TestFactoryImpl<(anonymous namespace)::CarbonTest_Constructor_Test>::~TestFactoryImpl() in test_SW_Carbon-066d1f.o
(anonymous namespace)::CarbonTest_Constructor_Test::~CarbonTest_Constructor_Test() in test_SW_Carbon-066d1f.o
...
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

I tried running with g++ but I received the same error.

@dschlaep
Copy link
Member

Which version of clang are you using? You may have an outdated version: this was a bug in clang that was fixed in 2016 (https://reviews.llvm.org/D24990 as explained here https://marc.info/?l=cfe-dev&m=147517413909285&w=2), but Apple has been shipping outdated clang versions, see https://stackoverflow.com/questions/40284307/undefined-symbols-when-using-clangs-undefined-sanitizer -- apparently, you need at least the toolchain from xcode 9.0 (https://developer.apple.com/documentation/code_diagnostics/undefined_behavior_sanitizer) for the -fsanitize=undefined to work with clang.

--> use target testci instead of test -- the latter uses sanitizers which apparently don't work on your setup which the former doesn't use, i.e.,

make clean testci test_run

instead of

make clean test_run

@CaitlinA
Copy link
Member Author

Running

make clean testci test_run

did allow for the tests to build and run when I was receiving the same errors as above. Luckily, updating my Command Line Tools for Xcode 9.2 has allowed me to build run tests with the normal

make clean test_run

I think we can close this issue and merge your pull request for branch bugfix_206.

@dschlaep
Copy link
Member

closed by PR #209

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants