[PATCH 0/3] syntax-check: Sync with gnulib, some tweaks

One might see patch 3/3 and think hey, it would actually be nice if developers were getting warned that some checks haven't been executed because a tool is missing! And I completely agree. However, since meson needs to see an exit code of 77 to mark a test as skipped, and make return codes are in the range 0-2, making that happen would require taking the cppi/flake8/black checks out of the current scaffolding and wrap them in some other way. I have no intention to shave that specific yak today O:-) Andrea Bolognani (3): all: Don't use 'grep -q' syntax-check: Sync with gnulib syntax-check: Drop 'syntax-check' target build-aux/syntax-check.mk | 33 ++++++++------------------------- libvirt.spec.in | 2 +- tests/virsh-uriprecedence | 2 +- tests/virt-aa-helper-test | 2 +- 4 files changed, 11 insertions(+), 28 deletions(-) -- 2.43.0

It's not portable. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- libvirt.spec.in | 2 +- tests/virsh-uriprecedence | 2 +- tests/virt-aa-helper-test | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 49ce717e1b..794645c9cc 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -281,7 +281,7 @@ Release: 1%{?dist} License: GPL-2.0-or-later AND LGPL-2.1-only AND LGPL-2.1-or-later AND OFL-1.1 URL: https://libvirt.org/ -%if %(echo %{version} | grep -q "\.0$"; echo $?) == 1 +%if %(echo %{version} | grep "\.0$" >/dev/null; echo $?) == 1 %define mainturl stable_updates/ %endif Source: https://download.libvirt.org/%{?mainturl}libvirt-%{version}.tar.xz diff --git a/tests/virsh-uriprecedence b/tests/virsh-uriprecedence index fd6ce108c0..f141d08dfd 100755 --- a/tests/virsh-uriprecedence +++ b/tests/virsh-uriprecedence @@ -15,7 +15,7 @@ mock_xdg_ || framework_failure is_uri_good() { - echo "$1" | grep -q -F "$good_uri" + echo "$1" | grep -F "$good_uri" >/dev/null } test_uri_internal() diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index 83f53acef6..9a97168330 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -139,7 +139,7 @@ testme() { rule_missing=0 if [ -n "$checkrule" ]; then - if ! grep -q "$checkrule" "$tmpout"; then + if ! grep "$checkrule" "$tmpout" >/dev/null; then echo "FAIL: missing rule '$checkrule'" >"$output" rule_missing=1 fi -- 2.43.0

The most notable change is the new 'sc_unportable_grep_q' rule. While importing it from gnulib, the rule has been tweaked slightly by adding superflous quotes so that syntax-check.mk itself doesn't trip it. This is similar to the tricks employed for the 'sc_prohibit_close' and 'sc_copyright_usage' rules, among many others. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- build-aux/syntax-check.mk | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index eaeb6d4f23..359dcbc5fb 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -1,13 +1,12 @@ # -# Rules for running syntax-check, derived from gnulib's -# maint.mk +# Rules for running syntax-check, derived from gnulib's top/maint.mk # # Specifically, all shared code should match gnulib commit # -# dd2503c8e73621e919e8e214a29c495ac89d8a92 (2022-05-21) +# d5191e456737661d4a0df5287f6c2064ab74dbbe (2024-02-15) # # Copyright (C) 2008-2019 Red Hat, Inc. -# Copyright (C) 2001-2022 Free Software Foundation, Inc. +# Copyright (C) 2001-2024 Free Software Foundation, Inc. # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -1083,7 +1082,8 @@ sc_prohibit_stdio--_without_use: @h='stdio--.h' re='\<((f(re)?|p)open|tmpfile) *\(' \ $(_sc_header_without_use) -_stddef_syms_re = NULL|offsetof|ptrdiff_t|size_t|wchar_t +_stddef_syms_re = \ + NULL|max_align_t|nullptr_t|offsetof|ptrdiff_t|size_t|unreachable|wchar_t # Prohibit the inclusion of stddef.h without an actual use. sc_prohibit_stddef_without_use: @h='stddef.h' \ @@ -1310,6 +1310,10 @@ sc_prohibit_path_max_allocation: halt='Avoid stack allocations of size PATH_MAX' \ $(_sc_search_regexp) +sc_unportable_grep_q: + @prohibit='grep ''-q' halt="unportable 'grep ""-q', use >/dev/null instead" \ + $(_sc_search_regexp) + ifneq ($(_gl-Makefile),) syntax-check: sc_spacing-check \ sc_prohibit-duplicate-header sc_mock-noinline sc_group-qemu-caps \ -- 2.43.0

