1:41 p.m.
Since v1: fix the gnulib bug I accidentally introduced, then
copy the gist of Roman's gnulib changes to also apply to our
cfg.mk syntax checks.
Eric Blake (3):
maint: update gnulib for syntax-check on BSD
maint: prefer $(GREP) in cfg.mk
maint: split long lines for BSD syntax-check
.gnulib | 2 +-
cfg.mk | 95 +++++++++++++++++++++++++++++----------------------------
2 files changed, 49 insertions(+), 48 deletions(-)
--
2.20.1
1:41 p.m.
New subject: [libvirt] [PATCH v2 1/3] maint: update gnulib for syntax-check on BSD
In particular, this incorporates Roman's patches to allow
'make syntax-check' to work on BSD with its exec argv
limitations that previously failed when trying to grep the
large number of files present in libvirt.
cfg.mk needs similar changes, but that will be tackled separately.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
.gnulib | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.gnulib b/.gnulib
index 4652c7bafa..70eb4687b1 160000
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit 4652c7bafa60332145f1e05a7de5f48e1bc56226
+Subproject commit 70eb4687b1d84ae780bbf2b8141bee07c85e6d3b
--
2.20.1
4:58 a.m.
New subject: [libvirt] [PATCH v2 1/3] maint: update gnulib for syntax-check on BSD
On Thu, Jan 03, 2019 at 01:41:57PM -0600, Eric Blake wrote:
In particular, this incorporates Roman's patches to allow
'make syntax-check' to work on BSD with its exec argv
limitations that previously failed when trying to grep the
large number of files present in libvirt.
cfg.mk needs similar changes, but that will be tackled separately.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
.gnulib | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1:41 p.m.
New subject: [libvirt] [PATCH v2 2/3] maint: prefer $(GREP) in cfg.mk
We already used $(GREP) in some places, but might as well use it
everywhere during syntax check, in line with similar recent gnulib
changes.
---
cfg.mk | 54 +++++++++++++++++++++++++++---------------------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 90ec8dfc60..9956d20034 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1,5 +1,5 @@
# Customize Makefile.maint. -*- makefile -*-
-# Copyright (C) 2008-2015 Red Hat, Inc.
+# Copyright (C) 2008-2019 Red Hat, Inc.
# Copyright (C) 2003-2008 Free Software Foundation, Inc.
# This program is free software: you can redistribute it and/or modify
@@ -305,7 +305,7 @@ sc_flags_usage:
$(srcdir)/include/libvirt/libvirt-qemu.h \
$(srcdir)/include/libvirt/libvirt-lxc.h \
$(srcdir)/include/libvirt/libvirt-admin.h \
- | grep -c '\(long\|unsigned\) flags')" != 4 && \
+ | $(GREP) -c '\(long\|unsigned\) flags')" != 4 && \
{ echo '$(ME): new API should use "unsigned int flags"' 1>&2;
\
exit 1; } || :
@prohibit=' flags ATTRIBUTE_UNUSED' \
@@ -639,10 +639,10 @@ sc_libvirt_unmarked_diagnostics:
exclude='_\(' \
halt='found unmarked diagnostic(s)' \
$(_sc_search_regexp)
- @{ grep -nE '\<$(func_re) *\(.*;$$' $$($(VC_LIST_EXCEPT)); \
- grep -A1 -nE '\<$(func_re) *\(.*,$$' $$($(VC_LIST_EXCEPT)); } \
+ @{ $(GREP) -nE '\<$(func_re) *\(.*;$$' $$($(VC_LIST_EXCEPT)); \
+ $(GREP) -A1 -nE '\<$(func_re) *\(.*,$$' $$($(VC_LIST_EXCEPT)); } \
| $(SED) 's/_("\([^\"]\|\\.\)\+"//;s/[ ]"%s"//' \
- | grep '[ ]"' && \
+ | $(GREP) '[ ]"' && \
{ echo '$(ME): found unmarked diagnostic(s)' 1>&2; \
exit 1; } || :
@@ -654,9 +654,9 @@ sc_libvirt_unmarked_diagnostics:
# there are functions to which this one applies but that do not get marked
# diagnostics.
sc_prohibit_newline_at_end_of_diagnostic:
- @grep -A2 -nE \
+ @$(GREP) -A2 -nE \
'\<$(func_re) *\(' $$($(VC_LIST_EXCEPT)) \
- | grep '\\n"' \
+ | $(GREP) '\\n"' \
&& { echo '$(ME): newline at end of message(s)' 1>&2; \
exit 1; } || :
@@ -664,12 +664,12 @@ sc_prohibit_newline_at_end_of_diagnostic:
# allow VIR_ERROR to do this, and ignore functions that take a single
# string rather than a format argument.
sc_prohibit_diagnostic_without_format:
- @{ grep -nE '\<$(func_re) *\(.*;$$' $$($(VC_LIST_EXCEPT)); \
- grep -A2 -nE '\<$(func_re) *\(.*,$$' $$($(VC_LIST_EXCEPT)); } \
+ @{ $(GREP) -nE '\<$(func_re) *\(.*;$$' $$($(VC_LIST_EXCEPT)); \
+ $(GREP) -A2 -nE '\<$(func_re) *\(.*,$$' $$($(VC_LIST_EXCEPT)); } \
| $(SED) -rn -e ':l; /[,"]$$/ {N;b l;}' \
-e '/(xenapiSessionErrorHandler|vah_(error|warning))/d' \
-e '/\<$(func_re)
*\([^"]*"([^%"]|"\n[^"]*")*"[,)]/p' \
- | grep -vE 'VIR_ERROR' && \
+ | $(GREP) -vE 'VIR_ERROR' && \
{ echo '$(ME): found diagnostic without %' 1>&2; \
exit 1; } || :
@@ -687,16 +687,16 @@ sc_prohibit_useless_translation:
# When splitting a diagnostic across lines, ensure that there is a space
# or \n on one side of the split.
sc_require_whitespace_in_translation:
- @grep -n -A1 '"$$' $$($(VC_LIST_EXCEPT)) \
+ @$(GREP) -n -A1 '"$$' $$($(VC_LIST_EXCEPT)) \
| $(SED) -ne ':l; /"$$/ {N;b l;}; s/"\n[^"]*"/""/g;
s/\\n/ /g' \
- -e '/_(.*[^\ ]""[^\ ]/p' | grep . && \
+ -e '/_(.*[^\ ]""[^\ ]/p' | $(GREP) . && \
{ echo '$(ME): missing whitespace at line split' 1>&2; \
exit 1; } || :
# Enforce recommended preprocessor indentation style.
sc_preprocessor_indentation:
@if cppi --version >/dev/null 2>&1; then \
- $(VC_LIST_EXCEPT) | grep -E '\.[ch](\.in)?$$' | xargs cppi -a -c \
+ $(VC_LIST_EXCEPT) | $(GREP) -E '\.[ch](\.in)?$$' | xargs cppi -a -c \
|| { echo '$(ME): incorrect preprocessor indentation' 1>&2; \
exit 1; }; \
else \
@@ -707,13 +707,13 @@ sc_preprocessor_indentation:
# (comment-only) C file that mirrors the same layout as the spec file.
sc_spec_indentation:
@if cppi --version >/dev/null 2>&1; then \
- for f in $$($(VC_LIST_EXCEPT) | grep '\.spec\.in$$'); do \
+ for f in $$($(VC_LIST_EXCEPT) | $(GREP) '\.spec\.in$$'); do \
$(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|"; \
- done | { if grep . >&2; then false; else :; fi; } \
+ done | { if $(GREP) . >&2; then false; else :; fi; } \
|| { echo '$(ME): incorrect preprocessor indentation' 1>&2; \
exit 1; }; \
else \
@@ -803,11 +803,11 @@ sc_prohibit_cross_inclusion:
# When converting an enum to a string, make sure that we track any new
# elements added to the enum by using a _LAST marker.
sc_require_enum_last_marker:
- @grep -A1 -nE '^[^#]*VIR_ENUM_IMPL *\(' $$($(VC_LIST_EXCEPT)) \
+ @$(GREP) -A1 -nE '^[^#]*VIR_ENUM_IMPL *\(' $$($(VC_LIST_EXCEPT)) \
| $(SED) -ne '/VIR_ENUM_IMPL[^,]*,$$/N' \
-e '/VIR_ENUM_IMPL[^,]*,[^,]*[^_,][^L,][^A,][^S,][^T,],/p' \
-e '/VIR_ENUM_IMPL[^,]*,[^,]\{0,4\},/p' \
- | grep . && \
+ | $(GREP) . && \
{ echo '$(ME): enum impl needs to use _LAST marker' 1>&2; \
exit 1; } || :
@@ -883,7 +883,7 @@ sc_prohibit_wrong_filename_in_comment:
if (fail == 1) { \
exit 1; \
} \
- }' $$($(VC_LIST_EXCEPT) | grep '\.[ch]$$') || fail=1; \
+ }' $$($(VC_LIST_EXCEPT) | $(GREP) '\.[ch]$$') || fail=1; \
if test $$fail -eq 1; then \
{ echo '$(ME): The file name in comments must match the' \
'actual file name' 1>&2; exit 1; } \
@@ -918,7 +918,7 @@ sc_require_if_else_matching_braces:
$(_sc_search_regexp)
sc_curly_braces_style:
- @files=$$($(VC_LIST_EXCEPT) | grep '\.[ch]$$'); \
+ @files=$$($(VC_LIST_EXCEPT) | $(GREP) '\.[ch]$$'); \
if $(GREP) -nHP \
'^\s*(?!([a-zA-Z_]*for_?each[a-zA-Z_]*) ?\()([_a-zA-Z0-9]+( [_a-zA-Z0-9]+)*
?\()?(\*?[_a-zA-Z0-9]+(,? \*?[_a-zA-Z0-9\[\]]+)+|void)\) ?\{' \
$$files; then \
@@ -931,7 +931,7 @@ sc_curly_braces_style:
fi
sc_prohibit_windows_special_chars_in_filename:
- @files=$$($(VC_LIST_EXCEPT) | grep '[:*?"<>|]'); \
+ @files=$$($(VC_LIST_EXCEPT) | $(GREP) '[:*?"<>|]'); \
test -n "$$files" && { echo '$(ME): Windows special chars' \
'in filename not allowed:' 1>&2; echo $$files 1>&2; exit 1; } ||
:
@@ -996,8 +996,8 @@ sc_prohibit_sysconf_pagesize:
$(_sc_search_regexp)
sc_prohibit_virSecurity:
- @grep -Pn 'virSecurityManager(?!Ptr)' $$($(VC_LIST_EXCEPT) | grep
'src/qemu/' | \
- grep -v 'src/qemu/qemu_security') && \
+ @$(GREP) -Pn 'virSecurityManager(?!Ptr)' $$($(VC_LIST_EXCEPT) | $(GREP)
'src/qemu/' | \
+ $(GREP) -v 'src/qemu/qemu_security') && \
{ echo '$(ME): prefer qemuSecurity wrappers' 1>&2; exit 1; } || :
sc_prohibit_pthread_create:
@@ -1128,24 +1128,24 @@ endif
# Don't include duplicate header in the source (either *.c or *.h)
prohibit-duplicate-header:
- $(AM_V_GEN)files=$$($(VC_LIST_EXCEPT) | grep '\.[chx]$$'); \
+ $(AM_V_GEN)files=$$($(VC_LIST_EXCEPT) | $(GREP) '\.[chx]$$'); \
$(PERL) -W $(top_srcdir)/build-aux/prohibit-duplicate-header.pl $$files
spacing-check:
- $(AM_V_GEN)files=`$(VC_LIST) | grep '\.c$$'`; \
+ $(AM_V_GEN)files=`$(VC_LIST) | $(GREP) '\.c$$'`; \
$(PERL) $(top_srcdir)/build-aux/check-spacing.pl $$files || \
{ echo '$(ME): incorrect formatting' 1>&2; exit 1; }
mock-noinline:
- $(AM_V_GEN)files=`$(VC_LIST) | grep '\.[ch]$$'`; \
+ $(AM_V_GEN)files=`$(VC_LIST) | $(GREP) '\.[ch]$$'`; \
$(PERL) $(top_srcdir)/build-aux/mock-noinline.pl $$files
header-ifdef:
- $(AM_V_GEN)files=`$(VC_LIST) | grep '\.[h]$$'`; \
+ $(AM_V_GEN)files=`$(VC_LIST) | $(GREP) '\.[h]$$'`; \
$(PERL) $(top_srcdir)/build-aux/header-ifdef.pl $$files
test-wrap-argv:
- $(AM_V_GEN)files=`$(VC_LIST) | grep -E '\.(ldargs|args)'`; \
+ $(AM_V_GEN)files=`$(VC_LIST) | $(GREP) -E '\.(ldargs|args)'`; \
$(PERL) $(top_srcdir)/tests/test-wrap-argv.pl --check $$files
group-qemu-caps:
--
2.20.1
4:58 a.m.
New subject: [libvirt] [PATCH v2 2/3] maint: prefer $(GREP) in cfg.mk
On Thu, Jan 03, 2019 at 01:41:58PM -0600, Eric Blake wrote:
We already used $(GREP) in some places, but might as well use it
everywhere during syntax check, in line with similar recent gnulib
changes.
---
cfg.mk | 54 +++++++++++++++++++++++++++---------------------------
1 file changed, 27 insertions(+), 27 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1:41 p.m.
New subject: [libvirt] [PATCH v2 3/3] maint: split long lines for BSD syntax-check
Similar to the gnulib changes we just incorporated into maint.mk,
it's time to use '$(VC_LIST) | xargs program' instead of
'program $$($(VC_LIST))', in order to bypass the problem of hitting
argv limits due to our large set of files.
Drop several uses of $$files as a temporary variable when we can
instead directly use xargs. While at it, fix a typo in the
prohibit_windows_special_chars error message.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
cfg.mk | 75 +++++++++++++++++++++++++++++-----------------------------
1 file changed, 38 insertions(+), 37 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 9956d20034..e2702d50c9 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -639,8 +639,10 @@ sc_libvirt_unmarked_diagnostics:
exclude='_\(' \
halt='found unmarked diagnostic(s)' \
$(_sc_search_regexp)
- @{ $(GREP) -nE '\<$(func_re) *\(.*;$$' $$($(VC_LIST_EXCEPT)); \
- $(GREP) -A1 -nE '\<$(func_re) *\(.*,$$' $$($(VC_LIST_EXCEPT)); } \
+ @{ $(VC_LIST_EXCEPT) | xargs \
+ $(GREP) -nE '\<$(func_re) *\(.*;$$' /dev/null; \
+ $(VC_LIST_EXCEPT) | xargs \
+ $(GREP) -A1 -nE '\<$(func_re) *\(.*,$$' /dev/null; } \
| $(SED) 's/_("\([^\"]\|\\.\)\+"//;s/[ ]"%s"//' \
| $(GREP) '[ ]"' && \
{ echo '$(ME): found unmarked diagnostic(s)' 1>&2; \
@@ -654,8 +656,8 @@ sc_libvirt_unmarked_diagnostics:
# there are functions to which this one applies but that do not get marked
# diagnostics.
sc_prohibit_newline_at_end_of_diagnostic:
- @$(GREP) -A2 -nE \
- '\<$(func_re) *\(' $$($(VC_LIST_EXCEPT)) \
+ @$(VC_LIST_EXCEPT) | xargs $(GREP) -A2 -nE \
+ '\<$(func_re) *\(' /dev/null \
| $(GREP) '\\n"' \
&& { echo '$(ME): newline at end of message(s)' 1>&2; \
exit 1; } || :
@@ -664,8 +666,10 @@ sc_prohibit_newline_at_end_of_diagnostic:
# allow VIR_ERROR to do this, and ignore functions that take a single
# string rather than a format argument.
sc_prohibit_diagnostic_without_format:
- @{ $(GREP) -nE '\<$(func_re) *\(.*;$$' $$($(VC_LIST_EXCEPT)); \
- $(GREP) -A2 -nE '\<$(func_re) *\(.*,$$' $$($(VC_LIST_EXCEPT)); } \
+ @{ $(VC_LIST_EXCEPT) | xargs \
+ $(GREP) -nE '\<$(func_re) *\(.*;$$' /dev/null; \
+ $(VC_LIST_EXCEPT) | xargs \
+ $(GREP) -A2 -nE '\<$(func_re) *\(.*,$$' /dev/null; } \
| $(SED) -rn -e ':l; /[,"]$$/ {N;b l;}' \
-e '/(xenapiSessionErrorHandler|vah_(error|warning))/d' \
-e '/\<$(func_re)
*\([^"]*"([^%"]|"\n[^"]*")*"[,)]/p' \
@@ -687,7 +691,7 @@ sc_prohibit_useless_translation:
# When splitting a diagnostic across lines, ensure that there is a space
# or \n on one side of the split.
sc_require_whitespace_in_translation:
- @$(GREP) -n -A1 '"$$' $$($(VC_LIST_EXCEPT)) \
+ @$(VC_LIST_EXCEPT) | xargs $(GREP) -n -A1 '"$$' /dev/null \
| $(SED) -ne ':l; /"$$/ {N;b l;}; s/"\n[^"]*"/""/g;
s/\\n/ /g' \
-e '/_(.*[^\ ]""[^\ ]/p' | $(GREP) . && \
{ echo '$(ME): missing whitespace at line split' 1>&2; \
@@ -803,7 +807,8 @@ sc_prohibit_cross_inclusion:
# When converting an enum to a string, make sure that we track any new
# elements added to the enum by using a _LAST marker.
sc_require_enum_last_marker:
- @$(GREP) -A1 -nE '^[^#]*VIR_ENUM_IMPL *\(' $$($(VC_LIST_EXCEPT)) \
+ @$(VC_LIST_EXCEPT) | xargs \
+ $(GREP) -A1 -nE '^[^#]*VIR_ENUM_IMPL *\(' /dev/null \
| $(SED) -ne '/VIR_ENUM_IMPL[^,]*,$$/N' \
-e '/VIR_ENUM_IMPL[^,]*,[^,]*[^_,][^L,][^A,][^S,][^T,],/p' \
-e '/VIR_ENUM_IMPL[^,]*,[^,]\{0,4\},/p' \
@@ -866,8 +871,7 @@ sc_prohibit_atoi:
$(_sc_search_regexp)
sc_prohibit_wrong_filename_in_comment:
- @fail=0; \
- awk 'BEGIN { \
+ @$(VC_LIST_EXCEPT) | $(GREP) '\.[ch]$$' | xargs awk 'BEGIN { \
fail=0; \
} FNR < 3 { \
n=match($$0, /[[:space:]][^[:space:]]*[.][ch][[:space:]:]/); \
@@ -883,11 +887,8 @@ sc_prohibit_wrong_filename_in_comment:
if (fail == 1) { \
exit 1; \
} \
- }' $$($(VC_LIST_EXCEPT) | $(GREP) '\.[ch]$$') || fail=1; \
- if test $$fail -eq 1; then \
- { echo '$(ME): The file name in comments must match the' \
- 'actual file name' 1>&2; exit 1; } \
- fi;
+ }' || { echo '$(ME): The file name in comments must match the' \
+ 'actual file name' 1>&2; exit 1; }
sc_prohibit_virConnectOpen_in_virsh:
@prohibit='\bvirConnectOpen[a-zA-Z]* *\(' \
@@ -918,22 +919,21 @@ sc_require_if_else_matching_braces:
$(_sc_search_regexp)
sc_curly_braces_style:
- @files=$$($(VC_LIST_EXCEPT) | $(GREP) '\.[ch]$$'); \
- if $(GREP) -nHP \
+ @if $(VC_LIST_EXCEPT) | $(GREP) '\.[ch]$$' | xargs $(GREP) -nHP \
'^\s*(?!([a-zA-Z_]*for_?each[a-zA-Z_]*) ?\()([_a-zA-Z0-9]+( [_a-zA-Z0-9]+)*
?\()?(\*?[_a-zA-Z0-9]+(,? \*?[_a-zA-Z0-9\[\]]+)+|void)\) ?\{' \
- $$files; then \
+ /dev/null; then \
echo '$(ME): Non-K&R style used for curly braces around' \
'function body' 1>&2; exit 1; \
fi; \
- if $(GREP) -A1 -En ' ((if|for|while|switch) \(|(else|do)\b)[^{]*$$'\
- $$files | $(GREP) '^[^ ]*- *{'; then \
+ if $(VC_LIST_EXCEPT) | $(GREP) '\.[ch]$$' | xargs \
+ $(GREP) -A1 -En ' ((if|for|while|switch) \(|(else|do)\b)[^{]*$$' \
+ /dev/null | $(GREP) '^[^ ]*- *{'; then \
echo '$(ME): Use hanging braces for compound statements' 1>&2; exit 1;
\
fi
sc_prohibit_windows_special_chars_in_filename:
- @files=$$($(VC_LIST_EXCEPT) | $(GREP) '[:*?"<>|]'); \
- test -n "$$files" && { echo '$(ME): Windows special chars' \
- 'in filename not allowed:' 1>&2; echo $$files 1>&2; exit 1; } ||
:
+ @$(VC_LIST_EXCEPT) | $(GREP) '[:*?"<>|]' && \
+ { echo '$(ME): Windows special chars in filename not allowed' 1>&2; echo
exit 1; } || :
sc_prohibit_mixed_case_abbreviations:
@prohibit='Pci|Usb|Scsi' \
@@ -949,11 +949,11 @@ sc_require_locale_h:
$(_sc_search_regexp)
sc_prohibit_empty_first_line:
- @awk 'BEGIN { fail=0; } \
+ @$(VC_LIST_EXCEPT) | xargs awk 'BEGIN { fail=0; } \
FNR == 1 { if ($$0 == "") { print FILENAME ":1:"; fail=1; } } \
END { if (fail == 1) { \
print "$(ME): Prohibited empty first line" > "/dev/stderr"; \
- } exit fail; }' $$($(VC_LIST_EXCEPT));
+ } exit fail; }'
sc_prohibit_paren_brace:
@prohibit='\)\{$$' \
@@ -996,8 +996,9 @@ sc_prohibit_sysconf_pagesize:
$(_sc_search_regexp)
sc_prohibit_virSecurity:
- @$(GREP) -Pn 'virSecurityManager(?!Ptr)' $$($(VC_LIST_EXCEPT) | $(GREP)
'src/qemu/' | \
- $(GREP) -v 'src/qemu/qemu_security') && \
+ @$(VC_LIST_EXCEPT) | $(GREP) 'src/qemu/' | \
+ $(GREP) -v 'src/qemu/qemu_security' | \
+ xargs $(GREP) -Pn 'virSecurityManager(?!Ptr)' /dev/null && \
{ echo '$(ME): prefer qemuSecurity wrappers' 1>&2; exit 1; } || :
sc_prohibit_pthread_create:
@@ -1128,25 +1129,25 @@ endif
# Don't include duplicate header in the source (either *.c or *.h)
prohibit-duplicate-header:
- $(AM_V_GEN)files=$$($(VC_LIST_EXCEPT) | $(GREP) '\.[chx]$$'); \
- $(PERL) -W $(top_srcdir)/build-aux/prohibit-duplicate-header.pl $$files
+ $(AM_V_GEN)$(VC_LIST_EXCEPT) | $(GREP) '\.[chx]$$' | xargs \
+ $(PERL) -W $(top_srcdir)/build-aux/prohibit-duplicate-header.pl
spacing-check:
- $(AM_V_GEN)files=`$(VC_LIST) | $(GREP) '\.c$$'`; \
- $(PERL) $(top_srcdir)/build-aux/check-spacing.pl $$files || \
+ $(AM_V_GEN)$(VC_LIST) | $(GREP) '\.c$$' | xargs \
+ $(PERL) $(top_srcdir)/build-aux/check-spacing.pl || \
{ echo '$(ME): incorrect formatting' 1>&2; exit 1; }
mock-noinline:
- $(AM_V_GEN)files=`$(VC_LIST) | $(GREP) '\.[ch]$$'`; \
- $(PERL) $(top_srcdir)/build-aux/mock-noinline.pl $$files
+ $(AM_V_GEN)$(VC_LIST) | $(GREP) '\.[ch]$$' | xargs \
+ $(PERL) $(top_srcdir)/build-aux/mock-noinline.pl
header-ifdef:
- $(AM_V_GEN)files=`$(VC_LIST) | $(GREP) '\.[h]$$'`; \
- $(PERL) $(top_srcdir)/build-aux/header-ifdef.pl $$files
+ $(AM_V_GEN)$(VC_LIST) | $(GREP) '\.[h]$$' | xargs \
+ $(PERL) $(top_srcdir)/build-aux/header-ifdef.pl
test-wrap-argv:
- $(AM_V_GEN)files=`$(VC_LIST) | $(GREP) -E '\.(ldargs|args)'`; \
- $(PERL) $(top_srcdir)/tests/test-wrap-argv.pl --check $$files
+ $(AM_V_GEN)$(VC_LIST) | $(GREP) -E '\.(ldargs|args)' | xargs \
+ $(PERL) $(top_srcdir)/tests/test-wrap-argv.pl --check
group-qemu-caps:
$(AM_V_GEN)$(PERL) $(top_srcdir)/tests/group-qemu-caps.pl --check $(top_srcdir)/
--
2.20.1
5 a.m.
New subject: [libvirt] [PATCH v2 3/3] maint: split long lines for BSD syntax-check
On Thu, Jan 03, 2019 at 01:41:59PM -0600, Eric Blake wrote:
Similar to the gnulib changes we just incorporated into maint.mk,
it's time to use '$(VC_LIST) | xargs program' instead of
'program $$($(VC_LIST))', in order to bypass the problem of hitting
argv limits due to our large set of files.
Drop several uses of $$files as a temporary variable when we can
instead directly use xargs. While at it, fix a typo in the
prohibit_windows_special_chars error message.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
cfg.mk | 75 +++++++++++++++++++++++++++++-----------------------------
1 file changed, 38 insertions(+), 37 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 9956d20034..e2702d50c9 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -639,8 +639,10 @@ sc_libvirt_unmarked_diagnostics:
exclude='_\(' \
halt='found unmarked diagnostic(s)' \
$(_sc_search_regexp)
- @{ $(GREP) -nE '\<$(func_re) *\(.*;$$' $$($(VC_LIST_EXCEPT)); \
- $(GREP) -A1 -nE '\<$(func_re) *\(.*,$$' $$($(VC_LIST_EXCEPT)); } \
+ @{ $(VC_LIST_EXCEPT) | xargs \
+ $(GREP) -nE '\<$(func_re) *\(.*;$$' /dev/null; \
+ $(VC_LIST_EXCEPT) | xargs \
+ $(GREP) -A1 -nE '\<$(func_re) *\(.*,$$' /dev/null; } \
Not sure why the /dev/null is needed.
If the syntax check rule were to operate on an empty list of files, we
can just delete it.
| $(SED) 's/_("\([^\"]\|\\.\)\+"//;s/[
]"%s"//' \
| $(GREP) '[ ]"' && \
{ echo '$(ME): found unmarked diagnostic(s)' 1>&2; \
With the /dev/null changes removed or justified:
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
9:11 a.m.
New subject: [libvirt] [PATCH v2 3/3] maint: split long lines for BSD syntax-check
On 1/7/19 5:00 AM, Ján Tomko wrote:
On Thu, Jan 03, 2019 at 01:41:59PM -0600, Eric Blake wrote:
> Similar to the gnulib changes we just incorporated into maint.mk,
> it's time to use '$(VC_LIST) | xargs program' instead of
> 'program $$($(VC_LIST))', in order to bypass the problem of hitting
> argv limits due to our large set of files.
>
> Drop several uses of $$files as a temporary variable when we can
> instead directly use xargs. While at it, fix a typo in the
> prohibit_windows_special_chars error message.
>
> - @{ $(GREP) -nE '\<$(func_re) *\(.*;$$'
$$($(VC_LIST_EXCEPT)); \
> - $(GREP) -A1 -nE '\<$(func_re) *\(.*,$$' $$($(VC_LIST_EXCEPT));
> } \
> + @{ $(VC_LIST_EXCEPT) | xargs \
> + $(GREP) -nE '\<$(func_re) *\(.*;$$' /dev/null; \
> + $(VC_LIST_EXCEPT) | xargs \
> + $(GREP) -A1 -nE '\<$(func_re) *\(.*,$$' /dev/null; } \
Not sure why the /dev/null is needed.
Because of grep semantics.
grep pattern file1
grep pattern file1 file2
have different output, and the syntax checkers depend on the grep output
that is produced when there are multiple files on the command line. The
canonical rewrite of 'grep pattern $(list of files)" into xargs uses
"list of files | xargs grep pattern /dev/null", because you presume that
'list of files' produced a non-empty list, but worry that xargs might do
a worst-case split into 'grep pattern all but one; grep pattern last',
where the different invocation of the last file name alone produces
unusual output that messes up the subsequent pipeline. Putting
/dev/null in the command line argument to grep ensures that we instead
get a split into 'grep pattern /dev/null all but one; grep pattern
/dev/null last' where all invocations are with at least two file names.
The case of 0 files is different - for that, you would normally use
'xargs -r' to ensure that grep is never called with 0 arguments. But
since grep'ing /dev/null will never produce output, we don't need to
worry about xargs -r, even if we could end up with an empty list. And
since the old code wasn't worrying about an empty list, I didn't see a
reason to worry about an empty list in the new code either.
If the syntax check rule were to operate on an empty list of files, we
can just delete it.
> | $(SED) 's/_("\([^\"]\|\\.\)\+"//;s/[
]"%s"//' \
> | $(GREP) '[ ]"' && \
> { echo '$(ME): found unmarked diagnostic(s)' 1>&2; \
With the /dev/null changes removed or justified:
I hope that was the justification you were looking for (and it matches
the gnulib rewrite that uses /dev/null in every conversion to xargs grep
where grep's output differs based on number of file names present - note
that 'grep -L' does not have different output, so it doesn't need the
/dev/null trick, but we didn't have uses of grep -L in cfg.mk).
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
9:39 a.m.
New subject: [libvirt] [PATCH v2 3/3] maint: split long lines for BSD syntax-check
On Mon, Jan 07, 2019 at 09:11:03AM -0600, Eric Blake wrote:
[...]
>> | $(SED)
's/_("\([^\"]\|\\.\)\+"//;s/[ ]"%s"//' \
>> | $(GREP) '[ ]"' && \
>> { echo '$(ME): found unmarked diagnostic(s)' 1>&2; \
>
> With the /dev/null changes removed or justified:
I hope that was the justification you were looking for (and it matches
Yes.
Jano
the gnulib rewrite that uses /dev/null in every conversion to xargs
grep
where grep's output differs based on number of file names present - note
that 'grep -L' does not have different output, so it doesn't need the
/dev/null trick, but we didn't have uses of grep -L in cfg.mk).
2:40 p.m.
New subject: [libvirt] [PATCH v2 3/3] maint: split long lines for BSD syntax-check
On 1/7/19 9:39 AM, Ján Tomko wrote:
On Mon, Jan 07, 2019 at 09:11:03AM -0600, Eric Blake wrote:
[...]
>>> | $(SED) 's/_("\([^\"]\|\\.\)\+"//;s/[
]"%s"//' \
>>> | $(GREP) '[ ]"' && \
>>> { echo '$(ME): found unmarked diagnostic(s)' 1>&2; \
>>
>> With the /dev/null changes removed or justified:
>
> I hope that was the justification you were looking for (and it matches
Yes.
Ok, I improved the commit message and pushed the series.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
1:34 a.m.
On Thu, Jan 03, 2019 at 01:41:56PM -0600, Eric Blake wrote:
Since v1: fix the gnulib bug I accidentally introduced, then
copy the gist of Roman's gnulib changes to also apply to our
cfg.mk syntax checks.
Eric Blake (3):
maint: update gnulib for syntax-check on BSD
maint: prefer $(GREP) in cfg.mk
maint: split long lines for BSD syntax-check
.gnulib | 2 +-
cfg.mk | 95 +++++++++++++++++++++++++++++----------------------------
2 files changed, 49 insertions(+), 48 deletions(-)
--
2.20.1
Hi Eric,
Thank you for making the changes. I'm sorry I was on vacation and wasn't
able to reply earlier.
I've run 'make syntax-check' on libvirt's HEAD. It succeeds on macOS.
Perhaps we can add syntax-check to .travis.yml?
Best regards,
Roman
4:49 a.m.
On Thu, 2019-01-10 at 10:34 +0300, Roman Bolshakov wrote:
I've run 'make syntax-check' on libvirt's HEAD. It
succeeds on macOS.
Perhaps we can add syntax-check to .travis.yml?
syntax-check passes on macOS, but it doesn't quite succeed:
[...]
unmarked_diagnostics
vulnerable_makefile_CVE-2009-4029
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
usage: grep [-abcDEFGHhIiJLlmnOoqRSsUVvwxZ] [-A num] [-B num] [-C[num]]
[-e pattern] [-f file] [--binary-files=value] [--color=when]
[--context[=num]] [--directories=action] [--label] [--line-buffered]
[--null] [pattern] [file ...]
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
maint.mk: skipping test sc_preprocessor_indentation: cppi not installed
grep: -: No such file or directory
maint.mk: skipping sc_prohibit_always-defined_macros: you lack GNU grep
grep: repetition-operator operand invalid
/usr/bin/sed: illegal option -- r
usage: sed script [-Ealn] [-i extension] [file ...]
sed [-Ealn] [-i extension] [-e script] ... [-f script_file] ... [file ...]
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
usage: grep [-abcDEFGHhIiJLlmnOoqRSsUVvwxZ] [-A num] [-B num] [-C[num]]
[-e pattern] [-f file] [--binary-files=value] [--color=when]
[--context[=num]] [--directories=action] [--label] [--line-buffered]
[--null] [pattern] [file ...]
maint.mk: skipping test sc_spec_indentation: cppi not installed
149.00 GFDL_version
150.00 TAB_in_indentation
[...]
(From https://travis-ci.org/andreabolognani/libvirt/jobs/477733819)
If you had time to look into those issues, that would be grand: we
are pretty close to being able to run syntax-check on macOS and
FreeBSD, and any help towards that goal is very much appreciated :)
--
Andrea Bolognani / Red Hat / Virtualization
7:20 a.m.
On Thu, Jan 10, 2019 at 11:49:35AM +0100, Andrea Bolognani wrote:
On Thu, 2019-01-10 at 10:34 +0300, Roman Bolshakov wrote:
> I've run 'make syntax-check' on libvirt's HEAD. It succeeds on
macOS.
> Perhaps we can add syntax-check to .travis.yml?
syntax-check passes on macOS, but it doesn't quite succeed:
[...]
unmarked_diagnostics
vulnerable_makefile_CVE-2009-4029
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
usage: grep [-abcDEFGHhIiJLlmnOoqRSsUVvwxZ] [-A num] [-B num] [-C[num]]
[-e pattern] [-f file] [--binary-files=value] [--color=when]
[--context[=num]] [--directories=action] [--label] [--line-buffered]
[--null] [pattern] [file ...]
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
maint.mk: skipping test sc_preprocessor_indentation: cppi not installed
grep: -: No such file or directory
maint.mk: skipping sc_prohibit_always-defined_macros: you lack GNU grep
grep: repetition-operator operand invalid
/usr/bin/sed: illegal option -- r
usage: sed script [-Ealn] [-i extension] [file ...]
sed [-Ealn] [-i extension] [-e script] ... [-f script_file] ... [file ...]
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
grep: empty (sub)expression
usage: grep [-abcDEFGHhIiJLlmnOoqRSsUVvwxZ] [-A num] [-B num] [-C[num]]
[-e pattern] [-f file] [--binary-files=value] [--color=when]
[--context[=num]] [--directories=action] [--label] [--line-buffered]
[--null] [pattern] [file ...]
maint.mk: skipping test sc_spec_indentation: cppi not installed
149.00 GFDL_version
150.00 TAB_in_indentation
[...]
(From https://travis-ci.org/andreabolognani/libvirt/jobs/477733819)
If you had time to look into those issues, that would be grand: we
are pretty close to being able to run syntax-check on macOS and
FreeBSD, and any help towards that goal is very much appreciated :)
Perhaps we should install grep, gnu-sed and cppi from homebrew in
.travis.yml to get it working. I have all of them installed on my laptop
and syntax-check passes without the errors for me.
grep provides GNU grep and all the "empty (sub)expression" statements
should be gone if it's installed.
sed on macOS doesn't support "-r flag" but supports "-E".
Novertheless,
prohibit_diagnostic_without_format relies on regular expressions that
are not supported in macOS:
prohibit_diagnostic_without_format
sed: 1: "/\<(|VIR_ERROR|lxcError ...": RE error: empty (sub)expression
We need gnu-sed for the rule.
Thanks,
Roman
9:02 a.m.
On Thu, 2019-01-10 at 16:20 +0300, Roman Bolshakov wrote:
Perhaps we should install grep, gnu-sed and cppi from homebrew in
.travis.yml to get it working. I have all of them installed on my laptop
and syntax-check passes without the errors for me.
grep provides GNU grep and all the "empty (sub)expression" statements
should be gone if it's installed.
sed on macOS doesn't support "-r flag" but supports "-E".
Novertheless,
prohibit_diagnostic_without_format relies on regular expressions that
are not supported in macOS:
prohibit_diagnostic_without_format
sed: 1: "/\<(|VIR_ERROR|lxcError ...": RE error: empty (sub)expression
We need gnu-sed for the rule.
Sure we can install cppi, but I think we should try to make our
tests portable rather than throwing in the towel and installing the
GNU tools, especially since making sure we pick those up instead of
the BSD equivalents, which at least on FreeBSD come first in $PATH,
would require adding even more autotools trickery...
--
Andrea Bolognani / Red Hat / Virtualization
2108
days inactive
2115
days old
13 comments
4 participants
participants (4)
-
Andrea Bolognani -
Eric Blake -
Ján Tomko -
Roman Bolshakov