From a258aa9a3b01975fe8df583005d931701663b054 Mon Sep 17 00:00:00 2001
From: Stephen Finucane <stephenfin@redhat.com>
Date: Fri, 6 Apr 2018 15:38:32 +0100
Subject: [PATCH] Default warning-is-error to True for non-legacy Sphinx
 projects

We're trying to move away from the legacy pbr 'build_sphinx' command.
However, without the relevant section in the 'setup.cfg' file, we have
no way to determine if the user wishes to use the '-W'
(warning-is-error) flag when building docs. The current behavior for
this is to default to False, meaning documentation for projects that
previously used the pbr functionality to enable warning-is-error is
liable to slowly break as time passes.

To resolve this, default to True for packages with no [build_sphinx]
section in setup.cfg. This ensures that projects migrating to the new
PTI (which, when fully implemented, involves removing the [build_sphinx]
section from setup.cfg) will be forced to fix any warnings prior to the
migration. However, packages that have not been converted will not be
broken. Only pacakges that have already fully switched over to the new
PTI but which did not have warning-is-error enabled are at risk of
unexpected breakage, but even for those the short-term pain should
ultimately be outweighed by the long-term gain.

Change-Id: I677afef96370ead5a45cba854ba483f18a8d1247
---
 roles/sphinx/README.rst                               |  6 ++++--
 roles/sphinx/library/sphinx_check_warning_is_error.py | 10 ++++++----
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/roles/sphinx/README.rst b/roles/sphinx/README.rst
index fcad72530..5a4929260 100644
--- a/roles/sphinx/README.rst
+++ b/roles/sphinx/README.rst
@@ -19,8 +19,10 @@ Run sphinx to generate documentation
 
 .. zuul:rolevar:: sphinx_warning_is_error
 
-   Whether to treat sphinx build warnings as errors. Defaults to undefined
-   which means to attempt to find the setting in a setup.cfg file.
+   Whether to treat sphinx build warnings as errors. Defaults to the value of
+   ``[build_sphinx] warning-is-error`` in ``setup.cfg`` if defined, ``False``
+   if the ``[build_sphinx]`` section is present but the ``warning-is-error``
+   option is undefined, or ``True`` if the entire section is undefined.
 
 .. zuul:rolevar:: zuul_work_virtualenv
    :default: ~/.venv
diff --git a/roles/sphinx/library/sphinx_check_warning_is_error.py b/roles/sphinx/library/sphinx_check_warning_is_error.py
index d067d2017..275d8c0a7 100644
--- a/roles/sphinx/library/sphinx_check_warning_is_error.py
+++ b/roles/sphinx/library/sphinx_check_warning_is_error.py
@@ -68,7 +68,7 @@ def main():
     )
     project_dir = module.params['project_dir']
 
-    warning_is_error = False
+    warning_is_error = True
     # TODO(mordred) Remove autodoc_index_modules logic  when we get OpenStack
     # projects off of the pbr autoindex
     autodoc_index_modules = False
@@ -91,9 +91,11 @@ def main():
             autodoc_index_modules=autodoc_index_modules,
             msg="Error reading setup.cfg, defaulting flags to false")
 
-    if (c.has_section('build_sphinx') and
-            c.has_option('build_sphinx', 'warning-is-error')):
-        warning_is_error = c.getboolean('build_sphinx', 'warning-is-error')
+    if c.has_section('build_sphinx'):
+        if c.has_option('build_sphinx', 'warning-is-error'):
+            warning_is_error = c.getboolean('build_sphinx', 'warning-is-error')
+        else:
+            warning_is_error = False
     if c.has_section('pbr') and c.has_option('pbr', 'autodoc_index_modules'):
         autodoc_index_modules = c.getboolean('pbr', 'autodoc_index_modules')
     if (c.has_section('pbr') and