[libcamera-devel] [PATCH v2 02/12] libcamera: add dependency on libudev

[Laurent Pinchart]

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 at ragnatech.se>
> >>> Reviewed-by: Laurent Pinchart <laurent.pinchart at 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)

-- 
Regards,

Laurent Pinchart