[libcamera-devel] [PATCH 0/3] libcamera: Add initial logger support

[Laurent Pinchart]

Hi Kieran,

On Monday, 10 December 2018 17:10:42 EET Kieran Bingham wrote:
> On 10/12/2018 14:48, Laurent Pinchart wrote:
> > On Monday, 10 December 2018 15:41:53 EET Kieran Bingham wrote:
> >> Hi Laurent,
> >> 
> >> I pushed the master branch with my gitlab runner on top and it failed
> >> the build tests I'm afraid :(
> >> 
> >> Failure at :
> >> 	https://gitlab.com/libcamera/libcamera/-/jobs/132294793
> >> 
> >> [1/9] Compiling C++ object
> >> 'src/libcamera/src at libcamera@@camera at sha/log.cpp.o'.
> >> FAILED: src/libcamera/src at libcamera@@camera at sha/log.cpp.o
> >> c++ -Isrc/libcamera/src at libcamera@@camera at sha -Isrc/libcamera
> >> -I../src/libcamera -Iinclude -I../include -I../src/libcamera/include
> >> -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall
> >> -Winvalid-pch -Wnon-virtual-dtor -O0 -g -Wall -Wextra -Werror
> >> -Wno-unused-parameter -fPIC  -MD -MQ
> >> 'src/libcamera/src at libcamera@@camera at sha/log.cpp.o' -MF
> >> 'src/libcamera/src at libcamera@@camera at sha/log.cpp.o.d' -o
> >> 'src/libcamera/src at libcamera@@camera at sha/log.cpp.o' -c
> >> ../src/libcamera/log.cpp
> >> ../src/libcamera/log.cpp: In constructor
> >> 'libcamera::LogMessage::LogMessage(const char*, unsigned int,
> >> libcamera::LogSeverity)':
> >> ../src/libcamera/log.cpp:69:39: error: 'basename' was not declared in
> >> this scope
> >> 
> >>   msgStream << " " << basename(fileName) << ":" << line << " ";
> > 
> > Which libc was that compiled with ? At this point I don't think we should
> > support other C libraries than glibc. We'll fix it eventually of course.
> 
> This is against a musl libc on alpine-linux.
> 
> I agree - this isn't really a priority for us right now - but perhaps
> highlights that at some point we should consider what toolchains the
> library will be built with.

Absolutely agreed. I think catering for the needs of multiple lib C 
implementations now would hinder development, but in the not too distant 
future we'll have to handle this task.

> I expect we should also try compiling with clang/LLVM when we get our
> automated test infrastructure set up.

I agree as well, and I think this could be addressed sooner than later.

> >> On 04/12/2018 22:11, Laurent Pinchart wrote:
> >>> Hello,
> >>> 
> >>> This patch series adds an initial logger to the library. It's so far
> >>> very dumb and is lacking many features, so volunteers are needed to
> >>> extend it :-)
> >>> 
> >>> Laurent Pinchart (3):
> >>>   Overhaul the directory structure
> >>>   libcamera: Add initial logger
> >>>   libcamera: Use the logger instead of cout
> >>>  
> >>>  include/libcamera/libcamera.h   |  4 ++
> >>>  lib/meson.build                 |  6 ---
> >>>  meson.build                     |  4 +-
> >>>  src/libcamera/include/log.h     | 38 ++++++++++++++++
> >>>  src/libcamera/include/utils.h   | 12 +++++
> >>>  src/libcamera/log.cpp           | 81 +++++++++++++++++++++++++++++++++
> >>>  {lib => src/libcamera}/main.cpp | 10 ++--
> >>>  src/libcamera/meson.build       | 14 ++++++
> >>>  src/meson.build                 |  1 +
> >>>  test/init.cpp                   |  2 +-
> >>>  test/meson.build                |  2 +-
> >>>  11 files changed, 160 insertions(+), 14 deletions(-)
> >>>  delete mode 100644 lib/meson.build
> >>>  create mode 100644 src/libcamera/include/log.h
> >>>  create mode 100644 src/libcamera/include/utils.h
> >>>  create mode 100644 src/libcamera/log.cpp
> >>>  rename {lib => src/libcamera}/main.cpp (68%)
> >>>  create mode 100644 src/libcamera/meson.build
> >>>  create mode 100644 src/meson.build

-- 
Regards,

Laurent Pinchart