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.

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

Add wayland and kmsdrm support to manylinux wheels #1997

Merged
merged 5 commits into from
Mar 5, 2024

Conversation

ankith26
Copy link
Member

@ankith26 ankith26 commented Mar 7, 2023

Continuation of upstream pygame/pygame#3482

@ankith26 ankith26 requested a review from a team as a code owner March 7, 2023 06:20
@Starbuck5
Copy link
Member

Starbuck5 commented Mar 10, 2023

Could you explain what this PR does and how it does it?

@ankith26
Copy link
Member Author

So basically, we compile wayland and friends (libdecor, protocols) so that SDL-compilation step finds it at compile time. These are not bundled in the wheels, and just like X11, is dynamically loaded on the users system if available.

I'm also adding mesa (and its deps) because it is needed for proper wayland support as I understand it (though in the long run someone more experienced than me will probably find more usecases for something like mesa being bundled in pygame wheels)

To test this PR, a linux user who has wayland can use SDL_VIDEODRIVER=wayland and run any pygame app (note that this would error previously if you are using a CI made wheel; locally made wheels are a different story)

@Starbuck5
Copy link
Member

Thanks for the clarification.

If the deps aren't bundled, what's the point of building it ourselves? Can't we get libwayland-devel our whatever through apt?

Also does this stuff build on mac too? It seems like it shouldn't.

@robertpfeiffer
Copy link
Contributor

robertpfeiffer commented Mar 12, 2023

If the deps aren't bundled, what's the point of building it ourselves? Can't we get libwayland-devel our whatever through apt?

Back when I filed pygame/pygame#2529, official pygame wheels would only use X11 on wayland (xwayland), but compiling on my own would fix this.

@ankith26
Copy link
Member Author

Can't we get libwayland-devel our whatever through apt?

The base image we use (manylinux2014) is old, wayland likely doesn't exist on those old centos repos, and even if it somehow did, would be quite outdated and SDL needs a new-ish wayland

Also does this stuff build on mac too? It seems like it shouldn't.

It doesn't build on macs, only manylinux

Copy link
Member

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

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

LGTM - from a scan through the scripts.

I think the only way we are going to be certain that this works is if we get in merged, get the wheels building and then get somebody with wayland & kmsdrm to test them out. That person will not be me as I do not have that hardware.

@Starbuck5
Copy link
Member

Shouldn't we be building against the oldest wayland stuff that still works? Because it will need to be linked at runtime on users systems, and that only happens if runtime available version >= compiled version? Looking at versions, I see we're building Mesa 23.0.0, which came out less than a month ago. So wouldn't that fail to link on anyone's system not on bleeding edge Mesa?

@MyreMylar
Copy link
Member

Linked to: #1301

@ankith26 ankith26 linked an issue Oct 1, 2023 that may be closed by this pull request
@ankith26 ankith26 marked this pull request as draft October 9, 2023 06:46
@pmp-p
Copy link
Member

pmp-p commented Feb 21, 2024

Shouldn't we be building against the oldest wayland stuff that still work

Nobody would use that, (old) wayland/pulseaudio/systemd are alpha prototypes stage that were (are) experimented on "live".

Wayland support allow for integration of pygame-ce in GNU/Linux rolling releases distributions.
Mesa should be always up to date or hardware drivers could fail.

@Starbuck5
Copy link
Member

But we don’t ship “Wayland”, we rely on what’s on the user’s system right? This is dynamic linking?

I don’t understand how we can compile against the latest and then have it work against not the latest. That’s not how libc works, famously. The PyInstaller docs say that you have to compile against the oldest libc you plan on supporting, why would this be different?

@ankith26 ankith26 marked this pull request as ready for review February 29, 2024 14:29
@ankith26
Copy link
Member Author

Some updates on this

  1. Building against the latest wayland+libdecor isn't problematic (as wayland negotiates the correct protocol version)
  2. the mesa stuff is apparently not being bundled in the wheels at all, which is a good thing because now it seems like it's going to pick whatever is on the users system (AKA retain same behaviour as before this PR)

Copy link
Member

@oddbookworm oddbookworm left a comment

Choose a reason for hiding this comment

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

LGTM!

@oddbookworm
Copy link
Member

Holding off on merge because I want @Starbuck5 to have the chance to voice any reservations he may still have after the most recent updates

@Starbuck5
Copy link
Member

I feel like with dependency updates like these the only way to really anticipate what happens is to see what happens, so lets merge this in.

@Starbuck5 Starbuck5 merged commit 9597f81 into main Mar 5, 2024
29 checks passed
@Starbuck5 Starbuck5 added this to the 2.5.0 milestone Mar 5, 2024
@ankith26 ankith26 deleted the ankith26-manylinux-wayland branch March 5, 2024 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No native wayland support in manylinux wheels (2529)
6 participants