diff mbox series

[1/2] package/re2: new package

Message ID 20200813144549.1262366-2-buildroot@heine.tech
State Accepted
Headers show
Series package/grpc: 1.31.0 | expand

Commit Message

Michael Nosthoff Aug. 13, 2020, 2:45 p.m. UTC
RE2 is a fast, safe, thread-friendly alternative to backtracking
regular expression engines like those used in PCRE, Perl, and
Python. It is a C++ library.

Signed-off-by: Michael Nosthoff <buildroot@heine.tech>
---
 package/Config.in     |  1 +
 package/re2/Config.in | 12 ++++++++++++
 package/re2/re2.hash  |  3 +++
 package/re2/re2.mk    | 25 +++++++++++++++++++++++++
 4 files changed, 41 insertions(+)
 create mode 100644 package/re2/Config.in
 create mode 100644 package/re2/re2.hash
 create mode 100644 package/re2/re2.mk

Comments

Thomas Petazzoni Aug. 16, 2020, 9:46 p.m. UTC | #1
On Thu, 13 Aug 2020 16:45:48 +0200
Michael Nosthoff via buildroot <buildroot@busybox.net> wrote:

> RE2 is a fast, safe, thread-friendly alternative to backtracking
> regular expression engines like those used in PCRE, Perl, and
> Python. It is a C++ library.
> 
> Signed-off-by: Michael Nosthoff <buildroot@heine.tech>

Thanks, I've applied to master, with a few changes.

> ---
>  package/Config.in     |  1 +
>  package/re2/Config.in | 12 ++++++++++++
>  package/re2/re2.hash  |  3 +++
>  package/re2/re2.mk    | 25 +++++++++++++++++++++++++
>  4 files changed, 41 insertions(+)

You forgot to add an entry in the DEVELOPERS file, so I've done so.

> diff --git a/package/re2/Config.in b/package/re2/Config.in
> new file mode 100644
> index 0000000000..d1dcb693ea
> --- /dev/null
> +++ b/package/re2/Config.in
> @@ -0,0 +1,12 @@
> +config BR2_PACKAGE_RE2
> +	bool "re2"
> +	depends on BR2_INSTALL_LIBSTDCPP
> +	depends on BR2_TOOLCHAIN_HAS_ATOMIC

Even though it uses std::atomic, I'm not sure it needs the atomic
intrinsics of the compiler. Indeed, I tried building for SPARC, where
the atomic built-ins are implemented in the libatomic.so library, and
even without linking to this library, re2 builds fine.

So I've dropped this dependency. We'll see if we have any build
failure, and we can adjust.

> +	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 # needs C++11
> +	help
> +	  RE2 is a fast, safe, thread-friendly alternative
> +	  to backtracking regular expression engines like
> +	  those used in PCRE, Perl, and Python.
> +	  It is a C++ library.
> +
> +	  https://github.com/google/re2

You forgot a Config.in comment about the dependency (C++, gcc >= 4.8),
so I've added that.


> +RE2_VERSION = 2020-08-01
> +RE2_SITE = $(call github,google,re2,$(RE2_VERSION))
> +RE2_LICENSE = BSD

BSD is not specific enough. In this case, it should have been BSD-3-Clause.

> +ifeq ($(BR2_SHARED_LIBS)$(BR2_SHARED_STATIC_LIBS),y)
> +RE2_CONF_OPTS += \
> +	-DBUILD_SHARED_LIBS=ON
> +HOST_RE2_CONF_OPTS += \
> +	-DBUILD_SHARED_LIBS=ON

This was not needed: BUILD_SHARED_LIBS is handled by the cmake-package
infrastructure.

Thanks, applied with those changes!

Thomas
Thomas Petazzoni Aug. 26, 2020, 9:30 p.m. UTC | #2
On Thu, 13 Aug 2020 16:45:48 +0200
Michael Nosthoff via buildroot <buildroot@busybox.net> wrote:

> RE2 is a fast, safe, thread-friendly alternative to backtracking
> regular expression engines like those used in PCRE, Perl, and
> Python. It is a C++ library.
> 
> Signed-off-by: Michael Nosthoff <buildroot@heine.tech>
> ---
>  package/Config.in     |  1 +
>  package/re2/Config.in | 12 ++++++++++++
>  package/re2/re2.hash  |  3 +++
>  package/re2/re2.mk    | 25 +++++++++++++++++++++++++
>  4 files changed, 41 insertions(+)
>  create mode 100644 package/re2/Config.in
>  create mode 100644 package/re2/re2.hash
>  create mode 100644 package/re2/re2.mk

It seems like there is an issue with this package:

  http://autobuild.buildroot.net/results/a98/a98d3203f68f0f929c544537244e7621e80ce0a1/build-end.log

Apparently, only a static version of libre2 gets installed, which is
built without -fPIC (makes sense for a static library). But then it
gets linked into a shared library, which booms as all the code in a
shared library must be compiled with -fPIC.

Perhaps the issue is that we're not explicitly passing
-DBUILD_SHARED_LIBS=ON to host CMake packages in package/pkg-cmake.mk ?

Could you have a look into this ?

Thanks!

