On Mon, 2019-06-17 at 14:13 +0200, Michal Privoznik wrote:
[...]
> @@ -695,6 +695,11 @@ sc_require_whitespace_in_translation:
> { echo '$(ME): missing whitespace at line split' 1>&2; \
> exit 1; } || :
>
> +cppi_banner = \
> + " *****************************************************\n" \
> + "* cppi not installed, some checks have been skipped *\n" \
> + "*****************************************************"
> +
> # Enforce recommended preprocessor indentation style.
> sc_preprocessor_indentation:
> @if cppi --version >/dev/null 2>&1; then \
> @@ -702,7 +707,7 @@ sc_preprocessor_indentation:
> || { echo '$(ME): incorrect preprocessor indentation' 1>&2; \
> exit 1; }; \
> else \
> - echo '$(ME): skipping test $@: cppi not installed' 1>&2; \
> + echo -e "$(ME): skipping test $@:\n"$(cppi_banner) 1>&2; \
> fi
>
> # Enforce similar spec file indentation style, by running cppi on a
> @@ -719,7 +724,7 @@ sc_spec_indentation:
> || { echo '$(ME): incorrect preprocessor indentation' 1>&2; \
> exit 1; }; \
> else \
> - echo '$(ME): skipping test $@: cppi not installed' 1>&2; \
> + echo -e "$(ME): skipping test $@:\n"$(cppi_banner) 1>&2; \
> fi
>
> # Nested conditionals are easier to understand if we enforce that endifs
How about this instead of all of the above?
----- 8< ----- 8< ----- 8< ----- 8< ----- 8< ----- 8< ----- 8< -----
@@ -1150,6 +1145,11 @@ ifneq ($(_gl-Makefile),)
syntax-check: spacing-check test-wrap-argv \
prohibit-duplicate-header mock-noinline group-qemu-caps \
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
endif
# Don't include duplicate header in the source (either *.c or *.h)
----- >8 ----- >8 ----- >8 ----- >8 ----- >8 ----- >8 ----- >8 -----
That is, keep briefly mentioning that a specific check has been
skipped because cppi is unavailable as we're already doing, but also
print a big fat warning *at the very end* so that you can't possibly
miss it even if you have been paying no attention while the command
was running.
[...]
> +# For 'make syntax-check'
> +%if 0%{?fedora}
> +BuildRequires: cppi
> +%endif
Pavel almost convinced me this is not appropriate for the spec file
by pointing out that we're not actually running 'make syntax-check'
when building the RPM, but Dan pointed out that it can be useful
for people using 'dnf builddep' so it's kind of a toss-up...
I'd probably slightly prefer dropping the hunk, especially once we
have the big fat warning.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
