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

[PATCH bpf-next v2 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.

v2:
- Pass rst2man option through a dedicated variable, use it to ask for a
  non-zero exit value on errors.
- Also build doc right after bpftool when building (not only running) the
  selftests.

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      |  3 ++-
 .../bpf/bpftool/Documentation/bpftool-btf.rst |  3 +++
 .../bpf/bpftool/Documentation/bpftool-gen.rst |  4 ++++
 .../bpf/bpftool/Documentation/bpftool-map.rst |  3 +++
 tools/testing/selftests/bpf/Makefile          |  5 +++++
 .../selftests/bpf/test_bpftool_build.sh       | 21 +++++++++++++++++++
 6 files changed, 38 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH bpf-next v2 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.

v2:
- Use "--verbose" instead of "-r 1" (same behaviour but more readable).
- Pass it through a RST2MAN_OPTS variable so we can easily pass other
  options too.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Acked-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/bpf/bpftool/Documentation/Makefile        | 3 ++-
 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, 12 insertions(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/Documentation/Makefile b/tools/bpf/bpftool/Documentation/Makefile
index 815ac9804aee..a45b51d98468 100644
--- a/tools/bpf/bpftool/Documentation/Makefile
+++ b/tools/bpf/bpftool/Documentation/Makefile
@@ -28,12 +28,13 @@ man: man8 helpers
 man8: $(DOC_MAN8)
 
 RST2MAN_DEP := $(shell command -v rst2man 2>/dev/null)
+RST2MAN_OPTS += --verbose
 
 $(OUTPUT)%.8: %.rst
 ifndef RST2MAN_DEP
 	$(error "rst2man not found, but required to generate man pages")
 endif
-	$(QUIET_GEN)rst2man $< > $@
+	$(QUIET_GEN)rst2man $(RST2MAN_OPTS) $< > $@
 
 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 v2 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. 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(+)

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

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

[Andrii Nakryiko]

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(+)
>

[...]

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

[Quentin Monnet]

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

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

[Andrii Nakryiko]

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

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

[Alexei Starovoitov]

On Wed, Sep 9, 2020 at 9:22 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.
>
> v2:
> - Pass rst2man option through a dedicated variable, use it to ask for a
>   non-zero exit value on errors.
> - Also build doc right after bpftool when building (not only running) the
>   selftests.

Applied. Thanks