Re: [LTP] [PATCH v2 1/2] doc/Makefile: Allow to create and use .venv

[Andrea Cervesato via ltp <ltp@lists.linux.it>]

Hi!

This somehow resolves the problem of not explicitly using virtualenv, 
but I have the impression that it has some critical issues.
virtualenv is not part of the buildsystem, but it's an external tool 
that collect python packages and it's up to the developer using it or 
not, and this makes it optional.  By introducing it in the Makefile, we 
end up mixing things a little bit.

Also, this patch complicates the Makefile, since we might need one 
command before running "make" (source our_venv/bin/activate) and two 
commands once in a while to create it (venv + pip).

In short, I have the impression there's no additional value to the 
patch, but mostly an over-engineered Makefile :-)

Andrea

On 2/12/25 00:35, Petr Vorel wrote:
> This is an optional target (not run by default).
> If .venv exists, it's used in other targets.
>
> This helps to use virtualenv for development, but avoid using it by
> default (readthedoc uses container with virtualenv, creating it would be
> waste of time).
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> v1: https://patchwork.ozlabs.org/project/ltp/patch/20250206143421.1571918-4-pvorel@suse.cz/
>
> Changes v1->v2:
> * virtualenv is optional
> * use variables
>
>   doc/Makefile | 18 +++++++++++++++---
>   1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/doc/Makefile b/doc/Makefile
> index a07df04d5c..909d9687b8 100644
> --- a/doc/Makefile
> +++ b/doc/Makefile
> @@ -5,14 +5,26 @@ top_srcdir		?= ..
>   
>   include $(top_srcdir)/include/mk/env_pre.mk
>   
> +PYTHON := python3
> +VENV_DIR := .venv
> +VENV_CMD := . $(VENV_DIR)/bin/activate
> +RUN_VENV := if [ -d $(VENV_DIR) ]; then $(VENV_CMD); fi
> +
> +# install sphinx only if needed
> +INSTALL_SPHINX := $(shell $(PYTHON) -c "import sphinx" 2>/dev/null && echo ":" || echo "pip install sphinx")
> +
> +$(VENV_DIR):
> +	$(PYTHON) -m virtualenv $(VENV_DIR)
> +	$(VENV_CMD) && pip install -r requirements.txt && $(INSTALL_SPHINX)
> +
>   ${abs_top_builddir}/metadata/ltp.json:
>   	$(MAKE) -C ${abs_top_builddir}/metadata
>   
>   all: ${abs_top_builddir}/metadata/ltp.json
> -	sphinx-build -b html . html
> +	$(RUN_VENV); sphinx-build -b html . html
>   
>   spelling:
> -	sphinx-build -b spelling -d build/doctree . build/spelling
> +	$(RUN_VENV); sphinx-build -b spelling -d build/doctree . build/spelling
>   
>   clean:
> -	rm -rf html/ build/ _static/syscalls.rst _static/tests.rst syscalls.tbl
> +	rm -rf html/ build/ _static/syscalls.rst _static/tests.rst syscalls.tbl $(VENV_DIR)

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp