| 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 |
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(+) > [...]
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
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 --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
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(+)