Skip to content

[sdl2] skip ibus on linux#14275

Merged
BillyONeal merged 1 commit intomicrosoft:masterfrom
jgilje:sdl2_skip_ibus
Oct 30, 2020
Merged

[sdl2] skip ibus on linux#14275
BillyONeal merged 1 commit intomicrosoft:masterfrom
jgilje:sdl2_skip_ibus

Conversation

@jgilje
Copy link
Contributor

@jgilje jgilje commented Oct 28, 2020

Describe the pull request

This patch removes all ibus detection on linux. When detected, ibus will add glib-2.0, gobject-2.0, gio-2.0 and ibus-1.0 as link libraries to the resulting sdl2 library. By skipping ibus entirely, we avoid having to declare these libraries as system libraries and adding a dependency on glib2.

The patch applies to a Linux-only part of sdl2's CMakeLists. Baseline updated before opening PR.

Yes

@JackBoosY JackBoosY added the category:port-bug The issue is with a library, which is something the port should already support label Oct 28, 2020
@JackBoosY
Copy link
Contributor

The gmp regression is not related to this PR's changes and will be fixed in #14003.

@JackBoosY JackBoosY added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Oct 29, 2020
@BillyONeal BillyONeal merged commit f543049 into microsoft:master Oct 30, 2020
@BillyONeal
Copy link
Member

Thanks for your contribution!

@BillyONeal BillyONeal changed the title skip ibus on linux [sdl2] skip ibus on linux Oct 30, 2020
@jgilje jgilje deleted the sdl2_skip_ibus branch November 11, 2020 10:07
@jgilje jgilje mentioned this pull request Nov 17, 2020
@PJB3005
Copy link
Contributor

PJB3005 commented Feb 11, 2023

I'm not an expert in all this stuff or how pkg-config works, but why were the ibus dependencies ending up in pkg-config in the first place? SDL2 dynamically loads everything at runtime and I can't see any references to these libraries in the pkg-config file for, say libsdl2-dev on Ubuntu.

@dg0yt
Copy link
Contributor

dg0yt commented Feb 11, 2023

That change was more two years ago, and the patch was dropped half a year ago.
And note that vcpkg defaults to building with static linkage on linux and osx. I wouldn't be surprised if the Ubuntu pc files don't cover this case.

@PJB3005
Copy link
Contributor

PJB3005 commented Feb 11, 2023

I did some more looking around and it does look like this older version of SDL didn't link dynamically at runtime, so I guess this was necessary at the time. However ibus is still disabled to this day in the port which is probably something worth fixing nowadays.

@PJB3005 PJB3005 mentioned this pull request Feb 12, 2023
7 tasks
PJB3005 added a commit to space-wizards/vcpkg that referenced this pull request Feb 12, 2023
This was disabled in microsoft#14275 because it caused the relevant libraries (ibus, glib, ...) to be depended on transitively.

Nowadays, SDL2 loads these completely at runtime, only needing the headers and such at compile time, so it's safe to re-enable this again.

Ibus is gated behind the "ibus" feature, which like the existing x11/wayland ones warn at installation that you need the necessary system packages to use it. The ibus feature is enabled by default.
JavierMatosD pushed a commit that referenced this pull request Feb 13, 2023
* [sdl2] Re-enable ibus as feature

This was disabled in #14275 because it caused the relevant libraries (ibus, glib, ...) to be depended on transitively.

Nowadays, SDL2 loads these completely at runtime, only needing the headers and such at compile time, so it's safe to re-enable this again.

Ibus is gated behind the "ibus" feature, which like the existing x11/wayland ones warn at installation that you need the necessary system packages to use it. The ibus feature is enabled by default.

* [sdl2] Update baseline
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-bug The issue is with a library, which is something the port should already support info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants