[libvirt PATCH 0/2] meson: Detect and reject invalid rst2html5 command
Previous attempt at solving this issue: https://listman.redhat.com/archives/libvir-list/2021-June/msg00097.html The solution presented here should be way more future-proof, though there's of course always the risk that the format used to report version information will change in a way that causes our detection to trip over... Andrea Bolognani (2): meson: Use 'rst2html5' instead of 'rst2html' everywhere meson: Detect and reject invalid rst2html5 command docs/go/meson.build | 2 +- docs/kbase/meson.build | 2 +- docs/manpages/meson.build | 2 +- docs/meson.build | 8 ++++---- meson.build | 30 +++++++++++++++++++++++++++++- 5 files changed, 36 insertions(+), 8 deletions(-) -- 2.31.1
We only use the HTML5 version these days. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/go/meson.build | 2 +- docs/kbase/meson.build | 2 +- docs/manpages/meson.build | 2 +- docs/meson.build | 8 ++++---- meson.build | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/go/meson.build b/docs/go/meson.build index d2accd322b..0ae0216ce3 100644 --- a/docs/go/meson.build +++ b/docs/go/meson.build @@ -12,7 +12,7 @@ foreach name : docs_go_files html_xslt_gen += { 'name': name, - 'file': docs_rst2html_gen.process(rst_file), + 'file': docs_rst2html5_gen.process(rst_file), 'source': 'docs' / 'go' / rst_file, } endforeach diff --git a/docs/kbase/meson.build b/docs/kbase/meson.build index 6d17a83d1d..8d8fd695ea 100644 --- a/docs/kbase/meson.build +++ b/docs/kbase/meson.build @@ -30,7 +30,7 @@ foreach name : docs_kbase_files html_xslt_gen += { 'name': name, - 'file': docs_rst2html_gen.process(rst_file), + 'file': docs_rst2html5_gen.process(rst_file), 'source': 'docs/kbase' / rst_file, } endforeach diff --git a/docs/manpages/meson.build b/docs/manpages/meson.build index 0fa93fad89..fe91a43614 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: [ rst2html5_prog, '--stylesheet=', '--strict', '@INPUT@' ], capture: true, ) diff --git a/docs/meson.build b/docs/meson.build index 4497f7270f..cbc138fa1f 100644 --- a/docs/meson.build +++ b/docs/meson.build @@ -183,8 +183,8 @@ docs_lxc_api_xml = docs_api_generated[1] docs_qemu_api_xml = docs_api_generated[2] docs_admin_api_xml = docs_api_generated[3] -docs_rst2html_gen = generator( - rst2html_prog, +docs_rst2html5_gen = generator( + rst2html5_prog, output: '@BASENAME@.html.in', arguments: [ '--stylesheet=', '--strict', '@INPUT@' ], capture: true, @@ -215,7 +215,7 @@ foreach name : docs_rst_files rst_file = '@0@.rst'.format(name) html_xslt_gen += { 'name': name, - 'file': docs_rst2html_gen.process(rst_file), + 'file': docs_rst2html5_gen.process(rst_file), 'source': 'docs' / rst_file, } endforeach @@ -252,7 +252,7 @@ html_xslt_gen += { 'file': hvsupport_html_in, } -news_html_in = docs_rst2html_gen.process(meson.source_root() / 'NEWS.rst') +news_html_in = docs_rst2html5_gen.process(meson.source_root() / 'NEWS.rst') html_xslt_gen += { 'name': 'news', 'file': news_html_in, diff --git a/meson.build b/meson.build index e25dc17fc8..32ad688c9c 100644 --- a/meson.build +++ b/meson.build @@ -790,7 +790,7 @@ required_programs = [ required_programs_groups = [ { 'name': 'rpcgen', 'prog': [ 'rpcgen', 'portable-rpcgen' ] }, - { 'name': 'rst2html', 'prog': [ 'rst2html5', 'rst2html5.py', 'rst2html5-3' ] }, + { 'name': 'rst2html5', 'prog': [ 'rst2html5', 'rst2html5.py', 'rst2html5-3' ] }, { 'name': 'rst2man', 'prog': [ 'rst2man', 'rst2man.py', 'rst2man-3' ] }, ] -- 2.31.1
The version coming from the rst2html5 package instead of the docutils package is unable to successfully generate the libvirt documentation. Examples of users encountering build issues because of the wrong version of rst2html5 being installed on their systems: https://gitlab.com/libvirt/libvirt/-/issues/40 https://gitlab.com/libvirt/libvirt/-/issues/139 https://gitlab.com/libvirt/libvirt/-/issues/169 https://gitlab.com/libvirt/libvirt/-/issues/195 Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- meson.build | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/meson.build b/meson.build index 32ad688c9c..02357a2678 100644 --- a/meson.build +++ b/meson.build @@ -813,6 +813,34 @@ foreach item : required_programs_groups set_variable('@0@_prog'.format(varname), prog) endforeach +# There are two versions of rst2html5 in the wild: one is the version +# coming from the docutils package, and the other is the one coming +# from the rst2html5 package. These versions are subtly different, +# and the libvirt documentation can only be successfully generated +# using the docutils version. Every now and then, users will report +# build failures that can be traced back to having the wrong version +# installed. +# +# The only reliable way to tell the two binaris apart seems to be +# looking look at their version information: the docutils version +# will report +# +# rst2html5 (Docutils ..., Python ..., on ...) +# +# whereas the rst2html5 version will report +# +# rst2html5 ... (Docutils ..., Python ..., on ...) +# +# with the additional bit of information being the version number for +# the rst2html5 package itself. +# +# Use this knowledge to detect the version that we know doesn't work +# for building libvirt and reject it +rst2html5_version = run_command(rst2html5_prog, '--version') +rst2html5_version = rst2html5_version.stdout().split('(') +if rst2html5_version[0] != 'rst2html5 ' + error('Please uninstall the rst2html5 package and install the docutils package') +endif # optional programs -- 2.31.1
On Mon, Aug 09, 2021 at 05:13:29PM +0200, Andrea Bolognani wrote:
The version coming from the rst2html5 package instead of the docutils package is unable to successfully generate the libvirt documentation.
Examples of users encountering build issues because of the wrong version of rst2html5 being installed on their systems:
https://gitlab.com/libvirt/libvirt/-/issues/40 https://gitlab.com/libvirt/libvirt/-/issues/139 https://gitlab.com/libvirt/libvirt/-/issues/169 https://gitlab.com/libvirt/libvirt/-/issues/195
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- meson.build | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/meson.build b/meson.build index 32ad688c9c..02357a2678 100644 --- a/meson.build +++ b/meson.build @@ -813,6 +813,34 @@ foreach item : required_programs_groups set_variable('@0@_prog'.format(varname), prog) endforeach
+# There are two versions of rst2html5 in the wild: one is the version +# coming from the docutils package, and the other is the one coming +# from the rst2html5 package. These versions are subtly different, +# and the libvirt documentation can only be successfully generated +# using the docutils version. Every now and then, users will report +# build failures that can be traced back to having the wrong version +# installed. +# +# The only reliable way to tell the two binaris apart seems to be +# looking look at their version information: the docutils version +# will report +# +# rst2html5 (Docutils ..., Python ..., on ...) +# +# whereas the rst2html5 version will report +# +# rst2html5 ... (Docutils ..., Python ..., on ...) +# +# with the additional bit of information being the version number for +# the rst2html5 package itself.
This feels way too fragile to me. Can't we just write out a 5 line rst doc showing the problem and try to run the command to detect brokeness ? 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 :|
On Tue, Aug 10, 2021 at 11:05:42AM +0100, Daniel P. Berrangé wrote:
On Mon, Aug 09, 2021 at 05:13:29PM +0200, Andrea Bolognani wrote:
The version coming from the rst2html5 package instead of the docutils package is unable to successfully generate the libvirt documentation.
Examples of users encountering build issues because of the wrong version of rst2html5 being installed on their systems:
https://gitlab.com/libvirt/libvirt/-/issues/40 https://gitlab.com/libvirt/libvirt/-/issues/139 https://gitlab.com/libvirt/libvirt/-/issues/169 https://gitlab.com/libvirt/libvirt/-/issues/195
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- meson.build | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/meson.build b/meson.build index 32ad688c9c..02357a2678 100644 --- a/meson.build +++ b/meson.build @@ -813,6 +813,34 @@ foreach item : required_programs_groups set_variable('@0@_prog'.format(varname), prog) endforeach
+# There are two versions of rst2html5 in the wild: one is the version +# coming from the docutils package, and the other is the one coming +# from the rst2html5 package. These versions are subtly different, +# and the libvirt documentation can only be successfully generated +# using the docutils version. Every now and then, users will report +# build failures that can be traced back to having the wrong version +# installed. +# +# The only reliable way to tell the two binaris apart seems to be +# looking look at their version information: the docutils version +# will report +# +# rst2html5 (Docutils ..., Python ..., on ...) +# +# whereas the rst2html5 version will report +# +# rst2html5 ... (Docutils ..., Python ..., on ...) +# +# with the additional bit of information being the version number for +# the rst2html5 package itself.
This feels way too fragile to me.
Can't we just write out a 5 line rst doc showing the problem and try to run the command to detect brokeness ?
Last time we discussed this, crafting a document that would trigger one of the known differences in behavior was also deemed "kind of fragile"[1]. Ultimately I don't think there's a way to tell the two binaries apart which isn't at least a bit susceptible to breaking down the line, especially because the rst2html5 package appears to be trying its best to be a drop-in replacement - except of course for the things that are different. That said, considering that the good rst2html5 binary comes from the docutils package itself I don't really foresee a scenario where they would have to introduce additional version information: it will naturally be versioned the same as docutils itself. So this feels like a reasonably future-proof detection method. Once again, I'm all hears when it comes to alternative ideas. AFAIK nobody has manged to come up with something that we're all 100% happy with, and I would hate to just leave things as they are because users keep hitting the issue - which incidentally also means that developers have to spend time debugging it and guiding them to the solution every single time. [1] https://listman.redhat.com/archives/libvir-list/2021-June/msg00196.html -- Andrea Bolognani / Red Hat / Virtualization
On Tue, Aug 10, 2021 at 05:02:54AM -0700, Andrea Bolognani wrote:
On Tue, Aug 10, 2021 at 11:05:42AM +0100, Daniel P. Berrangé wrote:
On Mon, Aug 09, 2021 at 05:13:29PM +0200, Andrea Bolognani wrote:
The version coming from the rst2html5 package instead of the docutils package is unable to successfully generate the libvirt documentation.
Examples of users encountering build issues because of the wrong version of rst2html5 being installed on their systems:
https://gitlab.com/libvirt/libvirt/-/issues/40 https://gitlab.com/libvirt/libvirt/-/issues/139 https://gitlab.com/libvirt/libvirt/-/issues/169 https://gitlab.com/libvirt/libvirt/-/issues/195
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- meson.build | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/meson.build b/meson.build index 32ad688c9c..02357a2678 100644 --- a/meson.build +++ b/meson.build @@ -813,6 +813,34 @@ foreach item : required_programs_groups set_variable('@0@_prog'.format(varname), prog) endforeach
+# There are two versions of rst2html5 in the wild: one is the version +# coming from the docutils package, and the other is the one coming +# from the rst2html5 package. These versions are subtly different, +# and the libvirt documentation can only be successfully generated +# using the docutils version. Every now and then, users will report +# build failures that can be traced back to having the wrong version +# installed. +# +# The only reliable way to tell the two binaris apart seems to be +# looking look at their version information: the docutils version +# will report +# +# rst2html5 (Docutils ..., Python ..., on ...) +# +# whereas the rst2html5 version will report +# +# rst2html5 ... (Docutils ..., Python ..., on ...) +# +# with the additional bit of information being the version number for +# the rst2html5 package itself.
This feels way too fragile to me.
Can't we just write out a 5 line rst doc showing the problem and try to run the command to detect brokeness ?
Last time we discussed this, crafting a document that would trigger one of the known differences in behavior was also deemed "kind of fragile"[1].
Ultimately I don't think there's a way to tell the two binaries apart which isn't at least a bit susceptible to breaking down the line, especially because the rst2html5 package appears to be trying its best to be a drop-in replacement - except of course for the things that are different.
That said, considering that the good rst2html5 binary comes from the docutils package itself I don't really foresee a scenario where they would have to introduce additional version information: it will naturally be versioned the same as docutils itself. So this feels like a reasonably future-proof detection method.
Once again, I'm all hears when it comes to alternative ideas. AFAIK nobody has manged to come up with something that we're all 100% happy with, and I would hate to just leave things as they are because users keep hitting the issue - which incidentally also means that developers have to spend time debugging it and guiding them to the solution every single time.
[1] https://listman.redhat.com/archives/libvir-list/2021-June/msg00196.html
Resurrecting an old thread. Dan, should I consider your criticism of the approach an explicit NACK? If not, I will pick up Pavel's ACK and push this. I think the latter is the right thing to do, on the basis that it will leave us in a place that's strictly better than the status quo and that - to the best of my knowledge - no alternative that doesn't itself suffer from known drawbacks has been proposed. -- Andrea Bolognani / Red Hat / Virtualization
On Wed, Oct 06, 2021 at 10:58:29AM -0500, Andrea Bolognani wrote:
Dan, should I consider your criticism of the approach an explicit NACK? If not, I will pick up Pavel's ACK and push this.
I think the latter is the right thing to do, on the basis that it will leave us in a place that's strictly better than the status quo and that - to the best of my knowledge - no alternative that doesn't itself suffer from known drawbacks has been proposed.
Since no actual NACK has been raised over the past three months, nor has an alternative implementation materialized, I went ahead and pushed the change. -- Andrea Bolognani / Red Hat / Virtualization
On Wed, Nov 03, 2021 at 04:15:49AM -0500, Andrea Bolognani wrote:
On Wed, Oct 06, 2021 at 10:58:29AM -0500, Andrea Bolognani wrote:
Dan, should I consider your criticism of the approach an explicit NACK? If not, I will pick up Pavel's ACK and push this.
I think the latter is the right thing to do, on the basis that it will leave us in a place that's strictly better than the status quo and that - to the best of my knowledge - no alternative that doesn't itself suffer from known drawbacks has been proposed.
Since no actual NACK has been raised over the past three months, nor has an alternative implementation materialized, I went ahead and pushed the change.
This broke FreeBSD CI https://gitlab.com/libvirt/libvirt/-/pipelines/400924210/failures 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 :|
On Wed, Nov 03, 2021 at 10:26:12AM +0000, Daniel P. Berrangé wrote:
On Wed, Nov 03, 2021 at 04:15:49AM -0500, Andrea Bolognani wrote:
Since no actual NACK has been raised over the past three months, nor has an alternative implementation materialized, I went ahead and pushed the change.
This broke FreeBSD CI
https://gitlab.com/libvirt/libvirt/-/pipelines/400924210/failures
Thanks for the heads up! I'll investigate right away. -- Andrea Bolognani / Red Hat / Virtualization
On Mon, Aug 09, 2021 at 05:13:27PM +0200, Andrea Bolognani wrote:
Previous attempt at solving this issue:
https://listman.redhat.com/archives/libvir-list/2021-June/msg00097.html
The solution presented here should be way more future-proof, though there's of course always the risk that the format used to report version information will change in a way that causes our detection to trip over...
This definitely improves the situation, let's hope they will not change the --version output to have it the same :).
Andrea Bolognani (2): meson: Use 'rst2html5' instead of 'rst2html' everywhere meson: Detect and reject invalid rst2html5 command
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
participants (3)
-
Andrea Bolognani -
Daniel P. Berrangé -
Pavel Hrdina