[LTP] [PATCH v3 0/4] Update doc related Makefile

[LTP] [PATCH v3 0/4] Update doc related Makefile

[Petr Vorel]

Hi,

we talked about this in the meetup yesterday.

I would also prefer to change 'make help' to add few reasonable targets
(e.g. make doc), but that'll be another effort.

Kind regards,
Petr

Changes since v2:
* Add new commit "doc/Makefile: Remove also metadata/ltp.json"
* Add distclean in "doc/Makefile: Allow to create and use .venv"
* Makefile: doc target: calls also '.venv' target
* Makefile: Add 'doc-clean' target

Link to v2:
https://patchwork.ozlabs.org/project/ltp/list/?series=443894&state=*
https://lore.kernel.org/ltp/20250211233552.1990618-1-pvorel@suse.cz/

Petr Vorel (4):
  doc/Makefile: Remove also metadata/ltp.json
  doc/Makefile: Allow to create and use .venv
  Makefile: Update 'doc' target, add 'doc-clean'
  doc: Note 'make doc' in the building doc

 Makefile                         | 10 ++++++++--
 doc/Makefile                     | 22 +++++++++++++++++++---
 doc/developers/documentation.rst |  2 ++
 3 files changed, 29 insertions(+), 5 deletions(-)

-- 
2.47.2


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

[LTP] [PATCH v3 1/4] doc/Makefile: Remove also metadata/ltp.json

[Petr Vorel]

Because there is no detection whether tests changed it's better to
remove JSON file on metadata cleanup.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v3.

 doc/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/doc/Makefile b/doc/Makefile
index a07df04d5c..3c5682ad00 100644
--- a/doc/Makefile
+++ b/doc/Makefile
@@ -15,4 +15,5 @@ spelling:
 	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 \
+		${abs_top_builddir}/metadata/ltp.json
-- 
2.47.2


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

[LTP] [PATCH v3 2/4] doc/Makefile: Allow to create and use .venv

[Petr Vorel]

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

Add 'distclean' target which removes also .venv/ directory.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes since v2:
* Add distclean in "doc/Makefile: Allow to create and use .venv"

 doc/Makefile | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/doc/Makefile b/doc/Makefile
index 3c5682ad00..3b8265d88e 100644
--- a/doc/Makefile
+++ b/doc/Makefile
@@ -5,15 +5,30 @@ 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 \
 		${abs_top_builddir}/metadata/ltp.json
+
+distclean: clean
+	rm -rf $(VENV_DIR)
-- 
2.47.2


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

[LTP] [PATCH v3 3/4] Makefile: Update 'doc' target, add 'doc-clean'

[Petr Vorel]

'doc' target previously run docparse documentation. Point it to doc/
directory so that it build sphinx docs. doc/ dir has metadata/ dir as
dependency, no need to specify it. Call also '.venv' target.

NOTE: it's still possible to avoid virtualenv by calling 'make -C doc'

Add 'doc-clean': to remove only generated data (not optional .venv).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes since v2:
* 'doc' target: call also '.venv' target
* Add 'doc-clean' target

 Makefile | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 5066789349..0c572248a4 100644
--- a/Makefile
+++ b/Makefile
@@ -169,8 +169,14 @@ INSTALL_TARGETS		+= $(addprefix $(DESTDIR)/$(bindir)/,$(BINDIR_INSTALL_SCRIPTS))
 
 $(INSTALL_TARGETS): $(INSTALL_DIR) $(DESTDIR)/$(bindir)
 
-.PHONY: doc
-doc: metadata-all
+.PHONY: doc doc-clean
+
+doc:
+	$(MAKE) -C $(abs_builddir)/doc .venv
+	$(MAKE) -C $(abs_builddir)/doc
+
+doc-clean:
+	$(MAKE) -C $(abs_builddir)/doc clean
 
 .PHONY: check
 check: $(CHECK_TARGETS)
-- 
2.47.2


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

[LTP] [PATCH v3 4/4] doc: Note 'make doc' in the building doc

[Petr Vorel]

Update doc by changes in previous commits.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v3.

 doc/developers/documentation.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/doc/developers/documentation.rst b/doc/developers/documentation.rst
index 27c847e125..2032409bcb 100644
--- a/doc/developers/documentation.rst
+++ b/doc/developers/documentation.rst
@@ -30,6 +30,8 @@ Before building, make sure you have python3 ``virtualenv`` module installed.
     # build documentation
     make
 
