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.

Sign up for GitHub

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

xkbcommon: Use PkgConfigDeps build context for wayland #13612

Merged
merged 5 commits into from
Oct 21, 2022
Merged

xkbcommon: Use PkgConfigDeps build context for wayland #13612

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0c5cc5d
xkbcommon: Use PkgConfigDeps build context for wayland
jwillikers Oct 19, 2022
638ec52
xkbcommon: finer grain requirements
ericLemanissier Oct 19, 2022
b9f8279
Place build context pkg-config files in a separate directory
jwillikers Oct 19, 2022
8cdc6d6
Move comment to a better spot
jwillikers Oct 19, 2022
05bab6b
Fix Apple shared install name and pkg-config conf
jwillikers Oct 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 62 additions & 35 deletions recipes/xkbcommon/all/conanfile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,24 @@
import os

from conan import ConanFile
from conan.tools.env import VirtualBuildEnv
from conan.tools.files import copy, get, replace_in_file, rmdir
from conan.tools.gnu import PkgConfigDeps
from conan.tools.layout import basic_layout
from conan.tools.meson import Meson, MesonToolchain
from conan.tools.scm import Version
from conan.errors import ConanInvalidConfiguration

required_conan_version = ">=1.50.0"
required_conan_version = ">=1.52.0"


class XkbcommonConan(ConanFile):
name = "xkbcommon"
description = "keymap handling library for toolkits and window systems"
topics = ("xkbcommon", "keyboard")
topics = ("keyboard")
url = "https://github.com/conan-io/conan-center-index"
homepage = "https://github.com/xkbcommon/libxkbcommon"
license = "MIT"

settings = "os", "arch", "compiler", "build_type"
options = {
"shared": [True, False],
Expand All @@ -34,7 +35,9 @@ class XkbcommonConan(ConanFile):
"xkbregistry": True,
}

generators = "PkgConfigDeps", "VirtualBuildEnv"
@property
def _has_build_profile(self):
return hasattr(self, "settings_build")

@property
def _has_xkbregistry_option(self):
Expand All @@ -48,64 +51,87 @@ def config_options(self):

def configure(self):
if self.options.shared:
del self.options.fPIC
del self.settings.compiler.libcxx
del self.settings.compiler.cppstd
try:
del self.options.fPIC
except Exception:
pass
try:
del self.settings.compiler.libcxx
except Exception:
pass
try:
del self.settings.compiler.cppstd
except Exception:
pass

def requirements(self):
self.requires("xorg/system")
if self.options.get_safe("xkbregistry"):
self.requires("libxml2/2.9.14")
if self.options.get_safe("with_wayland"):
self.requires("wayland/1.21.0")
self.requires("wayland-protocols/1.26") # FIXME: This should be a build-requires
if not self._has_build_profile:
self.requires("wayland-protocols/1.27")

def validate(self):
if self.settings.os not in ["Linux", "FreeBSD"]:
raise ConanInvalidConfiguration("This library is only compatible with Linux or FreeBSD")
if self.info.settings.os not in ["Linux", "FreeBSD"]:
raise ConanInvalidConfiguration(f"{self.ref} is only compatible with Linux and FreeBSD")

def build_requirements(self):
self.tool_requires("meson/0.63.1")
self.tool_requires("bison/3.7.6")
self.tool_requires("pkgconf/1.7.4")
if hasattr(self, "settings_build") and self.options.get_safe("with_wayland"):
self.tool_requires("meson/0.63.3")
self.tool_requires("bison/3.8.2")
self.tool_requires("pkgconf/1.9.3")
if self._has_build_profile and self.options.get_safe("with_wayland"):
self.tool_requires("wayland/1.21.0")
self.tool_requires("wayland-protocols/1.27")

def layout(self):
basic_layout(self, src_folder="src")

def source(self):
get(self, **self.conan_data["sources"][self.version], strip_root=True)
get(self, **self.conan_data["sources"][self.version],
destination=self.source_folder, strip_root=True)

def generate(self):
tc = MesonToolchain(self)

tc.project_options["libexecdir"] = "bin"
tc.project_options["libdir"] = "lib"
tc.project_options["datadir"] = "res"

tc.project_options["enable-docs"] = False
tc.project_options["enable-wayland"] = self.options.get_safe("with_wayland", False)
tc.project_options["enable-x11"] = self.options.with_x11

if self._has_xkbregistry_option:
tc.project_options["enable-xkbregistry"] = self.options.xkbregistry
if self._has_build_profile:
tc.project_options["build.pkg_config_path"] = self.generators_folder
tc.generate()

def build(self):
# Conan doesn't provide a `wayland-scanner.pc` file for the package in the _build_ context
meson_build_file = os.path.join(self.source_folder, "meson.build")
replace_in_file(self, meson_build_file,
"wayland_scanner_dep = dependency('wayland-scanner', required: false, native: true)",
"# wayland_scanner_dep = dependency('wayland-scanner', required: false, native: true)")
pkg_config_deps = PkgConfigDeps(self)
if self._has_build_profile and self.options.get_safe("with_wayland"):
pkg_config_deps.build_context_activated = ["wayland", "wayland-protocols"]
pkg_config_deps.build_context_suffix = {"wayland": "_BUILD"}
pkg_config_deps.generate()

