Re: [pacman-dev] [PATCH] Add meson.build files to build with meson
On 10/21/18 at 05:46pm, Dave Reisner wrote:
-- >8 -- (lots of words)
> diff --git a/meson.build b/meson.build
> new file mode 100644
> index ..3f9b2ae0
> --- /dev/null
> +++ b/meson.build
> @@ -0,0 +1,487 @@
-- >8 -- (many more words)
> +PYTHON = find_program('python')
This should look for python3, should it not?
-- >8 -- (I really hope this was mostly copy-paste)
> +libcommon = static_library(
> + 'common',
> + libcommon_sources,
> + install : false)
It's a mistake, but common/ini.c currently includes alpm.h, which
grabs the system alpm.h, or dies if it's not installed, because this
doesn't link_with libalpm. I'll send a patch to fix this particular
error, but I can imagine this sort of subtle error creeping in again.
Should we proactively link_with libalpm to prevent this from
recurring?
-- >8 -- (seriously, this patch is huge)
> diff --git a/test/pacman/meson.build b/test/pacman/meson.build
> new file mode 100644
> index ..dbdb429e
> --- /dev/null
> +++ b/test/pacman/meson.build
> @@ -0,0 +1,357 @@
> +pacman_tests = [
> + { 'name': 'tests/backup001.py' },
Having the test list and expected success/failure duplicated here is
almost certain to lead to meson and autotools getting out of sync.
Can/should we dynamically create this list at least for as long as
we're maintaining both build systems?
Re: [pacman-dev] [PATCH] Add meson.build files to build with meson
On Thu, Nov 01, 2018 at 01:03:27AM -0700, Andrew Gregory wrote:
> On 10/21/18 at 05:46pm, Dave Reisner wrote:
>
> -- >8 -- (lots of words)
>
> > diff --git a/meson.build b/meson.build
> > new file mode 100644
> > index ..3f9b2ae0
> > --- /dev/null
> > +++ b/meson.build
> > @@ -0,0 +1,487 @@
>
> -- >8 -- (many more words)
>
> > +PYTHON = find_program('python')
>
> This should look for python3, should it not?
>
> -- >8 -- (I really hope this was mostly copy-paste)
>
> > +libcommon = static_library(
> > + 'common',
> > + libcommon_sources,
> > + install : false)
>
> It's a mistake, but common/ini.c currently includes alpm.h, which
> grabs the system alpm.h, or dies if it's not installed, because this
> doesn't link_with libalpm. I'll send a patch to fix this particular
> error, but I can imagine this sort of subtle error creeping in again.
> Should we proactively link_with libalpm to prevent this from
> recurring?
>
I get what you're saying about ini.c wrongly including alpm.h, but I'm
not sure I follow about linking with libalpm. Shouldn't the includes be
fixed such that the inclusion of alpm.h comes from lib/libalpm rather
than /usr/include? I'm not clear on what linking with the local libalpm
accomplishes other than being an unnecessary dependency.
> -- >8 -- (seriously, this patch is huge)
>
> > diff --git a/test/pacman/meson.build b/test/pacman/meson.build
> > new file mode 100644
> > index ..dbdb429e
> > --- /dev/null
> > +++ b/test/pacman/meson.build
> > @@ -0,0 +1,357 @@
> > +pacman_tests = [
> > + { 'name': 'tests/backup001.py' },
>
> Having the test list and expected success/failure duplicated here is
> almost certain to lead to meson and autotools getting out of sync.
> Can/should we dynamically create this list at least for as long as
> we're maintaining both build systems?
I'll think about how to do this... I'm not crazy about the idea of
making configuration of the meson build anything more than just 'meson
build', and this all needs to ready for processing at the time meson
generates the inputs for ninja.
Re: [pacman-dev] [PATCH] Add meson.build files to build with meson
On 11/01/18 at 08:51pm, Dave Reisner wrote: > On Thu, Nov 01, 2018 at 01:03:27AM -0700, Andrew Gregory wrote: > > On 10/21/18 at 05:46pm, Dave Reisner wrote: ... > > > +libcommon = static_library( > > > + 'common', > > > + libcommon_sources, > > > + install : false) > > > > It's a mistake, but common/ini.c currently includes alpm.h, which > > grabs the system alpm.h, or dies if it's not installed, because this > > doesn't link_with libalpm. I'll send a patch to fix this particular > > error, but I can imagine this sort of subtle error creeping in again. > > Should we proactively link_with libalpm to prevent this from > > recurring? > > I get what you're saying about ini.c wrongly including alpm.h, but I'm > not sure I follow about linking with libalpm. Shouldn't the includes be > fixed such that the inclusion of alpm.h comes from lib/libalpm rather > than /usr/include? I'm not clear on what linking with the local libalpm > accomplishes other than being an unnecessary dependency. I've still not played with meson enough to fully understand exactly how it works. The use of link_with was just to get meson to use lib/libalpm as an include dir. If there's a better way to do that, great, I just want to make sure that if a common file includes alpm.h in the future, it doesn't sneakily use the system copy.