Our entry point for syntax-check rules is meson, which calls to each of them specifically; additionally, we have the 'all' target that warns users who try to use make directly. The 'syntax-check' target is not used by anything, and in fact it couldn't be even if one tried: its availability depends on the $(_gl-Makefile) variable, which in our case is never defined. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- build-aux/syntax-check.mk | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index 359dcbc5fb..0a10259bd8 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -1314,27 +1314,6 @@ sc_unportable_grep_q: @prohibit='grep ''-q' halt="unportable 'grep ""-q', use >/dev/null instead" \ $(_sc_search_regexp) -ifneq ($(_gl-Makefile),) -syntax-check: sc_spacing-check \ - sc_prohibit-duplicate-header sc_mock-noinline sc_group-qemu-caps \ - sc_header-ifdef - @if ! cppi --version >/dev/null 2>&1; then \ - echo "*****************************************************" >&2; \ - echo "* cppi not installed, some checks have been skipped *" >&2; \ - echo "*****************************************************" >&2; \ - fi; \ - if [ -z "$(FLAKE8)" ]; then \ - echo "*****************************************************" >&2; \ - echo "* flake8 not installed, sc_flake8 has been skipped *" >&2; \ - echo "*****************************************************" >&2; \ - fi - if [ -z "$(BLACK)" ]; then \ - echo "*****************************************************" >&2; \ - echo "* black not installed, sc_black has been skipped *" >&2; \ - echo "*****************************************************" >&2; \ - fi -endif - # Don't include duplicate header in the source (either *.c or *.h) sc_prohibit-duplicate-header: $(AM_V_GEN)$(VC_LIST_EXCEPT) | $(GREP) '\.[chx]$$' | $(RUNUTF8) xargs \ -- 2.43.0

On 2/16/24 15:47, Andrea Bolognani wrote:
One might see patch 3/3 and think hey, it would actually be nice if developers were getting warned that some checks haven't been executed because a tool is missing! And I completely agree.
However, since meson needs to see an exit code of 77 to mark a test as skipped, and make return codes are in the range 0-2, making that happen would require taking the cppi/flake8/black checks out of the current scaffolding and wrap them in some other way. I have no intention to shave that specific yak today O:-)
Andrea Bolognani (3): all: Don't use 'grep -q' syntax-check: Sync with gnulib syntax-check: Drop 'syntax-check' target
build-aux/syntax-check.mk | 33 ++++++++------------------------- libvirt.spec.in | 2 +- tests/virsh-uriprecedence | 2 +- tests/virt-aa-helper-test | 2 +- 4 files changed, 11 insertions(+), 28 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

On Fri, Feb 16, 2024 at 03:47:59PM +0100, Andrea Bolognani wrote:
One might see patch 3/3 and think hey, it would actually be nice if developers were getting warned that some checks haven't been executed because a tool is missing! And I completely agree.
However, since meson needs to see an exit code of 77 to mark a test as skipped, and make return codes are in the range 0-2, making that happen would require taking the cppi/flake8/black checks out of the current scaffolding and wrap them in some other way. I have no intention to shave that specific yak today O:-)
I have an idea how to do that somewhat elegantly, I'll post it soon. This is fine the way it is. Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Andrea Bolognani (3): all: Don't use 'grep -q' syntax-check: Sync with gnulib syntax-check: Drop 'syntax-check' target
build-aux/syntax-check.mk | 33 ++++++++------------------------- libvirt.spec.in | 2 +- tests/virsh-uriprecedence | 2 +- tests/virt-aa-helper-test | 2 +- 4 files changed, 11 insertions(+), 28 deletions(-)
-- 2.43.0 _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org
participants (3)
-
Andrea Bolognani
-
Martin Kletzander
-
Michal Prívozník