Re: [libvirt] [PATCH] Use SED variable for `sed` binary in src/Makefile

2010-09-10 Thread Eric Blake

On 09/09/2010 12:41 AM, Mitchell Hashimoto wrote:
 Hi,

 I've been trying to get libvirt (client) to cleanly/easily compile on
 BSD-based systems (in this case OS X). ./configure runs fine but the
 make caused an error with `sed` since BSD sed was reporting some 
sort of

 regex error. I realized that the SED variable was populated with gsed
 which worked properly. This made the make continue (failed again 
later, will

 address that when I can).


---
  src/Makefile.am |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index b321657..118c329 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1029,14 +1029,14 @@ libvirt.syms: libvirt_public.syms $(USED_SYM_FILES)
  libvirt.def: libvirt.syms
 $(AM_V_GEN)rm -f -- $...@-tmp $@ ; \
 printf 'EXPORTS\n'  $...@-tmp  \
-   sed -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/[
\t]*\(.*\)\;/\1/g' $^  $...@-tmp  \
+   $(SED) -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/[
\t]*\(.*\)\;/\1/g' $^  $...@-tmp  \


Thanks for the report.  Hmm - the real problem here is that we are 
relying on a non-portable sed construct.  Using $(SED) is a crutch that 
will let other users substitute gsed when needed, but the real fix is to 
correct the sed expression to be portable to POSIX rules in the first place.


Neither \} nor \t are portable:

$ printf 'a\tc\n}\n' | /usr/bin/sed '/\}/d; s/[\t]/ /'
sed: 1: /\}/d: RE error: parentheses not balanced
$ printf 'abc\n}\n' | /usr/bin/sed '/}/d; s/[\t]/ /'
a   c
$ printf 'abc\n}\n' | gsed '/\}/d; s/[\t]/ /'
a c

So, I'll propose a better patch that fixes the regex to be portable, at 
which point, I think we no longer need to worry about using $(SED) in 
the first place.


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] Use SED variable for `sed` binary in src/Makefile

2010-09-09 Thread Mitchell Hashimoto
Hi,

I've been trying to get libvirt (client) to cleanly/easily compile on
BSD-based systems (in this case OS X). ./configure runs fine but the
make caused an error with `sed` since BSD sed was reporting some sort of
regex error. I realized that the SED variable was populated with gsed
which worked properly. This made the make continue (failed again later, will
address that when I can).

This is my first contribution to a C-based project and I don't have much
experience with autotools other than using them as a consumer. Let me know
if anything can be improved.

Thank you,
Mitchell


0001-Use-SED-variable-for-sed-binary-in-src-Makefile.patch
Description: Binary data
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] Use SED variable for `sed` binary in src/Makefile

2010-09-09 Thread Mitchell Hashimoto
Hm, the patch attached as binary data. Here is the attached patch as text.
Apologies about that:

---
 src/Makefile.am |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index b321657..118c329 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1029,14 +1029,14 @@ libvirt.syms: libvirt_public.syms $(USED_SYM_FILES)
 libvirt.def: libvirt.syms
$(AM_V_GEN)rm -f -- $...@-tmp $@ ; \
printf 'EXPORTS\n'  $...@-tmp  \
-   sed -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/[
\t]*\(.*\)\;/\1/g' $^  $...@-tmp  \
+   $(SED) -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/[
\t]*\(.*\)\;/\1/g' $^  $...@-tmp  \
chmod a-w $...@-tmp  \
mv $...@-tmp libvirt.def

 libvirt_qemu.def: $(srcdir)/libvirt_qemu.syms
$(AM_V_GEN)rm -f -- $...@-tmp $@ ; \
printf 'EXPORTS\n'  $...@-tmp  \
-   sed -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/[
\t]*\(.*\)\;/\1/g' $^  $...@-tmp  \
+   $(SED) -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/[
\t]*\(.*\)\;/\1/g' $^  $...@-tmp  \
chmod a-w $...@-tmp  \
mv $...@-tmp libvirt_qemu.def

-- 
1.7.2.2

On Wed, Sep 8, 2010 at 11:39 PM, Mitchell Hashimoto 
mitchell.hashim...@gmail.com wrote:

 Hi,

 I've been trying to get libvirt (client) to cleanly/easily compile on
 BSD-based systems (in this case OS X). ./configure runs fine but the
 make caused an error with `sed` since BSD sed was reporting some sort of
 regex error. I realized that the SED variable was populated with gsed
 which worked properly. This made the make continue (failed again later, will
 address that when I can).

 This is my first contribution to a C-based project and I don't have much
 experience with autotools other than using them as a consumer. Let me know
 if anything can be improved.

 Thank you,
 Mitchell

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] Use SED variable for `sed` binary in src/Makefile

2010-09-09 Thread Richard W.M. Jones
On Wed, Sep 08, 2010 at 11:41:52PM -0700, Mitchell Hashimoto wrote:
 Hm, the patch attached as binary data. Here is the attached patch as text.
 Apologies about that:
 
 ---
  src/Makefile.am |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/src/Makefile.am b/src/Makefile.am
 index b321657..118c329 100644
 --- a/src/Makefile.am
 +++ b/src/Makefile.am
 @@ -1029,14 +1029,14 @@ libvirt.syms: libvirt_public.syms $(USED_SYM_FILES)
  libvirt.def: libvirt.syms
 $(AM_V_GEN)rm -f -- $...@-tmp $@ ; \
 printf 'EXPORTS\n'  $...@-tmp  \
 -   sed -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/[
 \t]*\(.*\)\;/\1/g' $^  $...@-tmp  \
 +   $(SED) -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/[
 \t]*\(.*\)\;/\1/g' $^  $...@-tmp  \
 chmod a-w $...@-tmp  \
 mv $...@-tmp libvirt.def
 
  libvirt_qemu.def: $(srcdir)/libvirt_qemu.syms
 $(AM_V_GEN)rm -f -- $...@-tmp $@ ; \
 printf 'EXPORTS\n'  $...@-tmp  \
 -   sed -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/[
 \t]*\(.*\)\;/\1/g' $^  $...@-tmp  \
 +   $(SED) -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/[
 \t]*\(.*\)\;/\1/g' $^  $...@-tmp  \
 chmod a-w $...@-tmp  \
 mv $...@-tmp libvirt_qemu.def

I think you also need to add AC_PROG_SED to configure.ac.  At the
moment it is being defined, but only incidentally because of some
other macro.

There are also uses of sed, at least one in configure.ac itself ...

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list