| Message ID | 20220119084545.3642950-1-james.hilliard1@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/1] package/re2: switch to generic-package make build | expand |
James, All, On 2022-01-19 01:45 -0700, James Hilliard spake thusly: > The cmake build appears to be missing features such as pkg-config > generation support, switch to the regular makefile based build > which appears to work better. Can't we just fix the CMakeList.txt and submit a patch upstream, instead? Also, see below... > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > --- > package/re2/re2.mk | 30 ++++++++++++++++++++++++++---- > 1 file changed, 26 insertions(+), 4 deletions(-) > > diff --git a/package/re2/re2.mk b/package/re2/re2.mk > index b562d5d7ef..144c82339f 100644 > --- a/package/re2/re2.mk > +++ b/package/re2/re2.mk > @@ -10,8 +10,30 @@ RE2_LICENSE = BSD-3-Clause > RE2_LICENSE_FILES = LICENSE > RE2_INSTALL_STAGING = YES > > -RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF > -HOST_RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF -DBUILD_SHARED_LIBS=ON > +define RE2_BUILD_CMDS > + $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \ > + -C $(@D) > +endef > > -$(eval $(cmake-package)) > -$(eval $(host-cmake-package)) > +define RE2_INSTALL_STAGING_CMDS > + $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \ > + DESTDIR="$(STAGING_DIR)" prefix=/usr -C $(@D) install > +endef > + > +define RE2_INSTALL_TARGET_CMDS > + $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \ > + DESTDIR="$(TARGET_DIR)" prefix=/usr -C $(@D) install The 'install' target will forcibly build and install static and shared libs: https://github.com/google/re2/blob/main/Makefile#L297 296: .PHONY: install 297: install: static-install shared-install So, this is probably going to not play nicely for builds where a shared build is not possible. AFAICS, the cmake-based buildsystem got that one correct, though, so it would be a bit of a shame to regress on that point... Regards, Yann E. MORIN. > +endef > + > +define HOST_RE2_BUILD_CMDS > + $(HOST_MAKE_ENV) $(MAKE) $(HOST_CONFIGURE_OPTS) \ > + -C $(@D) > +endef > + > +define HOST_RE2_INSTALL_CMDS > + $(HOST_MAKE_ENV) $(MAKE) $(HOST_CONFIGURE_OPTS) \ > + -C $(@D) DESTDIR="$(HOST_DIR)" prefix=/usr install > +endef > + > +$(eval $(generic-package)) > +$(eval $(host-generic-package)) > -- > 2.25.1 > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
On Wed, Jan 19, 2022 at 1:45 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote: > > James, All, > > On 2022-01-19 01:45 -0700, James Hilliard spake thusly: > > The cmake build appears to be missing features such as pkg-config > > generation support, switch to the regular makefile based build > > which appears to work better. > > Can't we just fix the CMakeList.txt and submit a patch upstream, > instead? It looked like this had been brought up before and there was some issue that was making this difficult to fix with cmake. See: https://github.com/google/re2/issues/349 > > Also, see below... > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > --- > > package/re2/re2.mk | 30 ++++++++++++++++++++++++++---- > > 1 file changed, 26 insertions(+), 4 deletions(-) > > > > diff --git a/package/re2/re2.mk b/package/re2/re2.mk > > index b562d5d7ef..144c82339f 100644 > > --- a/package/re2/re2.mk > > +++ b/package/re2/re2.mk > > @@ -10,8 +10,30 @@ RE2_LICENSE = BSD-3-Clause > > RE2_LICENSE_FILES = LICENSE > > RE2_INSTALL_STAGING = YES > > > > -RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF > > -HOST_RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF -DBUILD_SHARED_LIBS=ON > > +define RE2_BUILD_CMDS > > + $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \ > > + -C $(@D) > > +endef > > > > -$(eval $(cmake-package)) > > -$(eval $(host-cmake-package)) > > +define RE2_INSTALL_STAGING_CMDS > > + $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \ > > + DESTDIR="$(STAGING_DIR)" prefix=/usr -C $(@D) install > > +endef > > + > > +define RE2_INSTALL_TARGET_CMDS > > + $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \ > > + DESTDIR="$(TARGET_DIR)" prefix=/usr -C $(@D) install > > The 'install' target will forcibly build and install static and shared > libs: > > https://github.com/google/re2/blob/main/Makefile#L297 > > 296: .PHONY: install > 297: install: static-install shared-install > > So, this is probably going to not play nicely for builds where a shared > build is not possible. Sent a v2 which should fix this: https://patchwork.ozlabs.org/project/buildroot/patch/20220120003827.395469-1-james.hilliard1@gmail.com/ > > AFAICS, the cmake-based buildsystem got that one correct, though, so it > would be a bit of a shame to regress on that point... > > Regards, > Yann E. MORIN. > > > +endef > > + > > +define HOST_RE2_BUILD_CMDS > > + $(HOST_MAKE_ENV) $(MAKE) $(HOST_CONFIGURE_OPTS) \ > > + -C $(@D) > > +endef > > + > > +define HOST_RE2_INSTALL_CMDS > > + $(HOST_MAKE_ENV) $(MAKE) $(HOST_CONFIGURE_OPTS) \ > > + -C $(@D) DESTDIR="$(HOST_DIR)" prefix=/usr install > > +endef > > + > > +$(eval $(generic-package)) > > +$(eval $(host-generic-package)) > > -- > > 2.25.1 > > > > _______________________________________________ > > buildroot mailing list > > buildroot@buildroot.org > > https://lists.buildroot.org/mailman/listinfo/buildroot > > -- > .-----------------.--------------------.------------------.--------------------. > | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | > | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | > | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no | > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | > '------------------------------^-------^------------------^--------------------'
Hello James, Yann, On Wed, 19 Jan 2022 17:42:46 -0700, James Hilliard <james.hilliard1@gmail.com> wrote: > On Wed, Jan 19, 2022 at 1:45 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote: > > > > James, All, > > > > On 2022-01-19 01:45 -0700, James Hilliard spake thusly: > > > The cmake build appears to be missing features such as pkg-config > > > generation support, switch to the regular makefile based build > > > which appears to work better. > > > > Can't we just fix the CMakeList.txt and submit a patch upstream, > > instead? > > It looked like this had been brought up before and there was some issue that was > making this difficult to fix with cmake. > See: > https://github.com/google/re2/issues/349 > > > > > Also, see below... > > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > > --- > > > package/re2/re2.mk | 30 ++++++++++++++++++++++++++---- > > > 1 file changed, 26 insertions(+), 4 deletions(-) > > > > > > diff --git a/package/re2/re2.mk b/package/re2/re2.mk > > > index b562d5d7ef..144c82339f 100644 > > > --- a/package/re2/re2.mk > > > +++ b/package/re2/re2.mk > > > @@ -10,8 +10,30 @@ RE2_LICENSE = BSD-3-Clause > > > RE2_LICENSE_FILES = LICENSE > > > RE2_INSTALL_STAGING = YES > > > > > > -RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF > > > -HOST_RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF -DBUILD_SHARED_LIBS=ON > > > +define RE2_BUILD_CMDS > > > + $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \ > > > + -C $(@D) > > > +endef > > > > > > -$(eval $(cmake-package)) > > > -$(eval $(host-cmake-package)) > > > +define RE2_INSTALL_STAGING_CMDS > > > + $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \ > > > + DESTDIR="$(STAGING_DIR)" prefix=/usr -C $(@D) install > > > +endef > > > + > > > +define RE2_INSTALL_TARGET_CMDS > > > + $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \ > > > + DESTDIR="$(TARGET_DIR)" prefix=/usr -C $(@D) install > > > > The 'install' target will forcibly build and install static and shared > > libs: > > > > https://github.com/google/re2/blob/main/Makefile#L297 > > > > 296: .PHONY: install > > 297: install: static-install shared-install > > > > So, this is probably going to not play nicely for builds where a shared > > build is not possible. > > Sent a v2 which should fix this: > https://patchwork.ozlabs.org/project/buildroot/patch/20220120003827.395469-1-james.hilliard1@gmail.com/ > > > > > AFAICS, the cmake-based buildsystem got that one correct, though, so it > > would be a bit of a shame to regress on that point... > > Or keep the cmake build and do the following to install re2.pc (taken from #349, [1]): --- a/package/re2/re2.mk +++ b/package/re2/re2.mk @@ -9,9 +9,20 @@ RE2_SITE = $(call github,google,re2,$(RE2_VERSION)) RE2_LICENSE = BSD-3-Clause RE2_LICENSE_FILES = LICENSE RE2_INSTALL_STAGING = YES +# keep original Makefile (for re2.pc install) +RE2_SUPPORTS_IN_SOURCE_BUILD = NO RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF HOST_RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF -DBUILD_SHARED_LIBS=ON +# install re2.pc +define RE2_INSTALL_PKG_CONFIG + $(MAKE1) -C $(@D) DESTDIR=$(STAGING_DIR) \ + libdir=/usr/lib includedir=/usr/include \ + common-install +endef + +RE2_POST_INSTALL_STAGING_HOOKS += RE2_INSTALL_PKG_CONFIG + $(eval $(cmake-package)) $(eval $(host-cmake-package)) Regards, Peter [1] https://github.com/macports/macports-ports/pull/9836#issuecomment-807868854 > > Regards, > > Yann E. MORIN. > > > > > +endef > > > + > > > +define HOST_RE2_BUILD_CMDS > > > + $(HOST_MAKE_ENV) $(MAKE) $(HOST_CONFIGURE_OPTS) \ > > > + -C $(@D) > > > +endef > > > + > > > +define HOST_RE2_INSTALL_CMDS > > > + $(HOST_MAKE_ENV) $(MAKE) $(HOST_CONFIGURE_OPTS) \ > > > + -C $(@D) DESTDIR="$(HOST_DIR)" prefix=/usr install > > > +endef > > > + > > > +$(eval $(generic-package)) > > > +$(eval $(host-generic-package)) > > > -- > > > 2.25.1 > > > > > > _______________________________________________ > > > buildroot mailing list > > > buildroot@buildroot.org > > > https://lists.buildroot.org/mailman/listinfo/buildroot > > > > -- > > .-----------------.--------------------.------------------.--------------------. > > | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | > > | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | > > | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no | > > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | > > '------------------------------^-------^------------------^--------------------' > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
On Thu, Jan 20, 2022 at 2:29 PM Peter Seiderer <ps.report@gmx.net> wrote: > > Hello James, Yann, > > On Wed, 19 Jan 2022 17:42:46 -0700, James Hilliard <james.hilliard1@gmail.com> wrote: > > > On Wed, Jan 19, 2022 at 1:45 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote: > > > > > > James, All, > > > > > > On 2022-01-19 01:45 -0700, James Hilliard spake thusly: > > > > The cmake build appears to be missing features such as pkg-config > > > > generation support, switch to the regular makefile based build > > > > which appears to work better. > > > > > > Can't we just fix the CMakeList.txt and submit a patch upstream, > > > instead? > > > > It looked like this had been brought up before and there was some issue that was > > making this difficult to fix with cmake. > > See: > > https://github.com/google/re2/issues/349 > > > > > > > > Also, see below... > > > > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > > > --- > > > > package/re2/re2.mk | 30 ++++++++++++++++++++++++++---- > > > > 1 file changed, 26 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/package/re2/re2.mk b/package/re2/re2.mk > > > > index b562d5d7ef..144c82339f 100644 > > > > --- a/package/re2/re2.mk > > > > +++ b/package/re2/re2.mk > > > > @@ -10,8 +10,30 @@ RE2_LICENSE = BSD-3-Clause > > > > RE2_LICENSE_FILES = LICENSE > > > > RE2_INSTALL_STAGING = YES > > > > > > > > -RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF > > > > -HOST_RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF -DBUILD_SHARED_LIBS=ON > > > > +define RE2_BUILD_CMDS > > > > + $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \ > > > > + -C $(@D) > > > > +endef > > > > > > > > -$(eval $(cmake-package)) > > > > -$(eval $(host-cmake-package)) > > > > +define RE2_INSTALL_STAGING_CMDS > > > > + $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \ > > > > + DESTDIR="$(STAGING_DIR)" prefix=/usr -C $(@D) install > > > > +endef > > > > + > > > > +define RE2_INSTALL_TARGET_CMDS > > > > + $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \ > > > > + DESTDIR="$(TARGET_DIR)" prefix=/usr -C $(@D) install > > > > > > The 'install' target will forcibly build and install static and shared > > > libs: > > > > > > https://github.com/google/re2/blob/main/Makefile#L297 > > > > > > 296: .PHONY: install > > > 297: install: static-install shared-install > > > > > > So, this is probably going to not play nicely for builds where a shared > > > build is not possible. > > > > Sent a v2 which should fix this: > > https://patchwork.ozlabs.org/project/buildroot/patch/20220120003827.395469-1-james.hilliard1@gmail.com/ > > > > > > > > AFAICS, the cmake-based buildsystem got that one correct, though, so it > > > would be a bit of a shame to regress on that point... > > > > > Or keep the cmake build and do the following to install re2.pc (taken from #349, [1]): > > --- a/package/re2/re2.mk > +++ b/package/re2/re2.mk > @@ -9,9 +9,20 @@ RE2_SITE = $(call github,google,re2,$(RE2_VERSION)) > RE2_LICENSE = BSD-3-Clause > RE2_LICENSE_FILES = LICENSE > RE2_INSTALL_STAGING = YES > +# keep original Makefile (for re2.pc install) > +RE2_SUPPORTS_IN_SOURCE_BUILD = NO > > RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF > HOST_RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF -DBUILD_SHARED_LIBS=ON > > +# install re2.pc > +define RE2_INSTALL_PKG_CONFIG > + $(MAKE1) -C $(@D) DESTDIR=$(STAGING_DIR) \ > + libdir=/usr/lib includedir=/usr/include \ > + common-install > +endef > + > +RE2_POST_INSTALL_STAGING_HOOKS += RE2_INSTALL_PKG_CONFIG > + > $(eval $(cmake-package)) > $(eval $(host-cmake-package)) > > Regards, > Peter > > [1] https://github.com/macports/macports-ports/pull/9836#issuecomment-807868854 There's a problem with this approach, the makefile with common-install gets overwritten by the one generated by cmake if cmake is used. > > > > > Regards, > > > Yann E. MORIN. > > > > > > > +endef > > > > + > > > > +define HOST_RE2_BUILD_CMDS > > > > + $(HOST_MAKE_ENV) $(MAKE) $(HOST_CONFIGURE_OPTS) \ > > > > + -C $(@D) > > > > +endef > > > > + > > > > +define HOST_RE2_INSTALL_CMDS > > > > + $(HOST_MAKE_ENV) $(MAKE) $(HOST_CONFIGURE_OPTS) \ > > > > + -C $(@D) DESTDIR="$(HOST_DIR)" prefix=/usr install > > > > +endef > > > > + > > > > +$(eval $(generic-package)) > > > > +$(eval $(host-generic-package)) > > > > -- > > > > 2.25.1 > > > > > > > > _______________________________________________ > > > > buildroot mailing list > > > > buildroot@buildroot.org > > > > https://lists.buildroot.org/mailman/listinfo/buildroot > > > > > > -- > > > .-----------------.--------------------.------------------.--------------------. > > > | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | > > > | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | > > > | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no | > > > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | > > > '------------------------------^-------^------------------^--------------------' > > _______________________________________________ > > buildroot mailing list > > buildroot@buildroot.org > > https://lists.buildroot.org/mailman/listinfo/buildroot >
James, All, On 2022-01-20 16:57 -0700, James Hilliard spake thusly: > On Thu, Jan 20, 2022 at 2:29 PM Peter Seiderer <ps.report@gmx.net> wrote: > > Or keep the cmake build and do the following to install re2.pc (taken from #349, [1]): > > > > --- a/package/re2/re2.mk > > +++ b/package/re2/re2.mk > > @@ -9,9 +9,20 @@ RE2_SITE = $(call github,google,re2,$(RE2_VERSION)) > > RE2_LICENSE = BSD-3-Clause > > RE2_LICENSE_FILES = LICENSE > > RE2_INSTALL_STAGING = YES > > +# keep original Makefile (for re2.pc install) > > +RE2_SUPPORTS_IN_SOURCE_BUILD = NO > > > > RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF > > HOST_RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF -DBUILD_SHARED_LIBS=ON > > > > +# install re2.pc > > +define RE2_INSTALL_PKG_CONFIG > > + $(MAKE1) -C $(@D) DESTDIR=$(STAGING_DIR) \ > > + libdir=/usr/lib includedir=/usr/include \ > > + common-install > > +endef > > + > > +RE2_POST_INSTALL_STAGING_HOOKS += RE2_INSTALL_PKG_CONFIG > > + > > $(eval $(cmake-package)) > > $(eval $(host-cmake-package)) > > > > Regards, > > Peter > > > > [1] https://github.com/macports/macports-ports/pull/9836#issuecomment-807868854 > > There's a problem with this approach, the makefile with common-install > gets overwritten > by the one generated by cmake if cmake is used. It currently gets overwritten, because the build is done in-tree, but What Peter suggests is doing the build out-of-tree, with: RE2_SUPPORTS_IN_SOURCE_BUILD = NO However, I can't say that I am very fond of this option either. Misinx the two buildsystems looks icky... What bothers me a bit, is that common-install also installs the headers, not just the re2.pc, so this could, even if unlikely, conflict with what the cmake-side would do. But when looking at the various bugs about this issue: https://github.com/google/re2/issues/349 -> https://github.com/google/re2/issues/304#issuecomment-808219200 -> https://github.com/macports/macports-ports/pull/9836 re2: change to cmake build to support gprc, which currently needs a cmake version of re2 to build. Try to make it work like the previous autotools build worked. So, that last issue, against homebrew, is interesting: it seems that using the Makefile-based buildsystem is not enough, because it does not isntall the cmake equivalent of re2.pc, which breaks grpc. So, whether we keep the cmake build, or switch to the Makefile build, we would eventually still miss some pieces... Damn, back to square-one... So, I think that Peter's proposal is not so un-appealing, in then end. But then in such situation, I think it would be simpler for us to just apply something like: define RE2_INSTALL_PC $(INTALL) -d -m 0644 $(@D)/re2.pc \ $(STAGING_DIR)/usr/lib/pkgconfig/re2.pc $(SED) 's,@includedir@,/usr/include,; s,@libdir@,/usr/lib,' \ $(STAGING_DIR)/usr/lib/pkgconfig/re2.pc endef RE2_POST_STAGING_INSTALL_HOOKS += RE2_INSTALL_PC And be done with it: minimal change to the re2 package, no dual-buildsystem situation and thus no install conflict on the headers, usual and simple post-install hook. Thoughts? Regards, Yann E. MORIN.
On Fri, 21 Jan 2022 07:58:50 +0100 "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > So, I think that Peter's proposal is not so un-appealing, in then end. > But then in such situation, I think it would be simpler for us to just > apply something like: > > define RE2_INSTALL_PC > $(INTALL) -d -m 0644 $(@D)/re2.pc \ > $(STAGING_DIR)/usr/lib/pkgconfig/re2.pc > $(SED) 's,@includedir@,/usr/include,; s,@libdir@,/usr/lib,' \ > $(STAGING_DIR)/usr/lib/pkgconfig/re2.pc > endef > RE2_POST_STAGING_INSTALL_HOOKS += RE2_INSTALL_PC > > And be done with it: minimal change to the re2 package, no dual-buildsystem > situation and thus no install conflict on the headers, usual and simple > post-install hook. I might be missing something in the whole discussion, but why not patch the CMake build system so that it also installs this .pc file, and contribute this fix upstream ? Best regards, Thomas
Thomas, All, On 2022-01-21 08:41 +0100, Thomas Petazzoni spake thusly: > On Fri, 21 Jan 2022 07:58:50 +0100 > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > > So, I think that Peter's proposal is not so un-appealing, in then end. > > But then in such situation, I think it would be simpler for us to just > > apply something like: > > > > define RE2_INSTALL_PC > > $(INTALL) -d -m 0644 $(@D)/re2.pc \ > > $(STAGING_DIR)/usr/lib/pkgconfig/re2.pc > > $(SED) 's,@includedir@,/usr/include,; s,@libdir@,/usr/lib,' \ > > $(STAGING_DIR)/usr/lib/pkgconfig/re2.pc > > endef > > RE2_POST_STAGING_INSTALL_HOOKS += RE2_INSTALL_PC > > > > And be done with it: minimal change to the re2 package, no dual-buildsystem > > situation and thus no install conflict on the headers, usual and simple > > post-install hook. > > I might be missing something in the whole discussion, but why not patch > the CMake build system so that it also installs this .pc file, and > contribute this fix upstream ? Because upstream have not been very enthusisatic about it, as James pointed out when I suggested the same. ;-) Regards, Yann E. MORIN.
On Thu, Jan 20, 2022 at 11:58 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote: > > James, All, > > On 2022-01-20 16:57 -0700, James Hilliard spake thusly: > > On Thu, Jan 20, 2022 at 2:29 PM Peter Seiderer <ps.report@gmx.net> wrote: > > > Or keep the cmake build and do the following to install re2.pc (taken from #349, [1]): > > > > > > --- a/package/re2/re2.mk > > > +++ b/package/re2/re2.mk > > > @@ -9,9 +9,20 @@ RE2_SITE = $(call github,google,re2,$(RE2_VERSION)) > > > RE2_LICENSE = BSD-3-Clause > > > RE2_LICENSE_FILES = LICENSE > > > RE2_INSTALL_STAGING = YES > > > +# keep original Makefile (for re2.pc install) > > > +RE2_SUPPORTS_IN_SOURCE_BUILD = NO > > > > > > RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF > > > HOST_RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF -DBUILD_SHARED_LIBS=ON > > > > > > +# install re2.pc > > > +define RE2_INSTALL_PKG_CONFIG > > > + $(MAKE1) -C $(@D) DESTDIR=$(STAGING_DIR) \ > > > + libdir=/usr/lib includedir=/usr/include \ > > > + common-install > > > +endef > > > + > > > +RE2_POST_INSTALL_STAGING_HOOKS += RE2_INSTALL_PKG_CONFIG > > > + > > > $(eval $(cmake-package)) > > > $(eval $(host-cmake-package)) > > > > > > Regards, > > > Peter > > > > > > [1] https://github.com/macports/macports-ports/pull/9836#issuecomment-807868854 > > > > There's a problem with this approach, the makefile with common-install > > gets overwritten > > by the one generated by cmake if cmake is used. > > It currently gets overwritten, because the build is done in-tree, but > What Peter suggests is doing the build out-of-tree, with: > RE2_SUPPORTS_IN_SOURCE_BUILD = NO > > However, I can't say that I am very fond of this option either. Misinx > the two buildsystems looks icky... What bothers me a bit, is that > common-install also installs the headers, not just the re2.pc, so this > could, even if unlikely, conflict with what the cmake-side would do. > > But when looking at the various bugs about this issue: > > https://github.com/google/re2/issues/349 > -> https://github.com/google/re2/issues/304#issuecomment-808219200 > -> https://github.com/macports/macports-ports/pull/9836 > > re2: change to cmake build to support gprc, which currently needs a > cmake version of re2 to build. Try to make it work like the previous > autotools build worked. > > So, that last issue, against homebrew, is interesting: it seems that > using the Makefile-based buildsystem is not enough, because it does not > isntall the cmake equivalent of re2.pc, which breaks grpc. That appears to have been fixed: https://github.com/grpc/grpc/commit/45e413d2520795e7281e9a592af81620349bc186 > > So, whether we keep the cmake build, or switch to the Makefile build, we > would eventually still miss some pieces... > > Damn, back to square-one... > > So, I think that Peter's proposal is not so un-appealing, in then end. > But then in such situation, I think it would be simpler for us to just > apply something like: > > define RE2_INSTALL_PC > $(INTALL) -d -m 0644 $(@D)/re2.pc \ > $(STAGING_DIR)/usr/lib/pkgconfig/re2.pc > $(SED) 's,@includedir@,/usr/include,; s,@libdir@,/usr/lib,' \ > $(STAGING_DIR)/usr/lib/pkgconfig/re2.pc > endef > RE2_POST_STAGING_INSTALL_HOOKS += RE2_INSTALL_PC > > And be done with it: minimal change to the re2 package, no dual-buildsystem > situation and thus no install conflict on the headers, usual and simple > post-install hook. > > Thoughts? > > Regards, > Yann E. MORIN. > > -- > .-----------------.--------------------.------------------.--------------------. > | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | > | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | > | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no | > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | > '------------------------------^-------^------------------^--------------------'
On Fri, 21 Jan 2022 08:59:24 +0100 "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > I might be missing something in the whole discussion, but why not patch > > the CMake build system so that it also installs this .pc file, and > > contribute this fix upstream ? > > Because upstream have not been very enthusisatic about it, as James > pointed out when I suggested the same. ;-) Upstream is not enthusiastic about supporting pkg-config, about having their two possible build systems behave in the same way? What sort of odd upstream is that? Thomas
On Fri, Jan 21, 2022 at 6:26 AM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > On Fri, 21 Jan 2022 08:59:24 +0100 > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > > > I might be missing something in the whole discussion, but why not patch > > > the CMake build system so that it also installs this .pc file, and > > > contribute this fix upstream ? > > > > Because upstream have not been very enthusisatic about it, as James > > pointed out when I suggested the same. ;-) > > Upstream is not enthusiastic about supporting pkg-config, about having > their two possible build systems behave in the same way? What sort of > odd upstream is that? Best I can tell the cmake build is mostly intended for non-system builds/installs or something weird like that. > > Thomas > -- > Thomas Petazzoni, co-owner and CEO, Bootlin > Embedded Linux and Kernel engineering and training > https://bootlin.com
diff --git a/package/re2/re2.mk b/package/re2/re2.mk index b562d5d7ef..144c82339f 100644 --- a/package/re2/re2.mk +++ b/package/re2/re2.mk @@ -10,8 +10,30 @@ RE2_LICENSE = BSD-3-Clause RE2_LICENSE_FILES = LICENSE RE2_INSTALL_STAGING = YES -RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF -HOST_RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF -DBUILD_SHARED_LIBS=ON +define RE2_BUILD_CMDS + $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \ + -C $(@D) +endef -$(eval $(cmake-package)) -$(eval $(host-cmake-package)) +define RE2_INSTALL_STAGING_CMDS + $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \ + DESTDIR="$(STAGING_DIR)" prefix=/usr -C $(@D) install +endef + +define RE2_INSTALL_TARGET_CMDS + $(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \ + DESTDIR="$(TARGET_DIR)" prefix=/usr -C $(@D) install +endef + +define HOST_RE2_BUILD_CMDS + $(HOST_MAKE_ENV) $(MAKE) $(HOST_CONFIGURE_OPTS) \ + -C $(@D) +endef + +define HOST_RE2_INSTALL_CMDS + $(HOST_MAKE_ENV) $(MAKE) $(HOST_CONFIGURE_OPTS) \ + -C $(@D) DESTDIR="$(HOST_DIR)" prefix=/usr install +endef + +$(eval $(generic-package)) +$(eval $(host-generic-package))
The cmake build appears to be missing features such as pkg-config generation support, switch to the regular makefile based build which appears to work better. Signed-off-by: James Hilliard <james.hilliard1@gmail.com> --- package/re2/re2.mk | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-)