[PATCH bpf-next 0/2] bpf: detect build errors for man pages for bpftool and eBPF helpers

[PATCH bpf-next 0/2] bpf: detect build errors for man pages for bpftool and eBPF helpers

[Quentin Monnet]

This set aims at improving the checks for building bpftool's documentation
(including the man page for eBPF helper functions). The first patch lowers
the log-level from rst2man and fix the reported informational messages. The
second one extends the script used to build bpftool in the eBPF selftests,
so that we also check a documentation build.

This is after a suggestion from Andrii Nakryiko.

Quentin Monnet (2):
  tools: bpftool: log info-level messages when building bpftool man
    pages
  selftests, bpftool: add bpftool (and eBPF helpers) documentation build

 tools/bpf/bpftool/Documentation/Makefile      |  2 +-
 .../bpf/bpftool/Documentation/bpftool-btf.rst |  3 +++
 .../bpf/bpftool/Documentation/bpftool-gen.rst |  4 ++++
 .../bpf/bpftool/Documentation/bpftool-map.rst |  3 +++
 .../selftests/bpf/test_bpftool_build.sh       | 23 +++++++++++++++++++
 5 files changed, 34 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH bpf-next 1/2] tools: bpftool: log info-level messages when building bpftool man pages

[Quentin Monnet]

To build man pages for bpftool (and for eBPF helper functions), rst2man
can log different levels of information. Let's make it log all levels
to keep the RST files clean.

Doing so, rst2man complains about double colons, used for literal
blocks, that look like underlines for section titles. Let's add the
necessary blank lines.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
 tools/bpf/bpftool/Documentation/Makefile        | 2 +-
 tools/bpf/bpftool/Documentation/bpftool-btf.rst | 3 +++
 tools/bpf/bpftool/Documentation/bpftool-gen.rst | 4 ++++
 tools/bpf/bpftool/Documentation/bpftool-map.rst | 3 +++
 4 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/Documentation/Makefile b/tools/bpf/bpftool/Documentation/Makefile
index 815ac9804aee..e871f28a5312 100644
--- a/tools/bpf/bpftool/Documentation/Makefile
+++ b/tools/bpf/bpftool/Documentation/Makefile
@@ -33,7 +33,7 @@ $(OUTPUT)%.8: %.rst
 ifndef RST2MAN_DEP
 	$(error "rst2man not found, but required to generate man pages")
 endif
-	$(QUIET_GEN)rst2man $< > $@
+	$(QUIET_GEN)rst2man -r 1 $< > $@
 
 clean: helpers-clean
 	$(call QUIET_CLEAN, Documentation)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-btf.rst b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
index 896f4c6c2870..864553e62af4 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-btf.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
@@ -91,6 +91,7 @@ OPTIONS
 EXAMPLES
 ========
 **# bpftool btf dump id 1226**
+
 ::
 
   [1] PTR '(anon)' type_id=2
@@ -104,6 +105,7 @@ EXAMPLES
 This gives an example of default output for all supported BTF kinds.
 
 **$ cat prog.c**
+
 ::
 
   struct fwd_struct;
@@ -144,6 +146,7 @@ This gives an example of default output for all supported BTF kinds.
   }
 
 **$ bpftool btf dump file prog.o**
+
 ::
 
   [1] PTR '(anon)' type_id=2
diff --git a/tools/bpf/bpftool/Documentation/bpftool-gen.rst b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
index df85dbd962c0..d52b03a352d7 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-gen.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
@@ -146,6 +146,7 @@ OPTIONS
 EXAMPLES
 ========
 **$ cat example.c**
+
 ::
 
   #include <stdbool.h>
@@ -187,6 +188,7 @@ This is example BPF application with two BPF programs and a mix of BPF maps
 and global variables.
 
 **$ bpftool gen skeleton example.o**
+
 ::
 
   /* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
@@ -241,6 +243,7 @@ and global variables.
   #endif /* __EXAMPLE_SKEL_H__ */
 
 **$ cat example_user.c**
+
 ::
 
   #include "example.skel.h"
@@ -283,6 +286,7 @@ and global variables.
   }
 
 **# ./example_user**
+
 ::
 
   my_map name: my_map
diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
index 083db6c2fc67..8f187c6416cd 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
@@ -182,6 +182,7 @@ OPTIONS
 EXAMPLES
 ========
 **# bpftool map show**
+
 ::
 
   10: hash  name some_map  flags 0x0
@@ -203,6 +204,7 @@ The following three commands are equivalent:
 
 
 **# bpftool map dump id 10**
+
 ::
 
   key: 00 01 02 03  value: 00 01 02 03 04 05 06 07
