10:37 a.m.
Some syntax-check rules use GNU sed specific regexps, so allow
to override which sed would be used to fix 'syntax-check' for
non GNU-userland systems.
---
cfg.mk | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 207dfeb..bd984bd 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -614,7 +614,7 @@ sc_libvirt_unmarked_diagnostics:
$(_sc_search_regexp)
@{ grep -nE '\<$(func_re) *\(.*;$$' $$($(VC_LIST_EXCEPT)); \
grep -A1 -nE '\<$(func_re) *\(.*,$$' $$($(VC_LIST_EXCEPT)); } \
- | sed 's/_("\([^\"]\|\\.\)\+"//;s/[ ]"%s"//' \
+ | $(SED) 's/_("\([^\"]\|\\.\)\+"//;s/[ ]"%s"//' \
| grep '[ ]"' && \
{ echo '$(ME): found unmarked diagnostic(s)' 1>&2; \
exit 1; } || :
@@ -639,7 +639,7 @@ sc_prohibit_newline_at_end_of_diagnostic:
sc_prohibit_diagnostic_without_format:
@{ grep -nE '\<$(func_re) *\(.*;$$' $$($(VC_LIST_EXCEPT)); \
grep -A2 -nE '\<$(func_re) *\(.*,$$' $$($(VC_LIST_EXCEPT)); } \
- | sed -rn -e ':l; /[,"]$$/ {N;b l;}' \
+ | $(SED) -rn -e ':l; /[,"]$$/ {N;b l;}' \
-e '/(xenapiSessionErrorHandler|vah_(error|warning))/d' \
-e '/\<$(func_re)
*\([^"]*"([^%"]|"\n[^"]*")*"[,)]/p' \
| grep -vE 'VIR_ERROR' && \
@@ -661,7 +661,7 @@ sc_prohibit_useless_translation:
# or \n on one side of the split.
sc_require_whitespace_in_translation:
@grep -n -A1 '"$$' $$($(VC_LIST_EXCEPT)) \
- | sed -ne ':l; /"$$/ {N;b l;}; s/"\n[^"]*"/""/g;
s/\\n/ /g' \
+ | $(SED) -ne ':l; /"$$/ {N;b l;}; s/"\n[^"]*"/""/g;
s/\\n/ /g' \
-e '/_(.*[^\ ]""[^\ ]/p' | grep . && \
{ echo '$(ME): missing whitespace at line split' 1>&2; \
exit 1; } || :
@@ -681,11 +681,11 @@ sc_preprocessor_indentation:
sc_spec_indentation:
@if cppi --version >/dev/null 2>&1; then \
for f in $$($(VC_LIST_EXCEPT) | grep '\.spec\.in$$'); do \
- sed -e 's|#|// #|; s|%ifn*\(arch\)* |#if a // |' \
+ $(SED) -e 's|#|// #|; s|%ifn*\(arch\)* |#if a // |' \
-e 's/%\(else\|endif\|define\)/#\1/' \
-e 's/^\( *\)\1\1\1#/#\1/' \
-e 's|^\( *[^#/ ]\)|// \1|; s|^\( */[^/]\)|// \1|' $$f \
- | cppi -a -c 2>&1 | sed "s|standard input|$$f|"; \
+ | cppi -a -c 2>&1 | $(SED) "s|standard input|$$f|"; \
done | { if grep . >&2; then false; else :; fi; } \
|| { echo '$(ME): incorrect preprocessor indentation' 1>&2; \
exit 1; }; \
@@ -777,7 +777,7 @@ sc_prohibit_cross_inclusion:
# elements added to the enum by using a _LAST marker.
sc_require_enum_last_marker:
@grep -A1 -nE '^[^#]*VIR_ENUM_IMPL *\(' $$($(VC_LIST_EXCEPT)) \
- | sed -ne '/VIR_ENUM_IMPL[^,]*,$$/N' \
+ | $(SED) -ne '/VIR_ENUM_IMPL[^,]*,$$/N' \
-e '/VIR_ENUM_IMPL[^,]*,[^,]*[^_,][^L,][^A,][^S,][^T,],/p' \
-e '/VIR_ENUM_IMPL[^,]*,[^,]\{0,4\},/p' \
| grep . && \
@@ -878,7 +878,7 @@ ifeq (0,$(MAKELEVEL))
# b653eda3ac4864de205419d9f41eec267cb89eeb
#
# Keep this logic in sync with autogen.sh.
- _submodule_hash = sed 's/^[ +-]//;s/ .*//'
+ _submodule_hash = $(SED) 's/^[ +-]//;s/ .*//'
_update_required := $(shell \
cd '$(srcdir)'; \
test -d .git || { echo 0; exit; }; \
--
1.8.4.3
3:03 p.m.
On 01/27/2014 09:37 AM, Roman Bogorodskiy wrote:
Some syntax-check rules use GNU sed specific regexps, so allow
to override which sed would be used to fix 'syntax-check' for
non GNU-userland systems.
---
cfg.mk | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
[Adding bug-gnulib]
Hmm - this is an interesting situation. I imagine we will probably hit
cases where maint.mk also starts assuming GNU sed, so it becomes a trade
of whether it is easier to rewrite the expressions to portable sed that
works out of the box on BSD, or to rewrite the rules to use $(SED)
uniformly. And what happens if we use $(SED) but the configure.ac did
not request AC_PROG_SED in configure.ac? (In gnulib, m4/po.m4 requires
AC_PROG_SED, which means any package using gettext is safe, but we'd
have to hoist the requirement of AC_PROG_SED into gnulib's gl_INIT macro
if we wanted to work even for packages that don't use gettext).
diff --git a/cfg.mk b/cfg.mk
index 207dfeb..bd984bd 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -614,7 +614,7 @@ sc_libvirt_unmarked_diagnostics:
$(_sc_search_regexp)
@{ grep -nE '\<$(func_re) *\(.*;$$' $$($(VC_LIST_EXCEPT)); \
grep -A1 -nE '\<$(func_re) *\(.*,$$' $$($(VC_LIST_EXCEPT)); } \
- | sed 's/_("\([^\"]\|\\.\)\+"//;s/[ ]"%s"//' \
+ | $(SED) 's/_("\([^\"]\|\\.\)\+"//;s/[ ]"%s"//' \
Is the non-portable sed usage merely the fact that I used \+ here? If
so, \{1,\} is a more portable way to write the 1-or-more modifier.
| grep '[ ]"' && \
{ echo '$(ME): found unmarked diagnostic(s)' 1>&2; \
exit 1; } || :
@@ -639,7 +639,7 @@ sc_prohibit_newline_at_end_of_diagnostic:
sc_prohibit_diagnostic_without_format:
@{ grep -nE '\<$(func_re) *\(.*;$$' $$($(VC_LIST_EXCEPT)); \
grep -A2 -nE '\<$(func_re) *\(.*,$$' $$($(VC_LIST_EXCEPT)); } \
- | sed -rn -e ':l; /[,"]$$/ {N;b l;}' \
+ | $(SED) -rn -e ':l; /[,"]$$/ {N;b l;}' \
-e '/(xenapiSessionErrorHandler|vah_(error|warning))/d' \
-e '/\<$(func_re)
*\([^"]*"([^%"]|"\n[^"]*")*"[,)]/p' \
POSIX has decided to add support for 'sed -E' in a future version (GNU
sed supports that as an alias for 'sed -r', the alias has been around
for a long time, but was only recently documented as a result of the
POSIX decision):
http://austingroupbugs.net/view.php?id=528
Would using 'sed -En -e ...' fix it for you? Or should we just rewrite
this in basic rather than extended regex:
-e /\(xenapiSessionErrorHandler\|vah_\(error\|warning\)\)/d'
-e /\<$func_re)
*([^"*"\([^%"]\|"\n[^"\*"\)*"[,)]/p'
@@ -661,7 +661,7 @@ sc_prohibit_useless_translation:
# or \n on one side of the split.
sc_require_whitespace_in_translation:
@grep -n -A1 '"$$' $$($(VC_LIST_EXCEPT)) \
- | sed -ne ':l; /"$$/ {N;b l;}; s/"\n[^"]*"/""/g;
s/\\n/ /g' \
+ | $(SED) -ne ':l; /"$$/ {N;b l;};
s/"\n[^"]*"/""/g; s/\\n/ /g' \
-e '/_(.*[^\ ]""[^\ ]/p' | grep . && \
Hmm, I'm not seeing the non-portable aspect here. Wonder what I'm missing...
sc_spec_indentation:
@if cppi --version >/dev/null 2>&1; then \
for f in $$($(VC_LIST_EXCEPT) | grep '\.spec\.in$$'); do \
- sed -e 's|#|// #|; s|%ifn*\(arch\)* |#if a // |' \
+ $(SED) -e 's|#|// #|; s|%ifn*\(arch\)* |#if a // |' \
-e 's/%\(else\|endif\|define\)/#\1/' \
-e 's/^\( *\)\1\1\1#/#\1/' \
-e 's|^\( *[^#/ ]\)|// \1|; s|^\( */[^/]\)|// \1|' $$f \
Again, not seeing the non-portable use here, what am I missing?
- | cppi -a -c 2>&1 | sed "s|standard
input|$$f|"; \
+ | cppi -a -c 2>&1 | $(SED) "s|standard input|$$f|"; \
And I doubt this use needs $(SED), as that looks pretty bog-standard.
sc_require_enum_last_marker:
@grep -A1 -nE '^[^#]*VIR_ENUM_IMPL *\(' $$($(VC_LIST_EXCEPT)) \
- | sed -ne '/VIR_ENUM_IMPL[^,]*,$$/N' \
+ | $(SED) -ne '/VIR_ENUM_IMPL[^,]*,$$/N' \
-e '/VIR_ENUM_IMPL[^,]*,[^,]*[^_,][^L,][^A,][^S,][^T,],/p' \
-e '/VIR_ENUM_IMPL[^,]*,[^,]\{0,4\},/p' \
Again, not seeing a non-portable use here.
@@ -878,7 +878,7 @@ ifeq (0,$(MAKELEVEL))
# b653eda3ac4864de205419d9f41eec267cb89eeb
#
# Keep this logic in sync with autogen.sh.
- _submodule_hash = sed 's/^[ +-]//;s/ .*//'
+ _submodule_hash = $(SED) 's/^[ +-]//;s/ .*//'
And this one's portable as well. Or is the idea to replace ALL use of
s/sed/$(SED)/ even where it makes no difference? If so, we should
probably also do it in maint.mk. I'd like some feedback from the gnulib
community if favoring $(SED) is the right thing to do, or if I should
just rewrite libvirt's rules to stick to portable sed constructs.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:05 p.m.
On 01/27/2014 01:03 PM, Eric Blake wrote:
I'd like some feedback from the gnulib
community if favoring $(SED) is the right thing to do, or if I should
just rewrite libvirt's rules to stick to portable sed constructs.
My kneejerk reaction is that we'd probably have fewer problems using
'$(SED)' everywhere in a makefile, rather than using '$(SED)' some
places and 'sed' in others, because it'd be one less 'sed'
implementation to worry about.
1:28 a.m.
Paul Eggert wrote:
On 01/27/2014 01:03 PM, Eric Blake wrote:
> I'd like some feedback from the gnulib
> community if favoring $(SED) is the right thing to do, or if I should
> just rewrite libvirt's rules to stick to portable sed constructs.
My kneejerk reaction is that we'd probably have fewer problems using
'$(SED)' everywhere in a makefile, rather than using '$(SED)' some
places and 'sed' in others, because it'd be one less 'sed'
implementation to worry about.
Yes, that was the idea. That's why I replaced all the usage of 'sed' even
though not all of them are non-portable, as Eric pointed.
For sure, using portable regexps is better way, but I don't know how
much of effort that would be.
Roman Bogorodskiy
4:56 a.m.
Roman Bogorodskiy wrote:
Some syntax-check rules use GNU sed specific regexps, so allow
to override which sed would be used to fix 'syntax-check' for
non GNU-userland systems.
---
cfg.mk | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
Any updates on that?
Roman Bogorodskiy
3707
days inactive
3765
days old
4 comments
3 participants
participants (3)
-
Eric Blake -
Paul Eggert -
Roman Bogorodskiy