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

Update all gtk crates to 18.x #2391

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

atlanticaccent
Copy link

@atlanticaccent atlanticaccent commented Nov 15, 2023

Updates all GTK dependencies from 0.16.x to 0.18.x.

Fixes breaking changes introduced.
Also adds a top level allow for clippy::arc_with_non_send_sync as it's triggered pretty much everywhere in the project and is a massive undertaking to fix.

Currently using a git dependency for piet as it does not have a release including updates to its GTK dependencies.

@xStrom xStrom added shell/gtk concerns the GTK backend S-needs-review waits for review labels Nov 15, 2023
@xStrom
Copy link
Member

xStrom commented Feb 23, 2024

Sorry for letting this sit for so long. Are you still interested in moving this forward?

The master branch now has an updated CI script and Clippy issues are resolved there. So please rebase on master and that top level clippy::arc_with_non_send_sync should no longer be required.

Also, Piet now has the GTK crates at 0.19. I will soon make a Piet release too. Until then, we could use a git pin. If you're interested, feel free to change this PR to 0.19 with the latest Piet git pin. If you just want to get the update to 0.18 that is fine too.

@atlanticaccent
Copy link
Author

I'm happy to update this PR yes!

I'll need to check if updating to GTK 0.19 will break anything I'm working on (I have a bunch of other dependencies with GTK crates in their deps as well) so will default to only going to 0.18 for now.

@atlanticaccent atlanticaccent force-pushed the update-gtk branch 2 times, most recently from 08fa2c0 to cc7cb62 Compare February 26, 2024 20:29
cairo-rs = { version = "0.16.7", default-features = false, features = ["xcb"] }
cairo-sys-rs = { version = "0.16.3", default-features = false, optional = true }
cairo-rs = { version = "0.18.0", default-features = false, features = ["xcb"] }
cairo-sys-rs = { version = "0.18.0", default-features = false, optional = true }

Choose a reason for hiding this comment

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

you don't need to explicitly depend on this, you can get it from cairo::ffi

gtk-rs = { version = "0.16.2", package = "gtk", optional = true }
glib-sys = { version = "0.16.3", optional = true }
gtk-sys = { version = "0.16.0", optional = true }
gtk-rs = { version = "0.18.0", package = "gtk", optional = true }

Choose a reason for hiding this comment

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

from gtk, you can go to gtk::gdk::ffi for example, all those dependencies are not needed. You could just depend on gtk except for cairo where you enable xcb feature

use gtk::glib::translate::FromGlib;
use gtk::prelude::*;
use gtk::traits::SettingsExt;
use gtk::traits::GtkSettingsExt;

Choose a reason for hiding this comment

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

Suggested change
use gtk::traits::GtkSettingsExt;

already part of the prelude

@@ -427,7 +427,7 @@ impl WindowBuilder {
.connect_enter_notify_event(|widget, _| {
widget.grab_focus();

Inhibit(true)
cairo::glib::Propagation::Stop

Choose a reason for hiding this comment

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

you imported it above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-needs-review waits for review shell/gtk concerns the GTK backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants