Re: [PATCH 135/147] meson: sphinx-build
On 10/08/20 21:06, Paolo Bonzini wrote: >>> diff --git a/configure b/configure >>> index 21b9ed2..7e7b4d8 100755 >>> --- a/configure >>> +++ b/configure >>> @@ -7768,7 +7768,6 @@ echo "INSTALL_PROG=$install -c -m 0755" >> >>> $config_host_mak >>> echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak >>> echo "PYTHON=$python" >> $config_host_mak >>> echo "SPHINX_BUILD=$sphinx_build" >> $config_host_mak >>> -echo "SPHINX_WERROR=$sphinx_werror" >> $config_host_mak >> Shouldn't we also be deleting the code in configure that >> sets $sphinx_werror if we're no longer using it ? > Yes. > ... It's still used by has_sphinx_build, actually. Paolo
Re: [PATCH 135/147] meson: sphinx-build
Il lun 10 ago 2020, 21:57 Peter Maydell ha scritto: > > Since it's all handled internally by sphinx, I think you only need to > > add the man pages to the dictionary, and get rid of the corresponding > > Texinfo outputs in qapi/meson.build and qga/meson.build? > > The patchset has a fair amount of change to the makefiles: > Makefile | 86 +--- > rules.mak | 14 +- > > > > In other words, it should be just this: > > ...so if it's that simple that would be nice. > Hopefully. Of course you would also have to delete the Texinfo rules in meson.build but that's certainly more self explanatory then the qapi-gen.py bit. Paolo > -- PMM > >
Re: [PATCH 135/147] meson: sphinx-build
On Mon, 10 Aug 2020 at 20:46, Paolo Bonzini wrote: > > On 10/08/20 21:36, Peter Maydell wrote: > >> It should be possible and probably not too hard once I figure out how > >> Sphinx events work. It's a fair request since build_always_stale is > >> inferior and Meson requires no particular magic to include the depfile. > >> Maybe that will win you over. :) > >> > >> I can also leave out sphinx from the initial conversion. > > If we have a working-but-build-always conversion for Sphinx > > I'd be happy to take that and then upgrade it to processing > > the dependencies properly later. > > > > (The thing I'm not really looking forward to is updating > > the qapi-doc-to-rst patchset to Meson...) > > Since it's all handled internally by sphinx, I think you only need to > add the man pages to the dictionary, and get rid of the corresponding > Texinfo outputs in qapi/meson.build and qga/meson.build? The patchset has a fair amount of change to the makefiles: Makefile | 86 +--- rules.mak | 14 +- > In other words, it should be just this: ...so if it's that simple that would be nice. -- PMM
Re: [PATCH 135/147] meson: sphinx-build
On 10/08/20 21:36, Peter Maydell wrote:
>> It should be possible and probably not too hard once I figure out how
>> Sphinx events work. It's a fair request since build_always_stale is
>> inferior and Meson requires no particular magic to include the depfile.
>> Maybe that will win you over. :)
>>
>> I can also leave out sphinx from the initial conversion.
> If we have a working-but-build-always conversion for Sphinx
> I'd be happy to take that and then upgrade it to processing
> the dependencies properly later.
>
> (The thing I'm not really looking forward to is updating
> the qapi-doc-to-rst patchset to Meson...)
Since it's all handled internally by sphinx, I think you only need to
add the man pages to the dictionary, and get rid of the corresponding
Texinfo outputs in qapi/meson.build and qga/meson.build?
In other words, it should be just this:
diff --git a/qapi/meson.build b/qapi/meson.build
index de5b16f..315252f 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -96,7 +96,7 @@ foreach module : qapi_all_modules
endforeach
qapi_files = custom_target('shared QAPI source files',
- output: qapi_util_outputs + qapi_specific_outputs + qapi_nonmodule_outputs +
['qapi-doc.texi'],
+ output: qapi_util_outputs + qapi_specific_outputs + qapi_nonmodule_outputs,
input: [ files('qapi-schema.json') ],
command: [ qapi_gen, '-o', 'qapi', '-b', '@INPUT0@' ],
depend_files: [ qapi_inputs, qapi_gen_depends ])
@@ -120,5 +120,3 @@ foreach output : qapi_specific_outputs +
qapi_nonmodule_outputs
specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: qapi_files[i])
i = i + 1
endforeach
-
-qapi_doc_texi = qapi_files[i]
diff --git a/qga/meson.build b/qga/meson.build
index 6f6..741b683 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -16,7 +16,7 @@ qga_qapi_outputs = [
]
qga_qapi_files = custom_target('QGA QAPI files',
- output: qga_qapi_outputs +
['qga-qapi-doc.texi'],
+ output: qga_qapi_outputs,
input: 'qapi-schema.json',
command: [ qapi_gen, '-o', 'qga', '-p', 'qga-',
'@INPUT0@' ],
depend_files: qapi_gen_depends)
@@ -27,7 +27,6 @@ foreach output: qga_qapi_outputs
qga_ss.add(qga_qapi_files[i])
i = i + 1
endforeach
-qga_qapi_doc_texi = qga_qapi_files[i]
qga_ss.add(files(
'commands.c',
Paolo
Re: [PATCH 135/147] meson: sphinx-build
On Mon, 10 Aug 2020 at 20:31, Paolo Bonzini wrote: > > On 10/08/20 21:21, Peter Maydell wrote: > >> Yes, because the Makefile's approach is not maintainable in my opinion; > >> *.rst.inc files were already not included in the Makefile. I'll look > >> into using a Sphinx extension to produce a dependency file. > > > > Yeah, agreed that the makefile approach isn't great. (It lists > > some .rst.inc files but we added more without updating the > > dependencies, I think.) > > > > If Sphinx can be persuaded to output a dependency file that > > would certainly be the nicest approach; I hadn't thought > > of trying that. > > It should be possible and probably not too hard once I figure out how > Sphinx events work. It's a fair request since build_always_stale is > inferior and Meson requires no particular magic to include the depfile. > Maybe that will win you over. :) > > I can also leave out sphinx from the initial conversion. If we have a working-but-build-always conversion for Sphinx I'd be happy to take that and then upgrade it to processing the dependencies properly later. (The thing I'm not really looking forward to is updating the qapi-doc-to-rst patchset to Meson...) thanks -- PMM
Re: [PATCH 135/147] meson: sphinx-build
On 10/08/20 21:21, Peter Maydell wrote: >> Yes, because the Makefile's approach is not maintainable in my opinion; >> *.rst.inc files were already not included in the Makefile. I'll look >> into using a Sphinx extension to produce a dependency file. > > Yeah, agreed that the makefile approach isn't great. (It lists > some .rst.inc files but we added more without updating the > dependencies, I think.) > > If Sphinx can be persuaded to output a dependency file that > would certainly be the nicest approach; I hadn't thought > of trying that. It should be possible and probably not too hard once I figure out how Sphinx events work. It's a fair request since build_always_stale is inferior and Meson requires no particular magic to include the depfile. Maybe that will win you over. :) I can also leave out sphinx from the initial conversion. > It would be nice to note in the commit messages where the > conversion has made this kind of "we're going to do it a > different way" design decision rather than just being > a translation of the makefile logic into Meson. Yes, I'll do that for the final version (to be posted Friday or next Monday). Thanks for running these initial test, it looks encouraging. Paolo
Re: [PATCH 135/147] meson: sphinx-build
On Mon, 10 Aug 2020 at 20:06, Paolo Bonzini wrote: > > On 10/08/20 20:33, Peter Maydell wrote: > > Does "build_always_stale: true" do what I guess it does from the > > name? Does this mean we're discarding the makefile's approach of > > only running sphinx if it's necessary in favour of always running > > half a dozen sphinx invocations every build ? > > Yes, because the Makefile's approach is not maintainable in my opinion; > *.rst.inc files were already not included in the Makefile. I'll look > into using a Sphinx extension to produce a dependency file. Yeah, agreed that the makefile approach isn't great. (It lists some .rst.inc files but we added more without updating the dependencies, I think.) If Sphinx can be persuaded to output a dependency file that would certainly be the nicest approach; I hadn't thought of trying that. It would be nice to note in the commit messages where the conversion has made this kind of "we're going to do it a different way" design decision rather than just being a translation of the makefile logic into Meson. thanks -- PMM
Re: [PATCH 135/147] meson: sphinx-build
On 10/08/20 20:33, Peter Maydell wrote:
> On Mon, 10 Aug 2020 at 19:16, Paolo Bonzini wrote:
>>
>> Signed-off-by: Marc-André Lureau
>> Signed-off-by: Paolo Bonzini
>
>> diff --git a/configure b/configure
>> index 21b9ed2..7e7b4d8 100755
>> --- a/configure
>> +++ b/configure
>> @@ -7768,7 +7768,6 @@ echo "INSTALL_PROG=$install -c -m 0755" >>
>> $config_host_mak
>> echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak
>> echo "PYTHON=$python" >> $config_host_mak
>> echo "SPHINX_BUILD=$sphinx_build" >> $config_host_mak
>> -echo "SPHINX_WERROR=$sphinx_werror" >> $config_host_mak
>
> Shouldn't we also be deleting the code in configure that
> sets $sphinx_werror if we're no longer using it ?
Yes.
>> +these_man_pages = []
>> +install_dirs = []
>> +foreach page, section : man_pages.get(manual, {})
>> + these_man_pages += page
>> + install_dirs += section == '' ? false : get_option('mandir') / section
>> +endforeach
>> +if these_man_pages.length() > 0
>> + sphinxmans += custom_target(manual + ' man pages',
>> + build_always_stale: true,
>> + build_by_default: build_docs,
>> + output: these_man_pages,
>> + install: build_docs,
>> + install_dir: install_dirs,
>> + command: [SPHINX_ARGS, '-b', 'man', '-d',
>> private_dir,
>> + input_dir, meson.current_build_dir()])
>> +endif
>> + endforeach
>> + alias_target('sphinxdocs', sphinxdocs)
>> + alias_target('man', sphinxmans)
>
> Does "build_always_stale: true" do what I guess it does from the
> name? Does this mean we're discarding the makefile's approach of
> only running sphinx if it's necessary in favour of always running
> half a dozen sphinx invocations every build ?
Yes, because the Makefile's approach is not maintainable in my opinion;
*.rst.inc files were already not included in the Makefile. I'll look
into using a Sphinx extension to produce a dependency file.
Paolo
Paolo
Re: [PATCH 135/147] meson: sphinx-build
On Mon, 10 Aug 2020 at 19:16, Paolo Bonzini wrote:
>
> Signed-off-by: Marc-André Lureau
> Signed-off-by: Paolo Bonzini
> diff --git a/configure b/configure
> index 21b9ed2..7e7b4d8 100755
> --- a/configure
> +++ b/configure
> @@ -7768,7 +7768,6 @@ echo "INSTALL_PROG=$install -c -m 0755" >>
> $config_host_mak
> echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak
> echo "PYTHON=$python" >> $config_host_mak
> echo "SPHINX_BUILD=$sphinx_build" >> $config_host_mak
> -echo "SPHINX_WERROR=$sphinx_werror" >> $config_host_mak
Shouldn't we also be deleting the code in configure that
sets $sphinx_werror if we're no longer using it ?
> +these_man_pages = []
> +install_dirs = []
> +foreach page, section : man_pages.get(manual, {})
> + these_man_pages += page
> + install_dirs += section == '' ? false : get_option('mandir') / section
> +endforeach
> +if these_man_pages.length() > 0
> + sphinxmans += custom_target(manual + ' man pages',
> + build_always_stale: true,
> + build_by_default: build_docs,
> + output: these_man_pages,
> + install: build_docs,
> + install_dir: install_dirs,
> + command: [SPHINX_ARGS, '-b', 'man', '-d',
> private_dir,
> + input_dir, meson.current_build_dir()])
> +endif
> + endforeach
> + alias_target('sphinxdocs', sphinxdocs)
> + alias_target('man', sphinxmans)
Does "build_always_stale: true" do what I guess it does from the
name? Does this mean we're discarding the makefile's approach of
only running sphinx if it's necessary in favour of always running
half a dozen sphinx invocations every build ?
thanks
-- PMM
