[LTP] [PATCH v5 3/3] doc/Makefile: Allow to create and use .venv

[Andrea Cervesato]

Hi Petr,

some comments below.

On 4/7/25 17:01, Petr Vorel wrote:
> Add 'setup' target (alias to '.venv') to create virtualenv directory.
> This is an optional target (not run by default).
> If .venv exists, it's used in other targets, activation supports only
> fish and bash/zsh (known shells used by LTP developers, csh/tcsh is
> ignored atm).
>
> 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).
>
> Add 'distclean' target which removes also .venv/ directory.
>
> Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.com>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> * The same as v4
>
> NOTE: doc/Makefile should be rewritten to use generic_leaf_target.mk,
> then integration to the top level Makefile will not be a hack).
>
>   doc/Makefile | 22 ++++++++++++++++++++--
>   1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/doc/Makefile b/doc/Makefile
> index 3c5682ad00..2062d6e935 100644
> --- a/doc/Makefile
> +++ b/doc/Makefile
> @@ -5,15 +5,33 @@ top_srcdir		?= ..
>   
>   include $(top_srcdir)/include/mk/env_pre.mk
>   
> +PYTHON := python3
> +VENV_DIR := .venv
> +
> +# only fish and bash/zsh supported
> +VENV_CMD := if [ "x${FISH_VERSION}" != "x" ]; then . $(VENV_DIR)/bin/activate.fish; else . $(VENV_DIR)/bin/activate; fi
I had to think carefully about this, but I think you are right. It's 
better not to over-complicate this and to support other shells but bash.
make command can override environment variables, so it's better to use 
that feature instead of complicating Makefile that can be really messy 
when adding new features.
> +
> +RUN_VENV := if [ -d $(VENV_DIR) ]; then $(VENV_CMD); fi
> +
> +$(VENV_DIR):
> +	$(PYTHON) -m virtualenv $(VENV_DIR)
> +	$(VENV_CMD) && pip install -r requirements.txt
> +
> +.PHONY: setup
> +setup: $(VENV_DIR)
> +
>   ${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 \
>   		${abs_top_builddir}/metadata/ltp.json
> +
> +distclean: clean
> +	rm -rf $(VENV_DIR)

The rest looks fine.

Andrea