[PATCH] build-aux: use GNU sed for syntax-check on FreeBSD

Roman Bogorodskiy posted 1 patch 3 years, 8 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200805160241.17603-1-bogorodskiy@gmail.com
build-aux/syntax-check.mk | 8 ++++++++
1 file changed, 8 insertions(+)
[PATCH] build-aux: use GNU sed for syntax-check on FreeBSD
Posted by Roman Bogorodskiy 3 years, 8 months ago
BSD sed(1) and GNU sed(1) syntax are not compatible, and as
syntax-check.mk uses the GNU flavor, set SED variable to
'gsed' by default.

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
---
 build-aux/syntax-check.mk | 8 ++++++++
 1 file changed, 8 insertions(+)

I'm not sure if this requires a more comprehensive solution.
I have mixed feeling about this. If we try to just use gsed like in this
patch, it'll fail because we don't require gsed to be installed.

OTOH, an alternative solution like checking for gsed in meson.build,
and probably even generation of some files with variables to be sourced
by .mk files feels like too much of a hassle, esp. in this context.

diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 6eb59cf90e..bbfcb63152 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -28,7 +28,15 @@ ME := build-aux/syntax-check.mk
 # ignoring the module description.
 AWK ?= awk
 GREP ?= grep
+# FreeBSD (and probably some other OSes too) ships own version of sed(1), not
+# compatible with the GNU sed. GNU sed is available as gsed(1), so use this
+# instead
+UNAME := $(shell uname)
+ifeq ($(UNAME),FreeBSD)
+SED ?= gsed
+else
 SED ?= sed
+endif
 
 # Helper variables.
 _empty =
-- 
2.27.0

Re: [PATCH] build-aux: use GNU sed for syntax-check on FreeBSD
Posted by Roman Bogorodskiy 3 years, 8 months ago
  Roman Bogorodskiy wrote:

> BSD sed(1) and GNU sed(1) syntax are not compatible, and as
> syntax-check.mk uses the GNU flavor, set SED variable to
> 'gsed' by default.
> 
> Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
> ---
>  build-aux/syntax-check.mk | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 

ping

Roman Bogorodskiy
Re: [PATCH] build-aux: use GNU sed for syntax-check on FreeBSD
Posted by Roman Bogorodskiy 3 years, 7 months ago
  Roman Bogorodskiy wrote:

>   Roman Bogorodskiy wrote:
> 
> > BSD sed(1) and GNU sed(1) syntax are not compatible, and as
> > syntax-check.mk uses the GNU flavor, set SED variable to
> > 'gsed' by default.
> > 
> > Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
> > ---
> >  build-aux/syntax-check.mk | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> 
> ping

ping


Roman Bogorodskiy
Re: [PATCH] build-aux: use GNU sed for syntax-check on FreeBSD
Posted by Daniel P. Berrangé 3 years, 7 months ago
On Wed, Aug 05, 2020 at 08:02:41PM +0400, Roman Bogorodskiy wrote:
> BSD sed(1) and GNU sed(1) syntax are not compatible, and as
> syntax-check.mk uses the GNU flavor, set SED variable to
> 'gsed' by default.
> 
> Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
> ---
>  build-aux/syntax-check.mk | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> I'm not sure if this requires a more comprehensive solution.
> I have mixed feeling about this. If we try to just use gsed like in this
> patch, it'll fail because we don't require gsed to be installed.
> 
> OTOH, an alternative solution like checking for gsed in meson.build,
> and probably even generation of some files with variables to be sourced
> by .mk files feels like too much of a hassle, esp. in this context.

I think that's likely overkill for this usage.

> 
> diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
> index 6eb59cf90e..bbfcb63152 100644
> --- a/build-aux/syntax-check.mk
> +++ b/build-aux/syntax-check.mk
> @@ -28,7 +28,15 @@ ME := build-aux/syntax-check.mk
>  # ignoring the module description.
>  AWK ?= awk
>  GREP ?= grep
> +# FreeBSD (and probably some other OSes too) ships own version of sed(1), not
> +# compatible with the GNU sed. GNU sed is available as gsed(1), so use this
> +# instead
> +UNAME := $(shell uname)
> +ifeq ($(UNAME),FreeBSD)
> +SED ?= gsed
> +else
>  SED ?= sed
> +endif

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|