-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Glibmm v2 toolchain #14930
Glibmm v2 toolchain #14930
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
9554f99
to
e683952
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Hooks produced the following warnings for commit 3686c00glibmm/2.72.1
glibmm/2.74.0
glibmm/2.66.4
|
Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only a small comment, otherwise, it's really good to go
Co-authored-by: Uilian Ries <uilianries@gmail.com>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
recipes/glibmm/all/conanfile.py
Outdated
if hasattr(self, "settings_build") and cross_building(self): | ||
raise ConanInvalidConfiguration("Cross-building not implemented") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should work now with MesonToolchain. It was a limitation of the legacy Meson build helper.
recipes/glibmm/all/conanfile.py
Outdated
else: | ||
build.check_min_cppstd(self, 11) | ||
check_min_cppstd(self, 11) | ||
|
||
if self.options.shared and not self.options["glib"].shared: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if self.options.shared and not self.options["glib"].shared: | |
if self.options.shared and not self.dependencies["glib"].options.shared: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I strongly advise to always call VirtualBuildEnv first to avoid conflicts. VirtualBuildEnv is dumb, while other generators may sanitize similar env vars, so you don't want raw env vars of VirtualBuildEnv to override sanitized env vars of others clever generators.
files.get(self, **self.conan_data["sources"][self.version], strip_root=True, destination=self._source_subfolder) | ||
get(self, **self.conan_data["sources"][self.version], strip_root=True, destination=self.source_folder) | ||
|
||
def generate(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def generate(self): | |
def generate(self): | |
env = VirtualBuildEnv(self) | |
env.generate() |
recipes/glibmm/all/conanfile.py
Outdated
|
||
env = VirtualBuildEnv(self) | ||
env.generate() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
env = VirtualBuildEnv(self) | |
env.generate() |
recipes/glibmm/all/conanfile.py
Outdated
self.build_requires("meson/0.64.1") | ||
self.build_requires("pkgconf/1.9.3") | ||
self.tool_requires("meson/1.0.0") | ||
self.tool_requires("pkgconf/1.9.3") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.tool_requires("pkgconf/1.9.3") | |
if not self.conf.get("tools.gnu:pkg_config", check_type=str): | |
self.tool_requires("pkgconf/1.9.3") |
recipes/glibmm/all/conanfile.py
Outdated
if self._abi_version == "2.68": | ||
self.requires("libsigcpp/3.0.7") | ||
else: | ||
self.requires("libsigcpp/2.10.8") | ||
|
||
def source(self): | ||
files.get(self, **self.conan_data["sources"][self.version], strip_root=True, destination=self._source_subfolder) | ||
get(self, **self.conan_data["sources"][self.version], strip_root=True, destination=self.source_folder) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get(self, **self.conan_data["sources"][self.version], strip_root=True, destination=self.source_folder) | |
get(self, **self.conan_data["sources"][self.version], strip_root=True) |
recipes/glibmm/all/conanfile.py
Outdated
|
||
# when using cpp_std=c++NM the /permissive- flag is added which | ||
# attempts enforcing standard conformant c++ code | ||
# the problem is that older versions of Windows SDK is not standard | ||
# conformant! see: | ||
# https://developercommunity.visualstudio.com/t/error-c2760-in-combaseapih-with-windows-sdk-81-and/185399 | ||
files.replace_in_file(self, meson_build, "cpp_std=c++", "cpp_std=vc++") | ||
replace_in_file(self, meson_build, "cpp_std=c++", "cpp_std=vc++") | ||
|
||
def configure(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
configure()
appears so late when you read this recipe from top to bottom, with a logic to change glib shared option.
I wouldn't be against reordering of methods by order of execution: https://docs.conan.io/en/latest/reference/commands/creator/create.html#methods-execution-order
It's super hard to follow global logic of this recipe.
recipes/glibmm/all/conanfile.py
Outdated
|
||
self.cpp_info.components[giomm_component].set_property("pkg_config_name", giomm_component) | ||
self.cpp_info.components[giomm_component].libs = [giomm_component] | ||
self.cpp_info.components[giomm_component].includedirs = [ os.path.join("include", giomm_component)] | ||
self.cpp_info.components[giomm_component].includedirs = [os.path.join("include", giomm_component)] | ||
self.cpp_info.components[giomm_component].requires = [glibmm_component, "glib::gio-2.0"] | ||
|
||
def package_id(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't review the line below, but there is a if not self.options["glib"].shared
here to replace by if not self.dependencies["glib"].options.shared
recipes/glibmm/all/conanfile.py
Outdated
if self._abi_version == "2.68": | ||
self.cpp_info.components[glibmm_component].requires = ["glib::gobject-2.0", "libsigcpp::sigc++"] | ||
else: | ||
self.cpp_info.components[glibmm_component].requires = ["glib::gobject-2.0", "libsigcpp::sigc++-2.0"] | ||
self.cpp_info.components[glibmm_component].requires = ["glib::gobject-2.0", "libsigcpp::libsigcpp"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you look at libsigcpp recipe, this branching shouldn't exist, it should always be self.cpp_info.components[glibmm_component].requires = ["glib::gobject-2.0", "libsigcpp::libsigcpp"]
. Existence of a component in libsigcpp recipe is (or was) just a trick for target name of legacy cmake_find_package*.
recipes/glibmm/all/conanfile.py
Outdated
"build-examples": "false", | ||
"build-documentation": "false", | ||
"msvc14x-parallel-installable": "false", | ||
"default_library": "shared" if self.options.shared else "static" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"default_library": "shared" if self.options.shared else "static" |
already managed by MesonToolchain
Conan v1 pipeline ✔️All green in build 13 (
Conan v2 pipeline (informative, not required for merge) ❌
The v2 pipeline failed. Please, review the errors and note this will be required for pull requests to be merged in the near future. See details:Failure in build 13 ( An unexpected error happened and has been reported. Help is on its way! 🏇 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* [glibmm] add version 2.74.0 * [glibmm] update meson toolchain * [glibmm] update test packages * [glibmm] fix libsigcpp component requires * [glibmm] add v1 test package * [glibmm] disable static builds on msvc * [glibmm] bump glib version * Update recipes/glibmm/all/conanfile.py Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com> * Update recipes/glibmm/all/conanfile.py Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com> * [glibmm] add 2.75.0 * [glibmm] add and document patches for 2.75.0 * Update recipes/glibmm/all/conanfile.py Co-authored-by: Uilian Ries <uilianries@gmail.com> * [glibmm] enable cross compilation * [glibmm] reorder methods * [glibmm] review suggestions --------- Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com> Co-authored-by: Uilian Ries <uilianries@gmail.com>
Specify library name and version: glibmm