Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

treewide: internally avoid deprecated machine file uses of "pkgconfig" #12060

Merged

Conversation

eli-schwartz
Copy link
Member

We support this in a machine file:

[binaries]
pkgconfig = 'pkg-config'
pkg-config = 'pkg-config'

and you can use either one, because internally we look up both. If you only set one of them, this plays awkwardly with setting $PKG_CONFIG, since we don't know which one you set in the machine file and the other one will be initialized from the environment instead.

In commit 22df45a we changed program lookup of config-tool style dependencies to use the regular tool names and only fall back on the strange internal names. This affected the pkg-config class too.

The result is that instead of preferring pkgconfig = followed by $PKG_CONFIG followed by pkg-config =, we inverted the lookup order. This is a good idea anyway, because now it behaves consistently with find_program('pkg-config').

Unfortunately, we documented the wrong name in a bunch of places, and also used the wrong name in various testsuite bits, which meant that if you set $PKG_CONFIG and then ran the testsuite, it would fail.

Correct these references, because they are buggy.

@eli-schwartz
Copy link
Member Author

/cc @mhmdanas

@triallax
Copy link

triallax commented Aug 1, 2023

Thanks for the quick fix, can confirm that with this the tests pass. Cheers!

@xclaesse
Copy link
Member

xclaesse commented Aug 1, 2023

LGTM, but CI seems unhappy.

Should we also actually deprecate pkgconfig? Something along the line of d5f4228

@eli-schwartz
Copy link
Member Author

LGTM, but CI seems unhappy.

The failing test is... interesting.

It happens only during cross builds, and it happens because we test that pkgconfig generation works by inserting a manual requires on 'glib-2.0' as a string.

The test case uses find_program('pkg-config', required: false), which previously found nothing from the cross file, but now finds something because the lookup value in the global cross file for tests is pkgconfig -> pkg-config. This means that:

pkgconfig = find_program('pkg-config', required: false)

env = environment()
env.prepend('PKG_CONFIG_PATH', meson.current_build_dir() / 'meson-private')

test(
  'pkgconfig-validation',
  pkgconfig,
  args: ['--validate', 'simple'],
  env : env,
)

now runs a cross pkg-config test:

PKG_CONFIG_PATH='/__w/meson/meson/b 752d8a5fd7/meson-private' /usr/bin/x86_64-w64-mingw32-pkg-config --validate simple

whereas before, it ran a native pkg-config test:

PKG_CONFIG_PATH='/__w/meson/meson/b 752d8a5fd7/meson-private' /usr/bin/pkg-config --validate simple

Naturally, the cross environment doesn't have a mingw glib-2.0 installed...

We support this in a machine file:

```
[binaries]
pkgconfig = 'pkg-config'
pkg-config = 'pkg-config'
```

and you can use either one, because internally we look up both. If you
only set *one* of them, this plays awkwardly with setting $PKG_CONFIG,
since we don't know which one you set in the machine file and the
*other* one will be initialized from the environment instead.

In commit 22df45a we changed program
lookup of config-tool style dependencies to use the regular tool names
and only fall back on the strange internal names. This affected the
pkg-config class too.

The result is that instead of preferring `pkgconfig =` followed by
$PKG_CONFIG followed by `pkg-config =`, we inverted the lookup order.
This is a good idea anyway, because now it behaves consistently with
`find_program('pkg-config')`.

Unfortunately, we documented the wrong name in a bunch of places, and
also used the wrong name in various testsuite bits, which meant that if
you set $PKG_CONFIG and then ran the testsuite, it would fail.

Correct these references, because they are buggy.

One test case expected to find_program() a native copy for convenience
of testing against natively installed glib. Force it to resolve a native
copy.
@eli-schwartz eli-schwartz merged commit 13f8eba into mesonbuild:master Aug 2, 2023
37 checks passed
@eli-schwartz eli-schwartz deleted the pkgconfig-env+machinefile branch August 2, 2023 11:03
@eli-schwartz eli-schwartz added this to the 1.2.1 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants