On Sun, Dec 10, 2017 at 4:16 AM, Arun Raghavan <arun at arunraghavan.net> wrote:
> On Sat, 9 Dec 2017, at 10:58 PM, Felipe Sateler wrote:
>> On Sat, Dec 9, 2017 at 2:59 AM, Arun Raghavan <arun at arunraghavan.net>
>> wrote:
>> > +
>> > +apiversion = '1.0'
>> > +soversion = 0
>> > +# maintaining compatibility with the previous libtool versioning
>> > +# current = minor * 100 + micro
>> > +libversion = '@0 at .@1 at .0'.format(soversion, pa_version_minor.to_int() * 100 + pa_version_micro.to_int())
>>
>> This gives 0.9901.0 instead of 0.20.2
>
> Yeah, we need to fix it up to do the right thing. Let's fix this in a
> subsequent patch.
Well, then the comment should be prefixed with FIXME or removed, since
the code does not do what the comment says :)
>
> My intention is to not have this be complete, but to have it land in
> some usable state and incrementally make it better. I expect we'll be
> supporting autofoo + meson for a while until there is parity between the
> two (across platforms).
I agree that perfect is enemy of the good. Merging the meson support
as incomplete is fine.
I'm struggling to find a way to phrase this, but I think outputting a
different artifact (as opposed to a less featureful, or missing
artifact) is a bug, and not a missing feature.
>> > +
>> > +if cc.has_function('SYS_memfd_create', prefix : '#include <sys/syscall.h>')
>> > + cdata.set('HAVE_MEMFD', 1)
>> > +endif
>>
>> configure has the option to enable or disable this. This meson script
>> autodetects only. I think the ability to explicitly disable/enable
>> (and error out if dependencies are not found) is nice to keep when
>> moving to meson. That is, this should become something like:
>>
>> want_memfd = get_option('memfd')
>> if want_memfd != 'false'
>> has_memfd = cc.has_function('SYS_memfd_create', prefix : '#include
>> <sys/syscall.h>')
>> if has_memfd
>> cdata.set('HAVE_MEMFD', 1)
>> elif want_memfd == 'true'
>> error('Memfd was requested but it was not found')
>> endif
>> endif
>>
>> plus a meson_options.txt:
>>
>> option('memfd', type: 'combo', choices: ['auto', 'true', 'false'],
>> description: 'Enable Linux memfd shared memory')
>>
>> This applies to a lot of the checks.
>
> This may be a hard, but in the long run, I actually would like to remove
> automagic dependencies. This makes builds more consistent, and removes a
> lot of (imo unnecessary) if/else-ery.
I'm not at all opposed to this. But then the basic pattern should be
in place. This way, when new features are ported from autotools to
meson, the pattern can be copied.
>
> So we would have want_memfd on by default (maybe conditional on OS ==
> Linux), and then you could disable it at configure time if you don't
> want it.
Unfortunately I don't think meson allows OS-conditional options so it
will have to be implemented as if-else too (although it is likely it
is less than autodetection).
>> > +pax11publish_sources = [
>> > + 'pax11publish.c',
>> > +]
>> > +
>> > +# FIXME: man pages
>> > +executable('pax11publish',
>> > + pax11publish_sources,
>> > + install: true,
>> > + include_directories : [configinc, topinc],
>> > + link_with : [libpulsecommon, libpulse],
>> > + dependencies : [x11_dep],
>> > + c_args : pa_c_args,
>> > +)
>>
>> Shouldn't this be conditional on x11_dep being found?
>
> Yes, that makes sense. I wonder if meson automatically won't build a
> target whose dep is not found.
I don't think so. Otherwise conditionally enabling features would be
very annoying:
exe_deps = [required_dep]
if optional_dep.found()
exe_deps += [option_dep]
endif
# etc
--
Saludos,
Felipe Sateler
