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

["Ricardo B. Marli��re via ltp" <ltp@lists.linux.it>]

On Wed Apr 9, 2025 at 5:12 AM -03, Petr Vorel wrote:
> Hi Ricardo,
>
>> >> > -doc: metadata-all
>> >> > +doc:
>> >> > +	$(MAKE) -C $(abs_builddir)/doc setup
>> >> > +	$(MAKE) -C $(abs_builddir)/doc
>> >> > +
>> >> > +.PHONY: doc-clean
>> >> > +doc-clean:
>> >> > +	$(MAKE) -C $(abs_builddir)/doc clean
>
>> >> IMO the top-level clean and distclean targets should be responsible of
>> >> calling the respective targets in doc/Makefile, what do you think?
>
>> > Well, we have in the top level other clean targets:
>> > lib-clean, libs-clean, ac-clean, ac-distclean, ac-maintainer-clean.
>> > Therefore I thought add at least doc-clean would be good. I ignored
>> > doc-distclean.
>
>> > Most of LTP is compiled and therefore handled by
>> > include/mk/generic_leaf_target.mk. kirk and sparse are special (submodule anyway),
>> > but even they allows to use generic_leaf_target.mk. I'm not sure if I want to
>> > rewrite whole doc/Makefile to support generic_leaf_target.mk, I should, but my
>> > goal was to simplify creating venv. I guess I drop this atm and send v5 and
>> > without it and get to it later.
>
>
>> Right, I tried out something (below) which kind of works but it would require
>> the setup target becoming the default... The sad part is that even if
>> the user chooses not to use a virtualenv, linuxdoc does not seem to be
>> packaged in the major distros (I checked Tumbleweed, Debian and Fedora
>> only Fedora has it)
>
> I'm surprised that any distro package linuxdoc. And IMHO all linux distros
> require pip install to run inside virtualenv.
>

I was surprise it isn't :(

>> diff --git a/Makefile b/Makefile
>> index 506678934983..69a8016535a9 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -41,7 +41,7 @@ $(1):: | $$(abs_top_builddir)/$$(basename $$(subst -,.,$(1)))
>>  endif
>>  endef
>
>> -COMMON_TARGETS         += testcases tools metadata
>> +COMMON_TARGETS         += testcases tools metadata doc
>
> I was testing doc part of COMMON_TARGETS as well (that requires that
> generic_leaf_target.mk to be included). But that means that doc will be always
> built, which means that user either has linuxdoc and others installed as a
> package or via 'make -C doc setup'.
>

+1

>>  # Don't want to nuke the original files if we're installing in-build-tree.
>>  ifneq ($(BUILD_TREE_STATE),$(BUILD_TREE_SRCDIR_INSTALL))
>> @@ -169,8 +169,8 @@ INSTALL_TARGETS             += $(addprefix $(DESTDIR)/$(bindir)/,$(BINDIR_INSTALL_SCRIPTS))
>
>>  $(INSTALL_TARGETS): $(INSTALL_DIR) $(DESTDIR)/$(bindir)
>
>> -.PHONY: doc
>> -doc: metadata-all
>> +#.PHONY: doc
>> +#doc: metadata-all
>
> 'doc: metadata-all' is a part I suggested to be deleted (useless now).
> But IMHO it should be replaced by other phony target - my plan was to use
> generic_leaf_target.mk (reuse existing LTP build system instead writing from
> scratch), but don't have doc/ part of build from top level Makefile (avoid
> dealing with virtualenv). Therefore I guess at least doc target in this patchset
> is more or less correct. Maybe having doc as a part of CLEAN_TARGETS
> variable will work.

Adding it to CLEAN_TARGETS will work, as it will create the `doc-clean`
target.

>
>>  .PHONY: check
>>  check: $(CHECK_TARGETS)
>> diff --git a/doc/Makefile b/doc/Makefile
>> index 2062d6e93561..7f11e659cab8 100644
>> --- a/doc/Makefile
>> +++ b/doc/Makefile
>> @@ -23,15 +23,17 @@ setup: $(VENV_DIR)
>>  ${abs_top_builddir}/metadata/ltp.json:
>>         $(MAKE) -C ${abs_top_builddir}/metadata
>
>> -all: ${abs_top_builddir}/metadata/ltp.json
>> +all: ${abs_top_builddir}/metadata/ltp.json setup
>>         $(RUN_VENV); sphinx-build -b html . html
>
> Using virtualenv by default was something which Andrea did not want (equivalent
> of this was in my v1). And I agree we don't want packagers to deal with
> virtualenv (FYI atm there are at least SUSE, Buildroot and Yocto packages; Red
> hat plans to use it as well).
>

Hm, but if we have a top-level target "doc" which setup the venv by
default, it will be even more confusing... Maybe having both doc and
doc-setup would make sense?

With this, we can opt-in the venv by doing `make doc-setup` first. We
can clean it with `make doc-clean` or the top-level `make clean` and
`make distclean`

>>  spelling:
>>         $(RUN_VENV); sphinx-build -b spelling -d build/doctree . build/spelling
>
>> -clean:
>> +clean::
> +1, I did not have enough time to figure out '::'.
>
>>         rm -rf html/ build/ _static/syscalls.rst _static/tests.rst syscalls.tbl \
>>                 ${abs_top_builddir}/metadata/ltp.json
>
>> -distclean: clean
>> +distclean:: clean
>>         rm -rf $(VENV_DIR)
>> +
>> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
>
>> Thanks,
>> 		rbm
>
>> > Kind regards,
>> > Petr
>
>> > The rest of LTP final directories is handled by
>> > include/mk/generic_leaf_target.mk, but doc/ is somehow special, that's why I
>> > added these targets.
>
>> > If you call make clean or distclean
>
>
>> >> >  .PHONY: check
>> >> >  check: $(CHECK_TARGETS)


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