-
Notifications
You must be signed in to change notification settings - Fork 74
gtest: vendor and update to 1.7.0 #8
base: master
Are you sure you want to change the base?
Conversation
…oogletest/downloads/list no changes to existing code
…ored gtest-all.cpp builds cleanly (minus a couple warnings in the test code itself) on OS X 10.10.4 with “Apple LLVM version 6.1.0 (clang-602.0.53) (based on LLVM 3.6.0svn)”
previously, this was emitting the following warning (treated as error): `c++ -g -Wall -Werror -I. -Igtest-1.7.0/include -o capn-test.o -c capn-test.cpp capn-test.cpp:66:29: error: unused variable 'SUBSTRUCT_DEFAULT' [-Werror,-Wunused-const-variable] static const AlignedData<2> SUBSTRUCT_DEFAULT = {{0,0,0,0,1,0,0,0, 0,0,0,0,0,0,0,0}}; ^ capn-test.cpp:67:29: error: unused variable 'STRUCTLIST_ELEMENT_SUBSTRUCT_DEFAULT' [-Werror,-Wunused-const-variable] static const AlignedData<2> STRUCTLIST_ELEMENT_SUBSTRUCT_DEFAULT = ^`
Original build log @ https://gist.github.com/e7e563b1981170236562 |
@kylemanna - i've also included these fixes (slightly different from yours), and gotten an auto build setup at https://github.com/liamstask/c-capnproto in case you are interested in collaborating. based on msgs on the capnproto mailing list, i'm not sure this repo is likely to be updated any time soon, but would definitely hope to merge changes back in should that change in the future. |
@liamstask I took a look at your repo, but it still has a few issues:
Here's the build log: https://gist.github.com/f6e0977da030db7a7971 |
@kylemanna - it looks like that build is not using the same makefiles (or at least compiler flags). the build log i see is at https://travis-ci.org/liamstask/c-capnproto/jobs/74119871 what is your portability concern with gtest? the authors recommend vendoring it per project: https://code.google.com/p/googletest/wiki/FAQ#Why_is_it_not_recommended_to_install_a_pre-compiled_copy_of_Goog |
Your build is using an old version of gcc If you review my build log, you cannot argue with the warnings that are promoted to errors. The code doesn't lie: signed types are mixed and variables are uninitialized. The gcc compiler in your build is 4+ years old and is letting these errors pass by. My branch is more explicit.
The FAQ is interesting. I'd like to avoid the mess. At minimum gtest should be downloaded and extracted by the developer. At best it would be a git submodule pointing to the repository. Dumping 90k lines of code in to this repo with no good way to maintain history and the same clunky process for updating it seems like a bad approach. In the meantime I'll roll the dice with system pre-compiler gtest libs. |
The issue in the compiler flags that didn't look right to me was the fact that it wasn't passing There are indeed some remaining signed/unsigned comparison warnings in the test code raised by newer gcc versions, I'll definitely apply fixes for those - thanks for the heads up. For gtest, I'm not sure vendoring it in repo is so bad - why do you say there's no way to maintain history for it? Updating it is pretty simple, and should only need to happen very infrequently, if ever. |
I wasn't totally sure what the intention was, since it appeared the gtest sources had been combined into a single file, and were still referencing includes that were out of tree. Seems like the easiest way to ensure that the project is buildable out of the box is just to vendor the unmodified gtest release. This should make it easier to set up an autobuilder (travis-ci.org or similar) as well.
Let me know if you'd like to re-org this in any way, happy to arrange it to your liking.