[libvirt PATCH] meson: Ask rst2html to strip comments

Andrea Bolognani posted 1 patch 2 years, 10 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210604123108.150878-1-abologna@redhat.com
docs/manpages/meson.build | 2 +-
docs/meson.build          | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
[libvirt PATCH] meson: Ask rst2html to strip comments
Posted by Andrea Bolognani 2 years, 10 months ago
They can be problematic: in particular, the rst files generated
by keycodemapdb's keymap-gen contain things like

  To re-generate, run:
    keymap-gen --lang=rst --title=virkeycode-osx [...]

which result in xsltproc later choking with

  [1/12] Generating virkeyname-osx.html with a meson_exe.py custom command
  FAILED: docs/manpages/virkeyname-osx.html
  /usr/bin/meson --internal exe --capture docs/manpages/virkeyname-osx.html \
    /usr/bin/xsltproc [...] --nonet ../docs/subsite.xsl docs/manpages/virkeyname-osx.html.in
  docs/manpages/virkeyname-osx.html.in:17: parser error : Double hyphen within comment:
    keymap-gen --lang=rst --title=virkeyname-osx [...]
               ^

Stripping comments avoids the issue entirely.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 docs/manpages/meson.build | 2 +-
 docs/meson.build          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/manpages/meson.build b/docs/manpages/meson.build
index 0fa93fad89..eda0aa8441 100644
--- a/docs/manpages/meson.build
+++ b/docs/manpages/meson.build
@@ -118,7 +118,7 @@ foreach data : docs_man_files
     html_in_file,
     input: rst_file,
     output: html_in_file,
-    command: [ rst2html_prog, '--stylesheet=', '--strict', '@INPUT@' ],
+    command: [ rst2html_prog, '--strip-comments', '--stylesheet=', '--strict', '@INPUT@' ],
     capture: true,
   )
 
diff --git a/docs/meson.build b/docs/meson.build
index f550629d8e..c94a3c7f13 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -185,7 +185,7 @@ docs_admin_api_xml = docs_api_generated[3]
 docs_rst2html_gen = generator(
   rst2html_prog,
   output: '@BASENAME@.html.in',
-  arguments: [ '--stylesheet=', '--strict', '@INPUT@' ],
+  arguments: [ '--strip-comments', '--stylesheet=', '--strict', '@INPUT@' ],
   capture: true,
 )
 
-- 
2.31.1

Re: [libvirt PATCH] meson: Ask rst2html to strip comments
Posted by Michal Prívozník 2 years, 10 months ago
On 6/4/21 2:31 PM, Andrea Bolognani wrote:
> They can be problematic: in particular, the rst files generated
> by keycodemapdb's keymap-gen contain things like
> 
>   To re-generate, run:
>     keymap-gen --lang=rst --title=virkeycode-osx [...]
> 
> which result in xsltproc later choking with
> 
>   [1/12] Generating virkeyname-osx.html with a meson_exe.py custom command
>   FAILED: docs/manpages/virkeyname-osx.html
>   /usr/bin/meson --internal exe --capture docs/manpages/virkeyname-osx.html \
>     /usr/bin/xsltproc [...] --nonet ../docs/subsite.xsl docs/manpages/virkeyname-osx.html.in
>   docs/manpages/virkeyname-osx.html.in:17: parser error : Double hyphen within comment:
>     keymap-gen --lang=rst --title=virkeyname-osx [...]

I don't see this error. Do I need some very fresh version of something?

$ rst2html.py --version
rst2html.py (Docutils 0.16 [release], Python 3.9.4, on linux)

Michal

Re: [libvirt PATCH] meson: Ask rst2html to strip comments
Posted by Pavel Hrdina 2 years, 10 months ago
On Mon, Jun 07, 2021 at 04:39:15PM +0200, Michal Prívozník wrote:
> On 6/4/21 2:31 PM, Andrea Bolognani wrote:
> > They can be problematic: in particular, the rst files generated
> > by keycodemapdb's keymap-gen contain things like
> > 
> >   To re-generate, run:
> >     keymap-gen --lang=rst --title=virkeycode-osx [...]
> > 
> > which result in xsltproc later choking with
> > 
> >   [1/12] Generating virkeyname-osx.html with a meson_exe.py custom command
> >   FAILED: docs/manpages/virkeyname-osx.html
> >   /usr/bin/meson --internal exe --capture docs/manpages/virkeyname-osx.html \
> >     /usr/bin/xsltproc [...] --nonet ../docs/subsite.xsl docs/manpages/virkeyname-osx.html.in
> >   docs/manpages/virkeyname-osx.html.in:17: parser error : Double hyphen within comment:
> >     keymap-gen --lang=rst --title=virkeyname-osx [...]
> 
> I don't see this error. Do I need some very fresh version of something?
> 
> $ rst2html.py --version
> rst2html.py (Docutils 0.16 [release], Python 3.9.4, on linux)

