Message ID | 20181204221123.571-2-laurent.pinchart@ideasonboard.com |
---|---|
State | Accepted |
Commit | 913b3ee817a34bd78d5a8dc4fc9025920202db09 |
Headers | show |
Series |
|
Related | show |
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@ideasonboard.com> Either way: Reviewed-by: Kieran Bingham <kieran.bingham@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) >
Hi Kieran, On Thursday, 6 December 2018 16:21:58 EET Kieran Bingham wrote: > 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. It's a good idea. Let's do so when we'll add unit tests :-) > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Either way: > > Reviewed-by: Kieran Bingham <kieran.bingham@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)
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') 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'] +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) test('Initialisation test', test_init)
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. Signed-off-by: Laurent Pinchart <laurent.pinchart@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