CMake find_package dependency backend

[mensinda]

Added a CMake dependency method based on the CMake --find-package
option a CMakeLists.txt and the CMake trace output (--trace --trace-expand).

To extract the necessary information from the CMake trace a few CMake
functions were implemented to detect all variables and targets:

• set()
• unset()
• string() -- only partially
• add_executable() -- only for IMPORTED targets
• add_library() -- only for IMPORTED targets
• add_custom_target() -- only looks at the target name
• set_property()
• set_target_properties()

With these functions both the old-style <NAME>_LIBRARIES
variables, as well as imported targets, and version detection are supported.

Also, the CMake method will be used if the for the auto method when pkg-config fails.

Example:

# Implicitly uses CMake as a fallback and guesses a target
dep1 = dependency('KF5TextEditor')

# Manually specify one or more CMake targets to use
dep2 = dependency('ZLIB', method : 'cmake', modules : ['ZLIB::ZLIB'])

CMake is automatically used after pkg-config fails when
no method was provided in the dependency options.

The minimum CMake version is 3.4 because the variables are not expanded in older
versions in the trace output.

Extracted CMake trace information

Variables

variable	description
PACKAGE_FOUND	to detect if found
PACKAGE_VERSION	to detect the version of the dependency
<NAME>_VERSION	(see above)
<NAME>_VERSION_STRING	(see above)
PACKAGE_INCLUDE_DIRS	for old-style compile information
PACKAGE_LIBRARIES	for old-style linker information

Target properties

property	description
INTERFACE_INCLUDE_DIRECTORIES	include directories
INTERFACE_COMPILE_DEFINITIONS	setting defines (-D)
INTERFACE_COMPILE_OPTIONS	other misc compile options
IMPORTED_CONFIGURATIONS	only used internaly to detect other properties
INTERFACE_LINK_LIBRARIES	list of other target dependencies
IMPORTED_LINK_DEPENDENT_LIBRARIES	same as above
IMPORTED_LOCATION	path to the library

[ignatenkobrain]

[Flake8]

[E231] missing whitespace after ','

[mensinda]

Oh, I missed that. Should be fixed now.

[ignatenkobrain]

[Flake8]

[E203] whitespace before ','

[jpakkane]

The last time this was discussed someone found out that CMake's facility for giving out dependency information for third parties was deprecated. Because of that we chose not to support it. Is this still the case or is the mechanism this PR is using guaranteed to remain supported for the foreseeable future?

[mensinda]

Yes, it is deprecated but this is not a problem since the changes do not rely on one line pkg-config like output. Instead, the traced output CMake generates is parsed. Also, the --find-package essentially only executes the builtin CMake module CMakeFindPackageMode.cmake and prints the content of a few variables to stdout. See the output of the following command:

cmake --find-package --trace --trace-expand -DCOMPILER_ID=GNU -DLANGUAGE=CXX -DMODE=LINK -DNAME=ZLIB

Should the --find-package option be removed at some point, it can be replaced with other mechanisms of calling the find_package() CMake function. (At worst with a minimal CMakeLists.txt)

The only real problem would be the removal of the --trace and the --trace-expand options, but both of them are not deprecated (according to the docs)

[mensinda]

Should --find-package be removed in the future, a CMakeLists.txt with only the line find_package(${PKG_TO_FIND}) would be sufficient as a replacement, since the command cmake -DPKG_TO_FIND=ZLIB . also produces the required trace information.

This option would, of course, be a lot slower than the --find-package option since CMake will basically generate a full build environment, but the only way for CMake to kill this approach would be to remove --trace options.

[jeandet]

@jpakkane @mensinda, here is the discussion. Then I tried to look if I could generate a custom CMakeLists printing out find_package() results, I didn't find an easy solution and didn't like the fact that this would depend on CMake internal stuff.

Looking your PR, it seems it support the version arg which is nice.

[mensinda]

I did some further testing with the CMakeLists.txt approach here:

It is definitively possible to use the standard cmake .. approach to generate the required trace data. It is also possible to skip the timeconsuming compiler test CMake does with some tricks.

The dependency detection process would then be:

