glfw: limit xorg library dependencies #23272

johningve · 2024-03-26T13:17:12Z

Previously the glfw package would implicitly require linking with xorg::xorg, which includes all the libraries exposed by the xorg package.

This change explicitly declares the requirements of glfw, limiting the xorg libraries to only those needed.

From version 3.4 these dependencies are no longer necessary since glew implemented runtime platform detection.

Also removes vulkan_static option in version 3.4, since it is no longer supported by glfw.

Specify library name and version: glfw/all

I've read the contributing guidelines.
I've used a recent Conan client version close to the currently deployed.
I've tried at least one configuration locally with the conan-center hook activated.

Previously the glfw package would implicitly require linking with xorg::xorg, which includes all the libraries exposed by the xorg package. This commit explicitly declares the requirements of glfw, limiting the xorg libraries to only those needed.

The ability to link to a static Vulkan loader was removed in glfw version 3.4.

github-actions · 2024-03-26T13:17:59Z

🤖 Beep Boop! This pull request is making changes to 'recipes/glfw//'.

👋 @Hopobcn you might be interested. 😉

valgur · 2024-03-26T13:22:29Z

recipes/glfw/all/conanfile.py

+        if Version(self.version) < "3.4":
+            if self.options.get_safe("vulkan_static"):
+                self.cpp_info.requires.extend(["vulkan-loader::vulkan-loader"])
+            if self.settings.os in ["Linux", "FreeBSD"]:
+                if self.options.get_safe("with_x11", True):
+                    self.cpp_info.requires.extend(["xorg::x11"])
+            if self.options.get_safe("with_wayland"):
+                self.cpp_info.requires.extend(["wayland::wayland", "xkbcommon::xkbcommon"])


Suggested change

if Version(self.version) < "3.4":

if self.options.get_safe("vulkan_static"):

self.cpp_info.requires.extend(["vulkan-loader::vulkan-loader"])

if self.settings.os in ["Linux", "FreeBSD"]:

if self.options.get_safe("with_x11", True):

self.cpp_info.requires.extend(["xorg::x11"])

if self.options.get_safe("with_wayland"):

self.cpp_info.requires.extend(["wayland::wayland", "xkbcommon::xkbcommon"])

if self.options.get_safe("vulkan_static"):

self.cpp_info.requires.extend(["vulkan-loader::vulkan-loader"])

if self.settings.os in ["Linux", "FreeBSD"]:

if self.options.get_safe("with_x11", True):

self.cpp_info.requires.extend(["xorg::x11"])

if self.options.get_safe("with_wayland"):

self.cpp_info.requires.extend(["wayland::wayland", "xkbcommon::xkbcommon"])

I don't think the version check is necessary as it's already handled by the deleted options in configure(), no?

I think the version check is needed because even when you specify with_x11=True or with_wayland=True, the glfw library does not require linking with these after version 3.4, which implemented the runtime platform detection.

From version 3.4, glfw loads platform libraries at runtime, and so it is not necessary to link with these.

conan-center-bot · 2024-03-27T09:04:44Z

Conan v1 pipeline ✔️

All green in build 2 (c05ac9dbdb4ad9a5508a7f6c8575e8098ac60fbd):

glfw/3.3.8:
All packages built successfully! (All logs)
glfw/3.4:
All packages built successfully! (All logs)
glfw/3.3.7:
All packages built successfully! (All logs)
glfw/3.3.6:
All packages built successfully! (All logs)
glfw/3.3.5:
All packages built successfully! (All logs)
glfw/3.3.4:
All packages built successfully! (All logs)
glfw/3.3.3:
All packages built successfully! (All logs)
glfw/3.3.2:
All packages built successfully! (All logs)

Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 2 (c05ac9dbdb4ad9a5508a7f6c8575e8098ac60fbd):

glfw/3.3.6:
All packages built successfully! (All logs)
glfw/3.4:
All packages built successfully! (All logs)
glfw/3.3.5:
All packages built successfully! (All logs)
glfw/3.3.8:
All packages built successfully! (All logs)
glfw/3.3.4:
All packages built successfully! (All logs)
glfw/3.3.7:
All packages built successfully! (All logs)
glfw/3.3.2:
All packages built successfully! (All logs)
glfw/3.3.3:
All packages built successfully! (All logs)

perseoGI

LGTM

uilianries

LGTM: https://github.com/glfw/glfw/compare/3.3.8..3.4#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20a

valgur

Need to remove some redundant/conflicting logic in package_info() that was added by #25547, which got merged first.

valgur · 2024-11-26T16:12:32Z

@perseoGI Needs a follow-up PR.

perseoGI · 2024-11-26T16:31:59Z

@valgur Which logic are you refering?

valgur · 2024-11-26T18:46:37Z

The requires being added for vulkan, xorg and wayland on lines 230-251 are being duplicated by the logic added in this PR.

perseoGI · 2024-11-27T08:15:14Z

Okey I see. Lines

conan-center-index/recipes/glfw/all/conanfile.py

