[libcamera-devel] [PATCH] libcamera: Auto-generate libcamera.h

[Kieran Bingham]

Hi Laurent,

On 21/05/2019 17:45, Laurent Pinchart wrote:
> As shown by two missing includes, keeping the libcamera.h file in sync
> when adding or removing headers is an error-prone manual process.
> Automate it by generating the header automatically.

Great idea!

> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

With the shellcheck issues considered, and the tabs fixed up in meson.build:

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


> ---
>  include/libcamera/gen-header.sh | 26 ++++++++++++++++++++++++++
>  include/libcamera/libcamera.h   | 20 --------------------
>  include/libcamera/meson.build   | 10 +++++++++-
>  3 files changed, 35 insertions(+), 21 deletions(-)
>  create mode 100755 include/libcamera/gen-header.sh
>  delete mode 100644 include/libcamera/libcamera.h
> 
> diff --git a/include/libcamera/gen-header.sh b/include/libcamera/gen-header.sh
> new file mode 100755
> index 000000000000..ea5b79a62520
> --- /dev/null
> +++ b/include/libcamera/gen-header.sh

Hrm... shouldn't this live in a utils directory?

Although there are some strong arguments for keeping it in
include/libcamera, such as:

 - It keeps the directory layout correct in git
 - The helper is in the place that the header will be output so it's
   easy to find

> @@ -0,0 +1,26 @@
> +#!/bin/sh
> +
> +src_dir=$1
> +dst_file=$2
> +
> +cat <<EOF > $dst_file
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/* This file is auto-generated, do not edit! */
> +/*
> + * Copyright (C) 2018, Google Inc.

Why not add `date +'%Y'` as it's autogenerated ? :-D

> + *
> + * libcamera.h - libcamera public API
> + */
> +#ifndef __LIBCAMERA_LIBCAMERA_H__
> +#define __LIBCAMERA_LIBCAMERA_H__
> +
> +EOF
> +
> +for header in $src_dir/*.h ; do
> +	echo "#include <libcamera/$(basename $header)>" >> $dst_file
> +done


Are we ever expecting to have subdirs here?
I don't think so at the moment, and if we do - then we can update this
at that point

(I'm sure someone will notice if they can't include their headers :D)

> +
> +cat <<EOF >> $dst_file
> +
> +#endif /* __LIBCAMERA_LIBCAMERA_H__ */
> +EOF


Here's the output of running shellcheck on this script:

> shellcheck ./include/libcamera/gen-header.sh 
> 
> In ./include/libcamera/gen-header.sh line 6:
> cat <<EOF > $dst_file
>             ^-- SC2086: Double quote to prevent globbing and word splitting.
> 
> 
> In ./include/libcamera/gen-header.sh line 19:
> for header in $src_dir/*.h ; do
>               ^-- SC2231: Quote expansions in this for loop glob to prevent wordsplitting, e.g. "$dir"/*.txt .
> 
> 
> In ./include/libcamera/gen-header.sh line 20:
>         echo "#include <libcamera/$(basename $header)>" >> $dst_file
>                                              ^-- SC2086: Double quote to prevent globbing and word splitting.
>                                                            ^-- SC2086: Double quote to prevent globbing and word splitting.
> 
> 
> In ./include/libcamera/gen-header.sh line 23:
> cat <<EOF >> $dst_file
>              ^-- SC2086: Double quote to prevent globbing and word splitting.





> diff --git a/include/libcamera/libcamera.h b/include/libcamera/libcamera.h
> deleted file mode 100644
> index dda576e906fb..000000000000
> --- a/include/libcamera/libcamera.h
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -/* SPDX-License-Identifier: LGPL-2.1-or-later */
> -/*
> - * Copyright (C) 2018, Google Inc.
> - *
> - * libcamera.h - libcamera public API
> - */
> -#ifndef __LIBCAMERA_LIBCAMERA_H__
> -#define __LIBCAMERA_LIBCAMERA_H__
> -
> -#include <libcamera/buffer.h>
> -#include <libcamera/camera.h>
> -#include <libcamera/camera_manager.h>
> -#include <libcamera/event_dispatcher.h>
> -#include <libcamera/event_notifier.h>
> -#include <libcamera/request.h>
> -#include <libcamera/signal.h>
> -#include <libcamera/stream.h>
> -#include <libcamera/timer.h>
> -
> -#endif /* __LIBCAMERA_LIBCAMERA_H__ */
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index 83d226ac5078..46aff0b6bc2f 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -5,7 +5,6 @@ libcamera_api = files([
>      'event_dispatcher.h',
>      'event_notifier.h',
>      'geometry.h',
> -    'libcamera.h',
>      'object.h',
>      'request.h',
>      'signal.h',
> @@ -13,5 +12,14 @@ libcamera_api = files([
>      'timer.h',
>  ])
>  
> +gen_header = join_paths(meson.current_source_dir(), 'gen-header.sh')
> +
> +custom_target('gen-header',
> +	      input: meson.current_source_dir(),
> +              output: 'libcamera.h',
> +	      command: [gen_header, '@INPUT@', '@OUTPUT@'],
> +	      install: true,
> +	      install_dir: 'include/libcamera')

I think some tabs have snuck in there.




> +
>  install_headers(libcamera_api,
>                  subdir : 'libcamera')
> 

-- 
Regards
--
Kieran