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

[Petr Vorel <pvorel@suse.cz>]

> 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>

I hope you still agree with this ^. Or shell I wait for your ack?

> > 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.

This supports fish, bash and zsh. If anybody asks for t{,csh} or anything else
in .venv/bin/activate* it can be added.

> > +
> > +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.

Thanks!

Kind regards,
Petr

> Andrea


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