[libcamera-devel] [PATCH 1/3] Overhaul the directory structure

[Kieran Bingham]

Hi Laurent,

On 04/12/2018 22:11, Laurent Pinchart wrote:
> In order to match the directory structure of traditional projects,
> rename the top-level lib/ directory to src/libcamera/. Other libraries
> developed as part of the project will later find a home in src/.
> 
> Split the libcamera header files in three categories: public headers
> describing the public API in include/libcamera/, internal headers
> describing the internal API in src/libcamera/include/, and private
> headers local to one or a small number of compilation units along the
> corresponding .cpp files. As no internal header exists yet the
> src/libcamera/include/ directory is created empty as the build system
> would fail otherwise.
> 

I like how you've updated the includes so that the shared library has
protected header includes, but the tests only have public.

Perhaps we might need to distinguish this so that we can be more
selective on tests, and have a libcamera_public includes and a
libcamera_protected includes (which also includes public) so that we can
have tests on the protected API's too.

But - we don't have any protected tests yet - so not something worth
requiring a v2 unless you want to update.

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

Either way:

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

> ---
>  meson.build                        | 4 ++--
>  src/libcamera/include/.keep_empty  | 0
>  {lib => src/libcamera}/main.cpp    | 0
>  {lib => src/libcamera}/meson.build | 7 ++++++-
>  src/meson.build                    | 1 +
>  test/meson.build                   | 2 +-
>  6 files changed, 10 insertions(+), 4 deletions(-)
>  create mode 100644 src/libcamera/include/.keep_empty
>  rename {lib => src/libcamera}/main.cpp (100%)
>  rename {lib => src/libcamera}/meson.build (50%)
>  create mode 100644 src/meson.build
> 
> diff --git a/meson.build b/meson.build
> index e0aeefa30330..e6ed03f951c7 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -21,11 +21,11 @@ cpp_arguments = common_arguments
>  add_project_arguments(c_arguments, language: 'c')
>  add_project_arguments(cpp_arguments, language: 'cpp')
>  
> -inc = include_directories('include')
> +libcamera_includes = include_directories('include')

It might be worth naming this libcamera_public


>  
>  subdir('Documentation')
>  subdir('include')
> -subdir('lib')
> +subdir('src')
>  subdir('test')
>  subdir('utils')
>  
> diff --git a/src/libcamera/include/.keep_empty b/src/libcamera/include/.keep_empty
> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/lib/main.cpp b/src/libcamera/main.cpp
> similarity index 100%
> rename from lib/main.cpp
> rename to src/libcamera/main.cpp
> diff --git a/lib/meson.build b/src/libcamera/meson.build
> similarity index 50%
> rename from lib/meson.build
> rename to src/libcamera/meson.build
> index fcd738cc86d8..07d9cd448342 100644
> --- a/lib/meson.build
> +++ b/src/libcamera/meson.build
> @@ -1,6 +1,11 @@
>  sources = ['main.cpp']
>  

And adding:
libcamera_protected = include_directories('include')

here,

> +includes = [
> +    libcamera_includes,
> +    include_directories('include'),
> +]
> +
>  libcamera = shared_library('camera',
>                             sources,
>                             install : true,
> -                           include_directories : inc)
> +                           include_directories : includes)
> diff --git a/src/meson.build b/src/meson.build
> new file mode 100644
> index 000000000000..4ce9668caa7b
> --- /dev/null
> +++ b/src/meson.build
> @@ -0,0 +1 @@
> +subdir('libcamera')
> diff --git a/test/meson.build b/test/meson.build
> index afe9bd9a741c..924a26f1ca66 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -1,5 +1,5 @@
>  test_init = executable('test_init', 'init.cpp',
>                         link_with : libcamera,
> -                       include_directories : inc)
> +                       include_directories : libcamera_includes)

Then it would be clear that this was only able to access the public API.


>  
>  test('Initialisation test', test_init)
> 

-- 
Regards
--
Kieran