interpreter: Add dependency_fallbacks()

[xclaesse]

The main goal of the new dependency_fallbacks() function call is to be
able to express the common pattern: Check pkg-config first with
multiple names, then try with find_library with multiple names, and
finally fallback to a subproject. An extra niche use-case is to try with
has_function() too in the case the needed API is part of libc builtin in
the compiler (e.g. iconv in glib).

[xclaesse]

Since this is a huge refactoring that moves lots of code into a new file, I'm wondering if we should merge that first (excluding the new API) and then discuss on the actual API? IMHO the refactoring is valuable on its own.

[lgtm-com]

This pull request introduces 1 alert when merging 466b026 into b6d277c - view on LGTM.com

new alerts:

• 1 for Unused import

[mensinda]

I would also agree that we should do the code refactoring separately to keep the two topics as much separated as possible. All changes that reduce the size of interpreter.py are good changes.

[xclaesse]

Made the needed refactoring, without adding new API, in #8861.

[xclaesse]

Ok, this is now ready for review, at least getting opinions on the proposed API.

[mensinda]

I haven't looked at the new syntax yet, but I have some technical suggestions.

Also, I am aware that you don't like type annotations, but please use full type annotations and enable dependencyfallbacks.py in run_mypy.py, since this is new code and we will never get full type checking coverage otherwise..

[mensinda]

If you are using typed_pos_args you might as well also use typed_kwargs.

[xclaesse]

Because find_library() in Compiler object has not been ported yet, that should be done in another PR.

[tristan957]

Could you instead take a Union[List[str], str] for the first argument to dependency() where the list of strings is various dependency lookups and the single string is the current behavior?

[xclaesse]

That would make sense, yes. Easier syntax for the common case.

[tristan957]

I think the concept behind the PR makes a lot of sense. Hope you can get back to it eventually.

[xclaesse]

@tristan957 since that use-case was already fully implemented internally, I made a PR with just that. #9330

[dcbaker]

This needs a rebase something awful :)

I'm not entirely convinced by this approach, but one aboslutely glaring problem is see is that you should be passing the language as a string, not the compiler object, because cross compiling

cc = meson.get_compiler('c', native : false)
cc_build = meson.get_compiler('c', native : true)