1. Setup a temporary "build" directory with a CMakeLists.txt
2. Create some files to skip the CMake compiler detection (which would take 1-2 seconds)
3. Run cmake --trace-expand -DNAME=<PKG2FIND> ..
4. Parse and interpret the trace output
5. Remove the temporary build directory

I have tested this on Linux with CMake 3.12.3 and 3.4.0. It technically also works for CMake versions below 3.4.0, but the variables are not expanded in the trace output (even with --trace-expand). This should also work on other platforms (Mac, Windows) but I can not test it there.

The CMakeLists.txt approach has the advantage that we would be in full control of the find_package() script. However, meson would have to generate a temporary build directory, copy files around and clean everything up.

@jeandet yes, extracting the version from the CMake scripts is supported. It is generally possible to extract the content of (almost) any variable and any target property.

[mensinda]

I managed to test the CMakeLists.txt approach on Windows and it also works there.
A full description of the files needed to trick CMake not to run its compiler tests can be found here.

Some benchmarks with and without tricking CMake:

*	normal	skipped compiler test
time	1355ms	46ms
trace size	9039 lines	897 lines
parser time	154ms	12ms

Should I keep the --find-package approach or change it to a full CMakeLists.txt "project"? If we use a "project" then there are a few open questions:

1. Where is the "project" located (inside mesons build dir, tempfile.gettempdir(), etc.)
2. Should the Fake CMake compiler setup be used or is that too much of a hack
3. Do we reuse the "project" directories (may cause problems since CMake caches variables)
4. Are the "project" directories removed after CMake ran or do we keep them

Also, are there other variables and/or target properties that should be extracted?

[jon-turney]

These lines are alphabetically sorted. Please add cmake in the appropriate place.

[mensinda]

should be fixed with the last commit

[mensinda]

Any opinions regarding the points from above?

[mensinda]

I have replaced the --find-package approach with a "full" CMake project and fake Compiler information.
As a result, this pull request no longer relies on deprecated CMake functions.

One CMake "build directory" is created for each dependency. All build directories have a cmake_ prefix and are located in the meson-private directory. They are currently not removed after CMake ran.

[jpakkane]

What is the purpose of installing the CMake file?

[mensinda]

The CMake file is used as a "CMake project" to determine the dependency. This file is copied into each CMake "build" directory (in meson-private) so that the CMake find_package() function can be executed and meson can gather the dependency information from the CMake output. This "CMake project" structure basically replaces the deprecated --find-package option.

[jpakkane]

LGTM. Is this ready to merge or are there still open questions?

[mensinda]

Yes, it is ready to merge and there are no open questions on my part.

[jon-turney]

The clause 'with pkg-config if possible, as a framework (OSX only), and with library-specific fallback detection logic otherwise' in the description of dependency() in Reference-manual.md needs updating. Perhaps Dependencies.md needs a section to discuss the default search order, instead.

[mensinda]

Updated the documentation. Are there any other issues?

[jpakkane]

This seems to ⚔️ conflict now...

[mensinda]

Fixed the conflicts by rebasing.

[mensinda]

All CI checks are passing again. Should be ready to merge.

[jon-turney]

Try different CMake generators since specifying no generator may fail in cygwin for some reason

I think the failure here was because make is not installed in the cygwin environment used in CI.

[mensinda]

Right, but having the dependency detection fail just because CMake is unable to automatically use a different generator is still bad, so I think the change should stay. But I agree that this commit fixes a very niche issue.

[jpakkane]

Thanks.

[jon-turney]

This seems to have a path where an unbound variable exception occurs, see:

https://travis-ci.org/jon-turney/meson-corpus-test/jobs/458904519#L643
https://travis-ci.org/jon-turney/meson-corpus-test/jobs/458904548#L523

[jon-turney]

Right, but having the dependency detection fail just because CMake is unable to automatically use a different generator is still bad, so I think the change should stay

I agree. My point was merely that the comment alludes to mysterious "reasons" without details.

[mensinda]

Oh. I missed that. Should be fixed with #4549

[jon-turney]

After fixing the above, there is now a different failure https://travis-ci.org/jon-turney/meson-corpus-test/jobs/460388821#L552

The problematic line is:

  udevdir = dependency('udev', required : false).get_pkgconfig_variable('udevdir')

