Bug 109062

In meson we iterate through all of the libdrm versions required for each enabled target, then select the hightest version and check only for that version of libdrm and libdrm_*. We do this specifically to avoid picking up two different versions of libdrm. A little above this in the meson output there is a line similar to:

message: libdrm 2.4.95 needed because amdgpu has the highest requirement

I implemented this because Ilia had pointed out cases where pkg-config could load the libdrm from his system directory, but the libdrm_nouveau from a custom install directory because the version differences. Eric and I spent a lot of time trying to figure out the best way to handle this, and the message right before the version check was the best we could come up with. Ideas to make this clearer are welcome.

How about something like the following pseudo code.
AFAICT it should produce the correct output.

req_libdrm=...
req_libdrm_foo=...
req_libdrm_bar=...

foreach driver : gallium-drivers // or drivers in general
   // if meson cannot handle two deps/packages at the same time,
   // invoke dependency() twice and then fold into one variable
   dep_libdrm_$driver = dependency(libdrm >= req_libdrm_$driver,
                                   libdrm_$driver >= req_libdrm_$driver)


// other generic parts/checks for libdrm
dep_libdrm = dependency(libdrm >= req_libdrm)

(In reply to Emil Velikov from comment #2)
> How about something like the following pseudo code.
[...]
>    dep_libdrm_$driver = dependency(libdrm >= req_libdrm_$driver,
>                                    libdrm_$driver >= req_libdrm_$driver)

Yeah, I don't see any reason this wouldn't work, and it would give a clearer error on failure; I'll give it a shot after lunch.

That's going to increase the number of calls to pkg-config considerably. If we take this approach please check the changes in the time it takes to run an initial meson; I have a feeling it could be bad.

The other option I guess would be to just pass all of the versions at once, meson does support `version : ['>= 1.0', '>= 1.3']`

On my 3+ y/o laptop shows virtually no difference in runtime between the following two commands.

pkg-config --libs "libdrm >= 2.4.20" "libdrm_nouveau >= 2.4.20"
pkg-config --libs "libdrm_nouveau >= 2.4.20"

So in all fairness, I'm not sure if the micro optimisations are worth the effort or code complexity.

that's not the right comparison though. Meson doesn't support querying two dependencies at the same time, that's something that only pkg-config can do (you can't do that with windows find-a-dll, mac's frameworks, cmake searching, or the wrappers for the various *-config tools. So what you're actually doing is:

pkg-config --libs "libdrm_nouveau >= 2.4.20"

vs

pkg-config --libs "libdrm >= 2.4.20"
pkg-config --libs "libdrm_nouveau >= 2.4.20"

that will take more time, I think it's worth while to figure out how much time. I fell like spending less time configuring is more important than getting a slightly better error message.

but if the time difference is negligible then thats a fair trade for a better error message :)

time for i in `seq 1 100`; do
  pkg-config --libs "libdrm_nouveau >= 2.4.20" &>/dev/null;
  pkg-config --libs "libdrm >= 2.4.20" &>/dev/null;
done

real    0m0.738s
user    0m0.476s
sys     0m0.270s


time for i in `seq 1 100`; do
  pkg-config --libs "libdrm >= 2.4.20" "libdrm_nouveau >= 2.4.20" &>/dev/null; done

real    0m0.418s
user    0m0.298s
sys     0m0.123s


That said, I think meson should be extended and check 2+ packages via pkg-config.
Adding workarounds in every project that uses meson doesn't scale :-(

tl;dr: that only works if you can *always know* that you wont need a fallback and that the only way to discover all of the dependencies is via pkg-config and not via cmake or a *-config tool.

imagine you have something like this:

dep1 = dependency('foo', version : ['>= 1.0', '< 2.0'], fallback : ['foo', 'dep_foo'])
dep2 = dependency('bar', version : '> 2.1', fallback : ['bar', 'dep_bar'])
deps = [dep1, dep2]

Which says, find foo and bar, and if you can't find them (and fallbacks aren't disabled) build subproject foo and get the declare_dependency object dep_foo and return that, and do the same for bar.

Now, we could probably write this as:
deps = dependency(['foo', 'bar'], version : [['>= 1.0', '< 2.0'], '> 2.1'],  fallback : [['foo', 'dep_foo'], ['bar', 'dep_bar']])

