[libvirt] [PATCH v3] Produce more verbose error if cppi not found

Andrea Bolognani abologna at redhat.com
Mon Jun 17 16:40:14 UTC 2019


On Mon, 2019-06-17 at 18:21 +0200, Michal Privoznik wrote:
> It's fairly easy (especially for new contributors) to not spot
> the 'cppi not installed' line in the syntax-check output. Turn it
> into a banner that is more visible

Good so far.

> and at the same time add it as
> a build dependency. Unfortunately, RHEL doesn't ship cppi so we
> can add the dependency only for Fedora.

Depending on the outcome of the discussion below, this part might
need to be dropped.

> Since it's v1 this has effectively became code copied over from
> Andrea's review suggestions.

I don't feel like this remark belongs in the git history.

[...]
>  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

ACK to this hunk.

[...]
> +# For 'make syntax-check'
> +%if 0%{?fedora}
> +BuildRequires: cppi
> +%endif

CC'ing Dan and Martin, who argued for having this, and Pavel, who
argued against and made me switch sides. Let's see what ends up
being its fate :)

-- 
Andrea Bolognani / Red Hat / Virtualization




More information about the libvir-list mailing list