[v1] Documentation: Fix `INCLUDE_PATH` doxygen configuration option
diff mbox series

Message ID 20250526120355.2023227-1-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • [v1] Documentation: Fix `INCLUDE_PATH` doxygen configuration option
Related show

Commit Message

Barnabás Pőcze May 26, 2025, 12:03 p.m. UTC
libcamera header files should be included using the `libcamera/...` prefix.
However, `INCLUDE_PATH` is currently set to `@TOP_SRCDIR@/include/libcamera`
meaning that doxygen, when encountering `libcamera/x.h`, will try to open
`@TOP_SRCDIR@/include/libcamera/libcamera/x.h`, which is not the correct
path.

Fix that by using `@TOP_{BUILD,SRC}DIR@/include`. This removes the extra
`libcamera` component from the path and adds the corresponding directory
from the build directory as well since that is an implicit include
directory added by meson.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 Documentation/Doxyfile-common.in | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Kieran Bingham May 26, 2025, 12:19 p.m. UTC | #1
Quoting Barnabás Pőcze (2025-05-26 13:03:55)
> libcamera header files should be included using the `libcamera/...` prefix.
> However, `INCLUDE_PATH` is currently set to `@TOP_SRCDIR@/include/libcamera`
> meaning that doxygen, when encountering `libcamera/x.h`, will try to open
> `@TOP_SRCDIR@/include/libcamera/libcamera/x.h`, which is not the correct
> path.
> 
> Fix that by using `@TOP_{BUILD,SRC}DIR@/include`. This removes the extra
> `libcamera` component from the path and adds the corresponding directory
> from the build directory as well since that is an implicit include
> directory added by meson.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

This sounds very reasonable - I wonder when this crept in. I recall it
was a long time ago that we decided to keep libcamera headers under the
"/libcamera/*" include hierarchy - so I wonder if this was missed back
then or added since.

Anyway - it won't matter much - and I believe this sounds right.

Is there anything in the output doxygen that this fixes or that can
demonstrate doxygen is now doing the right thing that was missing
before?

Either way, I'd still give this an ack...

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  Documentation/Doxyfile-common.in | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/Doxyfile-common.in b/Documentation/Doxyfile-common.in
> index 045c19dd6..58afea1cd 100644
> --- a/Documentation/Doxyfile-common.in
> +++ b/Documentation/Doxyfile-common.in
> @@ -57,7 +57,8 @@ GENERATE_LATEX         = NO
>  MACRO_EXPANSION        = YES
>  EXPAND_ONLY_PREDEF     = YES
>  
> -INCLUDE_PATH           = "@TOP_SRCDIR@/include/libcamera"
> +INCLUDE_PATH           = "@TOP_BUILDDIR@/include" \
> +                         "@TOP_SRCDIR@/include"
>  INCLUDE_FILE_PATTERNS  = *.h
>  
>  IMAGE_PATH             = "@TOP_SRCDIR@/Documentation/images"
> -- 
> 2.49.0
>
Barnabás Pőcze May 26, 2025, 1:25 p.m. UTC | #2
2025. 05. 26. 14:19 keltezéssel, Kieran Bingham írta:
> Quoting Barnabás Pőcze (2025-05-26 13:03:55)
>> libcamera header files should be included using the `libcamera/...` prefix.
>> However, `INCLUDE_PATH` is currently set to `@TOP_SRCDIR@/include/libcamera`
>> meaning that doxygen, when encountering `libcamera/x.h`, will try to open
>> `@TOP_SRCDIR@/include/libcamera/libcamera/x.h`, which is not the correct
>> path.
>>
>> Fix that by using `@TOP_{BUILD,SRC}DIR@/include`. This removes the extra
>> `libcamera` component from the path and adds the corresponding directory
>> from the build directory as well since that is an implicit include
>> directory added by meson.
>>
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> 
> This sounds very reasonable - I wonder when this crept in. I recall it
> was a long time ago that we decided to keep libcamera headers under the
> "/libcamera/*" include hierarchy - so I wonder if this was missed back
> then or added since.
> 
> Anyway - it won't matter much - and I believe this sounds right.
> 
> Is there anything in the output doxygen that this fixes or that can
> demonstrate doxygen is now doing the right thing that was missing
> before?

