diff mbox series

[bpf-next,v2,2/2] selftests, bpftool: add bpftool (and eBPF helpers) documentation build

Message ID 20200909162251.15498-3-quentin@isovalent.com
State Accepted
Delegated to: BPF Maintainers
Headers show
Series bpf: detect build errors for man pages for bpftool and eBPF helpers | expand

Commit Message

Quentin Monnet Sept. 9, 2020, 4:22 p.m. UTC
eBPF selftests include a script to check that bpftool builds correctly
with different command lines. Let's add one build for bpftool's
documentation so as to detect errors or warning reported by rst2man when
compiling the man pages. Also add a build to the selftests Makefile to
make sure we build bpftool documentation along with bpftool when
building the selftests.

This also builds and checks warnings for the man page for eBPF helpers,
which is built along bpftool's documentation.

This change adds rst2man as a dependency for selftests (it comes with
Python's "docutils").

v2:
- Use "--exit-status=1" option for rst2man instead of counting lines
  from stderr.
- Also build bpftool as part as the selftests build (and not only when
  the tests are actually run).

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
 tools/testing/selftests/bpf/Makefile          |  5 +++++
 .../selftests/bpf/test_bpftool_build.sh       | 21 +++++++++++++++++++
 2 files changed, 26 insertions(+)

Comments

Andrii Nakryiko Sept. 9, 2020, 4:45 p.m. UTC | #1
On Wed, Sep 9, 2020 at 9:22 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> eBPF selftests include a script to check that bpftool builds correctly
> with different command lines. Let's add one build for bpftool's
> documentation so as to detect errors or warning reported by rst2man when
> compiling the man pages. Also add a build to the selftests Makefile to
> make sure we build bpftool documentation along with bpftool when
> building the selftests.
>
> This also builds and checks warnings for the man page for eBPF helpers,
> which is built along bpftool's documentation.
>
> This change adds rst2man as a dependency for selftests (it comes with
> Python's "docutils").
>
> v2:
> - Use "--exit-status=1" option for rst2man instead of counting lines
>   from stderr.

It's a sane default to have non-zero exit code on error/warning, so
why not specifying it all the time?

> - Also build bpftool as part as the selftests build (and not only when
>   the tests are actually run).
>
> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
> ---

LGTM.

Acked-by: Andrii Nakryiko <andriin@fb.com>

>  tools/testing/selftests/bpf/Makefile          |  5 +++++
>  .../selftests/bpf/test_bpftool_build.sh       | 21 +++++++++++++++++++
>  2 files changed, 26 insertions(+)
>

[...]
Quentin Monnet Sept. 9, 2020, 4:51 p.m. UTC | #2
On 09/09/2020 17:45, Andrii Nakryiko wrote:
> On Wed, Sep 9, 2020 at 9:22 AM Quentin Monnet <quentin@isovalent.com> wrote:
>>
>> eBPF selftests include a script to check that bpftool builds correctly
>> with different command lines. Let's add one build for bpftool's
>> documentation so as to detect errors or warning reported by rst2man when
>> compiling the man pages. Also add a build to the selftests Makefile to
>> make sure we build bpftool documentation along with bpftool when
>> building the selftests.
>>
>> This also builds and checks warnings for the man page for eBPF helpers,
>> which is built along bpftool's documentation.
>>
>> This change adds rst2man as a dependency for selftests (it comes with
>> Python's "docutils").
>>
>> v2:
>> - Use "--exit-status=1" option for rst2man instead of counting lines
>>   from stderr.
> 
> It's a sane default to have non-zero exit code on error/warning, so
> why not specifying it all the time?

I hesitated to do so. I held off because a non-zero exit stops man pages
generation (rst2man does pursue the creation of the current man page
unless the error level is too high, but the Makefile will exit and not
produce the following man pages). This sounds desirable for developers,
but if distributions automatically build the doc to package it, I
thought it would be better to carry on and build the other man pages
rather than stopping the whole process.

But I can change it as a follow-up if you think it would be best.

Quentin
Andrii Nakryiko Sept. 9, 2020, 4:52 p.m. UTC | #3
On Wed, Sep 9, 2020 at 9:51 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> On 09/09/2020 17:45, Andrii Nakryiko wrote:
> > On Wed, Sep 9, 2020 at 9:22 AM Quentin Monnet <quentin@isovalent.com> wrote:
> >>
> >> eBPF selftests include a script to check that bpftool builds correctly
> >> with different command lines. Let's add one build for bpftool's
> >> documentation so as to detect errors or warning reported by rst2man when
> >> compiling the man pages. Also add a build to the selftests Makefile to
> >> make sure we build bpftool documentation along with bpftool when
> >> building the selftests.
> >>
> >> This also builds and checks warnings for the man page for eBPF helpers,
> >> which is built along bpftool's documentation.
> >>
> >> This change adds rst2man as a dependency for selftests (it comes with
> >> Python's "docutils").
> >>
> >> v2:
> >> - Use "--exit-status=1" option for rst2man instead of counting lines
> >>   from stderr.
> >
> > It's a sane default to have non-zero exit code on error/warning, so
> > why not specifying it all the time?
>
> I hesitated to do so. I held off because a non-zero exit stops man pages
> generation (rst2man does pursue the creation of the current man page
> unless the error level is too high, but the Makefile will exit and not
> produce the following man pages). This sounds desirable for developers,
> but if distributions automatically build the doc to package it, I
> thought it would be better to carry on and build the other man pages
> rather than stopping the whole process.
>
> But I can change it as a follow-up if you think it would be best.
>

I don't really care, leave it as is then.

> Quentin
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 65d3d9aaeb31..05798c2b5c67 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -176,6 +176,11 @@  $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile)    \
 	$(Q)$(MAKE) $(submake_extras)  -C $(BPFTOOLDIR)			       \
 		    OUTPUT=$(BUILD_DIR)/bpftool/			       \
 		    prefix= DESTDIR=$(SCRATCH_DIR)/ install
+	$(Q)mkdir -p $(BUILD_DIR)/bpftool/Documentation
+	$(Q)RST2MAN_OPTS="--exit-status=1" $(MAKE) $(submake_extras)	       \
+		    -C $(BPFTOOLDIR)/Documentation			       \
+		    OUTPUT=$(BUILD_DIR)/bpftool/Documentation/		       \
+		    prefix= DESTDIR=$(SCRATCH_DIR)/ install
 
 $(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile)		       \
 	   ../../../include/uapi/linux/bpf.h                                   \
diff --git a/tools/testing/selftests/bpf/test_bpftool_build.sh b/tools/testing/selftests/bpf/test_bpftool_build.sh
index ac349a5cea7e..2db3c60e1e61 100755
--- a/tools/testing/selftests/bpf/test_bpftool_build.sh
+++ b/tools/testing/selftests/bpf/test_bpftool_build.sh
@@ -85,6 +85,23 @@  make_with_tmpdir() {
 	echo
 }
 
+make_doc_and_clean() {
+	echo -e "\$PWD:    $PWD"
+	echo -e "command: make -s $* doc >/dev/null"
+	RST2MAN_OPTS="--exit-status=1" make $J -s $* doc
+	if [ $? -ne 0 ] ; then
+		ERROR=1
+		printf "FAILURE: Errors or warnings when building documentation\n"
+	fi
+	(
+		if [ $# -ge 1 ] ; then
+			cd ${@: -1}
+		fi
+		make -s doc-clean
+	)
+	echo
+}
+
 echo "Trying to build bpftool"
 echo -e "... through kbuild\n"
 
@@ -145,3 +162,7 @@  make_and_clean
 make_with_tmpdir OUTPUT
 
 make_with_tmpdir O
+
+echo -e "Checking documentation build\n"
+# From tools/bpf/bpftool
+make_doc_and_clean