+Instead of the above it's possible call ``make doc`` in the top level directory.
+
 Once the procedure has been completed, documentation will be visible at
 ``doc/html/index.html``.
 
-- 
2.47.2


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

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

[Andrea Cervesato via ltp]

Hi,

On 3/25/25 00:40, 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).
>
> Add 'distclean' target which removes also .venv/ directory.
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Changes since v2:
> * Add distclean in "doc/Makefile: Allow to create and use .venv"
>
>   doc/Makefile | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/doc/Makefile b/doc/Makefile
> index 3c5682ad00..3b8265d88e 100644
> --- a/doc/Makefile
> +++ b/doc/Makefile
> @@ -5,15 +5,30 @@ top_srcdir		?= ..
>   
>   include $(top_srcdir)/include/mk/env_pre.mk
>   
> +PYTHON := python3
> +VENV_DIR := .venv
> +VENV_CMD := . $(VENV_DIR)/bin/activate
This will cut off all shells not supporting "activate" script, such as 
fish or csh.
Quite possible this Makefile would work only on CI, since developers 
often customize their own shells, unless you override VENV_CMD. And, in 
that case, virtualenv creation is 1 command away and it makes this 
Makefile feature superfluous.
> +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")
This can be added to requirements.txt, there's no need to handle it in 
Makefile.
> +
> +$(VENV_DIR):
"setup" stage is more clear than using virtualenv directory name.
> +	$(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 \
>   		${abs_top_builddir}/metadata/ltp.json
> +
> +distclean: clean
> +	rm -rf $(VENV_DIR)
Andrea

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

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

[Petr Vorel]

Hi Andrea,

first, thanks for your review.

> Hi,

> On 3/25/25 00:40, 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).

> > Add 'distclean' target which removes also .venv/ directory.

> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> > Changes since v2:
> > * Add distclean in "doc/Makefile: Allow to create and use .venv"

> >   doc/Makefile | 19 +++++++++++++++++--
> >   1 file changed, 17 insertions(+), 2 deletions(-)

> > diff --git a/doc/Makefile b/doc/Makefile
> > index 3c5682ad00..3b8265d88e 100644
> > --- a/doc/Makefile
> > +++ b/doc/Makefile
> > @@ -5,15 +5,30 @@ top_srcdir		?= ..
> >   include $(top_srcdir)/include/mk/env_pre.mk
> > +PYTHON := python3
> > +VENV_DIR := .venv
> > +VENV_CMD := . $(VENV_DIR)/bin/activate
> This will cut off all shells not supporting "activate" script, such as fish
> or csh.

I would guess csh is not much used nowadays in Linux. I would dare to say bash
is much more popular than modern fish (I know more people using zsh), but sure,
how about:

VENV_CMD := . $(VENV_DIR)/bin/activate || . venv/bin/activate.fish

Or even add also activate.csh if you think people are using it.
I would ignore the rest of activate.* scripts

> Quite possible this Makefile would work only on CI, since developers often
> customize their own shells, unless you override VENV_CMD. And, in that case,
> virtualenv creation is 1 command away and it makes this Makefile feature
> superfluous.

I would say in with the above it will work for most of the users (I have heavily
customised shell and venv always worked) and yes, it allows to overwrite.

make VENV_CMD=". .venv/bin/activate.csh"

> > +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")
> This can be added to requirements.txt, there's no need to handle it in
> Makefile.

I added this as it speeds up the installation. Sure, I can remove it.

> > +
> > +$(VENV_DIR):
> "setup" stage is more clear than using virtualenv directory name.

Using directory name gives us detection if it's needed for free. You complained
about Makefile getting complicated, using setup would complicate it more. Do you
want that?

FYI I expect people will just call 'make doc' in toplevel directory (see
following commits). These people will not even know about name of the .venv
target.

And people doing packaging and still want to build the documentation (IMHO
nobody currently does: SUSE, Buildroot and Yocto), it's still possible via
'make -C doc'.

Kind regards,
Petr

