Re: [libvirt PATCH 3/3] syntax-check: Run flake8 on all Python scripts
On Mon, 2021-03-22 at 07:50 +0100, Erik Skultety wrote:
On Fri, Mar 19, 2021 at 06:39:31PM +0100, Andrea Bolognani wrote:
> sc_flake8:
> @if [ -n "$(FLAKE8)" ]; then \
> - $(VC_LIST_EXCEPT) | $(GREP) '\.py$$' | xargs \
> - $(FLAKE8) --ignore $(FLAKE8_IGNORE) --show-source; \
> + 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); \
Not that I'd be against ^this, but I think it might be worth (even for
consistency reasons) to mandate that all Python scripts to use the '.py'
extension explicitly instead and rename the ones that violate this. To support
my argument, there are 34 scripts that use a suffix and 2 (one of which is the
latest CI helper) that don't.
It's a time-honored tradition to omit the suffix for scripts which
are called directly by the user, which is why you don't install
packages using dnf.py or build software using meson.py :)
If you don't limit yourself to Python specifically, you'll find several more
examples of this happening in libvirt:
$ git grep -lE '^#!/' | sed -E 's/\.in$//g' | grep -Ev
'\.[^.]+$'
build-aux/useless-if-before-free
build-aux/vc-list-files
ci/helper
examples/sh/virt-lxc-convert
run
tests/libvirtd-fail
tests/libvirtd-pool
tests/qemucapsfixreplies
tests/qemuvhostuserdata/usr/libexec/qemu/vhost-user/test-vhost-user-gpu
tests/virsh-auth
tests/virsh-checkpoint
tests/virsh-cpuset
tests/virsh-define-dev-segfault
tests/virsh-int-overflow
tests/virsh-optparse
tests/virsh-output
tests/virsh-output-commands
tests/virsh-read-bufsiz
tests/virsh-read-non-seekable
tests/virsh-schedinfo
tests/virsh-self-test
tests/virsh-snapshot
tests/virsh-start
tests/virsh-undefine
tests/virsh-uriprecedence
tests/virsh-vcpupin
tests/virt-aa-helper-test
tools/virt-pki-validate
tools/virt-sanlock-cleanup
tools/virt-xml-validate
Finally, if we wanted to enforce the convention that all Python
script in the repository have to be named something.py, then we'd
have to introduce a new syntax-check rule for that...
--
Andrea Bolognani / Red Hat / Virtualization