[libcamera-devel] [RFC PATCH 1/8] meson: Simplify pkg_mod.generate

[Kieran Bingham]

Hi Laurent,

On 01/12/2020 17:12, Laurent Pinchart wrote:
> On Tue, Dec 01, 2020 at 05:01:46PM +0000, Kieran Bingham wrote:
>> Hi Laurent,
>>
>> On 01/12/2020 16:58, Laurent Pinchart wrote:
>>> Hi Kieran,
>>>
>>> On Tue, Dec 01, 2020 at 04:56:56PM +0000, Kieran Bingham wrote:
>>>> On 01/12/2020 16:50, Laurent Pinchart wrote:
>>>>> On Mon, Nov 23, 2020 at 04:43:12PM +0000, Kieran Bingham wrote:
>>>>>> Later versions of meson allow for the first positional argument to
>>>>>> specificy the defaults.  Specify the libcamera library as the first
>>>>>> argument, and remove the filebase.
>>>>>>
>>>>>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>>>>> ---
>>>>>>  meson.build | 3 +--
>>>>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/meson.build b/meson.build
>>>>>> index 55cf36e15f57..ced4afa7d726 100644
>>>>>> --- a/meson.build
>>>>>> +++ b/meson.build
>>>>>> @@ -146,10 +146,9 @@ run_command('ln', '-fsT', meson.source_root(),
>>>>>>  configure_file(output : 'config.h', configuration : config_h)
>>>>>>  
>>>>>>  pkg_mod = import('pkgconfig')
>>>>>> -pkg_mod.generate(libraries : libcamera,
>>>>>> +pkg_mod.generate(libcamera,
>>>>>>                   version : '1.0',
>>>>>>                   name : 'libcamera',
>>>>>> -                 filebase : 'camera',
>>>>>>                   description : 'Complex Camera Support Library',
>>>>>>                   subdirs : 'libcamera')
>>>>>>  
>>>>>
>>>>> The documentation states
>>>>>
>>>>> Since 0.46 a StaticLibrary or SharedLibrary object can optionally be
>>>>> passed as first positional argument. If one is provided a default value
>>>>> will be provided for all required fields of the pc file:
>>>>>
>>>>> - install_dir is set to pkgconfig folder in the same location than the
>>>>>   provided library.
>>>>> - description is set to the project's name followed by the library's
>>>>>   name.
>>>>> - name is set to the library's name.
>>>>>
>>>>>
>>>>> Is it the "name" argument that can be dropped, not "filebase" ? filebase
>>>>> seems to default to name, so it would be set to "libcamera", not
>>>>> "camera". If we drop the name argument then name will default to the
>>>>> shared library name, which is "camera", and filebase will then default
>>>>> to name. There thus seems to be a change of behaviour, which should be
>>>>> documented in the commit message (and verified to ensure it's correct).
>>>>
>>>>
>>>> I'll have to check, but this was a patch I had stuck around for ages,
>>>> which I created upon a recommendation from one of the meson devs.
>>>>
>>>> I haven't seen any change in behaviour, but I'll double check again
>>>> later, meanwhile: an install from this build seems to show everything
>>>> still correctly named 'libcamera' ...
>>>>
>>>> /tmp/libcamera-build-prefix/usr/share/libcamera
>>>> /tmp/libcamera-build-prefix/usr/share/doc/libcamera-0.0.11
>>>> /tmp/libcamera-build-prefix/usr/lib/x86_64-linux-gnu/libcamera.so
>>>> /tmp/libcamera-build-prefix/usr/include/libcamera/libcamera/version.h
>>>
>>> I expect this patch to only affect the .pc file generation :-) Could you
>>> diff its contents, and check if its name changes ?
>>
>> diff
>> /tmp/libcamera-build-prefix/usr/lib/x86_64-linux-gnu/pkgconfig/libcamera.pc
>> /usr/lib/x86_64-linux-gnu/pkgconfig/libcamera.pc
>>
>> <blank> ;-)
> 
> That's strange, here I have

I must have installed a 'patched' version at some point which broke my
comparison.

Anyway, this patch is a distraction - so I've split out the meson fixes
to a new series and dropped this patch for now.

There's no bug/issue here - it was only a recommendation from meson
developers.

We can always revisit it later.



> diff -Nur a/usr/local/lib64/pkgconfig/camera.pc b/usr/local/lib64/pkgconfig/camera.pc
> --- a/usr/local/lib64/pkgconfig/camera.pc       2020-12-01 17:37:07.077026434 +0200
> +++ b/usr/local/lib64/pkgconfig/camera.pc       1970-01-01 02:00:00.000000000 +0200
> @@ -1,9 +0,0 @@
> -prefix=/usr/local
> -libdir=${prefix}/lib64
> -includedir=${prefix}/include
> -
> -Name: libcamera
> -Description: Complex Camera Support Library
> -Version: 1.0
> -Libs: -L${libdir} -lcamera
> -Cflags: -I${includedir}/libcamera
> diff -Nur a/usr/local/lib64/pkgconfig/libcamera.pc b/usr/local/lib64/pkgconfig/libcamera.pc
> --- a/usr/local/lib64/pkgconfig/libcamera.pc    1970-01-01 02:00:00.000000000 +0200
> +++ b/usr/local/lib64/pkgconfig/libcamera.pc    2020-12-01 19:04:21.415247132 +0200
> @@ -0,0 +1,9 @@
> +prefix=/usr/local
> +libdir=${prefix}/lib64
> +includedir=${prefix}/include
> +
> +Name: libcamera
> +Description: Complex Camera Support Library
> +Version: 1.0
> +Libs: -L${libdir} -lcamera
> +Cflags: -I${includedir}/libcamera
> 
> Contents are the same, but the .pc file got renamed from camera.pc to
> libcamera.pc. No necessarily an issue, but pkg-config will need to be

I see, I must have missed that, as the builds will have still found the
old version.

Personally, I'd actually rather see the library specified as 'libcamera'
to pkg-config, as that's our name though.



> run with an updated name, requiring an update to simple-cam I believe.
> 
> If we additionally drop the name argument, the file name is preserved,
> but its contents change to
> 
> --- a/usr/local/lib64/pkgconfig/camera.pc	2020-12-01 17:37:07.077026434 +0200
> +++ b/usr/local/lib64/pkgconfig/camera.pc	2020-12-01 19:09:47.299260873 +0200
> @@ -2,7 +2,7 @@
>  libdir=${prefix}/lib64
>  includedir=${prefix}/include
>  
> -Name: libcamera
> +Name: camera

Ok, so we don't want that bit though ;-)

>  Description: Complex Camera Support Library
>  Version: 1.0
>  Libs: -L${libdir} -lcamera
> 
> 
> I'm not a pkg-config expert so I don't know if we should include the lib
> prefix in the file name and/or the Name.

I think that's up to us isn't it ?

However I suspect most libraries do not prefix with 'lib' ... but most
libraries do not have 'lib' in their name. (Except for the fact that
they are libraries).

We're a bit different because we are 'libcamera' as opposed to say
'gtk', which is not known widely as 'libgtk', that's just it's library.

-- 
Regards
--
Kieran