> > +	$(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 \
> >   		${abs_top_builddir}/metadata/ltp.json
> > +
> > +distclean: clean
> > +	rm -rf $(VENV_DIR)
> Andrea

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

Re: [LTP] [PATCH v3 3/4] Makefile: Update 'doc' target, add 'doc-clean'

[Ricardo B. Marli��re via ltp]

On Mon Mar 24, 2025 at 8:40 PM -03, Petr Vorel wrote:
> 'doc' target previously run docparse documentation. Point it to doc/
> directory so that it build sphinx docs. doc/ dir has metadata/ dir as
> dependency, no need to specify it. Call also '.venv' target.
>
> NOTE: it's still possible to avoid virtualenv by calling 'make -C doc'

From the root dir ?

$ make -C doc
make: Entering directory '/mnt/ext/src/linux/ltp/mail/doc'
make -C /mnt/ext/src/linux/ltp/mail/metadata
make[1]: Entering directory '/mnt/ext/src/linux/ltp/mail/metadata'
/mnt/ext/src/linux/ltp/mail/metadata/parse.sh > ltp.json
make[1]: Leaving directory '/mnt/ext/src/linux/ltp/mail/metadata'
if [ -d .venv ]; then . .venv/bin/activate; fi; sphinx-build -b html . html
/bin/sh: line 1: sphinx-build: command not found
make: *** [Makefile:24: all] Error 127
make: Leaving directory '/mnt/ext/src/linux/ltp/mail/doc'

>
> Add 'doc-clean': to remove only generated data (not optional .venv).
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Changes since v2:
> * 'doc' target: call also '.venv' target
> * Add 'doc-clean' target
>
>  Makefile | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 5066789349..0c572248a4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -169,8 +169,14 @@ INSTALL_TARGETS		+= $(addprefix $(DESTDIR)/$(bindir)/,$(BINDIR_INSTALL_SCRIPTS))
>  
>  $(INSTALL_TARGETS): $(INSTALL_DIR) $(DESTDIR)/$(bindir)
>  
> -.PHONY: doc
> -doc: metadata-all
> +.PHONY: doc doc-clean
> +
> +doc:
> +	$(MAKE) -C $(abs_builddir)/doc .venv
> +	$(MAKE) -C $(abs_builddir)/doc
> +
> +doc-clean:
> +	$(MAKE) -C $(abs_builddir)/doc clean
>  
>  .PHONY: check
>  check: $(CHECK_TARGETS)


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

Re: [LTP] [PATCH v3 3/4] Makefile: Update 'doc' target, add 'doc-clean'

[Andrea Cervesato via ltp]

Hi Ricardo,

On 3/27/25 12:50, Ricardo B. Marli��re via ltp wrote:
> On Mon Mar 24, 2025 at 8:40 PM -03, Petr Vorel wrote:
>> 'doc' target previously run docparse documentation. Point it to doc/
>> directory so that it build sphinx docs. doc/ dir has metadata/ dir as
>> dependency, no need to specify it. Call also '.venv' target.
>>
>> NOTE: it's still possible to avoid virtualenv by calling 'make -C doc'
>  From the root dir ?
>
> $ make -C doc
> make: Entering directory '/mnt/ext/src/linux/ltp/mail/doc'
> make -C /mnt/ext/src/linux/ltp/mail/metadata
> make[1]: Entering directory '/mnt/ext/src/linux/ltp/mail/metadata'
> /mnt/ext/src/linux/ltp/mail/metadata/parse.sh > ltp.json
> make[1]: Leaving directory '/mnt/ext/src/linux/ltp/mail/metadata'
> if [ -d .venv ]; then . .venv/bin/activate; fi; sphinx-build -b html . html
> /bin/sh: line 1: sphinx-build: command not found
> make: *** [Makefile:24: all] Error 127
> make: Leaving directory '/mnt/ext/src/linux/ltp/mail/doc'
Yeah, sphinx should be included in the requirements.txt, otherwise it's 
not possible to access to it via virtualenv.

Andrea


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

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

[Andrea Cervesato via ltp]

Hi Petr,

On 3/25/25 10:48, Petr Vorel wrote:
> Hi Andrea,
>
> first, thanks for your review.
>
>> Hi,
>> On 3/25/25 00:40, 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).
>>> Add 'distclean' target which removes also .venv/ directory.
>>> Signed-off-by: Petr Vorel <pvorel@suse.cz>
>>> ---
>>> Changes since v2:
>>> * Add distclean in "doc/Makefile: Allow to create and use .venv"
>>>    doc/Makefile | 19 +++++++++++++++++--
>>>    1 file changed, 17 insertions(+), 2 deletions(-)
>>> diff --git a/doc/Makefile b/doc/Makefile
>>> index 3c5682ad00..3b8265d88e 100644
>>> --- a/doc/Makefile
>>> +++ b/doc/Makefile
>>> @@ -5,15 +5,30 @@ top_srcdir		?= ..
>>>    include $(top_srcdir)/include/mk/env_pre.mk
>>> +PYTHON := python3
>>> +VENV_DIR := .venv
>>> +VENV_CMD := . $(VENV_DIR)/bin/activate
>> This will cut off all shells not supporting "activate" script, such as fish
>> or csh.
> I would guess csh is not much used nowadays in Linux. I would dare to say bash
> is much more popular than modern fish (I know more people using zsh), but sure,
> how about:
>
> VENV_CMD := . $(VENV_DIR)/bin/activate || . venv/bin/activate.fish
>
> Or even add also activate.csh if you think people are using it.
> I would ignore the rest of activate.* scripts
Yeah, somehow we need to handle those shells.
>> Quite possible this Makefile would work only on CI, since developers often
>> customize their own shells, unless you override VENV_CMD. And, in that case,
>> virtualenv creation is 1 command away and it makes this Makefile feature
>> superfluous.
> I would say in with the above it will work for most of the users (I have heavily
> customised shell and venv always worked) and yes, it allows to overwrite.
>
> make VENV_CMD=". .venv/bin/activate.csh"
>
>>> +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")
>> This can be added to requirements.txt, there's no need to handle it in
>> Makefile.
> I added this as it speeds up the installation. Sure, I can remove it.
>
>>> +
>>> +$(VENV_DIR):
>> "setup" stage is more clear than using virtualenv directory name.
> Using directory name gives us detection if it's needed for free. You complained
> about Makefile getting complicated, using setup would complicate it more. Do you
> want that?
"make setup" is not complicated. Fixing command based on the folder name 
is: none cares how the virtualenv folder is named and none really wants 
to remember how it's called.
If the goal is to keep things simple, exposing Makefile implementations 
to the users sounds weird and counterintuitive.

