[PATCH 03/16] build-aux: introduce 'black' tool for python formatting

[Michal Prívozník]

On 3/8/23 17:39, Daniel P. Berrangé wrote:
> The 'black' tool is intended to be an opinionated formatting
> tool for python code. It is complementary to flake8 which
> validates coding bad practices, but (mostly) ignores code
> layout issues.
> 
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>  build-aux/Makefile.in     |  1 +
>  build-aux/meson.build     |  5 +++++
>  build-aux/syntax-check.mk | 18 ++++++++++++++++++
>  meson.build               |  1 +
>  4 files changed, 25 insertions(+)
> 
> diff --git a/build-aux/Makefile.in b/build-aux/Makefile.in
> index 7ee4680847..db44ee36a4 100644
> --- a/build-aux/Makefile.in
> +++ b/build-aux/Makefile.in
> @@ -3,6 +3,7 @@
>  top_srcdir = @top_srcdir@
>  top_builddir = @top_builddir@
>  FLAKE8 = @flake8_path@
> +BLACK = @black_path@
>  RUNUTF8 = @runutf8@
>  PYTHON = @PYTHON3@
>  GREP = @GREP@
> diff --git a/build-aux/meson.build b/build-aux/meson.build
> index 16d085505d..0330f2940c 100644
> --- a/build-aux/meson.build
> +++ b/build-aux/meson.build
> @@ -2,6 +2,10 @@ flake8_path = ''
>  if flake8_prog.found()
>    flake8_path = flake8_prog.full_path()
>  endif
> +black_path = ''
> +if black_prog.found()
> +  black_path = black_prog.full_path()
> +endif
>  
>  if host_machine.system() == 'freebsd' or host_machine.system() == 'darwin'
>    make_prog = find_program('gmake')
> @@ -32,6 +36,7 @@ syntax_check_conf = configuration_data({
>    'top_srcdir': meson.project_source_root(),
>    'top_builddir': meson.project_build_root(),
>    'flake8_path': flake8_path,
> +  'black_path': black_path,
>    'runutf8': ' '.join(runutf8),
>    'PYTHON3': python3_prog.full_path(),
>    'GREP': grep_prog.full_path(),
> diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
> index 158f14e77e..6d82a4301a 100644
> --- a/build-aux/syntax-check.mk
> +++ b/build-aux/syntax-check.mk
> @@ -595,6 +595,16 @@ sc_flake8:
>  		echo 'skipping test $@: flake8 not installed' 1>&2; \
>  	fi
>  
> +sc_black:
> +	if [ -n "$(BLACK)" ]; then \
> +		DOT_PY=$$($(VC_LIST_EXCEPT) | $(GREP) '\.py$$'); \
> +		BANG_PY=$$($(VC_LIST_EXCEPT) | xargs grep -l '^#!/usr/bin/env python3$$'); \
> +		ALL_PY=$$(printf "%s\n%s" "$$DOT_PY" "$$BANG_PY" | sort -u); \
> +		echo "$$ALL_PY" | xargs $(BLACK) --check; \

At this point, there's no file passed to $(BLACK) (because of
exclude_file_... below) and thus it fails. It starts working after next
patch when new python scripts are introduced.

Nevertheless, it may still be worth running this as:

  echo "$$ALL_PY" | xargs --no-run-if-empty -- $(BLACK) --check;

(the same could be done to flake8 from which you copied these lines)

> +	else \
> +		echo 'skipping test $@: black not installed' 1>&2; \
> +	fi
> +
>  # mymain() in test files should use return, not exit, for nicer output
>  sc_prohibit_exit_in_tests:
>  	@prohibit='\<exit *\(' \
> @@ -1303,6 +1313,11 @@ syntax-check: sc_spacing-check \
>  		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)
> @@ -1479,6 +1494,9 @@ exclude_file_name_regexp--sc_prohibit_select = \
>    ^build-aux/syntax-check\.mk|src/util/vireventglibwatch\.c|tests/meson\.build$$
>  
>  
> +exclude_file_name_regexp--sc_black = \
> +  ^tools/|src/|tests/|ci/|run\.in|scripts/[^/]*\.py
> +

Michal