diff mbox series

[2/2] doc: pass "werror" setting through to doc build

Message ID 20200117104820.71403-2-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [1/2] doc: fix doc build when sphinx reports version to stderr |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/travis-robot warning Travis build: failed
ci/Intel-compilation success Compilation OK

Commit Message

Bruce Richardson Jan. 17, 2020, 10:48 a.m. UTC 
  When werror is set for the build, we should pass that flag through to
sphinx so that it can flag warnings as errors too.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 buildtools/call-sphinx-build.py | 5 ++---
 doc/guides/meson.build          | 4 ++++
 2 files changed, 6 insertions(+), 3 deletions(-)

Comments

Aaron Conole Jan. 17, 2020, 1:16 p.m. UTC | #1 
Bruce Richardson <bruce.richardson@intel.com> writes:

> When werror is set for the build, we should pass that flag through to
> sphinx so that it can flag warnings as errors too.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---

I see that this actually works to generate the errors... BUT

if we merge this it will break the build.  Can you also insert a patch
to address the warning so that the series could be merged?

>  buildtools/call-sphinx-build.py | 5 ++---
>  doc/guides/meson.build          | 4 ++++
>  2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/buildtools/call-sphinx-build.py b/buildtools/call-sphinx-build.py
> index 85c9e0156..fc4e9d040 100755
> --- a/buildtools/call-sphinx-build.py
> +++ b/buildtools/call-sphinx-build.py
> @@ -9,12 +9,11 @@
>  from subprocess import run, PIPE, STDOUT
>  from distutils.version import StrictVersion
>  
> -(sphinx, src, dst) = sys.argv[1:]  # assign parameters to variables
> +(*sphinx_cmd, src, dst) = sys.argv[1:]  # assign parameters to variables
>  
>  # for sphinx version >= 1.7 add parallelism using "-j auto"
> -ver = run([sphinx, '--version'], stdout=PIPE,
> +ver = run(sphinx_cmd + ['--version'], stdout=PIPE,
>            stderr=STDOUT).stdout.decode().split()[-1]
> -sphinx_cmd = [sphinx]
>  if StrictVersion(ver) >= StrictVersion('1.7'):
>      sphinx_cmd += ['-j', 'auto']
>  
> diff --git a/doc/guides/meson.build b/doc/guides/meson.build
> index 732e7ad3a..5a2b854e8 100644
> --- a/doc/guides/meson.build
> +++ b/doc/guides/meson.build
> @@ -7,6 +7,10 @@ if not sphinx.found()
>  	subdir_done()
>  endif
>  
> +if get_option('werror')
> +	sphinx = [sphinx, '-W']
> +endif
> +
>  htmldir = join_paths(get_option('datadir'), 'doc', 'dpdk')
>  html_guides = custom_target('html_guides',
>  	input: files('index.rst'),
Bruce Richardson Jan. 17, 2020, 1:25 p.m. UTC | #2 
On Fri, Jan 17, 2020 at 08:16:55AM -0500, Aaron Conole wrote:
> Bruce Richardson <bruce.richardson@intel.com> writes:
> 
> > When werror is set for the build, we should pass that flag through to
> > sphinx so that it can flag warnings as errors too.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> 
> I see that this actually works to generate the errors... BUT
> 
> if we merge this it will break the build.  Can you also insert a patch
> to address the warning so that the series could be merged?
> 

Ok, I didn't have any warnings in my setup, which is why I didn't see any
problems. I assume that the warnings are showing up in travis? Anywhere
else?

/Bruce
Aaron Conole Jan. 17, 2020, 1:34 p.m. UTC | #3 
Bruce Richardson <bruce.richardson@intel.com> writes:

> On Fri, Jan 17, 2020 at 08:16:55AM -0500, Aaron Conole wrote:
>> Bruce Richardson <bruce.richardson@intel.com> writes:
>> 
>> > When werror is set for the build, we should pass that flag through to
>> > sphinx so that it can flag warnings as errors too.
>> >
>> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>> > ---
>> 
>> I see that this actually works to generate the errors... BUT
>> 
>> if we merge this it will break the build.  Can you also insert a patch
>> to address the warning so that the series could be merged?
>> 
>
> Ok, I didn't have any warnings in my setup, which is why I didn't see any
> problems. I assume that the warnings are showing up in travis? Anywhere
> else?

I only saw them on Travis.

> /Bruce
Bruce Richardson Jan. 17, 2020, 5:42 p.m. UTC | #4 
On Fri, Jan 17, 2020 at 08:34:01AM -0500, Aaron Conole wrote:
> Bruce Richardson <bruce.richardson@intel.com> writes:
> 
> > On Fri, Jan 17, 2020 at 08:16:55AM -0500, Aaron Conole wrote:
> >> Bruce Richardson <bruce.richardson@intel.com> writes:
> >> 
> >> > When werror is set for the build, we should pass that flag through to
> >> > sphinx so that it can flag warnings as errors too.
> >> >
> >> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> >> > ---
> >> 
> >> I see that this actually works to generate the errors... BUT
> >> 
> >> if we merge this it will break the build.  Can you also insert a patch
> >> to address the warning so that the series could be merged?
> >> 
> >
> > Ok, I didn't have any warnings in my setup, which is why I didn't see any
> > problems. I assume that the warnings are showing up in travis? Anywhere
> > else?
> 
> I only saw them on Travis.
> 
The error from sphinx in travis looks like a false positive that is fixed
in later versions of sphinx. The error I see is:

/home/travis/build/bruce-richardson/dpdk/doc/guides/linux_gsg/eal_args.include.rst:: WARNING: document isn't included in any toctree

However, that file is an include one that is included in both the linux and
freebsd eal parameters docs, and so is not missing though not included in
the index. What is the best approach to deal with this, do you think?

* rework so it has a toctree entry e.g. by creating a new section for
  common parameters
* other workaround in the code, e.g. rename the file to not end in .rst
* can we update sphinx in the travis build to avoid the warning altogether?

/Bruce
Aaron Conole Jan. 17, 2020, 5:59 p.m. UTC | #5 
Bruce Richardson <bruce.richardson@intel.com> writes:

> On Fri, Jan 17, 2020 at 08:34:01AM -0500, Aaron Conole wrote:
>> Bruce Richardson <bruce.richardson@intel.com> writes:
>> 
>> > On Fri, Jan 17, 2020 at 08:16:55AM -0500, Aaron Conole wrote:
>> >> Bruce Richardson <bruce.richardson@intel.com> writes:
>> >> 
>> >> > When werror is set for the build, we should pass that flag through to
>> >> > sphinx so that it can flag warnings as errors too.
>> >> >
>> >> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>> >> > ---
>> >> 
>> >> I see that this actually works to generate the errors... BUT
>> >> 
>> >> if we merge this it will break the build.  Can you also insert a patch
>> >> to address the warning so that the series could be merged?
>> >> 
>> >
>> > Ok, I didn't have any warnings in my setup, which is why I didn't see any
>> > problems. I assume that the warnings are showing up in travis? Anywhere
>> > else?
>> 
>> I only saw them on Travis.
>> 
> The error from sphinx in travis looks like a false positive that is fixed
> in later versions of sphinx. The error I see is:
>
> /home/travis/build/bruce-richardson/dpdk/doc/guides/linux_gsg/eal_args.include.rst::
> WARNING: document isn't included in any toctree
>
> However, that file is an include one that is included in both the linux and
> freebsd eal parameters docs, and so is not missing though not included in
> the index. What is the best approach to deal with this, do you think?
>
> * rework so it has a toctree entry e.g. by creating a new section for
>   common parameters
> * other workaround in the code, e.g. rename the file to not end in .rst
> * can we update sphinx in the travis build to avoid the warning altogether?

I think the third option is best.  Distributions can always disable
werror on doc builds or upgrade their own sphinx packages.

If that isn't acceptable to anyone else, then the 1st option is my
next choice.  I dislike the approach of renaming the file.

> /Bruce
diff mbox series

Patch

diff --git a/buildtools/call-sphinx-build.py b/buildtools/call-sphinx-build.py
index 85c9e0156..fc4e9d040 100755
--- a/buildtools/call-sphinx-build.py
+++ b/buildtools/call-sphinx-build.py
@@ -9,12 +9,11 @@ 
 from subprocess import run, PIPE, STDOUT
 from distutils.version import StrictVersion
 
-(sphinx, src, dst) = sys.argv[1:]  # assign parameters to variables
+(*sphinx_cmd, src, dst) = sys.argv[1:]  # assign parameters to variables
 
 # for sphinx version >= 1.7 add parallelism using "-j auto"
-ver = run([sphinx, '--version'], stdout=PIPE,
+ver = run(sphinx_cmd + ['--version'], stdout=PIPE,
           stderr=STDOUT).stdout.decode().split()[-1]
-sphinx_cmd = [sphinx]
 if StrictVersion(ver) >= StrictVersion('1.7'):
     sphinx_cmd += ['-j', 'auto']
 
diff --git a/doc/guides/meson.build b/doc/guides/meson.build
index 732e7ad3a..5a2b854e8 100644
--- a/doc/guides/meson.build
+++ b/doc/guides/meson.build
@@ -7,6 +7,10 @@  if not sphinx.found()
 	subdir_done()
 endif
 
+if get_option('werror')
+	sphinx = [sphinx, '-W']
+endif
+
 htmldir = join_paths(get_option('datadir'), 'doc', 'dpdk')
 html_guides = custom_target('html_guides',
 	input: files('index.rst'),