replace_in_file(self, meson_build_file,
"if not wayland_client_dep.found() or not wayland_protocols_dep.found() or not wayland_scanner_dep.found()",
"if not wayland_client_dep.found() or not wayland_protocols_dep.found()")
virtual_build_env = VirtualBuildEnv(self)
virtual_build_env.generate()

replace_in_file(self, meson_build_file,
"wayland_scanner = find_program(wayland_scanner_dep.get_pkgconfig_variable('wayland_scanner'))",
"wayland_scanner = find_program('wayland-scanner')")
def build(self):
if self.options.get_safe("with_wayland"):
meson_build_file = os.path.join(self.source_folder, "meson.build")
if self._has_build_profile:
replace_in_file(self, meson_build_file,
"wayland_scanner_dep = dependency('wayland-scanner', required: false, native: true)",
"wayland_scanner_dep = dependency('wayland-scanner_BUILD', required: false, native: true)")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you dont ass the context suffix could this be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll get an error if I don't set the build_context_suffix for the wayland package because this library requires the wayland-client library from the wayland package as a regular requires while simultaneously requiring the wayland-scanner tool from the wayland package as a tool_requires.

If we were able to place the build-context pkg-config files in a separate directory from the others, this would avoid naming conflicts and the necessary patching to find the tool_requires dependencies. Meson would be able to handle this gracefully just by setting the standard project option build.pkg_config_path to a value including the directory for the build-context pkg-config files.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've gone ahead and re-worked this PR to use a separate directory for the pkg-config files in the build context. This doesn't require using replace_in_file when a build profile is present.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes I ask questions for myself to learn. Don't feel obligated to make changes the community (aka you) know more than me and that's a good thing.

Reading and looking qt the code I feel like it's a limitation on the client, like there's nothing in the layout for separation... seems like a useful feature

Maybe we can open an issue again the main repo and see what the team thinks?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that'd be a good idea, even if it's only useful for Meson. Your question hit on something that'd been bothering me, too. I'm glad you brought it up. It really seems more maintainable if we don't have to patch the build system in order to use the pkg-config files in the build context. I think the changes here will be a great demonstration of the desired functionality. I'll take care of opening the issue tomorrow unless you get to it first.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created conan-io/conan#12342 but feel free to correct me if I misunderstood or left something important out :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I just realized my workaround doesn't actually work... I'd have to not only generate pkg-config files for the wayland dependency, but also for everything listed as a requires for the wayland::wayland-scanner component. Without the pkg-config files available for the dependencies, pkg-config reports that the dependency was not found.

else:
# Conan doesn't provide a `wayland-scanner.pc` file for the package in the _build_ context
replace_in_file(self, meson_build_file,
"wayland_scanner_dep = dependency('wayland-scanner', required: false, native: true)",
"# wayland_scanner_dep = dependency('wayland-scanner', required: false, native: true)")

replace_in_file(self, meson_build_file,
"if not wayland_client_dep.found() or not wayland_protocols_dep.found() or not wayland_scanner_dep.found()",
"if not wayland_client_dep.found() or not wayland_protocols_dep.found()")

replace_in_file(self, meson_build_file,
"wayland_scanner = find_program(wayland_scanner_dep.get_pkgconfig_variable('wayland_scanner'))",
"wayland_scanner = find_program('wayland-scanner')")

meson = Meson(self)
meson.configure()
Expand Down Expand Up @@ -133,10 +159,11 @@ def package_info(self):
self.cpp_info.components["libxkbregistry"].libs = ["xkbregistry"]
self.cpp_info.components["libxkbregistry"].requires = ["libxml2::libxml2"]
if self.options.get_safe("with_wayland", False):
# FIXME: This generates just executable, but I need to use the requirements to pass Conan checks
self.cpp_info.components["xkbcli-interactive-wayland"].libs = []
self.cpp_info.components["xkbcli-interactive-wayland"].includedirs = []
self.cpp_info.components["xkbcli-interactive-wayland"].requires = ["wayland::wayland", "wayland-protocols::wayland-protocols"]
self.cpp_info.components["xkbcli-interactive-wayland"].requires = ["wayland::wayland-client"]
if not self._has_build_profile:
self.cpp_info.components["xkbcli-interactive-wayland"].requires.append("wayland-protocols::wayland-protocols")

if Version(self.version) >= "1.0.0":
bindir = os.path.join(self.package_folder, "bin")
Expand Down
1 change: 1 addition & 0 deletions recipes/xkbcommon/all/test_package/conanfile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
class TestPackageConan(ConanFile):
settings = "os", "arch", "compiler", "build_type"
generators = "CMakeToolchain", "CMakeDeps", "VirtualRunEnv"
test_type = "explicit"

def requirements(self):
self.requires(self.tested_reference_str)
Expand Down