Andrea


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

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

[Petr Vorel]

Hi Andrea,

> Hi Petr,

> On 3/25/25 10:48, Petr Vorel wrote:
> > Hi Andrea,

> > first, thanks for your review.

> > > Hi,
> > > On 3/25/25 00:40, 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).
> > > > Add 'distclean' target which removes also .venv/ directory.
> > > > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > > > ---
> > > > Changes since v2:
> > > > * Add distclean in "doc/Makefile: Allow to create and use .venv"
> > > >    doc/Makefile | 19 +++++++++++++++++--
> > > >    1 file changed, 17 insertions(+), 2 deletions(-)
> > > > diff --git a/doc/Makefile b/doc/Makefile
> > > > index 3c5682ad00..3b8265d88e 100644
> > > > --- a/doc/Makefile
> > > > +++ b/doc/Makefile
> > > > @@ -5,15 +5,30 @@ top_srcdir		?= ..
> > > >    include $(top_srcdir)/include/mk/env_pre.mk
> > > > +PYTHON := python3
> > > > +VENV_DIR := .venv
> > > > +VENV_CMD := . $(VENV_DIR)/bin/activate
> > > This will cut off all shells not supporting "activate" script, such as fish
> > > or csh.
> > I would guess csh is not much used nowadays in Linux. I would dare to say bash
> > is much more popular than modern fish (I know more people using zsh), but sure,
> > how about:

> > VENV_CMD := . $(VENV_DIR)/bin/activate || . venv/bin/activate.fish

> > Or even add also activate.csh if you think people are using it.
> > I would ignore the rest of activate.* scripts
> Yeah, somehow we need to handle those shells.

I don't have openSUSE statistics, thus using Debian popcon:

* bash: 99.98%
https://qa.debian.org/popcon.php?package=bash

* zsh (compatible with bash):  7.32%
https://qa.debian.org/popcon.php?package=zsh

* fish (you're using): 1.95%
https://qa.debian.org/popcon.php?package=fish

* csh + tcsh: 0.67% + 1.91%
https://qa.debian.org/popcon.php?package=tcsh
https://qa.debian.org/popcon.php?package=csh

I'm not really convinced anybody uses {t}csh on shell development, but ok,
giving up and add also this support :).

> > > Quite possible this Makefile would work only on CI, since developers often
> > > customize their own shells, unless you override VENV_CMD. And, in that case,
> > > virtualenv creation is 1 command away and it makes this Makefile feature
> > > superfluous.
> > I would say in with the above it will work for most of the users (I have heavily
> > customised shell and venv always worked) and yes, it allows to overwrite.

> > make VENV_CMD=". .venv/bin/activate.csh"

> > > > +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")
> > > This can be added to requirements.txt, there's no need to handle it in
> > > Makefile.
> > I added this as it speeds up the installation. Sure, I can remove it.

