[Buildroot] [PATCH v2] gtest/gmock: bump to version 1.8.0

Romain Naour romain.naour at gmail.com
Mon Feb 6 15:43:17 UTC 2017


Hi Carlos, Fabrice, All,

I removed the last remaining gtest related patch [1] from patchwork since the
gtest/gmock 1.8.0 is now available.

What's the status of this patch bump?
Can you send an updated version?

Thanks!

Best regards,
Romain

[1] http://patchwork.ozlabs.org/patch/667729/

Le 11/09/2016 à 14:09, Arnout Vandecappelle a écrit :
>  Big patch, so lots of comments on this one...
> 
> 
> On 08-09-16 01:16, Carlos Santos wrote:
>> From: Fabrice Fontaine <fontaine.fabrice at gmail.com>
>>
>> Version 1.8.0 now includes gmock so merge gmock package inside gtest.
>>
>> In this merge:
>>  - Add gmock as a suboption of gtest (BR2_PACKAGE_GTEST_GMOCK)
>>    following advice from Arnout Vandecappelle
>>  - Make gmock a virtual package that selects BR2_PACKAGE_GTEST and
>>    BR2_PACKAGE_GTEST_GMOCK, to provide backward compatibility
> 
>  I don't think this is a good idea.
> 
> * It means we have to keep on supporting the BR2_PACKAGE_GMOCK option for some
> time. Let's say two years down the line we decide to remove it, it means we have
> to move it to legacy at that time. So long term, we gain exactly nothing.
> * The legacy approach works perfectly. The Config.in.legacy entry will make sure
> that the package remains selected. If an external package depends on gmock,
> you'll get an immediate build termination because make doesn't know how to build
> gmock. Since the user has just removed gmock from the legacy menu, it should be
> quite obvious what needs to be done.
> * This is abusing the virtual package infra for something it wasn't meant for.
> We want to avoid adding bad examples to buildroot.
> 
> 
>>  - Use cmake to install libraries and headers and add missing files
>>    (gtest.pc, gtest-config, gmock.pc) in
>>    GTEST_POST_INSTALL_STAGING_HOOKS instead of redefining
>>    GTEST_INSTALL_STAGING_CMDS
>>  - Remove patch on Python as gmock/gtest now supports python 3.0
>>    (commit 456fc2b5c4e9ebf05a5987dfe1ff0ac9ffeb53cc)
>>  - Add the correct license in HOST_GTEST_LICENSE as all python code in
>>    googlemock/scripts/generator is licensed under Apache-2.0 and not
>>    BSD-3c
>>  - Fix URL of gtest project in Config.in
>>
>> Signed-off-by: Fabrice Fontaine <fabrice.fontaine at orange.com>
>> Signed-off-by: Carlos Santos <casantos at datacom.ind.br>
>>
>> ---
>> Changes v1 -> v2
>>   - Add mirtual package for host-gmock, as pointed-out by Arnout
>>     Vandecappelle.
> 
>  Well, no, I never mentioned virtual-package, I said to add a legacy entry.
> 
>>
>> Signed-off-by: Carlos Santos <casantos at datacom.ind.br>
>> ---
>>  package/gmock/0001-force-use-python2.patch | 20 ----------
>>  package/gmock/Config.in                    | 26 +++++--------
>>  package/gmock/gmock.hash                   |  2 -
>>  package/gmock/gmock.mk                     | 60 +-----------------------------
>>  package/gtest/Config.in                    | 28 +++++++++++++-
>>  package/{gmock => gtest}/gmock.pc          |  2 +-
>>  package/gtest/gtest.hash                   |  2 +-
>>  package/gtest/gtest.mk                     | 52 +++++++++++++++++++++-----
>>  package/gtest/gtest.pc                     |  2 +-
>>  9 files changed, 84 insertions(+), 110 deletions(-)
>>  delete mode 100644 package/gmock/0001-force-use-python2.patch
>>  delete mode 100644 package/gmock/gmock.hash
>>  rename package/{gmock => gtest}/gmock.pc (93%)
> [snip]
>> diff --git a/package/gtest/gtest.mk b/package/gtest/gtest.mk
>> index eb30905..249971e 100644
>> --- a/package/gtest/gtest.mk
>> +++ b/package/gtest/gtest.mk
>> @@ -4,13 +4,27 @@
>>  #
>>  ################################################################################
>>  
>> -# Make sure this remains the same version as the gmock one
>> -GTEST_VERSION = release-1.7.0
>> +GTEST_VERSION = release-1.8.0
>>  GTEST_SITE = $(call github,google,googletest,$(GTEST_VERSION))
>>  GTEST_INSTALL_STAGING = YES
>>  GTEST_INSTALL_TARGET = NO
>>  GTEST_LICENSE = BSD-3c
>> -GTEST_LICENSE_FILES = LICENSE
>> +GTEST_LICENSE_FILES = googletest/LICENSE
>> +
>> +ifeq ($(BR2_PACKAGE_GTEST_GMOCK),y)
>> +GTEST_LICENSE_FILES += googlemock/LICENSE
> 
>  It has the same content, so it doesn't make sense adding it. Since it's now a
> single package, a single license file is definitely enough.
> 
>> +GTEST_DEPENDENCIES = host-gtest
>> +
>> +HOST_GTEST_LICENSE = Apache-2.0
> 
>  This is so weird that it deserves a separate comment.
> 
> # HOST_GTEST is just the generator script, which has a different license
> # than the rest of gtest. Also, the generator script is not installed on
> # target so its license doesn't need to be mentioned for the target package.
> 
>> +HOST_GTEST_LICENSE_FILES = googlemock/scripts/generator/LICENSE
>> +HOST_GTEST_DEPENDENCIES = host-python
> 
>  Is the host-python dependency still needed? If python-2.6 doesn't work, yes it
> is, but then there should be a comment. But the README file says "Python 2.3.5
> or later"...
> 
>  All this HOST_GTEST stuff should be defined unconditionally.
> 
>> +else
>> +# By default, gtest also builds gmock
>> +GTEST_SUBDIR = googletest
> 
>  Actually, the top-level CMakeLists has the options BUILD_GTEST and BUILD_GMOCK
> (the latter implies the former in a rather complicated way). It's better to use
> those rather than messing with SUBDIR.
> 
>> +endif
>> +
>> +HOST_GTEST_GMOCK_PYTHONPATH = \
>> +        $(HOST_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)/site-packages
>>  
>>  # While it is possible to build gtest as shared library, using this gtest shared
>>  # library requires to set some special configure option in the project using
>> @@ -21,11 +35,7 @@ GTEST_LICENSE_FILES = LICENSE
>>  # the gtest sources.
>>  GTEST_CONF_OPTS = -DBUILD_SHARED_LIBS=OFF
>>  
>> -define GTEST_INSTALL_STAGING_CMDS
>> -	$(INSTALL) -D -m 0755 $(@D)/libgtest.a $(STAGING_DIR)/usr/lib/libgtest.a
>> -	$(INSTALL) -D -m 0755 $(@D)/libgtest_main.a $(STAGING_DIR)/usr/lib/libgtest_main.a
>> -	$(INSTALL) -d -m 0755 $(STAGING_DIR)/usr/include/gtest/
>> -	cp -rp $(@D)/include/gtest/* $(STAGING_DIR)/usr/include/gtest/
>> +define GTEST_INSTALL_MISSING_FILES
>>  	$(INSTALL) -D -m 0644 package/gtest/gtest.pc \
>>  		$(STAGING_DIR)/usr/lib/pkgconfig/gtest.pc
>>  	# Generate the gtest-config script manually, since the CMake
>> @@ -39,9 +49,33 @@ define GTEST_INSTALL_STAGING_CMDS
>>  		s%@bindir@%$(STAGING_DIR)/usr/bin%;\
>>  		s%@PTHREAD_CFLAGS@%%;\
>>  		s%@PTHREAD_LIBS@%-lpthread%;' \
>> -		$(@D)/scripts/gtest-config.in \
>> +		$(@D)/googletest/scripts/gtest-config.in \
>>  		> $(STAGING_DIR)/usr/bin/gtest-config
>>  	chmod +x $(STAGING_DIR)/usr/bin/gtest-config
>>  endef
>>  
>> +GTEST_POST_INSTALL_STAGING_HOOKS += GTEST_INSTALL_MISSING_FILES
>> +
>> +ifeq ($(BR2_PACKAGE_GTEST_GMOCK),y)
>> +define GTEST_GMOCK_INSTALL_MISSING_FILE
>> +	$(INSTALL) -D -m 0755 package/gtest/gmock.pc \
>> +		$(STAGING_DIR)/usr/lib/pkgconfig/gmock.pc
>> +endef
>> +
>> +GTEST_POST_INSTALL_STAGING_HOOKS += GTEST_GMOCK_INSTALL_MISSING_FILE
>> +
>> +define HOST_GTEST_INSTALL_CMDS
> 
>  This shouldn't be inside the GTEST_GMOCK condition.
> 
>> +	$(INSTALL) -D -m 0755 $(@D)/googlemock/scripts/generator/gmock_gen.py \
>> +		$(HOST_DIR)/usr/bin/gmock_gen.py
>> +	ln -sf gmock_gen.py $(HOST_DIR)/usr/bin/gmock_gen
>> +	cp -rp $(@D)/googlemock/scripts/generator/cpp \
>> +		$(HOST_GTEST_GMOCK_PYTHONPATH)
> 
>  This is just copied from the current implementation, so OK. However, now I look
> like it, this seems a really twisted way of installing this script. The normal
> approach would be
> 
> 	mkdir -p $(HOST_DIR)/usr/lib/gmock
> 	cp -r $(@D)/googlemock/scripts/generator/* $(HOST_DIR)/usr/lib/gmock
> 	ln -sf ../lib/gmock/gmock_gen.py $(HOST_DIR)/usr/bin/gmock_gen
> 
>  I.e. install the script itself together with the libraries it imports, and
> symlink to it from $PATH. This way, the script will find the libraries by itself
> (it extends sys.path with $0).
> 
>  That way, the script is also usable with the system's python. So, unless you
> make this change, the host-python dependency really _is_ needed, otherwise the
> gmock_gen imports won't be found when executing the script.
> 
>> +endef
>> +endif
>> +
>>  $(eval $(cmake-package))
>> +ifeq ($(BR2_PACKAGE_GTEST_GMOCK),y)
> 
>  This should _definitely_ not be in the GTEST_GMOCK condition. We never let the
> package definition itself depend on the config options.
> 
> 
>  Regards,
>  Arnout
> 
>> +# The host package does not build anything, just installs gmock_gen stuff, so
>> +# it does not need to be a host-cmake-package.
>> +$(eval $(host-generic-package))
>> +endif
>> diff --git a/package/gtest/gtest.pc b/package/gtest/gtest.pc
>> index b7a8aa4..594e79d 100644
>> --- a/package/gtest/gtest.pc
>> +++ b/package/gtest/gtest.pc
>> @@ -5,7 +5,7 @@ includedir=${prefix}/include
>>  
>>  Name: gtest
>>  Description: Google C++ Testing Framework
>> -Version: 1.7.0
>> +Version: 1.8.0
>>  Libs: -L${libdir} -lgtest
>>  Libs.private: -lpthread
>>  Cflags: -I${includedir}
>>
> 



More information about the buildroot mailing list