| Message ID | 20210802123300.564709-1-arnout@mind.be |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [for-next] package/gtest: remove partial host package | expand |
Am Mo., 2. Aug. 2021 um 14:33 Uhr schrieb Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>: > > gmock (which is part of the gtest package) has a host variant that is > not actually the full host package, but instead just installs a single > python script. > > This script, however, is no longer maintained and is not needed for most > practical uses of gmock. Even if it is used, its output is meant to be > hand-edited, so it shouldn't be used as part of the build flow. > > Therefore, remove the host-gtest package. A proper, full host package > may make sense (for building other host packages that use gtest), but > this single script really doesn't. > > Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> > Cc: Norbert Lange <nolange79@gmail.com> > --- > Since we're so close to the 2021.08 release, this should probably be > delayed a little. > --- > package/gtest/gtest.mk | 27 --------------------------- > 1 file changed, 27 deletions(-) > > diff --git a/package/gtest/gtest.mk b/package/gtest/gtest.mk > index 6447954e36..a9ac49957b 100644 > --- a/package/gtest/gtest.mk > +++ b/package/gtest/gtest.mk > @@ -13,23 +13,6 @@ GTEST_LICENSE_FILES = LICENSE > GTEST_CPE_ID_VENDOR = google > GTEST_CPE_ID_PRODUCT = google_test > > -ifeq ($(BR2_PACKAGE_GTEST_GMOCK),y) > -GTEST_DEPENDENCIES += host-gtest > -endif > - > -HOST_GTEST_LICENSE = Apache-2.0 > -HOST_GTEST_LICENSE_FILES = googlemock/scripts/generator/LICENSE > -ifeq ($(BR2_PACKAGE_PYTHON3),y) > -HOST_GTEST_PYTHON_VERSION = $(PYTHON3_VERSION_MAJOR) > -HOST_GTEST_DEPENDENCIES += host-python3 > -else > -HOST_GTEST_PYTHON_VERSION = $(PYTHON_VERSION_MAJOR) > -HOST_GTEST_DEPENDENCIES += host-python > -endif > - > -HOST_GTEST_GMOCK_PYTHONPATH = \ > - $(HOST_DIR)/lib/python$(HOST_GTEST_PYTHON_VERSION)/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 > # gtest. > @@ -52,14 +35,4 @@ else > GTEST_CONF_OPTS += -DBUILD_GMOCK=OFF > endif > > -define HOST_GTEST_INSTALL_CMDS > - $(INSTALL) -D -m 0755 $(@D)/googlemock/scripts/generator/gmock_gen.py \ > - $(HOST_DIR)/bin/gmock_gen > - cp -rp $(@D)/googlemock/scripts/generator/cpp \ > - $(HOST_GTEST_GMOCK_PYTHONPATH) > -endef > - > $(eval $(cmake-package)) add this here: $(eval $(host-cmake-package)) It works, and I see no particular reason not to allow the host package. > -# 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)) > -- > 2.31.1 > With this change the patch is identical to my local version, and thus Reviewed-by: Norbert Lange <nolange79@gmail.com> Norbert
On 02/08/2021 15:09, Norbert Lange wrote: > Am Mo., 2. Aug. 2021 um 14:33 Uhr schrieb Arnout Vandecappelle > (Essensium/Mind) <arnout@mind.be>: >> >> gmock (which is part of the gtest package) has a host variant that is >> not actually the full host package, but instead just installs a single >> python script. >> >> This script, however, is no longer maintained and is not needed for most >> practical uses of gmock. Even if it is used, its output is meant to be >> hand-edited, so it shouldn't be used as part of the build flow. >> >> Therefore, remove the host-gtest package. A proper, full host package >> may make sense (for building other host packages that use gtest), but >> this single script really doesn't. >> >> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> >> Cc: Norbert Lange <nolange79@gmail.com> >> --- >> Since we're so close to the 2021.08 release, this should probably be >> delayed a little. >> --- >> package/gtest/gtest.mk | 27 --------------------------- >> 1 file changed, 27 deletions(-) >> >> diff --git a/package/gtest/gtest.mk b/package/gtest/gtest.mk >> index 6447954e36..a9ac49957b 100644 >> --- a/package/gtest/gtest.mk >> +++ b/package/gtest/gtest.mk >> @@ -13,23 +13,6 @@ GTEST_LICENSE_FILES = LICENSE >> GTEST_CPE_ID_VENDOR = google >> GTEST_CPE_ID_PRODUCT = google_test >> >> -ifeq ($(BR2_PACKAGE_GTEST_GMOCK),y) >> -GTEST_DEPENDENCIES += host-gtest >> -endif >> - >> -HOST_GTEST_LICENSE = Apache-2.0 >> -HOST_GTEST_LICENSE_FILES = googlemock/scripts/generator/LICENSE >> -ifeq ($(BR2_PACKAGE_PYTHON3),y) >> -HOST_GTEST_PYTHON_VERSION = $(PYTHON3_VERSION_MAJOR) >> -HOST_GTEST_DEPENDENCIES += host-python3 >> -else >> -HOST_GTEST_PYTHON_VERSION = $(PYTHON_VERSION_MAJOR) >> -HOST_GTEST_DEPENDENCIES += host-python >> -endif >> - >> -HOST_GTEST_GMOCK_PYTHONPATH = \ >> - $(HOST_DIR)/lib/python$(HOST_GTEST_PYTHON_VERSION)/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 >> # gtest. >> @@ -52,14 +35,4 @@ else >> GTEST_CONF_OPTS += -DBUILD_GMOCK=OFF >> endif >> >> -define HOST_GTEST_INSTALL_CMDS >> - $(INSTALL) -D -m 0755 $(@D)/googlemock/scripts/generator/gmock_gen.py \ >> - $(HOST_DIR)/bin/gmock_gen >> - cp -rp $(@D)/googlemock/scripts/generator/cpp \ >> - $(HOST_GTEST_GMOCK_PYTHONPATH) >> -endef >> - >> $(eval $(cmake-package)) > > add this here: > $(eval $(host-cmake-package)) > > It works, and I see no particular reason not to allow the host package. We only allow host packages that are either a dependency of an in-tree package, or have a Config.in.host entry. Otherwise, it's basically dead code. 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)) >> -- >> 2.31.1 >> > > With this change the patch is identical to my local version, and thus > Reviewed-by: Norbert Lange <nolange79@gmail.com> > > Norbert >
Hello Norbert, On Mon, 2 Aug 2021 15:09:06 +0200 Norbert Lange <nolange79@gmail.com> wrote: > > $(eval $(cmake-package)) > > add this here: > $(eval $(host-cmake-package)) > > It works, and I see no particular reason not to allow the host package. > > > -# 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)) > > -- > > 2.31.1 > > > > With this change the patch is identical to my local version, and thus > Reviewed-by: Norbert Lange <nolange79@gmail.com> Are you saying that you are using the gmock_gen script in your workflow? If so, how does that fit with the commit description from Arnout, which says: This script, however, is no longer maintained and is not needed for most practical uses of gmock. Even if it is used, its output is meant to be hand-edited, so it shouldn't be used as part of the build flow. Thanks for your feedback, Thomas
Am Do., 19. Aug. 2021 um 23:31 Uhr schrieb Thomas Petazzoni <thomas.petazzoni@bootlin.com>: > > Hello Norbert, > > On Mon, 2 Aug 2021 15:09:06 +0200 > Norbert Lange <nolange79@gmail.com> wrote: > > > > $(eval $(cmake-package)) > > > > add this here: > > $(eval $(host-cmake-package)) > > > > It works, and I see no particular reason not to allow the host package. > > > > > -# 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)) > > > -- > > > 2.31.1 > > > > > > > With this change the patch is identical to my local version, and thus > > Reviewed-by: Norbert Lange <nolange79@gmail.com> > > Are you saying that you are using the gmock_gen script in your > workflow? If so, how does that fit with the commit description from > Arnout, which says: I am saying that I agree with the patch, not that I use the script. > > This script, however, is no longer maintained and is not needed for most > practical uses of gmock. Even if it is used, its output is meant to be > hand-edited, so it shouldn't be used as part of the build flow. My original patch [2] did only install the script if python2/3 was built with buildroot. I had a similar commit message then, suggesting the removal. The script is only used to create some boilerplate code automatically, but any reference of it is already removed from the docs [1]. To be clear, this was used manually and only one time when writing a test. Given that the script is old and unmaintained, C++ is evolving, its only a matter of time when parsing using this script turns into a hit-and-miss. Norbert [1] - https://github.com/google/googletest/blob/master/docs/gmock_for_dummies.md#writing-the-mock-class [2] - http://lists.busybox.net/pipermail/buildroot/2021-July/616335.html
Arnout, All, On 2021-08-02 14:33 +0200, Arnout Vandecappelle (Essensium/Mind) spake thusly: > gmock (which is part of the gtest package) has a host variant that is > not actually the full host package, but instead just installs a single > python script. > > This script, however, is no longer maintained and is not needed for most > practical uses of gmock. Even if it is used, its output is meant to be > hand-edited, so it shouldn't be used as part of the build flow. > > Therefore, remove the host-gtest package. A proper, full host package > may make sense (for building other host packages that use gtest), but > this single script really doesn't. > > Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> > Cc: Norbert Lange <nolange79@gmail.com> > Reviewed-by: Norbert Lange <nolange79@gmail.com> In fact, we applied a different patch that makes host-gtests a proper host package; https://git.buildroot.org/buildroot/commit/?id=c0f9e2447d57194f0b02edb93cd16a677ebb326c I marked your patch as rejected, then. Thanks! ;-) Regards, Yann E. MORIN. > --- > Since we're so close to the 2021.08 release, this should probably be > delayed a little. > --- > package/gtest/gtest.mk | 27 --------------------------- > 1 file changed, 27 deletions(-) > > diff --git a/package/gtest/gtest.mk b/package/gtest/gtest.mk > index 6447954e36..a9ac49957b 100644 > --- a/package/gtest/gtest.mk > +++ b/package/gtest/gtest.mk > @@ -13,23 +13,6 @@ GTEST_LICENSE_FILES = LICENSE > GTEST_CPE_ID_VENDOR = google > GTEST_CPE_ID_PRODUCT = google_test > > -ifeq ($(BR2_PACKAGE_GTEST_GMOCK),y) > -GTEST_DEPENDENCIES += host-gtest > -endif > - > -HOST_GTEST_LICENSE = Apache-2.0 > -HOST_GTEST_LICENSE_FILES = googlemock/scripts/generator/LICENSE > -ifeq ($(BR2_PACKAGE_PYTHON3),y) > -HOST_GTEST_PYTHON_VERSION = $(PYTHON3_VERSION_MAJOR) > -HOST_GTEST_DEPENDENCIES += host-python3 > -else > -HOST_GTEST_PYTHON_VERSION = $(PYTHON_VERSION_MAJOR) > -HOST_GTEST_DEPENDENCIES += host-python > -endif > - > -HOST_GTEST_GMOCK_PYTHONPATH = \ > - $(HOST_DIR)/lib/python$(HOST_GTEST_PYTHON_VERSION)/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 > # gtest. > @@ -52,14 +35,4 @@ else > GTEST_CONF_OPTS += -DBUILD_GMOCK=OFF > endif > > -define HOST_GTEST_INSTALL_CMDS > - $(INSTALL) -D -m 0755 $(@D)/googlemock/scripts/generator/gmock_gen.py \ > - $(HOST_DIR)/bin/gmock_gen > - cp -rp $(@D)/googlemock/scripts/generator/cpp \ > - $(HOST_GTEST_GMOCK_PYTHONPATH) > -endef > - > $(eval $(cmake-package)) > -# 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))
diff --git a/package/gtest/gtest.mk b/package/gtest/gtest.mk index 6447954e36..a9ac49957b 100644 --- a/package/gtest/gtest.mk +++ b/package/gtest/gtest.mk @@ -13,23 +13,6 @@ GTEST_LICENSE_FILES = LICENSE GTEST_CPE_ID_VENDOR = google GTEST_CPE_ID_PRODUCT = google_test -ifeq ($(BR2_PACKAGE_GTEST_GMOCK),y) -GTEST_DEPENDENCIES += host-gtest -endif - -HOST_GTEST_LICENSE = Apache-2.0 -HOST_GTEST_LICENSE_FILES = googlemock/scripts/generator/LICENSE -ifeq ($(BR2_PACKAGE_PYTHON3),y) -HOST_GTEST_PYTHON_VERSION = $(PYTHON3_VERSION_MAJOR) -HOST_GTEST_DEPENDENCIES += host-python3 -else -HOST_GTEST_PYTHON_VERSION = $(PYTHON_VERSION_MAJOR) -HOST_GTEST_DEPENDENCIES += host-python -endif - -HOST_GTEST_GMOCK_PYTHONPATH = \ - $(HOST_DIR)/lib/python$(HOST_GTEST_PYTHON_VERSION)/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 # gtest. @@ -52,14 +35,4 @@ else GTEST_CONF_OPTS += -DBUILD_GMOCK=OFF endif -define HOST_GTEST_INSTALL_CMDS - $(INSTALL) -D -m 0755 $(@D)/googlemock/scripts/generator/gmock_gen.py \ - $(HOST_DIR)/bin/gmock_gen - cp -rp $(@D)/googlemock/scripts/generator/cpp \ - $(HOST_GTEST_GMOCK_PYTHONPATH) -endef - $(eval $(cmake-package)) -# 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))
gmock (which is part of the gtest package) has a host variant that is not actually the full host package, but instead just installs a single python script. This script, however, is no longer maintained and is not needed for most practical uses of gmock. Even if it is used, its output is meant to be hand-edited, so it shouldn't be used as part of the build flow. Therefore, remove the host-gtest package. A proper, full host package may make sense (for building other host packages that use gtest), but this single script really doesn't. Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> Cc: Norbert Lange <nolange79@gmail.com> --- Since we're so close to the 2021.08 release, this should probably be delayed a little. --- package/gtest/gtest.mk | 27 --------------------------- 1 file changed, 27 deletions(-)