Thomas
Michael Nosthoff Aug. 30, 2020, 2:34 p.m. UTC | #3
Hi Thomas,

On 26.08.20 23:30, Thomas Petazzoni wrote:
> On Thu, 13 Aug 2020 16:45:48 +0200
> Michael Nosthoff via buildroot <buildroot@busybox.net> wrote:
>
>> RE2 is a fast, safe, thread-friendly alternative to backtracking
>> regular expression engines like those used in PCRE, Perl, and
>> Python. It is a C++ library.
>>
>> Signed-off-by: Michael Nosthoff <buildroot@heine.tech>
>> ---
>>   package/Config.in     |  1 +
>>   package/re2/Config.in | 12 ++++++++++++
>>   package/re2/re2.hash  |  3 +++
>>   package/re2/re2.mk    | 25 +++++++++++++++++++++++++
>>   4 files changed, 41 insertions(+)
>>   create mode 100644 package/re2/Config.in
>>   create mode 100644 package/re2/re2.hash
>>   create mode 100644 package/re2/re2.mk
> It seems like there is an issue with this package:
>
>    http://autobuild.buildroot.net/results/a98/a98d3203f68f0f929c544537244e7621e80ce0a1/build-end.log
that's not good...
>
> Apparently, only a static version of libre2 gets installed, which is
> built without -fPIC (makes sense for a static library). But then it
> gets linked into a shared library, which booms as all the code in a
> shared library must be compiled with -fPIC.
>
> Perhaps the issue is that we're not explicitly passing
> -DBUILD_SHARED_LIBS=ON to host CMake packages in package/pkg-cmake.mk ?
>
> Could you have a look into this ?


What I'm wondering: Should all host packages always be built as shared libs?


 From the CMake Documentation [1] (which is a bit unclear) I assume that 
as default

CMake will build libs as static. So it might be a coincidence that all 
existing host cmake packages

built as shared as their default.

When comparing to package/pkg-autotools.mk I see that for host packages

     --enable-shared --disable-static

is always set. So it would make sense to also add

    -DBUILD_SHARED_LIBS=ON


to cmake host build flags.



I'm currently on vacation with no access to a proper machine for 
building. So I would

be able to take care of this further earliest next week.


Regards,

Michael



[1] https://cmake.org/cmake/help/latest/variable/BUILD_SHARED_LIBS.html
diff mbox series

Patch

diff --git a/package/Config.in b/package/Config.in
index c637b5427f..f6e9524a0f 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -1942,6 +1942,7 @@  menu "Text and terminal handling"
 	source "package/pcre/Config.in"
 	source "package/pcre2/Config.in"
 	source "package/popt/Config.in"
+	source "package/re2/Config.in"
 	source "package/readline/Config.in"
 	source "package/slang/Config.in"
 	source "package/tclap/Config.in"
diff --git a/package/re2/Config.in b/package/re2/Config.in
new file mode 100644
index 0000000000..d1dcb693ea
--- /dev/null
+++ b/package/re2/Config.in
@@ -0,0 +1,12 @@ 
+config BR2_PACKAGE_RE2
+	bool "re2"
+	depends on BR2_INSTALL_LIBSTDCPP
+	depends on BR2_TOOLCHAIN_HAS_ATOMIC
+	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 # needs C++11
+	help
+	  RE2 is a fast, safe, thread-friendly alternative
+	  to backtracking regular expression engines like
+	  those used in PCRE, Perl, and Python.
+	  It is a C++ library.
+
+	  https://github.com/google/re2
diff --git a/package/re2/re2.hash b/package/re2/re2.hash
new file mode 100644
index 0000000000..b8a80513b7
--- /dev/null
+++ b/package/re2/re2.hash
@@ -0,0 +1,3 @@ 
+# locally calculated
+sha256  6f4c8514249cd65b9e85d3e6f4c35595809a63ad71c5d93083e4d1dcdf9e0cd6  re2-2020-08-01.tar.gz
+sha256  6040cda75d90b1738292a631d89934c411ef7ffd543c4d6a1b7edfc8edf29449  LICENSE
diff --git a/package/re2/re2.mk b/package/re2/re2.mk
new file mode 100644
index 0000000000..d2791a460b
--- /dev/null
+++ b/package/re2/re2.mk
@@ -0,0 +1,25 @@ 
+################################################################################
+#
+# Google RE2
+#
+################################################################################
+
+RE2_VERSION = 2020-08-01
+RE2_SITE = $(call github,google,re2,$(RE2_VERSION))
+RE2_LICENSE = BSD
+RE2_LICENSE_FILES = LICENSE
+
+RE2_INSTALL_STAGING = YES
+
+RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF
+HOST_RE2_CONF_OPTS += -DRE2_BUILD_TESTING=OFF
+
+ifeq ($(BR2_SHARED_LIBS)$(BR2_SHARED_STATIC_LIBS),y)
+RE2_CONF_OPTS += \
+	-DBUILD_SHARED_LIBS=ON
+HOST_RE2_CONF_OPTS += \
+	-DBUILD_SHARED_LIBS=ON
+endif
+
+$(eval $(cmake-package))
+$(eval $(host-cmake-package))