Lines 230 to 251 in dc3c064

    
           self.cpp_info.requires = ["opengl::opengl"] 
        
           if self.options.get_safe("vulkan_static"): 
        
               self.cpp_info.requires.append("vulkan-loader::vulkan-loader") 
        
           if self.settings.os in ["Linux", "FreeBSD"]: 
        
               if self.options.get_safe("with_x11", True): 
        
                   # https://github.com/glfw/glfw/blob/3.4/src/CMakeLists.txt#L181-L218 
        
                   # https://github.com/glfw/glfw/blob/3.3.2/CMakeLists.txt#L196-L233 
        
                   self.cpp_info.requires.extend([ 
        
                       "xorg::x11", # Also includes Xkb and Xshape 
        
                       "xorg::xrandr", 
        
                       "xorg::xinerama", 
        
                       "xorg::xcursor", 
        
                       "xorg::xi", 
        
                   ]) 
        
           if self.options.get_safe("with_wayland"): 
        
               # https://github.com/glfw/glfw/blob/3.4/src/CMakeLists.txt#L163-L167 
        
               self.cpp_info.requires.extend([ 
        
                   "wayland::wayland-client", 
        
                   "wayland::wayland-cursor", 
        
                   "wayland::wayland-egl", 
        
                   "xkbcommon::xkbcommon" 
        
               ])

seems to be more complete.
But in this PR the version control was added. Should we keep the previous lines with the version control?
Could you @johningve or @valgur open a PR to fix this?

Co-authored-by: PerseoGI <perseog@jfrog.com>

johningve added 2 commits March 26, 2024 13:47

glfw: limit xorg library requirements

925ca6f

Previously the glfw package would implicitly require linking with xorg::xorg, which includes all the libraries exposed by the xorg package. This commit explicitly declares the requirements of glfw, limiting the xorg libraries to only those needed.

glfw: remove vulkan_static option for version 3.4

58f9f7e

The ability to link to a static Vulkan loader was removed in glfw version 3.4.

valgur reviewed Mar 26, 2024

View reviewed changes

conan-center-bot added the Failed label Mar 26, 2024

This comment has been minimized.

Sign in to view

glfw: don't link platform libraries from version 3.4

c05ac9d

From version 3.4, glfw loads platform libraries at runtime, and so it is not necessary to link with these.

johningve force-pushed the fix-glfw-cpp-info-requirements branch from 18ad9b7 to c05ac9d Compare March 27, 2024 07:59

conan-center-bot removed the Failed label Mar 27, 2024

prince-chrismc mentioned this pull request Mar 27, 2024

Pull Requests Ready to be Reviewed prince-chrismc/conan-center-index-pending-review#1

Open

jwillikers approved these changes Apr 9, 2024

View reviewed changes

This was referenced Apr 9, 2024

[service] Potentially conflicting recipes #3571

Closed

glfw: Use libglvnd on FreeBSD and Linux #23439

Closed

ghost mentioned this pull request Apr 26, 2024

glfw: use dependencies.build instead of deps_cpp_info #23779

Merged

3 tasks

conan-center-bot requested a review from uilianries May 7, 2024 11:41

conan-center-bot requested a review from jcar87 June 7, 2024 11:40

conan-center-bot requested a review from franramirez688 October 18, 2024 11:38

perseoGI added 2 commits November 26, 2024 15:56

Conan v2 refactor

67dc5b9

Conan v2 refactor

a8d4222

perseoGI previously approved these changes Nov 26, 2024

View reviewed changes

Make GLFW_VULKAN_STATIC cache_variable to take effect

5403358

perseoGI dismissed their stale review via 5403358 November 26, 2024 15:43

uilianries approved these changes Nov 26, 2024

View reviewed changes

perseoGI approved these changes Nov 26, 2024

View reviewed changes

perseoGI merged commit d167a8d into conan-io:master Nov 26, 2024
9 checks passed

valgur suggested changes Nov 26, 2024

View reviewed changes

OMGtechy pushed a commit to OMGtechy/conan-center-index that referenced this pull request Dec 31, 2024

glfw: limit xorg library dependencies (conan-io#23272)

d99a71e

Co-authored-by: PerseoGI <perseog@jfrog.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

glfw: limit xorg library dependencies #23272

glfw: limit xorg library dependencies #23272

johningve commented Mar 26, 2024

github-actions bot commented Mar 26, 2024

valgur Mar 26, 2024

johningve Mar 26, 2024

This comment has been minimized.

conan-center-bot commented Mar 27, 2024

perseoGI left a comment

uilianries left a comment

valgur left a comment

valgur commented Nov 26, 2024

perseoGI commented Nov 26, 2024

valgur commented Nov 26, 2024

perseoGI commented Nov 27, 2024

glfw: limit xorg library dependencies #23272

glfw: limit xorg library dependencies #23272

Conversation

johningve commented Mar 26, 2024

github-actions bot commented Mar 26, 2024

valgur Mar 26, 2024

Choose a reason for hiding this comment

johningve Mar 26, 2024

Choose a reason for hiding this comment

This comment has been minimized.

conan-center-bot commented Mar 27, 2024

Conan v1 pipeline ✔️

Conan v2 pipeline ✔️

perseoGI left a comment

Choose a reason for hiding this comment

uilianries left a comment

Choose a reason for hiding this comment

valgur left a comment

Choose a reason for hiding this comment

valgur commented Nov 26, 2024

perseoGI commented Nov 26, 2024

valgur commented Nov 26, 2024

perseoGI commented Nov 27, 2024