> On Aug 15, 2019, at 3:22 PM, Christian Gruber <christian.gru...@posteo.de> 
> wrote:
> 
> Am 15.08.19 um 00:23 schrieb Christian Gruber:
>> 
>>> I think our distro packagers would object to any option other than relying 
>>> on tools outside of the build system to provide the googletest sources and 
>>> maybe a prebuilt static lib. It's a one-off for the casual builder and 
>>> easily scripted for everyone else. Building gtest is also sufficiently 
>>> trivial that it's not all that interesting to call out to guests own 
>>> CMakeLists.txt instead of just building the two libraries in ours. OTOH 
>>> it's possible that might change at some point, so
>> 
>> Ok, so you want to keep build process as it is at the moment. In this case 
>> at least bug 797344 should be fixed. I'll provide a PR. And I recommend to 
>> use library target gtest directly instead of variables GTEST_LIB, 
>> GTEST_INCLUDE_DIR as stated above. The same I recommend for library target 
>> gmock. I'll add this to the same PR.
>> 
>> Finally I recommend to add a few more notes on the Wiki page 
>> https://wiki.gnucash.org/wiki/Google_Test. For me not familiar with the 
>> GnuCash build system it was not obvious at the beginning, that gtest is 
>> built inside GnuCash build system, because this is a little bit unusual. It 
>> could for instance be explained, how to build gtest and gmock from inside 
>> GnuCash build system via "make gtest" and "make gmock" and that this way one 
>> can test the correct CMake configuration for GnuCash regarding variables 
>> GTEST_ROOT and GMOCK_ROOT. The Wiki page only explains how to "test the 
>> installation" from inside the GoogleTest build system and tells that this is 
>> "not used in practice". But this doesn't really test the CMake configuration 
>> for GnuCash.
>> 
>>> 
>>> I don't think anyone's actually tried doing a mock with GMock yet. There 
>>> are hand-rolled ones in the QOF tests, but they use the old Glib test 
>>> facility. Most of the C++ work so far has been at the lowest levels so that 
>>> the C++ classes don't have any mockable dependencies. That will change when 
>>> we get to redoing the engine as we'll want to use mock for at least the 
>>> backend. We don't want to remove GMock from the dependencies.
>>> 
>>> I agree about libgtest_main.a. Would you like to make a PR to remove all of 
>>> the inclusion of gtest_main.cc and GTEST_SRC?
>> I opened PR https://github.com/Gnucash/gnucash/pull/552.
>>> 
>>> Regards,
>>> John Ralls
>>> 
>> Regards,
>> Christian
>> 
> I just realized now, after PR https://github.com/Gnucash/gnucash/pull/552 
> failed to build, that there is a significant difference between using 
> prebuilt GoogleTest libraries and building GoogleTest from source repository. 
> In the latter case libgtest_main.a doesn't exist at all, it isn't built 
> anywhere. And GTEST_LIB contains only libgtest.a in this case. Therefore I 
> created a new pull request https://github.com/Gnucash/gnucash/pull/555, which 
> removes GTEST_SRC and additionally adds source file gtest_main.cc to the 
> sources of libgtest.a. I also thought about creating a second target 
> gtest_main for building libgtest_main.a as in GoogleTest source repository, 
> but in the end I thought that's exaggerated, because both libraries are 
> always used in combination in GnuCash.

That's fine, and I've merged your PR.

Regards,
John Ralls

_______________________________________________
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Reply via email to