[PATCH] conf.py: silence cve role already registered

[PATCH] conf.py: silence cve role already registered

[Julien Stephan]

Commit bd9add8a0 - conf.py: add macro for Mitre CVE links
added a new macro for mitre cve links called "cve_mitre" which conflicts
with the already existing "cve" role.

  WARNING: role 'cve' is already registered, it will be overridden [app.add_role]

The warning is turned into an error making "make html" fail, but
documentation is still successfully built.

Since documentation is still correctly built, just suppress the warning.

Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
 documentation/conf.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/documentation/conf.py b/documentation/conf.py
index 35c5c14535ba67aae3554819b55f0fb52a9d0027..aaddf2c789196fc26a734f06dbe005f147b9bbd3 100644
--- a/documentation/conf.py
+++ b/documentation/conf.py
@@ -118,6 +118,9 @@ intersphinx_mapping = {
 # Suppress "WARNING: unknown mimetype for ..."
 suppress_warnings = ['epub.unknown_project_files']
 
+# Suppress "WARNING: role 'cve' is already registered, it will be overridden [app.add_role]"
+suppress_warnings.append('app.add_role')
+
 # -- Options for HTML output -------------------------------------------------
 
 # The theme to use for HTML and HTML Help pages.  See the documentation for

---
base-commit: 9563855ccd92e21fb6f8320c96a3a83e115c947e
change-id: 20241031-fix-cve-add-role-warning-1147c223787a

Best regards,
-- 
Julien Stephan <jstephan@baylibre.com>

Re: [docs] [PATCH] conf.py: silence cve role already registered

[Antonin Godard]

Hi Julien,

On Thu Oct 31, 2024 at 5:52 PM CET, Julien Stephan wrote:
> Commit bd9add8a0 - conf.py: add macro for Mitre CVE links
> added a new macro for mitre cve links called "cve_mitre" which conflicts
> with the already existing "cve" role.
>
>   WARNING: role 'cve' is already registered, it will be overridden [app.add_role]
>
> The warning is turned into an error making "make html" fail, but
> documentation is still successfully built.
>
> Since documentation is still correctly built, just suppress the warning.
>
> Signed-off-by: Julien Stephan <jstephan@baylibre.com>
> ---
>  documentation/conf.py | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/documentation/conf.py b/documentation/conf.py
> index 35c5c14535ba67aae3554819b55f0fb52a9d0027..aaddf2c789196fc26a734f06dbe005f147b9bbd3 100644
> --- a/documentation/conf.py
> +++ b/documentation/conf.py
> @@ -118,6 +118,9 @@ intersphinx_mapping = {
>  # Suppress "WARNING: unknown mimetype for ..."
>  suppress_warnings = ['epub.unknown_project_files']
>  
> +# Suppress "WARNING: role 'cve' is already registered, it will be overridden [app.add_role]"
> +suppress_warnings.append('app.add_role')
> +
>  # -- Options for HTML output -------------------------------------------------
>  
>  # The theme to use for HTML and HTML Help pages.  See the documentation for
>
> ---
> base-commit: 9563855ccd92e21fb6f8320c96a3a83e115c947e
> change-id: 20241031-fix-cve-add-role-warning-1147c223787a
>
> Best regards,

Thanks, we're also updating Sphinx at the moment, so this comes at the right
time! I do get the warning/error on 8.1.3, and this fixes the error and make the
links point to NIST, as expected.

Also verified and it is backward compatible with previous versions of Sphinx.

Reviewed-by: Antonin Godard <antonin.godard@bootlin.com>
Tested-by: Antonin Godard <antonin.godard@bootlin.com>

Cheers,
Antonin

-- 
Antonin Godard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [docs] [PATCH] conf.py: silence cve role already registered

[Quentin Schulz]

Hi Julien,

On 10/31/24 5:52 PM, Julien Stephan via lists.yoctoproject.org wrote:
> Commit bd9add8a0 - conf.py: add macro for Mitre CVE links
> added a new macro for mitre cve links called "cve_mitre" which conflicts
> with the already existing "cve" role.
> 
>    WARNING: role 'cve' is already registered, it will be overridden [app.add_role]
> 
> The warning is turned into an error making "make html" fail, but
> documentation is still successfully built.
> 
> Since documentation is still correctly built, just suppress the warning.
> 

I don't understand the issue and why Sphinx complains about it.

I'm also very wary about just silencing a warning instead of tackling it.

I don't understand why this triggers it, extlinks contains an alias with 
a base URL and a caption. The only overlap we have is the CVE-%s 
caption. But why would that be an issue, it's just text!

Also not sure what Antonin meant with "make the
links point to NIST, as expected.". Which ones? What's the actual issue 
here we're trying to fix?

Cheers,
Quentin

Re: [docs] [PATCH] conf.py: silence cve role already registered

[Antonin Godard]

Hi Quentin,

On Mon Nov 4, 2024 at 11:33 AM CET, Quentin Schulz via lists.yoctoproject.org wrote:
> Hi Julien,
>
> On 10/31/24 5:52 PM, Julien Stephan via lists.yoctoproject.org wrote:
>> Commit bd9add8a0 - conf.py: add macro for Mitre CVE links
>> added a new macro for mitre cve links called "cve_mitre" which conflicts
>> with the already existing "cve" role.
>> 
>>    WARNING: role 'cve' is already registered, it will be overridden [app.add_role]
>> 
>> The warning is turned into an error making "make html" fail, but
>> documentation is still successfully built.
>> 
>> Since documentation is still correctly built, just suppress the warning.
>> 
>
> I don't understand the issue and why Sphinx complains about it.
>
> I'm also very wary about just silencing a warning instead of tackling it.

The issue is that newer Sphinx versions are already defining a :cve: role that
points to cve.org, instead of the role we defined in conf.py that points to
nvd.nist.gov. So we have different solutions:

- either override the role to make it point to nvd.nist.gov, which is a bit of a
  hack but works:

  from sphinx.roles import CVE
  CVE._BASE_URL ='https://nvd.nist.gov/vuln/detail/CVE-'

  Also that may not be backwards compatible. Context: we already noticed last
  week that old releases don't build with newer versions of Sphinx, we were thus
  planning on keeping an old Sphinx version for building old doc versions.

- just point to cve.org instead, I'm not sure how important pointing to the nist
  database is. May pose a problem with older sphinx versions, though.

- remove the warning as Julien did. Has the advantage of being backward
  compatible, although not nice to suppress a warning of course.

> I don't understand why this triggers it, extlinks contains an alias with 
> a base URL and a caption. The only overlap we have is the CVE-%s 
> caption. But why would that be an issue, it's just text!

I guess extlinks doesn't really act as an "override existing roles" behavior but
an "append to existing roles and warn if a role already exists".

> Also not sure what Antonin meant with "make the
> links point to NIST, as expected.". Which ones? What's the actual issue
> here we're trying to fix?
>
> Cheers,
> Quentin

Cheers,
Antonin

-- 
Antonin Godard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [docs] [PATCH] conf.py: silence cve role already registered

[Quentin Schulz]

Hi Antonin,

On 11/4/24 12:11 PM, Antonin Godard wrote:
> Hi Quentin,
> 
> On Mon Nov 4, 2024 at 11:33 AM CET, Quentin Schulz via lists.yoctoproject.org wrote:
>> Hi Julien,
>>
>> On 10/31/24 5:52 PM, Julien Stephan via lists.yoctoproject.org wrote:
>>> Commit bd9add8a0 - conf.py: add macro for Mitre CVE links
>>> added a new macro for mitre cve links called "cve_mitre" which conflicts
>>> with the already existing "cve" role.
>>>
>>>     WARNING: role 'cve' is already registered, it will be overridden [app.add_role]
>>>
>>> The warning is turned into an error making "make html" fail, but
>>> documentation is still successfully built.
>>>
>>> Since documentation is still correctly built, just suppress the warning.
>>>
>>
>> I don't understand the issue and why Sphinx complains about it.
>>
>> I'm also very wary about just silencing a warning instead of tackling it.
> 
> The issue is that newer Sphinx versions are already defining a :cve: role that
> points to cve.org, instead of the role we defined in conf.py that points to
> nvd.nist.gov. So we have different solutions:


Ah, there it is. Thanks for the explanation, this should at the very 
least be added to the commit log :)
> 
> - either override the role to make it point to nvd.nist.gov, which is a bit of a
>    hack but works:
> 
>    from sphinx.roles import CVE
>    CVE._BASE_URL ='https://nvd.nist.gov/vuln/detail/CVE-'
> 
>    Also that may not be backwards compatible. Context: we already noticed last
>    week that old releases don't build with newer versions of Sphinx, we were thus
>    planning on keeping an old Sphinx version for building old doc versions.
> 
> - just point to cve.org instead, I'm not sure how important pointing to the nist
>    database is. May pose a problem with older sphinx versions, though.
> 
> - remove the warning as Julien did. Has the advantage of being backward
>    compatible, although not nice to suppress a warning of course.
> 

- rename :cve: to :cve_nist:

Easy to backport, easy to replace with sed (s/:cve:/:cve_nist:/) no need 
to get rid of warnings or implement non-portable hacks.

I'm biased, I vote for the fourth option :)

Cheers,
Quentin

Re: [docs] [PATCH] conf.py: silence cve role already registered

[Antonin Godard]

Hi Quentin,

On Mon Nov 4, 2024 at 1:21 PM CET, Quentin Schulz wrote:
[...]
>
> - rename :cve: to :cve_nist:
>
> Easy to backport, easy to replace with sed (s/:cve:/:cve_nist:/) no need 
> to get rid of warnings or implement non-portable hacks.
>
> I'm biased, I vote for the fourth option :)

Actually, sound like a reasonable approach to me too! :) I'm working on updating
Sphinx atm, so a patch should come sooner or later.

> Cheers,
> Quentin

Cheers,
Antonin

-- 
Antonin Godard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com