You can see the difference if you run `doxygen -d preprocessor ...`, e.g.

   #include libcamera/base/bound_method.h: already processed! skipping...
   #include libcamera/base/message.h: parsing...
     #include atomic: not found! skipping...
     #include libcamera/base/private.h: parsing...
     #include libcamera/base/bound_method.h: already processed! skipping...

vs.

   #include libcamera/base/bound_method.h: already processed! skipping...
   #include libcamera/base/message.h: not found! skipping...


I don't believe it changes anything in practice, or at least I have not
noticed any changes. But I thought it's better to be correct even if it
does not really do anything.


Regards,
Barnabás Pőcze


> 
> Either way, I'd still give this an ack...
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
>> ---
>>   Documentation/Doxyfile-common.in | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/Doxyfile-common.in b/Documentation/Doxyfile-common.in
>> index 045c19dd6..58afea1cd 100644
>> --- a/Documentation/Doxyfile-common.in
>> +++ b/Documentation/Doxyfile-common.in
>> @@ -57,7 +57,8 @@ GENERATE_LATEX         = NO
>>   MACRO_EXPANSION        = YES
>>   EXPAND_ONLY_PREDEF     = YES
>>   
>> -INCLUDE_PATH           = "@TOP_SRCDIR@/include/libcamera"
>> +INCLUDE_PATH           = "@TOP_BUILDDIR@/include" \
>> +                         "@TOP_SRCDIR@/include"
>>   INCLUDE_FILE_PATTERNS  = *.h
>>   
>>   IMAGE_PATH             = "@TOP_SRCDIR@/Documentation/images"
>> -- 
>> 2.49.0
>>
Kieran Bingham May 26, 2025, 1:40 p.m. UTC | #3
Quoting Barnabás Pőcze (2025-05-26 14:25:08)
> 2025. 05. 26. 14:19 keltezéssel, Kieran Bingham írta:
> > Quoting Barnabás Pőcze (2025-05-26 13:03:55)
> >> libcamera header files should be included using the `libcamera/...` prefix.
> >> However, `INCLUDE_PATH` is currently set to `@TOP_SRCDIR@/include/libcamera`
> >> meaning that doxygen, when encountering `libcamera/x.h`, will try to open
> >> `@TOP_SRCDIR@/include/libcamera/libcamera/x.h`, which is not the correct
> >> path.
> >>
> >> Fix that by using `@TOP_{BUILD,SRC}DIR@/include`. This removes the extra
> >> `libcamera` component from the path and adds the corresponding directory
> >> from the build directory as well since that is an implicit include
> >> directory added by meson.
> >>
> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> > 
> > This sounds very reasonable - I wonder when this crept in. I recall it
> > was a long time ago that we decided to keep libcamera headers under the
> > "/libcamera/*" include hierarchy - so I wonder if this was missed back
> > then or added since.
> > 
> > Anyway - it won't matter much - and I believe this sounds right.
> > 
> > Is there anything in the output doxygen that this fixes or that can
> > demonstrate doxygen is now doing the right thing that was missing
> > before?
> 
> You can see the difference if you run `doxygen -d preprocessor ...`, e.g.
> 
>    #include libcamera/base/bound_method.h: already processed! skipping...
>    #include libcamera/base/message.h: parsing...
>      #include atomic: not found! skipping...
>      #include libcamera/base/private.h: parsing...
>      #include libcamera/base/bound_method.h: already processed! skipping...
> 
> vs.
> 
>    #include libcamera/base/bound_method.h: already processed! skipping...
>    #include libcamera/base/message.h: not found! skipping...
> 
> 
> I don't believe it changes anything in practice, or at least I have not
> noticed any changes. But I thought it's better to be correct even if it
> does not really do anything.