You need the incorrect rst2html tool, see this explanation for example:

https://gitlab.com/libvirt/libvirt/-/issues/139#note_528736524

I'm not so sure about this patch. I think we should instead try to
detect what rst2html is used and error out if it is not the one provided
by docutils project.

Pavel
Re: [libvirt PATCH] meson: Ask rst2html to strip comments
Posted by Andrea Bolognani 2 years, 10 months ago
On Mon, Jun 07, 2021 at 04:52:41PM +0200, Pavel Hrdina wrote:
> On Mon, Jun 07, 2021 at 04:39:15PM +0200, Michal Prívozník wrote:
> > On 6/4/21 2:31 PM, Andrea Bolognani wrote:
> > >   [1/12] Generating virkeyname-osx.html with a meson_exe.py custom command
> > >   FAILED: docs/manpages/virkeyname-osx.html
> > >   /usr/bin/meson --internal exe --capture docs/manpages/virkeyname-osx.html \
> > >     /usr/bin/xsltproc [...] --nonet ../docs/subsite.xsl docs/manpages/virkeyname-osx.html.in
> > >   docs/manpages/virkeyname-osx.html.in:17: parser error : Double hyphen within comment:
> > >     keymap-gen --lang=rst --title=virkeyname-osx [...]
> >
> > I don't see this error. Do I need some very fresh version of something?
> >
> > $ rst2html.py --version
> > rst2html.py (Docutils 0.16 [release], Python 3.9.4, on linux)
>
> You need the incorrect rst2html tool, see this explanation for example:
>
> https://gitlab.com/libvirt/libvirt/-/issues/139#note_528736524

Yeah, use of the incorrect rst2html version is my guess too; I
should, however, point out that I have not actually confirmed this
with the user... See [1] for the full conversation.

> I'm not so sure about this patch. I think we should instead try to
> detect what rst2html is used and error out if it is not the one provided
> by docutils project.

This is the simplest possible patch which solves the issue that the
user reported. In general, stripping the comments shouldn't cause any
problems, so I don't think applying this patch would be wrong.

That said, if you can come up with a sensible way to detect whether
the rst2html binary found on the system is the correct one and abort
otherwise, that would indeed be great! Do you want to give it a try
and see whether it's feasible with a reasonable amount of effort?


[1] https://listman.redhat.com/archives/libvirt-users/2021-June/msg00008.html
-- 
Andrea Bolognani / Red Hat / Virtualization


Re: [libvirt PATCH] meson: Ask rst2html to strip comments
Posted by Daniel P. Berrangé 2 years, 10 months ago
On Mon, Jun 07, 2021 at 04:52:41PM +0200, Pavel Hrdina wrote:
> On Mon, Jun 07, 2021 at 04:39:15PM +0200, Michal Prívozník wrote:
> > On 6/4/21 2:31 PM, Andrea Bolognani wrote:
> > > They can be problematic: in particular, the rst files generated
> > > by keycodemapdb's keymap-gen contain things like
> > > 
> > >   To re-generate, run:
> > >     keymap-gen --lang=rst --title=virkeycode-osx [...]
> > > 
> > > which result in xsltproc later choking with
> > > 
> > >   [1/12] Generating virkeyname-osx.html with a meson_exe.py custom command
> > >   FAILED: docs/manpages/virkeyname-osx.html
> > >   /usr/bin/meson --internal exe --capture docs/manpages/virkeyname-osx.html \
> > >     /usr/bin/xsltproc [...] --nonet ../docs/subsite.xsl docs/manpages/virkeyname-osx.html.in
> > >   docs/manpages/virkeyname-osx.html.in:17: parser error : Double hyphen within comment:
> > >     keymap-gen --lang=rst --title=virkeyname-osx [...]
> > 
> > I don't see this error. Do I need some very fresh version of something?
> > 
> > $ rst2html.py --version
> > rst2html.py (Docutils 0.16 [release], Python 3.9.4, on linux)
> 
> You need the incorrect rst2html tool, see this explanation for example:
> 
> https://gitlab.com/libvirt/libvirt/-/issues/139#note_528736524
> 
> I'm not so sure about this patch. I think we should instead try to
> detect what rst2html is used and error out if it is not the one provided
> by docutils project.

I've looked detecting the wrong rst2html "binary" before, but I didn't
see any attractive ways todo it.

