[libvirt] [PATCH] maint: Use pep8 to implement sc_prohibit_semicolon_at_eol_in_python

[Daniel P. Berrangé]

On Thu, Sep 12, 2019 at 05:55:38PM +0800, Shi Lei wrote:
> Now sc_prohibit_semicolon_at_eol_in_python can't handle semicolon
> within multiline strings(comments) properly.
> 
> I suggest that we could use pep8 to check python code style error, such
> as 'statement ends with a semicolon'. In future, we could use '--select'
> to introduce other rules.
> 
> Signed-off-by: Shi Lei <shi_lei at massclouds.com>
> ---
>  cfg.mk       | 6 ++----
>  configure.ac | 4 ++++
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/cfg.mk b/cfg.mk
> index 42e1abf0..c8eaf74e 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -813,10 +813,8 @@ sc_require_enum_last_marker:
>  
>  # In Python files we don't want to end lines with a semicolon like in C
>  sc_prohibit_semicolon_at_eol_in_python:
> -	@prohibit='^[^#].*\;$$' \
> -	in_vc_files='\.py$$' \
> -	halt='python does not require to end lines with a semicolon' \
> -	  $(_sc_search_regexp)
> +	@$(VC_LIST_EXCEPT) | $(GREP) '\.py$$' | xargs $(PEP8) --select E703 \
> +		| $(GREP) . && { exit 1; } || :
>  
>  # mymain() in test files should use return, not exit, for nicer output
>  sc_prohibit_exit_in_tests:
> diff --git a/configure.ac b/configure.ac
> index bf9e7681..37fa9924 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -704,6 +704,10 @@ AC_PATH_PROGS([PYTHON], [python3 python2 python])
>  if test -z "$PYTHON"; then
>      AC_MSG_ERROR(['python3', 'python2' or 'python' binary is required to build libvirt])
>  fi
> +AC_PATH_PROG([PEP8], [pep8])
> +if test -z "$PEP8"; then
> +    AC_MSG_ERROR(['pep8' binary is required to check python code style])
> +fi
>  AC_PATH_PROG([PERL], [perl])
>  if test -z "$PERL"; then
>           AC_MSG_ERROR(['perl' binary is required to build libvirt])

Using pep8 is an interesting idea. Especially with my series to
standardize on using python for all build scripts, it will be
valuable to have much more advanced python style checks.

The only thing I wonder about is whether its reasonable to make
it a mandatory requirement or not, since it is a separate package
from python itself we can't assume it is present I think. It is
on the various Linux we care about and FreeBSD too, but I'm not
seeing it for macOS via homebrew.

Also on my host 'pep8' is a python2 impl, you need 'pep8-3' for
the python3 impl. Except that when I run it, it warns that
it is renamed to pycodestyle upstream and 'pep8' will be dropped
in future.

IOW, I think we'll need to check for existence of the first available
bniary from the list in this order:

  pycodestyle-3 pycodestyle pycodestyle-2 pep8-3 pep8 pep8-2


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|