| Message ID | 20190521164536.20920-1-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@ideasonboard.com> With the shellcheck issues considered, and the tabs fixed up in meson.build: Reviewed-by: Kieran Bingham <kieran.bingham@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') >
Hi Laurent, Thanks for your work. On 2019-05-22 08:39:44 +0100, Kieran Bingham wrote: > 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@ideasonboard.com> > > With the shellcheck issues considered, and the tabs fixed up in meson.build: > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> With Kieran's comments addressed, Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > > --- > > 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 > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
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. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@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 > @@ -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. > + * > + * 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 > + > +cat <<EOF >> $dst_file > + > +#endif /* __LIBCAMERA_LIBCAMERA_H__ */ > +EOF > 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') > + Hrm... should this rebuild if libcamera_api changes at all ? Also - Note that the following warning has appeared in my build with this patch applied: WARNING: Custom target input '.../libcamera/include/libcamera' can't be converted to File object(s). This will become a hard error in the future. > install_headers(libcamera_api, > subdir : 'libcamera') >
Hi Kieran, On Wed, May 22, 2019 at 02:25:59PM +0100, Kieran Bingham wrote: > 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. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@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 > > @@ -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. > > + * > > + * 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 > > + > > +cat <<EOF >> $dst_file > > + > > +#endif /* __LIBCAMERA_LIBCAMERA_H__ */ > > +EOF > > 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') > > + > > Hrm... should this rebuild if libcamera_api changes at all ? It should, but from my tests this is already ensured indirectly. Changing libcamera_api changes meson.build, which triggers a regeneration of all the related ninja files. As the timestamp of the ninja files changes, they rebuild all the rules they contain. > Also - Note that the following warning has appeared in my build with > this patch applied: > > > WARNING: Custom target input '.../libcamera/include/libcamera' can't be > converted to File object(s). > This will become a hard error in the future. Which version of meson are you using ? > > install_headers(libcamera_api, > > subdir : 'libcamera')
Hi Kieran, On Wed, May 22, 2019 at 08:39:44AM +0100, Kieran Bingham wrote: > 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@ideasonboard.com> > > With the shellcheck issues considered, and the tabs fixed up in meson.build: > > Reviewed-by: Kieran Bingham <kieran.bingham@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 I considered both options and found it more logical to keep the file there. It doesn't get installed, so it shouldn't be an issue. If we later extend the script to generate more headers, or more files, than libcamera.h, then we could move it. > > @@ -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 Good point. I however wonder how to treat copyright on auto-generated files. The generation process doesn't create any additional work that can be attributed to an entity entitled to copyrights as far as I understand, so the copyright on libcamera.h would come from the copyright on its source. In this case the source is the combination of the directory layout (hardly copyrightable I believe) and this script. It thus seems that the copyright year should be fixed in the script (and should be 2018-2019), and should only change when the script is significantly modified. I'm no expert in this field though. > > + * > > + * 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) Yes, let's handle this later :-) > > + > > +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. I'll fix these, thank you. > > 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. My bad :-( Will fix too. > > + > > install_headers(libcamera_api, > > subdir : 'libcamera') > >
Hi Laurent, On 22/05/2019 15:26, Laurent Pinchart wrote: > Hi Kieran, > > On Wed, May 22, 2019 at 02:25:59PM +0100, Kieran Bingham wrote: >> 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. >>> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@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 >>> @@ -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. >>> + * >>> + * 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 >>> + >>> +cat <<EOF >> $dst_file >>> + >>> +#endif /* __LIBCAMERA_LIBCAMERA_H__ */ >>> +EOF >>> 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') >>> + >> >> Hrm... should this rebuild if libcamera_api changes at all ? > > It should, but from my tests this is already ensured indirectly. > Changing libcamera_api changes meson.build, which triggers a > regeneration of all the related ninja files. As the timestamp of the > ninja files changes, they rebuild all the rules they contain. OK - that sounds fine then. >> Also - Note that the following warning has appeared in my build with >> this patch applied: >> >> >> WARNING: Custom target input '.../libcamera/include/libcamera' can't be >> converted to File object(s). >> This will become a hard error in the future. > > Which version of meson are you using ? $ meson --version 0.49.0 > >>> install_headers(libcamera_api, >>> subdir : 'libcamera') >
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 @@ -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. + * + * 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 + +cat <<EOF >> $dst_file + +#endif /* __LIBCAMERA_LIBCAMERA_H__ */ +EOF 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') + install_headers(libcamera_api, subdir : 'libcamera')
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. Signed-off-by: Laurent Pinchart <laurent.pinchart@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