I wonder if the docutils python API is considered stable ?  If it is,
then we could skip calling the system rst2html5 entirely, and just copy
its contents into scripts/rst2html5. Then all we need to check is that
docutils python API is installed, and we'll know we run the right thing.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [libvirt PATCH] meson: Ask rst2html to strip comments
Posted by Michal Prívozník 2 years, 10 months ago
On 6/7/21 5:03 PM, Daniel P. Berrangé wrote:
> On Mon, Jun 07, 2021 at 04:52:41PM +0200, Pavel Hrdina wrote:
>> On Mon, Jun 07, 2021 at 04:39:15PM +0200, Michal Prívozník wrote:
>>> On 6/4/21 2:31 PM, Andrea Bolognani wrote:
>>>> They can be problematic: in particular, the rst files generated
>>>> by keycodemapdb's keymap-gen contain things like
>>>>
>>>>   To re-generate, run:
>>>>     keymap-gen --lang=rst --title=virkeycode-osx [...]
>>>>
>>>> which result in xsltproc later choking with
>>>>
>>>>   [1/12] Generating virkeyname-osx.html with a meson_exe.py custom command
>>>>   FAILED: docs/manpages/virkeyname-osx.html
>>>>   /usr/bin/meson --internal exe --capture docs/manpages/virkeyname-osx.html \
>>>>     /usr/bin/xsltproc [...] --nonet ../docs/subsite.xsl docs/manpages/virkeyname-osx.html.in
>>>>   docs/manpages/virkeyname-osx.html.in:17: parser error : Double hyphen within comment:
>>>>     keymap-gen --lang=rst --title=virkeyname-osx [...]
>>>
>>> I don't see this error. Do I need some very fresh version of something?
>>>
>>> $ rst2html.py --version
>>> rst2html.py (Docutils 0.16 [release], Python 3.9.4, on linux)
>>
>> You need the incorrect rst2html tool, see this explanation for example:
>>
>> https://gitlab.com/libvirt/libvirt/-/issues/139#note_528736524
>>
>> I'm not so sure about this patch. I think we should instead try to
>> detect what rst2html is used and error out if it is not the one provided
>> by docutils project.
> 
> I've looked detecting the wrong rst2html "binary" before, but I didn't
> see any attractive ways todo it.
> 

Well, aren't we looking at one example how to distinguish the two? I
mean, we can detect at configure time whether double hyphens within a
comment work and if not then we found broken rst2html tool.

> I wonder if the docutils python API is considered stable ?  If it is,
> then we could skip calling the system rst2html5 entirely, and just copy
> its contents into scripts/rst2html5. Then all we need to check is that
> docutils python API is installed, and we'll know we run the right thing.

Sounds good.

Michal

Re: [libvirt PATCH] meson: Ask rst2html to strip comments
Posted by Daniel P. Berrangé 2 years, 10 months ago
On Mon, Jun 07, 2021 at 08:35:39PM +0200, Michal Prívozník wrote:
> On 6/7/21 5:03 PM, Daniel P. Berrangé wrote:
> > On Mon, Jun 07, 2021 at 04:52:41PM +0200, Pavel Hrdina wrote:
> >> On Mon, Jun 07, 2021 at 04:39:15PM +0200, Michal Prívozník wrote:
> >>> On 6/4/21 2:31 PM, Andrea Bolognani wrote:
> >>>> They can be problematic: in particular, the rst files generated
> >>>> by keycodemapdb's keymap-gen contain things like
> >>>>
> >>>>   To re-generate, run:
> >>>>     keymap-gen --lang=rst --title=virkeycode-osx [...]
> >>>>
> >>>> which result in xsltproc later choking with
> >>>>
> >>>>   [1/12] Generating virkeyname-osx.html with a meson_exe.py custom command
> >>>>   FAILED: docs/manpages/virkeyname-osx.html
> >>>>   /usr/bin/meson --internal exe --capture docs/manpages/virkeyname-osx.html \
> >>>>     /usr/bin/xsltproc [...] --nonet ../docs/subsite.xsl docs/manpages/virkeyname-osx.html.in
> >>>>   docs/manpages/virkeyname-osx.html.in:17: parser error : Double hyphen within comment:
> >>>>     keymap-gen --lang=rst --title=virkeyname-osx [...]
> >>>
> >>> I don't see this error. Do I need some very fresh version of something?
> >>>
> >>> $ rst2html.py --version
> >>> rst2html.py (Docutils 0.16 [release], Python 3.9.4, on linux)
> >>
> >> You need the incorrect rst2html tool, see this explanation for example:
> >>
> >> https://gitlab.com/libvirt/libvirt/-/issues/139#note_528736524
> >>
> >> I'm not so sure about this patch. I think we should instead try to
> >> detect what rst2html is used and error out if it is not the one provided
> >> by docutils project.
> > 
> > I've looked detecting the wrong rst2html "binary" before, but I didn't
> > see any attractive ways todo it.
> > 
> 
> Well, aren't we looking at one example how to distinguish the two? I
> mean, we can detect at configure time whether double hyphens within a
> comment work and if not then we found broken rst2html tool.

Yes, there are various bugs you can hook on, but they all feel kind
of fragile as things to bet on long term. I was more thinking about
a way to detect the difference at a conceptual level 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|