Re: [dpdk-dev] [PATCH] doc: support building HTML guides with meson

[Luca Boccassi <bluca@debian.org>]

On Wed, 2018-09-12 at 10:36 +0100, Bruce Richardson wrote:
> On Tue, Sep 11, 2018 at 10:47:58PM +0100, Luca Boccassi wrote:
> > On Tue, 2018-09-11 at 21:36 +0100, Luca Boccassi wrote:
> > > On Tue, 2018-09-11 at 17:13 +0100, Bruce Richardson wrote:
> > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > > ---
> > > > NOTE: this patch depends upon:
> > > > http://patches.dpdk.org/project/dpdk/list/?series=1232
> > > 
> > > Just a reminder that's v2, and the patch won't apply cleanly on
> > > the
> > > latest revision
> > > 
> > > >  doc/api/meson.build    |  3 ++-
> > > >  doc/guides/meson.build | 16 ++++++++++++++++
> > > >  doc/meson.build        | 11 +++++++++++
> > > >  3 files changed, 29 insertions(+), 1 deletion(-)
> > > >  create mode 100644 doc/guides/meson.build
> > > > 
> > > > diff --git a/doc/api/meson.build b/doc/api/meson.build
> > > > index 5dfa0fe04..f9bee4dac 100644
> > > > --- a/doc/api/meson.build
> > > > +++ b/doc/api/meson.build
> > > > @@ -50,5 +50,6 @@ if doxygen.found()
> > > >  		install_dir: htmldir,
> > > >  		build_by_default: false)
> > > >  
> > > > -	run_target('doc', command: 'true', depends:
> > > > doxy_build)
> > > > +	doc_targets += doxy_build
> > > > +	doc_target_names += 'Doxygen_API'
> > > >  endif
> > > > diff --git a/doc/guides/meson.build b/doc/guides/meson.build
> > > > new file mode 100644
> > > > index 000000000..6d1e2990d
> > > > --- /dev/null
> > > > +++ b/doc/guides/meson.build
> > > > @@ -0,0 +1,16 @@
> > > > +# SPDX-License-Identifier: BSD-3-Clause
> > > > +# Copyright(c) 2017 Intel Corporation
> > > 
> > > s/2017/2018
> > > 
> > > > +sphinx = find_program('sphinx-build', required:
> > > > get_option('enable_docs'))
> > > > +
> > > > +if sphinx.found()
> > > > +	html_guides_build = custom_target('html_guides_build',
> > > > +		input: meson.current_source_dir(),
> > > > +		output: 'index.html',
> > > 
> > > As we discussed I don't have a good solution, but right now
> > > running
> > > ninja will rebuild these sphinx docs again and again, which will
> > > be a
> > > bit annoying when using enable_docs=true as it will always
> > > happen.
> > > Changing output to 'html' fixes the issue and it makes it build
> > > only
> > > once, but then they won't rebuild if the docs change as you
> > > noted.
> > > 
> > > > +		command: [sphinx, '-b', 'html', '@INPUT@',
> > > > meson.current_build_dir() + '/html'],
> > > > +		build_by_default: false,
> > > > +		install: get_option('enable_docs'))
> > > > +
> > > > +	doc_targets += html_guides_build
> > > > +	doc_target_names += 'HTML_Guides'
> > > > +endif
> > > > diff --git a/doc/meson.build b/doc/meson.build
> > > > index afca2e713..c5410d85d 100644
> > > > --- a/doc/meson.build
> > > > +++ b/doc/meson.build
> > > > @@ -1,4 +1,15 @@
> > > >  # SPDX-License-Identifier: BSD-3-Clause
> > > >  # Copyright(c) 2018 Luca Boccassi <bluca@debian.org>
> > > >  
> > > > +doc_targets = []
> > > > +doc_target_names = []
> > > >  subdir('api')
> > > > +subdir('guides')
> > > > +
> > > > +if doc_targets.length() == 0
> > > > +	message = 'No docs targets found'
> > > > +else
> > > > +	message = 'Building docs:'
> > > > +endif
> > > > +run_target('doc', command: ['echo', message,
> > > > doc_target_names],
> > > > +	depends: doc_targets)
> > > 
> > > One thing that's missing is the install_dir, without which ninja
> > > install doesn't work (actually errors out for the whole tree).
> > > 
> > > To keep the install the same as the legacy makefiles this diff is
> > > needed (I need to change the outdir in the doxygen stuff too, v5
> > > coming) (in the build dir it will be slightly awkward as it's
> > > build/doc/guides/guides and build/doc/api/api, but I can't find
> > > another
> > > way to get it to work and install in the expected directories):
> > > 
> > > --- a/doc/guides/meson.build
> > > +++ b/doc/guides/meson.build
> > > @@ -4,12 +4,14 @@
> > >  sphinx = find_program('sphinx-build', required:
> > > get_option('enable_docs'))
> > >  
> > >  if sphinx.found()
> > > +       htmldir = join_paths('share', 'doc', 'dpdk')
> > >         html_guides_build = custom_target('html_guides_build',
> > >                 input: meson.current_source_dir(),
> > > -               output: 'index.html',
> > > -               command: [sphinx, '-b', 'html', '@INPUT@',
> > > meson.current_build_dir() + '/html'],
> > > +               output: 'guides',
> > > +               command: [sphinx, '-b', 'html', '@INPUT@',
> > > meson.current_build_dir() + '/guides'],
> > >                 build_by_default: false,
> > > -               install: get_option('enable_docs'))
> > > +               install: get_option('enable_docs'),
> > > +               install_dir: htmldir)
> > >  
> > >         doc_targets += html_guides_build
> > >         doc_target_names += 'HTML_Guides'
> > 
> > Couple more issues: I diffed the installed doc with ye olde make
> > and
> > this one, and:
> > 
> > 1) This one will install sphinx temporary cache files, which among
> > other things will screw up reproducible builds. It's easy to get
> > rid of
> > the .doctrees by adding the following to the sphinx command:
> > 
> >  '-d', meson.current_build_dir() + '/.doctrees'
> > 
> > so that the doctrees are stored in the build directory, rather than
> > in
> > the output directory, and so they will not be installed.
> > But the .buildinfo file, with some hashes (and so more
> > reproducibility
> > problems) is still installed and I can't see how to get rid of it.
> > 
> > 2) The custom.css file is installed manually by ye olde makefiles
> > and
> > thus is missing from guides/_static/css/custom.css
> > 
> 
> Great, thanks for all the reviews. I'll attempt to fix as much as
> possible
> and do a V2.
> 
> /Bruce

For the last 2 things, the only thing I can think of is having a little
bash script like for doxygen, that calls sphinx, copies the css file
and then deletes the .buildinfo. Not the cleanest, but I can't think of
other options - I looked into sphinx-build and I can't see an option to
determine where .buildinfo is saved to unfortunately

-- 
Kind regards,
Luca Boccassi