Check for, and prefer, pkgconf when looking for pkg-config #8551
Conversation
|
@dcbaker At least on Ubuntu pkgconf package conflicts with pkg-config, so you can install only one of them. And if you install pkgconf you get a symlink |
|
Nix at least allows you to install both simultaneously and doesn't create a symlink, I'm sure Gentoo does the same. |
dcbaker
force-pushed
the
submit/use-pkgconf
branch
3 times, most recently
from
4d330aa
to
07b2dca
Compare
|
That test failure is suspicious as it only happens on this MR. |
dcbaker
force-pushed
the
submit/use-pkgconf
branch
from
07b2dca
to
d39e74d
Compare
|
This pull request introduces 2 alerts when merging d39e74d into 5f3c712 - view on LGTM.com new alerts:
|
Codecov Report
@@ Coverage Diff @@
## master #8551 +/- ##
==========================================
- Coverage 68.82% 67.33% -1.49%
==========================================
Files 406 203 -203
Lines 88020 44019 -44001
Branches 19550 9775 -9775
==========================================
- Hits 60580 29641 -30939
+ Misses 22868 12125 -10743
+ Partials 4572 2253 -2319
Continue to review full report at Codecov.
|
Rather than a big complicated set of monkey patches (that don't use mock) a single mock patch that simply sets `ExternalProgram.found()` to always return False will suffice.
dcbaker
force-pushed
the
submit/use-pkgconf
branch
from
d39e74d
to
0880f3b
Compare
|
This pull request introduces 1 alert when merging 0880f3b into 5f3c712 - view on LGTM.com new alerts:
|
| if shutil.which('pkg-config'): | ||
| if not shutil.which('sdl2-config'): | ||
| self.assertMesonRaises("dependency('sdl2', method : 'sdlconfig')", self.dnf) | ||
| if not shutil.which('pkg-config'): |
There was a problem hiding this comment.
So... Apparently this was always broken? This deserves a call-out in the commit message.
There was a problem hiding this comment.
Does the revised commit message look better?
There was a problem hiding this comment.
Ah, I was thinking about the fact that this test only passes if sdl2-config skips... because if it gets to the pkg-config check, it only runs when pkg-config is found, and then it asserts that the dependency must not be found, which seems... confusing.
The test is currently all but useless because it skips completely if `sdl-config` is found in $PATH. There are other pieces of the test, however, than can still be run even if `sdl-config` is found, and only that portion should be skipped.
This also means that we'll detect pkgconf by default, which we currently don't.
dcbaker
force-pushed
the
submit/use-pkgconf
branch
from
0880f3b
to
4b78bff
Compare
|
This pull request introduces 1 alert when merging 4b78bff into ec388fe - view on LGTM.com new alerts:
|
Some distros (debian and nix) provide pkg-config and pkgconf, but don't link pkgconf to pkg-config. Others do link pkg-config to pkgconf (arch). pkgconf is just better than pkg-config is pretty much every way, so we should prefer pkgconf if it's installed. Right now we don't even search for pkgconf.
This series fixes that, and also centralizes the names to look for in a single place.