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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list