I'm guessing the type of the returned object has changed, from a not-found pkgconfig dependency to a not-found cmake dependency.

per http://mesonbuild.com/Reference-manual.html#dependency-object, it's an error to call get_pkgconfig_variable() on a non-pkgconfig dependency, although this is unhelpful for writing reliable meson.build files (and see #2324 (comment) et seq.)

[mensinda]

What would be the desired behavior in this case? The main problem here is that with method: 'auto' multiple dependency backends can be tried. I also don't know if returning an empty string or just silently ignoring get_pkgconfig_variable would be a good idea.

Maybe enforcing method: 'pkg-config' for cases like this would be a good idea. This way the dependency object is guaranteed to be of the right type.

[jpakkane]

If CMake does not provide a way to get equivalent data, then there's nothing else you can realistically do.

[mensinda]

I do have access to every variable CMake has ever set, but this would be completely different from get_pkgconfig_variable and I don't know if this would be worth implementing.

My idea would be to restrict the use of get_pkgconfig_variable, etc. to dependency objects where the method was explicitly specified. This will certainly break a lot of code now, but would prevent stuff like this happening in the future.

[jpakkane]

Waitaminute:

udevdir = dependency('udev', required : false).get_pkgconfig_variable('udevdir')

How has this passed earlier? It should fail because you are trying to get a variable of a dependency that does not exist. The documentation says:

get the pkg-config variable specified, or, if invoked on a non pkg-config dependency, error out.

That should be an error because a not-found dependency is not really a pkg-config dependency. I checked the code and in base.py it will return an empty string if the dependency has not been found. It's hard to tell if that is intentional or not. Git blame says it was last edited by @jon-turney. Any comments?

I guess the obvious follow up question is whether we can change this or do we need to preserve the current behaviour to keep backwards compatibility.

[jon-turney]

Waitaminute:

udevdir = dependency('udev', required : false).get_pkgconfig_variable('udevdir')

How has this passed earlier? It should fail because you are trying to get a variable of a dependency that does not exist.

I think that's what I wrote...

That should be an error because a not-found dependency is not really a pkg-config dependency. I checked the code and in base.py it will return an empty string if the dependency has not been found. It's hard to tell if that is intentional or not. Git blame says it was last edited by @jon-turney. Any comments?

Without a line number or commit I have no idea what you are blaming me for.

Regardless... it doesn't matter what NotFoundDependency objects do with get_pkg_config_variable() calls: find_external_dependency() returns the last failing dependency object (if required: is false, otherwise it's going to raise an error). I'm pretty sure I didn't change that, although it's now in one place, rather than every subclass. (In this case, that used to be a not-found pkg-config dependency, but is now a not-found cmake dependency).

Maybe it should return a NotFoundDependency object in this case, instead.

I guess the obvious follow up question is whether we can change this or do we need to preserve the current behaviour to keep backwards compatibility.

Yeah. This looks dangerous, but I haven't looked at what casync does with this value.

There's possibly another bug here in that calling get_pkgconfig_variable on a not-found pkgconfig dependency shouldn't be allowed at all.

[jon-turney]

Something else which could do with a bit of polish:

https://travis-ci.org/jon-turney/meson-corpus-test/jobs/461405180#L522

|Found CMake: NO
|Dependency alsa found: NO (tried pkgconfig)
subprojects/gvc/meson.build:60:2: ERROR:  Dependency alsa not found: CMake not found.

The actual error here is due to a package dependency change, but the way it's reported is very confusing.

This is a consquence of some logic in find_external_dependency, where an exception in the 2nd method tried (cmake) is considered more noteworthy than a simple failure in the 1st method tried (pkgconfig).

I'm not sure what the correct thing to do here is.

meson/mesonbuild/dependencies/base.py

Lines 2071 to 2080 in 0c23e65

	if required:
	# if exception(s) occurred, re-raise the first one (on the grounds that
	# it came from a preferred dependency detection method)
	if pkg_exc:
	raise pkg_exc
	# we have a list of failed ExternalDependency objects, so we can report
	# the methods we tried to find the dependency
	raise DependencyException('Dependency "%s" not found' % (name) +
	(', tried %s' % (tried) if tried else ''))