| Message ID | 20181229032855.26249-3-niklas.soderlund@ragnatech.se |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, Laurent, On 29/12/2018 03:28, Niklas Söderlund wrote: > The device enumeration will depend on libudev, add the dependency to the > build system. This should be turned into a optional dependency once a > device enumerator not using udev is supported. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/meson.build | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index 46591069aa5f8beb..52b556a8ed4050cb 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -16,7 +16,10 @@ includes = [ > libcamera_internal_includes, > ] > > +libudev = dependency('libudev') > + The website currently states that we will not depend on anything except the standard libraries [0]. I guess this will be updated when we move the website to build from the repo, although I can't see anywhere within Documentation listing anything similar. Is there a section missing in our source level docs? [0] http://www.libcamera.org/docs.html#technical-requirements -- Kieran > libcamera = shared_library('camera', > libcamera_sources, > install : true, > - include_directories : includes) > + include_directories : includes, > + dependencies : libudev) >
Hi Kieran, On Wednesday, 9 January 2019 19:07:29 EET Kieran Bingham wrote: > On 29/12/2018 03:28, Niklas Söderlund wrote: > > The device enumeration will depend on libudev, add the dependency to the > > build system. This should be turned into a optional dependency once a > > device enumerator not using udev is supported. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > > > src/libcamera/meson.build | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > index 46591069aa5f8beb..52b556a8ed4050cb 100644 > > --- a/src/libcamera/meson.build > > +++ b/src/libcamera/meson.build > > @@ -16,7 +16,10 @@ includes = [ > > libcamera_internal_includes, > > ] > > > > +libudev = dependency('libudev') > > + > > The website currently states that we will not depend on anything except > the standard libraries [0]. It also states "If other dependencies are deemed to be useful during development they shall be proposed and reviewed." :-) Note that we should make this dependency optional. > I guess this will be updated when we move the website to build from the > repo, although I can't see anywhere within Documentation listing > anything similar. Is there a section missing in our source level docs? The rest of the section has been added to the coding style document, but this particular bullet point is missing. It doesn't belong to the coding style in my opinion. Any idea on who to structure the documentation to give it a home ? > [0] http://www.libcamera.org/docs.html#technical-requirements > > > libcamera = shared_library('camera', > > libcamera_sources, > > install : true, > > - include_directories : includes) > > + include_directories : includes, > > + dependencies : libudev)
Hi Laurent, On 10/01/2019 13:39, Laurent Pinchart wrote: > Hi Kieran, > > On Wednesday, 9 January 2019 19:07:29 EET Kieran Bingham wrote: >> On 29/12/2018 03:28, Niklas Söderlund wrote: >>> The device enumeration will depend on libudev, add the dependency to the >>> build system. This should be turned into a optional dependency once a >>> device enumerator not using udev is supported. >>> >>> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>> --- >>> >>> src/libcamera/meson.build | 5 ++++- >>> 1 file changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build >>> index 46591069aa5f8beb..52b556a8ed4050cb 100644 >>> --- a/src/libcamera/meson.build >>> +++ b/src/libcamera/meson.build >>> @@ -16,7 +16,10 @@ includes = [ >>> libcamera_internal_includes, >>> ] >>> >>> +libudev = dependency('libudev') >>> + >> >> The website currently states that we will not depend on anything except >> the standard libraries [0]. > > It also states "If other dependencies are deemed to be useful during > development they shall be proposed and reviewed." :-) Note that we should make > this dependency optional. Ah yes - I'm not disagreeing on the inclusion of the dependency - just that it leaves the documentation out-dated. Agreed on making it optional at some point but we can call that an optimisation at the moment :) (and I don't think it can be optional until there is a sysfs fallback option) >> I guess this will be updated when we move the website to build from the >> repo, although I can't see anywhere within Documentation listing >> anything similar. Is there a section missing in our source level docs? > > The rest of the section has been added to the coding style document, but this > particular bullet point is missing. It doesn't belong to the coding style in > my opinion. Any idea on who to structure the documentation to give it a home ? We have a top level README.md which describes how to build the project. That file itself could do with some attention at somepoint, but I think project dependencies should go with build instructions. I think we should keep a top-level README.md - but we might want to integrate that somehow into the website documentation as well in a manner which doesn't duplicate the text if possible. Can we progress your patches that build the website from the source tree? I think at least bcd64a88e307...e164951f08 inclusive from your documentation branch could be posted and integrated already... Would you like to do so ? or shall I steal the patches ? :) Personally I'd say just push them: Documentation: Add custom theme Documentation: Make the toctree more web-friendly Documentation: Link to the API documentation generated by Doxygen Can have an Acked-by: tag from me if you like. I realise there is a bit of a hack/workaround in that last patch - but it works and if we dislike it so much we can solve it later, along with looking at the sphinx/breathe integration at some point. I'd rather see the patches progress upstream as they are as it will allow us to tie in the website documentation directly and improve incrementally. -- Kieran >> [0] http://www.libcamera.org/docs.html#technical-requirements >> >>> libcamera = shared_library('camera', >>> libcamera_sources, >>> install : true, >>> - include_directories : includes) >>> + include_directories : includes, >>> + dependencies : libudev) >
Hi Kieran, On Thursday, 10 January 2019 18:34:26 EET Kieran Bingham wrote: > On 10/01/2019 13:39, Laurent Pinchart wrote: > > On Wednesday, 9 January 2019 19:07:29 EET Kieran Bingham wrote: > >> On 29/12/2018 03:28, Niklas Söderlund wrote: > >>> The device enumeration will depend on libudev, add the dependency to the > >>> build system. This should be turned into a optional dependency once a > >>> device enumerator not using udev is supported. > >>> > >>> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>> --- > >>> > >>> src/libcamera/meson.build | 5 ++++- > >>> 1 file changed, 4 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > >>> index 46591069aa5f8beb..52b556a8ed4050cb 100644 > >>> --- a/src/libcamera/meson.build > >>> +++ b/src/libcamera/meson.build > >>> @@ -16,7 +16,10 @@ includes = [ > >>> libcamera_internal_includes, > >>> ] > >>> > >>> +libudev = dependency('libudev') > >>> + > >> > >> The website currently states that we will not depend on anything except > >> the standard libraries [0]. > > > > It also states "If other dependencies are deemed to be useful during > > development they shall be proposed and reviewed." :-) Note that we should > > make this dependency optional. > > Ah yes - I'm not disagreeing on the inclusion of the dependency - just > that it leaves the documentation out-dated. > > Agreed on making it optional at some point but we can call that an > optimisation at the moment :) (and I don't think it can be optional > until there is a sysfs fallback option) Agreed, that's on the todo list. > >> I guess this will be updated when we move the website to build from the > >> repo, although I can't see anywhere within Documentation listing > >> anything similar. Is there a section missing in our source level docs? > > > > The rest of the section has been added to the coding style document, but > > this particular bullet point is missing. It doesn't belong to the coding > > style in my opinion. Any idea on who to structure the documentation to > > give it a home ? > > We have a top level README.md which describes how to build the project. > That file itself could do with some attention at somepoint, but I think > project dependencies should go with build instructions. Don't they also belong to a design document that would explain the rationale ? > I think we should keep a top-level README.md - but we might want to > integrate that somehow into the website documentation as well in a manner > which doesn't duplicate the text if possible. I'm all for de-duplicating :-) > Can we progress your patches that build the website from the source tree? > > I think at least bcd64a88e307...e164951f08 inclusive from your > documentation branch could be posted and integrated already... > > Would you like to do so ? or shall I steal the patches ? :) Done :-) > Personally I'd say just push them: > Documentation: Add custom theme > Documentation: Make the toctree more web-friendly > Documentation: Link to the API documentation generated by Doxygen > > Can have an Acked-by: tag from me if you like. I've updated those patches to add support for a search box and to remove the toc tree on the first page. Could you have a look ? > I realise there is a bit of a hack/workaround in that last patch - but > it works and if we dislike it so much we can solve it later, along with > looking at the sphinx/breathe integration at some point. I also believe we will at some point need to move documentation to a subsection of the website, and generate the rest using a different tool. There's no urgency, but if we want to add other services (such as a bugtracker for instance), or extend the site with more dynamic content (such as blogging) we won't be able to generate it all using sphinx. > I'd rather see the patches progress upstream as they are as it will > allow us to tie in the website documentation directly and improve > incrementally. I agree. > >> [0] http://www.libcamera.org/docs.html#technical-requirements > >> > >>> libcamera = shared_library('camera', > >>> libcamera_sources, > >>> install : true, > >>> - include_directories : includes) > >>> + include_directories : includes, > >>> + dependencies : libudev)
diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index 46591069aa5f8beb..52b556a8ed4050cb 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -16,7 +16,10 @@ includes = [ libcamera_internal_includes, ] +libudev = dependency('libudev') + libcamera = shared_library('camera', libcamera_sources, install : true, - include_directories : includes) + include_directories : includes, + dependencies : libudev)