> > > > +
> > > > +$(VENV_DIR):
> > > "setup" stage is more clear than using virtualenv directory name.
> > Using directory name gives us detection if it's needed for free. You complained
> > about Makefile getting complicated, using setup would complicate it more. Do you
> > want that?
> "make setup" is not complicated. Fixing command based on the folder name is:
> none cares how the virtualenv folder is named and none really wants to
> remember how it's called.
> If the goal is to keep things simple, exposing Makefile implementations to
> the users sounds weird and counterintuitive.

I'm ok to have phony target 'setup' which points to '.venv' target (less code
than to have only setup target bug check for directory presence). I suppose you
talk only about the make target, not about the virtualenv directory name.

About the directory name: I felt '.venv' is kind of the default directory which
is used by people for virtualenv. Looking at:
https://codesearch.debian.net/search?q=%5C.venv&perpkg=1&page=1
People mostly use: .venv, then venv, .venv*, venv*, .env, env, .eggs

That's why it make sense to me create the directory as '.venv'. Also it's good
that it's a hidden directory (starts with '.'). Hope you agree.

Kind regards,
Petr

> Andrea

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

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

[Andrea Cervesato via ltp]

Hi Petyr,

On 3/27/25 15:57, Petr Vorel wrote:
> Hi Andrea,
>
>> Hi Petr,
>> On 3/25/25 10:48, Petr Vorel wrote:
>>> Hi Andrea,
>>> first, thanks for your review.
>>>> Hi,
>>>> On 3/25/25 00:40, 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).
>>>>> Add 'distclean' target which removes also .venv/ directory.
>>>>> Signed-off-by: Petr Vorel <pvorel@suse.cz>
>>>>> ---
>>>>> Changes since v2:
>>>>> * Add distclean in "doc/Makefile: Allow to create and use .venv"
>>>>>     doc/Makefile | 19 +++++++++++++++++--
>>>>>     1 file changed, 17 insertions(+), 2 deletions(-)
>>>>> diff --git a/doc/Makefile b/doc/Makefile
>>>>> index 3c5682ad00..3b8265d88e 100644
>>>>> --- a/doc/Makefile
>>>>> +++ b/doc/Makefile
>>>>> @@ -5,15 +5,30 @@ top_srcdir		?= ..
>>>>>     include $(top_srcdir)/include/mk/env_pre.mk
>>>>> +PYTHON := python3
>>>>> +VENV_DIR := .venv
>>>>> +VENV_CMD := . $(VENV_DIR)/bin/activate
>>>> This will cut off all shells not supporting "activate" script, such as fish
>>>> or csh.
>>> I would guess csh is not much used nowadays in Linux. I would dare to say bash
>>> is much more popular than modern fish (I know more people using zsh), but sure,
>>> how about:
>>> VENV_CMD := . $(VENV_DIR)/bin/activate || . venv/bin/activate.fish
>>> Or even add also activate.csh if you think people are using it.
>>> I would ignore the rest of activate.* scripts
>> Yeah, somehow we need to handle those shells.
> I don't have openSUSE statistics, thus using Debian popcon:
>
> * bash: 99.98%
> https://qa.debian.org/popcon.php?package=bash
>
> * zsh (compatible with bash):  7.32%
> https://qa.debian.org/popcon.php?package=zsh
>
> * fish (you're using): 1.95%
> https://qa.debian.org/popcon.php?package=fish
>
> * csh + tcsh: 0.67% + 1.91%
> https://qa.debian.org/popcon.php?package=tcsh
> https://qa.debian.org/popcon.php?package=csh
>
> I'm not really convinced anybody uses {t}csh on shell development, but ok,
> giving up and add also this support :).
Thanks for the stats. Let's keep bash only then. Default will be good.
>>>> Quite possible this Makefile would work only on CI, since developers often
>>>> customize their own shells, unless you override VENV_CMD. And, in that case,
>>>> virtualenv creation is 1 command away and it makes this Makefile feature
>>>> superfluous.
>>> I would say in with the above it will work for most of the users (I have heavily
>>> customised shell and venv always worked) and yes, it allows to overwrite.
>>> make VENV_CMD=". .venv/bin/activate.csh"
>>>>> +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")
>>>> This can be added to requirements.txt, there's no need to handle it in
>>>> Makefile.
>>> I added this as it speeds up the installation. Sure, I can remove it.
>>>>> +
>>>>> +$(VENV_DIR):
>>>> "setup" stage is more clear than using virtualenv directory name.
>>> Using directory name gives us detection if it's needed for free. You complained
>>> about Makefile getting complicated, using setup would complicate it more. Do you
>>> want that?
>> "make setup" is not complicated. Fixing command based on the folder name is:
>> none cares how the virtualenv folder is named and none really wants to
>> remember how it's called.
>> If the goal is to keep things simple, exposing Makefile implementations to
>> the users sounds weird and counterintuitive.
> I'm ok to have phony target 'setup' which points to '.venv' target (less code
> than to have only setup target bug check for directory presence). I suppose you
> talk only about the make target, not about the virtualenv directory name.
>
> About the directory name: I felt '.venv' is kind of the default directory which
> is used by people for virtualenv. Looking at:
> https://codesearch.debian.net/search?q=%5C.venv&perpkg=1&page=1
> People mostly use: .venv, then venv, .venv*, venv*, .env, env, .eggs
>
> That's why it make sense to me create the directory as '.venv'. Also it's good
> that it's a hidden directory (starts with '.'). Hope you agree.
>
> Kind regards,
> Petr
>
>> Andrea
Yes I meant "make setup" target, not the virtualenv directory name. That 
can be anything and ".venv" is fine.