df = dependency_fallback('a', 'b', 'c)
df.find_library(cc, 'foo')
dependency(df, native : true)

Sure, you could have some logic to check that you pass the right cc, but that might not get noticed in testing if it only affects a host != build configuration and you don't have one of those in your CI (or the right options set). passing the language as a string though lets meson just get the right compiler every time.

Here's my main concern with the approach: meson tries really hard not to have mutable objects, but this is a mutable object. So we should think really hard about whether this is a good idea or not. Your example doesn't give my any case where something like

dependency('foo', 'bar', find_library : ['foo'], has_function : 'bar', had_header : 'foo.h', language : 'c')

wouldn't already do exactly what we want without introducing more types and more biolerplate, as well as a mutable object. or, alternatively could we construct another immutable object to use instead?

lf = library_fallback('foo', 'bar', has_function : 'bar', has_header : 'foo.h', prefix : '#define _GNU_SOURCE', langauge : c')
dependency('foo', 'bar', find_library : lf, fallback : 'foo')

(or cc.library_fallback, though again, you have the target machine mismatch issue). The other thing about this appoarch, is it keeps the existing fallback keyword argument, and provides a similar approach with a find_library keyword

[jpakkane]

this is a mutable object. So we should think really hard about whether this is a good idea or not.

I don't like those either. At the very least it should be made more explicit, perhaps by calling it a fallback_builder or something and then doing the usual thing where you need to call fb.get_fallback() to get the actual object (and make the builder so that calling anything else on it is a hard failure).

Alternatively we could try something a bit more immutable like:

arr = [dep_lookup(type:'library', has_headers: 'foo.h'),
   dep_lookup(type: 'normal', name: 'foobar'),
   ...
]

final_dep = multistage_dep_lookup(arr)

The syntax is a bit wonky but you get the idea.

[tristan957]

dependency('foo', 'bar', find_library : ['foo'], has_function : 'bar', had_header : 'foo.h', language : 'c')

Seems not bad

[xclaesse]

Sure, you could have some logic to check that you pass the right cc, but that might not get noticed in testing if it only affects a host != build configuration and you don't have one of those in your CI (or the right options set). passing the language as a string though lets meson just get the right compiler every time.

Very good point, I like that.

dependency('foo', 'bar', find_library : ['foo'], has_function : 'bar', had_header : 'foo.h', language : 'c')

Finding the right API is the difficult part indeed, we had many other proposals in the past.

Your proposal has a few issues too:

• it clusters kwargs on dependency function that already has many.
• we would have to hardcore an order in which we do the lookup. Probably not too bad?
• when you pass arrays to find_library, does it mean AND or OR ?
• what if different lib would need to check for different header?
• does it mean you need the lib with that func, or func and fallback to lib?
• what if you need to pass include_directories, or any other kwargs of find_library or has_function?

One thing I like about my proposal is each method has the same API as the corresponding function. But I agree mutable is not great. Maybe we should introduce real builder syntax:

df = dependency_fallbacks().find_library().has_function()
# df is now immutable

Another idea I vaguely had is using a module:

df = import('dependency-fallbacks')
dependency('foo', df.find_library(), df.has_function(), ...)

[eli-schwartz]

IMHO we need to support the case where a dependency can be found by two or more mutually exclusive find_library fallbacks, each with a different library name and header include. There are actual real world cases where that is needed.

[dcbaker]

it clusters kwargs on dependency function that already has many.

Indeed, that's one of the reasons I suggested that having a new, immutable, object like cc.library_fallback() or dependency_fallback() instead of putting all of the arguments in dependency

when you pass arrays to find_library, does it mean AND or OR ?

I would expect that dependency(a, b, c) and dependency(..., find_library : ['a', 'b', 'c']) would have the same behavior, try a, if that fails try b, etc.

what if different lib would need to check for different header?

There must be at some point that the dependeices you're checking are so non related that just doing the required : false dance becomes the right solution. synactic sugare should make the common case easy, not try to make every case easy. I think solving the "library X has many different names, and we want to try them all" is the common case we should be solving for, ie, is is "libz", "libz-1", "z", "z-1", "libz-1.0", "libz-in-rust-1.0", etc.

The only other case that I can think of, is that of "there are seven different libraries that provide a function that does X, and I can use any of them. Which is probably best solved with a for loop anyway:

for args : [['ssl', {has_function : 'ssl_sha1', has_header : 'ssl.h'}],
                ['openssl', {has_header : 'openssl.h']}],
                ['nettle', {'has_function : 'nettle_sha1', has_header : 'nettle.h'}]]
    lib = cc.find_library(args[0], kwargs : args[1], required : false)
    if lib.found()
        conf.set('HAVE_@0@'.format(d.name().upper()))
        break
    endif
endfor

really isn't that big of a difference against

df = dependency_fallbacks()
df.find_library('ssl', has_function : 'ssl_sha1')
df.find_library('openssl', ...)
df.find_library('nettle', ...')
d = dependency(df)
conf.set('HAVE_@0@'.format(args[0].upper()))

And, it works with any version of meson already

df = dependency_fallbacks().find_library().has_function()

How does meson know that you're done with it though? that's introducing significant to assignment, which is something we don't do generally (except with values that copy-on-assign instead of reference-on-assign). From meson's perspective

def = dependency_fallbacks()
def = def.find_library()

is equivalent to

def = dependency_fallbacks().find_library()

[xclaesse]

How does meson know that you're done with it though? that's introducing significant to assignment, which is something we don't do generally (except with values that copy-on-assign instead of reference-on-assign)

It's easy for meson to set a immutable flag in assign. That would be new concept, but simple to implement. But to be fair I'm not seriously proposing doing it, I prefer mutable than that.

There must be at some point that the dependeices you're checking are so non related that just doing the required : false dance becomes the right solution. synactic sugare should make the common case easy, not try to make every case easy.

That's true but it's still worth investigating if we can find a solution that solves as many cases as possible, and not stop thinking too soon. I think the most important is having something flexible that we can extend. Trying to fit everything into a single function kwargs is going to be hard to extend. That's why I like having a mutable object, or even a whole module, we can always add more methods.

Now that I think again about this proposal, I think I like the most the module API.