Re: [PATCH v6 22/25] python: add Makefile for some common tasks

[Cleber Rosa <crosa@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4014 bytes --]

On Wed, May 12, 2021 at 07:12:38PM -0400, John Snow wrote:
> Add "make venv" to create the pipenv-managed virtual environment that
> contains our explicitly pinned dependencies.
> 
> Add "make check" to run the python linters [in the host execution
> environment].
> 
> Add "make venv-check" which combines the above two: create/update the
> venv, then run the linters in that explicitly managed environment.
> 
> Add "make develop" which canonizes the runes needed to get both the
> linting pre-requisites (the "[devel]" part), and the editable
> live-install (the "-e" part) of these python libraries.
> 
> make clean: delete miscellaneous python packaging output possibly
> created by pipenv, pip, or other python packaging utilities
> 
> make distclean: delete the above, the .venv, and the editable "qemu"
> package forwarder (qemu.egg-info) if there is one.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/README.rst |  3 +++
>  python/Makefile   | 42 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+)
>  create mode 100644 python/Makefile
> 
> diff --git a/python/README.rst b/python/README.rst
> index e107bd12a69..3e09d20c23c 100644
> --- a/python/README.rst
> +++ b/python/README.rst
> @@ -35,6 +35,9 @@ Files in this directory
>  - ``qemu/`` Python package source directory.
>  - ``tests/`` Python package tests directory.
>  - ``avocado.cfg`` Configuration for the Avocado test-runner.
> +  Used by ``make check`` et al.
> +- ``Makefile`` provides some common testing/installation invocations.
> +  Try ``make help`` to see available targets.
>  - ``MANIFEST.in`` is read by python setuptools, it specifies additional files
>    that should be included by a source distribution.
>  - ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org.
> diff --git a/python/Makefile b/python/Makefile
> new file mode 100644
> index 00000000000..184f59e5634
> --- /dev/null
> +++ b/python/Makefile
> @@ -0,0 +1,42 @@
> +.PHONY: help venv venv-check check clean distclean develop
> +
> +help:
> +	@echo "python packaging help:"
> +	@echo ""
> +	@echo "make venv:       Create pipenv's virtual environment."
> +	@echo "    NOTE: Requires Python 3.6 and pipenv."
> +	@echo "          Will download packages from PyPI."
> +	@echo "    Hint: (On Fedora): 'sudo dnf install python36 pipenv'"
> +	@echo ""
> +	@echo "make venv-check: run linters using pipenv's virtual environment."
> +	@echo "    Hint: If you don't know which test to run, run this one!"
> +	@echo ""
> +	@echo "make develop:    Install deps for 'make check', and"
> +	@echo "                 the qemu libs in editable/development mode."
> +	@echo ""
> +	@echo "make check:      run linters using the current environment."
> +	@echo ""

Let's observe how this will be used (or misused).  I fear most people
will jump into `make check`, even though you have described `make
venv-check` as the primary choice.

We have a precedent with `make check-acceptance` that will create a
venv and use it by default, so we can consider that as a fallback
strategy based on user feedback.

> +	@echo "make clean:      remove build output."
> +	@echo ""
> +	@echo "make distclean:  remove venv files, qemu package forwarder, and"
> +	@echo "                 everything from 'make clean'."
> +
> +venv: .venv
> +.venv: Pipfile.lock
> +	@PIPENV_VENV_IN_PROJECT=1 pipenv sync --dev --keep-outdated
> +	@touch .venv
> +
> +venv-check: venv
> +	@pipenv run make check
> +
> +develop:
> +	pip3 install -e .[devel]
> +
> +check:
> +	@avocado --config avocado.cfg run tests/
> +
> +clean:
> +	rm -rf build/ dist/
> +

Usually `python3 setup.py clean --all` would be the better choice here,
but, it doesn't clean `dist/`, so I'm OK with this.

> +distclean: clean
> +	rm -rf qemu.egg-info/ .venv/
> -- 
> 2.30.2
> 
> 

Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]