Andrea


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

Re: [LTP] [PATCH v3 3/4] Makefile: Update 'doc' target, add 'doc-clean'

[Petr Vorel]

> Hi Ricardo,

> On 3/27/25 12:50, Ricardo B. Marli��re via ltp wrote:
> > On Mon Mar 24, 2025 at 8:40 PM -03, Petr Vorel wrote:
> > > 'doc' target previously run docparse documentation. Point it to doc/
> > > directory so that it build sphinx docs. doc/ dir has metadata/ dir as
> > > dependency, no need to specify it. Call also '.venv' target.

> > > NOTE: it's still possible to avoid virtualenv by calling 'make -C doc'
> >  From the root dir ?

> > $ make -C doc
> > make: Entering directory '/mnt/ext/src/linux/ltp/mail/doc'
> > make -C /mnt/ext/src/linux/ltp/mail/metadata
> > make[1]: Entering directory '/mnt/ext/src/linux/ltp/mail/metadata'
> > /mnt/ext/src/linux/ltp/mail/metadata/parse.sh > ltp.json
> > make[1]: Leaving directory '/mnt/ext/src/linux/ltp/mail/metadata'
> > if [ -d .venv ]; then . .venv/bin/activate; fi; sphinx-build -b html . html
> > /bin/sh: line 1: sphinx-build: command not found
> > make: *** [Makefile:24: all] Error 127
> > make: Leaving directory '/mnt/ext/src/linux/ltp/mail/doc'
> Yeah, sphinx should be included in the requirements.txt, otherwise it's not
> possible to access to it via virtualenv.

OK, I'll add it in the next version.

Kind regards,
Petr

> Andrea


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

Re: [LTP] [PATCH v3 3/4] Makefile: Update 'doc' target, add 'doc-clean'

[Petr Vorel]

Hi Ricardo,

> On Mon Mar 24, 2025 at 8:40 PM -03, Petr Vorel wrote:
> > 'doc' target previously run docparse documentation. Point it to doc/
> > directory so that it build sphinx docs. doc/ dir has metadata/ dir as
> > dependency, no need to specify it. Call also '.venv' target.

> > NOTE: it's still possible to avoid virtualenv by calling 'make -C doc'

Thanks for testing.

> From the root dir ?

Yes, this is noted in the 4th patch
https://patchwork.ozlabs.org/project/ltp/patch/20250324234016.367228-5-pvorel@suse.cz/

> $ make -C doc
> make: Entering directory '/mnt/ext/src/linux/ltp/mail/doc'
> make -C /mnt/ext/src/linux/ltp/mail/metadata
> make[1]: Entering directory '/mnt/ext/src/linux/ltp/mail/metadata'
> /mnt/ext/src/linux/ltp/mail/metadata/parse.sh > ltp.json
> make[1]: Leaving directory '/mnt/ext/src/linux/ltp/mail/metadata'
> if [ -d .venv ]; then . .venv/bin/activate; fi; sphinx-build -b html . html
> /bin/sh: line 1: sphinx-build: command not found
> make: *** [Makefile:24: all] Error 127
> make: Leaving directory '/mnt/ext/src/linux/ltp/mail/doc'

As I wrote, I'll add sphinx to requrements.txt (as Andrea suggested).

Kind regards,
Petr

> > Add 'doc-clean': to remove only generated data (not optional .venv).

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