[2/2] meson: do not automatically build documentation if sphinx-build-3 is found
diff mbox series

Message ID 20250404161237.1298653-2-foss+libcamera@0leil.net
State New
Headers show
Series
  • [1/2] meson: make the default value of "documentation" feature explicit
Related show

Commit Message

Quentin Schulz April 4, 2025, 4:12 p.m. UTC
From: Quentin Schulz <quentin.schulz@cherry.de>

Commit aba567338b25 ("Documentation: Move all dependencies into
features") did an incomplete migration of the documentation boolean
option into a documentation feature.

If sphinx-build-3 binary is found on the host system, the documentation
is built, regardless of the value of the feature option.

This makes sure that sphinx-build-3 presence is only checked if the
documentation feature is not disabled (which is the default, as it's
"auto" by default).

This is essential for reproducibility for build systems where
sphinx-build-3 may or may not be present when libcamera is built, and
also to declutter the generated package if documentation isn't desired.

Fixes: aba567338b25 ("Documentation: Move all dependencies into features")
Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 Documentation/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Laurent Pinchart April 4, 2025, 5:21 p.m. UTC | #1
Hi Quentin,

Thank you for the patch.

On Fri, Apr 04, 2025 at 06:12:35PM +0200, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> Commit aba567338b25 ("Documentation: Move all dependencies into
> features") did an incomplete migration of the documentation boolean
> option into a documentation feature.
> 
> If sphinx-build-3 binary is found on the host system, the documentation
> is built, regardless of the value of the feature option.
> 
> This makes sure that sphinx-build-3 presence is only checked if the
> documentation feature is not disabled (which is the default, as it's
> "auto" by default).
> 
> This is essential for reproducibility for build systems where
> sphinx-build-3 may or may not be present when libcamera is built, and
> also to declutter the generated package if documentation isn't desired.
> 
> Fixes: aba567338b25 ("Documentation: Move all dependencies into features")
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  Documentation/meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/meson.build b/Documentation/meson.build
> index 6158320e..c59849f6 100644
> --- a/Documentation/meson.build
> +++ b/Documentation/meson.build
> @@ -116,7 +116,7 @@ endif
>  # Sphinx
>  #
>  
> -sphinx = find_program('sphinx-build-3', required : false)
> +sphinx = find_program('sphinx-build-3', required : get_option('documentation'))
>  if not sphinx.found()
>      sphinx = find_program('sphinx-build', required : get_option('documentation'))
>  endif
Barnabás Pőcze April 4, 2025, 5:50 p.m. UTC | #2
Hi


2025. 04. 04. 18:12 keltezéssel, Quentin Schulz írta:
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> Commit aba567338b25 ("Documentation: Move all dependencies into
> features") did an incomplete migration of the documentation boolean
> option into a documentation feature.
> 
> If sphinx-build-3 binary is found on the host system, the documentation
> is built, regardless of the value of the feature option.
> 
> This makes sure that sphinx-build-3 presence is only checked if the
> documentation feature is not disabled (which is the default, as it's
> "auto" by default).
> 
> This is essential for reproducibility for build systems where
> sphinx-build-3 may or may not be present when libcamera is built, and
> also to declutter the generated package if documentation isn't desired.
> 
> Fixes: aba567338b25 ("Documentation: Move all dependencies into features")
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---
>   Documentation/meson.build | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/meson.build b/Documentation/meson.build
> index 6158320e..c59849f6 100644
> --- a/Documentation/meson.build
> +++ b/Documentation/meson.build
> @@ -116,7 +116,7 @@ endif
>   # Sphinx
>   #
>   
> -sphinx = find_program('sphinx-build-3', required : false)
> +sphinx = find_program('sphinx-build-3', required : get_option('documentation'))

I think this is not entirely ideal. If `documentation=enabled`, `sphinx-build-3`
is not found, but `sphinx-build` is, then the above call will abort with an error,
and meson will never check for `sphinx-build`.

I think

   sphinx = find_program(['sphinx-build-3', 'sphinx-build'], required : get_option('documentation')

might work.


>   if not sphinx.found()
>       sphinx = find_program('sphinx-build', required : get_option('documentation'))
>   endif


Regards,
Barnabás Pőcze
Laurent Pinchart April 4, 2025, 6:10 p.m. UTC | #3
On Fri, Apr 04, 2025 at 07:50:06PM +0200, Barnabás Pőcze wrote:
> 2025. 04. 04. 18:12 keltezéssel, Quentin Schulz írta:
> > From: Quentin Schulz <quentin.schulz@cherry.de>
> > 
> > Commit aba567338b25 ("Documentation: Move all dependencies into
> > features") did an incomplete migration of the documentation boolean
> > option into a documentation feature.
> > 
> > If sphinx-build-3 binary is found on the host system, the documentation
> > is built, regardless of the value of the feature option.
> > 
> > This makes sure that sphinx-build-3 presence is only checked if the
> > documentation feature is not disabled (which is the default, as it's
> > "auto" by default).
> > 
> > This is essential for reproducibility for build systems where
> > sphinx-build-3 may or may not be present when libcamera is built, and
> > also to declutter the generated package if documentation isn't desired.
> > 
> > Fixes: aba567338b25 ("Documentation: Move all dependencies into features")
> > Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> > ---
> >   Documentation/meson.build | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/meson.build b/Documentation/meson.build
> > index 6158320e..c59849f6 100644
> > --- a/Documentation/meson.build
> > +++ b/Documentation/meson.build
> > @@ -116,7 +116,7 @@ endif
> >   # Sphinx
> >   #
> >   
> > -sphinx = find_program('sphinx-build-3', required : false)
> > +sphinx = find_program('sphinx-build-3', required : get_option('documentation'))
> 
> I think this is not entirely ideal. If `documentation=enabled`, `sphinx-build-3`
> is not found, but `sphinx-build` is, then the above call will abort with an error,
> and meson will never check for `sphinx-build`.

Good point.

> I think
> 
>    sphinx = find_program(['sphinx-build-3', 'sphinx-build'], required : get_option('documentation')
> 
> might work.

Or

    sphinx = find_program('sphinx-build-3', 'sphinx-build', required : get_option('documentation')

Quentin, could you check if that fixes your issue ?

> >   if not sphinx.found()
> >       sphinx = find_program('sphinx-build', required : get_option('documentation'))
> >   endif

Patch
diff mbox series

diff --git a/Documentation/meson.build b/Documentation/meson.build
index 6158320e..c59849f6 100644
--- a/Documentation/meson.build
+++ b/Documentation/meson.build
@@ -116,7 +116,7 @@  endif
 # Sphinx
 #
 
-sphinx = find_program('sphinx-build-3', required : false)
+sphinx = find_program('sphinx-build-3', required : get_option('documentation'))
 if not sphinx.found()
     sphinx = find_program('sphinx-build', required : get_option('documentation'))
 endif