Re: [PATCH v1 1/1] Add support for meson building

From: Ariel D'Alessandro <ariel@vanguardiasur.com.ar>
To: Gregor Jasny <gjasny@googlemail.com>,
	Xavier Claessens <xavier.claessens@collabora.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Ezequiel Garcia <ezequiel@collabora.com>
Cc: kieran.bingham@ideasonboard.com, linux-media@vger.kernel.org,
	hverkuil@xs4all.nl, sean@mess.org, p.zabel@pengutronix.de,
	nicolas@ndufresne.ca,
	nicolas Dufresne <nicolas.dufresne@collabora.com>
Subject: Re: [PATCH v1 1/1] Add support for meson building
Date: Thu, 25 Jun 2020 15:32:04 -0300	[thread overview]
Message-ID: <c8d8a2fe-6abb-796d-803c-672106bc9587@vanguardiasur.com.ar> (raw)
In-Reply-To: <4d8eecf2-3ce5-927b-bb3d-d950d123f0e0@googlemail.com>

Hey Gregor,

On 6/24/20 4:46 PM, Gregor Jasny wrote:
> Hello,
> 
> thanks for your helpful suggestions. I made a PR from those: Feel free to squash
> it into yours:
> https://gitlab.com/adalessandro/v4l-utils/-/merge_requests/1
> 
> When looking at your quoted code with the fixes I'm wondering if you forgot to
> push your branch?

Merged and pushed, thanks.

> 
> On 6/23/20 7:26 PM, Ariel D'Alessandro wrote:
>> On 6/22/20 4:09 PM, Gregor Jasny wrote:
[snip]
> I have some more comments:
> * The project version is still at 1.19.0 (should be 1.21.0 for the merge)

Got it.

> 
> * As long as we support autotools and use 'make dist' to create the tarball we'd
> have to add the meson.build and meson_options.txt to the EXTRA_DIST of the
> respective Makefile.am.

Makes sense, will do that.

BTW, did you try 'meson dist'? Have any comments on that?

> 
> * for ir-keytable the following variables are set and forwarded to the source
> code as a preprocessor define:
> 
> ir_keytable_system_dir = udevdir
> ir_keytable_user_dir = get_option('sysconfdir') / 'rc_keymaps'
> 
> It would be nice if we could re-use those as the install-dir base here:
> udevdir / 'rc_keymaps'

Sounds good, let's re-use those variables as much as possible.

> 
> (But that's nice to have)
> 
> * Right now on install we create an empty directory at the ir_keytable_user_dir.
> Are you aware of any mechanism to create this with meson? I found
> https://github.com/mesonbuild/meson/issues/2904 and it makes me think that there
> is no easy way to do so. I could imaging to work-around by putting a readme file
> inside of that directory to have it created. That would also explain its reason
> for existence.

Hmm, nope. I don't think there's a way for creating an empty directory with
meson right now. We can achieve what you say using install_subdir() and exclude
the README file:

    install_subdir('user_dir_protocols',
                   exclude_files : 'README.md',
                   strip_directory : true,
                   install_dir : ir_keytable_user_dir / 'protocols')

See
https://gitlab.com/adalessandro/v4l-utils/-/commit/144545f0a2901e825de44a57704ed43e08f667fc

> 
> I'm still very fascinated of the very high build speed improvement. For me it's
> down to 8s (with cold cache). Very impressive. Thank you, again!

Great! Yep, meson rocks.

Ariel