Correctly check for pkg-config

[sardemff7]

It is a tool, and needs the correct prefix for cross-compiling.
Also, if $PKG_CONFIG is set, either by the user or AC_PATH_TOOL, we can
consider it will be executable.
A bare test -x is not enough for package managers which set
$PKG_CONFIG to <prefix>-pkg-config (not absolute).

[krakjoe]

@remicollet can I ask that you review this please ?

[orlitzky]

You switched from -x to -n to catch the case where $PKG_CONFIG is not an absolute path, but isn't the AC_PATH_TOOL macro guaranteed to give you an absolute path?

[orlitzky]

Nevermind, I see it now: the AC_PATH_TOOL macro is only used when $PKG_CONFIG is empty. I guess you can define $PKG_CONFIG yourself to force a particular implementation? In that case, I would probably either resolve a relative path to an absolute one, or simply state that $PKG_CONFIG is an absolute path and let the customer figure it out.

[sardemff7]

$PKG_CONFIG can be used when cross-compiling with an already existing tool that exports it.
There is no value to checking if $PKG_CONFIG can run, since AC_PATH_TOOL guarantees that, and only specific tools and package managers set the variable directly. If the user sets it to some silly value, the call right after the && will catch the error.
Requiring the -x test means modifying existing tools to set the absolute path or not to export the variable, while only these tools (made specifically to cross-compile, and thus knowing what they do to the variable) are likely to use this variable.

What kind of failures are you trying to catch here? This check just seems annoying, since every other good user of Autotools just use the PKG_PROG_PKG_CONFIG macro which allows that behaviour.

[orlitzky]

You're right that the actual invocation of pkgconfig will fail if it's not executable, I just think it's a slightly better design to decide up front what the $PKG_CONFIG variable should contain. If we say it contains an absolute path, then either the user has to give us one, or we have to convert relative to absolute early on. But from that point on, the script can rely on the fact that $PKG_CONFIG is an absolute path.

If, on the other hand, we let $PKG_CONFIG be either relative or absolute, then every place in the script that it's used needs to be aware of that fact and handle both cases. I think this approach is more likely to cause bugs in the future, if (for example) somebody adds another line like the -x test that we have now -- which somebody might do completely independently five years from now, not realizing that the path can be relative.

I just happened across this issue by accident and decided to look over the change, so my suggestion isn't the law =)

I do however agree with the goal.

[sardemff7]

I see. However, $PKG_CONFIG is a standard variable used across countless projects, just based on the upstream PKG_PROG_PKG_CONFIG macro (used by PKG_CHECK_MODULES and PKG_CHECK_EXISTS internally at least).
Let’s see what other peoples think about it.

[KalleZ]

@remicollet with the recent changes to use pkg-config in curl I think it was, I suppose this could be merged as well?

[cmb69]

@eli-schwartz Is this PR obsolete due to PR #3632 and #3654?

[eli-schwartz]

Yes.

• calls to AC_PATH_PROG are obsolete since PKG_PROG_PKG_CONFIG is run during early configure

• $PKG_CONFIG --exists can regardless be replaced universally with PKG_CHECK_EXISTS

• these checks should be (and in some cases have been) recreated with PKG_CHECK_MODULES, which combines PKG_CHECK_EXISTS and the actual definition of various *_CFLAGS variables