-
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
gdk-pixbuf: bump deps and several fixes #21137
Conversation
I detected other pull requests that are modifying gdk-pixbuf/all recipe: This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there. |
This comment has been minimized.
This comment has been minimized.
I've taken the opportunity to include another PR on top of this one as it needed this fix to pass, hopefully it does not cause any issues for you :) |
This reverts commit 3fe80de.
Reverting as it currently does not build for v2, let's have this one go thru and get merged and then we can take care of re-enabling mac in the other PR :) |
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.
As far as I can see, this is not reported upstream (https://github.com/conan-io/conan-center-index/pull/21137/files#diff-471daaafe56f12730848ec964c04a3badcf52a304703057c2ef62b443ef628d1R169), is it? I think that we should start reporting it before merging this. Wdyt @uilianries?
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.
Not directly related to your changes, but I was trying to create a conan package for openslide too and got stuck on gdk-pixbuf recipe.
I think line 205 "gdk_pixbuf_binarydir": "${libdir1}/gdk-pixbuf-2.0/2.10" contains a typo, should say ${libdir}. For me the typo resulted in not being able to use the gdk-pixbuf package from another meson package.
This comment has been minimized.
This comment has been minimized.
You're right. Since Conan 2.0.8, diff --git a/recipes/gdk-pixbuf/all/conanfile.py b/recipes/gdk-pixbuf/all/conanfile.py
index 4f44d79742..1ffbfc55c7 100644
--- a/recipes/gdk-pixbuf/all/conanfile.py
+++ b/recipes/gdk-pixbuf/all/conanfile.py
@@ -12,7 +12,7 @@ from conan.tools.scm import Version
import os
-required_conan_version = ">=1.56.0 <2 || >=2.0.6"
+required_conan_version = ">=1.56.0 <2 || >=2.0.8"
class GdkPixbufConan(ConanFile):
@@ -195,10 +195,12 @@ class GdkPixbufConan(ConanFile):
self.cpp_info.exelinkflags = ldflags
self.cpp_info.sharedlinkflags = ldflags
+ # Related to https://github.com/conan-io/conan/pull/14233
+ libdir_variable = "libdir1" if Version(conan_version) < "2.0" else "libdir"
pkgconfig_variables = {
"bindir": "${prefix}/bin",
"gdk_pixbuf_binary_version": "2.10.0",
- "gdk_pixbuf_binarydir": "${libdir1}/gdk-pixbuf-2.0/2.10",
+ "gdk_pixbuf_binarydir": "${%s}/gdk-pixbuf-2.0/2.10" % libdir_variable,
"gdk_pixbuf_moduledir": "${gdk_pixbuf_binarydir}/loaders",
"gdk_pixbuf_cache_file": "${gdk_pixbuf_binarydir}/loaders.cache",
"gdk_pixbuf_csource": "${bindir}/gdk-pixbuf-csource", @valgur could you add those changes too? Otherwise, we could open another PR to solve that. |
Oh, it's not a typo but a version incompatibility. Thanks for explaining! |
…workaround for the breaking change to pkgconfig_variables
Hi @valgur I just added a couple of changes, including the macOS validation. I'm curious why it was there, so let's see if it passes now or if that validation could be more limited. UPDATED: If everything passes, I'll update the PR description to add the rest of the changes that I introduced 😁 |
This comment has been minimized.
This comment has been minimized.
@franramirez688 Looks like |
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.
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
Sorry for moving this to DRAFT... I'd like to see if we could simplify the tons of patches applied before merging it. |
This comment has been minimized.
This comment has been minimized.
@franramirez688 What's the status of this PR? I assume something is still blocking it? |
This comment has been minimized.
This comment has been minimized.
@valgur I'd like to talk to the team about the possibility of simplifying all the patches that we're using in this recipe. I think I can give it another try in these coming days 😁 Thanks for the reminder! |
Conan v1 pipeline ✔️All green in build 17 (
Conan v2 pipeline ✔️
All green in build 18 ( |
Is it possible to drop the version I took a look in Ubuntu and Arch package to gdk-pixbuf and indeed they use patches to build with meson and autotools, but of course, nothing related like us, because we have a full house of profiles. |
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
This PR contains several fixes and bumping dependencies. Among them:
pkg_config_custom_content
property overwrites variables conan#14233)shared=True
:custom_target
is failing due to lack of environment variables).Related failing PRs: #21126.