I agree, thanks for the insights above.

--
Kieran


> 
> 
> Regards,
> Barnabás Pőcze
> 
> 
> > 
> > Either way, I'd still give this an ack...
> > 
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > 
> >> ---
> >>   Documentation/Doxyfile-common.in | 3 ++-
> >>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/Doxyfile-common.in b/Documentation/Doxyfile-common.in
> >> index 045c19dd6..58afea1cd 100644
> >> --- a/Documentation/Doxyfile-common.in
> >> +++ b/Documentation/Doxyfile-common.in
> >> @@ -57,7 +57,8 @@ GENERATE_LATEX         = NO
> >>   MACRO_EXPANSION        = YES
> >>   EXPAND_ONLY_PREDEF     = YES
> >>   
> >> -INCLUDE_PATH           = "@TOP_SRCDIR@/include/libcamera"
> >> +INCLUDE_PATH           = "@TOP_BUILDDIR@/include" \
> >> +                         "@TOP_SRCDIR@/include"
> >>   INCLUDE_FILE_PATTERNS  = *.h
> >>   
> >>   IMAGE_PATH             = "@TOP_SRCDIR@/Documentation/images"
> >> -- 
> >> 2.49.0
> >>
>
Laurent Pinchart May 28, 2025, 2:04 p.m. UTC | #4
Hi Barnabás,

Thank you for the patch.

On Mon, May 26, 2025 at 02:03:55PM +0200, Barnabás Pőcze wrote:
> libcamera header files should be included using the `libcamera/...` prefix.
> However, `INCLUDE_PATH` is currently set to `@TOP_SRCDIR@/include/libcamera`
> meaning that doxygen, when encountering `libcamera/x.h`, will try to open
> `@TOP_SRCDIR@/include/libcamera/libcamera/x.h`, which is not the correct
> path.
> 
> Fix that by using `@TOP_{BUILD,SRC}DIR@/include`. This removes the extra
> `libcamera` component from the path and adds the corresponding directory
> from the build directory as well since that is an implicit include
> directory added by meson.

Generating documentation from files in the build directory isn't very
nice, as doxygen will list build directory names in the generated HTML.
We should fix it at some point, but that's an existing issue that isn't
introduced by this patch, so

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

> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  Documentation/Doxyfile-common.in | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/Doxyfile-common.in b/Documentation/Doxyfile-common.in
> index 045c19dd6..58afea1cd 100644
> --- a/Documentation/Doxyfile-common.in
> +++ b/Documentation/Doxyfile-common.in
> @@ -57,7 +57,8 @@ GENERATE_LATEX         = NO
>  MACRO_EXPANSION        = YES
>  EXPAND_ONLY_PREDEF     = YES
>  
> -INCLUDE_PATH           = "@TOP_SRCDIR@/include/libcamera"
> +INCLUDE_PATH           = "@TOP_BUILDDIR@/include" \
> +                         "@TOP_SRCDIR@/include"
>  INCLUDE_FILE_PATTERNS  = *.h
>  
>  IMAGE_PATH             = "@TOP_SRCDIR@/Documentation/images"

Patch
diff mbox series

diff --git a/Documentation/Doxyfile-common.in b/Documentation/Doxyfile-common.in
index 045c19dd6..58afea1cd 100644
--- a/Documentation/Doxyfile-common.in
+++ b/Documentation/Doxyfile-common.in
@@ -57,7 +57,8 @@  GENERATE_LATEX         = NO
 MACRO_EXPANSION        = YES
 EXPAND_ONLY_PREDEF     = YES
 
-INCLUDE_PATH           = "@TOP_SRCDIR@/include/libcamera"
+INCLUDE_PATH           = "@TOP_BUILDDIR@/include" \
+                         "@TOP_SRCDIR@/include"
 INCLUDE_FILE_PATTERNS  = *.h
 
 IMAGE_PATH             = "@TOP_SRCDIR@/Documentation/images"