but that's a pretty confusing syntax, and if pkg-config fails then we still have to either re-run it once for each dependency and put all of that together, or parse the human readable output and try to figure out which dependencies were missing, and rerun with on the the ones that were found and then fallback to the rest.

The imagine that bar only provides a BarConfig.CMake file, or that bar provides both cmake and pkg-config, or that it only provides a bar-config script.

Relevant to this change proposal though, lets say we have:

/usr/lib/libdrm*.2.4.75; /usr/local/lib/libdrm*.2.4.95
and
PKG_CONFIG_PATH=/usr/lib/pkg-config:/usr/local/lib/pkg-config

if our dependencies are:
libdrm_intel = 2.4.75
libdrm_radeonsi = 2.4.95

And we configure i915 and radeonsi, under your proposal wouldn't we end up with both -L/usr/lib -ldrm -ldrm_intel and -L/usr/local/lib -ldrm -ldrm_amdgpu in the link args of any target that requires both libdrm_intel and libdrm_amdgpu since pkg-config will come along and say, "libdrm in /usr/lib is good enough for intel", but then get to radeonsi and go, "nope we need the one in /usr/local/lib for amdgpu"?

Right, let's keep multiple [pkg-config] dependencies out of the topic for now.
As you pointed out, there are some glitches that need sorting out.

Fwiw my suggestion is to follow the autotools build, which was explicitly requested by Ilia, Dave and Chad - commit 2b4eaabff01a3a8ea0c4742ac481492092c1ab4f.

Above ensures that both core and $vendor libs meet the $vendor version requirement.

AFAICT meson seems to pick the highest version and enforces it everywhere - core and all used vendors. So for example:
A local nouveau v10, and system amdgpu v5, will meet the requirements, yet meson will error out expecting amdgpu v10 :-(

That said, as long the guys are happy, I'm fine either way.

(In reply to Emil Velikov from comment #12)
> Fwiw my suggestion is to follow the autotools build, which was explicitly
> requested by Ilia, Dave and Chad - commit
> 2b4eaabff01a3a8ea0c4742ac481492092c1ab4f.

FWIW, not to dump on Dylan too much, I suspect that the current meson behavior may be my doing too -- we had this discussion earlier on, and the thing that meson currently does was the compromise --

It's unreasonable for a system to have different versions of libdrm and libdrm_foo -- they should all be the same.

Similarly, it is unreasonable to require me to have version X (> Y), when all I'm doing is building a driver which needs version Y.

So looking at your original issue, you're trying to configure with intel + amdgpu, and your version isn't enough for amdgpu.

Solution: either update, or don't build with amdgpu.

I suppose the problem is that all you know is "update" -- you don't know that by simply removing amdgpu, all can be well (and perhaps you don't want amdgpu in the first place). Not sure what to do about that.

(Did I at least understand the problem correctly?)

Ilia's explanation is my understanding of the problem, and how we got here. We do print a message saying that "version x.y.z was required because $driver requires it" maybe we can make the message better? Such as:

```
libdrm requirements:
  intel : >= 2.4.75
  amdgpu  : >= 2.4.95

globally requiring libdrm 2.4.95 because intel and amd drivers enabled.
```

It started with a confusing error message and upon closer look behaviour seemed fairly different from what you guys wanted.

Since the current solution is OK you guys, please ignore me.

Thanks for the input Ilia. Closing as NOTABUG

(In reply to Emil Velikov from comment #15)
> It started with a confusing error message and upon closer look behaviour
> seemed fairly different from what you guys wanted.
> 
> Since the current solution is OK you guys, please ignore me.
> 
> Thanks for the input Ilia. Closing as NOTABUG

The other thing to note is that the only realistic way for this scenario to occur is to have libdrm v1 in one place and libdrm v2 in another place. If you allow that to go forth, you would pick libdrm_a from v1, libdrm_b from v2, and libdrm from which version? I don't think there's a right answer to that question generically, so IMHO this scenario should just be avoided.

The fix in autotools you referenced was to ensure that e.g. intel wanting a thing in libdrm core would actually get picked up by the intel_min_version thing rather than having to bump the super-global libdrm requirement (as that would affect all drivers).