@@ -210,6 +212,7 @@ The following three commands are equivalent:
   Found 2 elements
 
 **# bpftool map getnext id 10 key 0 1 2 3**
+
 ::
 
   key:
-- 
2.25.1

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

[Quentin Monnet]

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.

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

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
 .../selftests/bpf/test_bpftool_build.sh       | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_bpftool_build.sh b/tools/testing/selftests/bpf/test_bpftool_build.sh
index ac349a5cea7e..22fbf1bf6eec 100755
--- a/tools/testing/selftests/bpf/test_bpftool_build.sh
+++ b/tools/testing/selftests/bpf/test_bpftool_build.sh
@@ -85,6 +85,25 @@ make_with_tmpdir() {
 	echo
 }
 
+make_doc_and_clean() {
+	echo -e "\$PWD:    $PWD"
+	echo -e "command: make -s $* doc >/dev/null"
+	# rst2man returns 0 even in case of warnings/errors, so we check that
+	# stderr is empty.
+	make $J -s $* doc |& tee /dev/stderr | [ $(wc -l) -eq 0 ] || false
+	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 +164,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
-- 
2.25.1

Re: [PATCH bpf-next 0/2] bpf: detect build errors for man pages for bpftool and eBPF helpers

[Andrii Nakryiko]

On Mon, Sep 7, 2020 at 9:40 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> This set aims at improving the checks for building bpftool's documentation
> (including the man page for eBPF helper functions). The first patch lowers
> the log-level from rst2man and fix the reported informational messages. The
> second one extends the script used to build bpftool in the eBPF selftests,
> so that we also check a documentation build.
>
> This is after a suggestion from Andrii Nakryiko.
>
> Quentin Monnet (2):
>   tools: bpftool: log info-level messages when building bpftool man
>     pages
>   selftests, bpftool: add bpftool (and eBPF helpers) documentation build
>
>  tools/bpf/bpftool/Documentation/Makefile      |  2 +-
>  .../bpf/bpftool/Documentation/bpftool-btf.rst |  3 +++
>  .../bpf/bpftool/Documentation/bpftool-gen.rst |  4 ++++
>  .../bpf/bpftool/Documentation/bpftool-map.rst |  3 +++
>  .../selftests/bpf/test_bpftool_build.sh       | 23 +++++++++++++++++++
>  5 files changed, 34 insertions(+), 1 deletion(-)
>
> --
> 2.25.1
>

LGTM.

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

But this still won't be run every time someone makes selftests. We do
build bpftool during selftests build, so it would be good to run doc
build there as well to ensure everyone is executing this. But this is
a good first step for sure.

Re: [PATCH bpf-next 0/2] bpf: detect build errors for man pages for bpftool and eBPF helpers

[Quentin Monnet]

On 09/09/2020 04:48, Andrii Nakryiko wrote:
> On Mon, Sep 7, 2020 at 9:40 AM Quentin Monnet <quentin@isovalent.com> wrote:
>>
>> This set aims at improving the checks for building bpftool's documentation
>> (including the man page for eBPF helper functions). The first patch lowers
>> the log-level from rst2man and fix the reported informational messages. The
>> second one extends the script used to build bpftool in the eBPF selftests,
>> so that we also check a documentation build.
>>
>> This is after a suggestion from Andrii Nakryiko.
>>
>> Quentin Monnet (2):
>>   tools: bpftool: log info-level messages when building bpftool man
>>     pages
>>   selftests, bpftool: add bpftool (and eBPF helpers) documentation build
>>
>>  tools/bpf/bpftool/Documentation/Makefile      |  2 +-
>>  .../bpf/bpftool/Documentation/bpftool-btf.rst |  3 +++
>>  .../bpf/bpftool/Documentation/bpftool-gen.rst |  4 ++++
>>  .../bpf/bpftool/Documentation/bpftool-map.rst |  3 +++
>>  .../selftests/bpf/test_bpftool_build.sh       | 23 +++++++++++++++++++
>>  5 files changed, 34 insertions(+), 1 deletion(-)
>>
>> --
>> 2.25.1
>>
> 
> LGTM.
> 
> Acked-by: Andrii Nakryiko <andriin@fb.com>
> 
> But this still won't be run every time someone makes selftests. We do
> build bpftool during selftests build, so it would be good to run doc
> build there as well to ensure everyone is executing this. But this is
> a good first step for sure.
> 

I see, indeed with this patch we would only build the doc on "make
run_tests", not when building the selftests. I'll send another version
that builds the doc at the same time as we build bpftool then.

I also had another look at rst2man's options, and there's probably
cleaner a way to get rid of the stderr redirection/line counting that
I'm doing in this version. I'll fix that too.

Thanks for the feedback.
Quentin