Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

Gtk+ goes quartz-only #39868

Closed
wants to merge 60 commits into from
Closed

Conversation

tschoonj
Copy link
Contributor

@MikeMcQuaid

This branch represents an effort to remove the X11 dependency completely from gtk+ and its dependencies cairo and pango.

The rationale behind this move is:

  1. It is a major pain for homebrew to support simultaneously the X11 and quartz backends of gtk+. Some formulas require the quartz backend explicitly (like gtk-mac-integration, which for this reason is still not included in homebrew). See also e.g. without-x11 option removed in cairo, cairomm, pango and pangomm #38969 for a discussion about this (there are a lot more PRs and issues where this problem was brought up!)
  2. In gtk+3, the X11 backend is not even supported anymore
  3. X11 is very un-Mac OS X anyway and delivers an inferior user experience (IMHO)
  4. Are there any packages that really need the gtk+ X11 backend to work properly anyway? Apparently gtkglarea does, it will be moved to the boneyard...

My plan is basically to:

  1. Remove X11 from the dependency list of cairo, pango and gtk+. Modify the configure argument list to ensure that no X11 bindings will get built. Explicitly enable quartz support. Update the revision number of these formulas
  2. Update the revision number of all formulas in Homebrew/homebrew that depend on gtk+. This will be the most work since many of these formulas will not be audit --strict compliant (missing tests).
  3. Update the revision number of all formulas in all other Homebrew taps that depend on gtk+. Will create separate pull request for each tap which should be pulled in when this PR is merged in.
    1. science: Homebrew/homebrew-science#2311
    2. x11: Homebrew/homebrew-x11#83
    3. games: Homebrew/homebrew-games#268

It is possible that some formulas that depend only on cairo and/or pango will also need a revision bump if they depend explicitly on the cairo/pango dylibs that are linked to X11. Not sure how to identify these though.

TO DO list (will get longer...):

  • cairo
  • pango
  • gtk+
  • cairomm
  • pangomm
  • gtkmm
  • bokken: 1.7 currently does not build on Yosemite (bokken: fix build on Yosemite #36052 and bokken failed to build on OSX 10.10 due to Python 2.5 dependency #32016). Since it uses pygtk, and not gtk+ directly there is no need to rebuild the package anyway. It should work as soon as the build problem is fixed.
  • diffuse
  • gabedit
  • gbdfed
  • gdmap
  • gnome-icon-theme: lost its gtk+ dependency by updating librsvg
  • goffice: lost its gtk+ dependency by updating librsvg
  • gqview
  • grsync
  • gst-python
  • gtk-engines
  • gtk-gnutella
  • gtk-murrine-engine
  • gtkdatabox
  • gtkextra
  • gtkglarea: needs gtk+ with x11 backend, won't work with quartz. Used by one formula: fsv. Will be moved to the boneyard
  • gtkglext
  • gtksourceview: gtk-mac-integration is required first + patches
  • gwyddion
  • homebank
  • jigdo
  • lablgtk
  • libglade
  • libglademm
  • libgnomecanvas
  • libgnomecanvasmm
  • libinfinity
  • librsvg: removed incorrect gtk+ dependency
  • mdk: had incorrect gtk+3 dependency, should have been gtk+
  • minbif
  • open-scene-graph
  • pidgin
  • pygtk
  • pygtkglext
  • pygtksourceview
  • qiv: going to the boneyard. Needs gtk+ with X11 and uses X11 directly too...
  • synfigstudio
  • vte
  • vte3
  • xplanetfx
  • xsane
  • cogl
  • fontforge: formula updated but there's one audit --strict problem I cannot seem to fix
  • gtk-mac-integration 2.0.8: new formula added!

@@ -22,7 +21,6 @@ class Cairo < Formula
depends_on "libpng"
depends_on "pixman"
depends_on "glib"
depends_on :x11 => :recommended
Copy link
Member

Choose a reason for hiding this comment

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

:optional, probably?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I vote no. Let's slay the X11 monster and keep it from showing its ugly head ever again in the gtk+ dependency graph.

Copy link
Member

Choose a reason for hiding this comment

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

I'm OK with that.

depends_on "gobject-introspection"
option "with-quartz-relocation"
Copy link
Member

Choose a reason for hiding this comment

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

Let's move the option above the dependencies, with space between the two blocks.

Copy link
Member

Choose a reason for hiding this comment

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

Also, option should have some explain message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

taken care of in latest commit

@xu-cheng
Copy link
Member

Personally I think it's bad to put all of these inside on PR. It consumes too many ci resources.

@tschoonj
Copy link
Contributor Author

Wouldn't it be better to configure ci to build only the last commit?

It's much easier to do this in one commit since this update will need to happen simultaneously for all packages involved

@@ -1,10 +1,9 @@
require "formula"

class Cairo < Formula
homepage "http://cairographics.org/"
url "http://cairographics.org/releases/cairo-1.14.2.tar.xz"
mirror "http://www.mirrorservice.org/sites/ftp.netbsd.org/pub/pkgsrc/distfiles/cairo-1.14.2.tar.xz"
Copy link
Contributor

Choose a reason for hiding this comment

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

@DomT4
Copy link
Member

DomT4 commented May 18, 2015

It's probably sensible to tuck them all inside the same PR so we can test they all actually work against the new changes, since a potential breakage and revert would be a nightmare given how deep GTK+ runs.

It's best to not push each change as you make it though, make them locally and then push the end result. Means we only need to whack the CI a few times rather than 40+ times.

@tschoonj
Copy link
Contributor Author

fair enough, will make only pushes when I have to from now on.

still I think the best solution would be for the build-bot to look only to the latest commit

@MikeMcQuaid
Copy link
Member

It does but if it's already started a job when you've pushed another it doesn't know to stop the existing job.

@xu-cheng
Copy link
Member

Wouldn't it be better to configure ci to build only the last commit?

It's not current configure. @MikeMcQuaid may have some opinion on this.

. probably sensible to tuck them all inside the same PR so we can test they all actually work against the new changes, since a potential breakage and revert would be a nightmare given how deep GTK+ runs.

I think the magnitude may be over the capacity that one PR can handle. As one of jacknagel's old advice, I would suggest to create a temporary tap and move all of gtk related formulae there. Therefore, we can test each commit one by one using Travis CI. After that, we can move these formulae back to core.

@DomT4
Copy link
Member

DomT4 commented May 18, 2015

I think the magnitude may be over the capacity that one PR can handle.

Disagree personally. It handled OpenSSL before, and OpenSSL was a bigger PR with some significantly heftier formulae. I would be surprised if all the GTK+ changes here in the end stress the bot more than one Rust build does. We would have a similar, if not more PITA, problem with Travis in that the build would be likely to timeout or exceed the 5MB log limit imposed on free Travis.

If someone wants to follow Jack's advice I'm fine with that too, but I think Jack's suggestion was to not actually remove them from the core, just to copy them, remove the x11 tie in, and then merge them back in to core?

@tschoonj
Copy link
Contributor Author

By the way: if someone wants to help out, I would really appreciate it if someone would move all Formulas from homebrew-x11 that depend on either gtk+ or gtk+3 to the homebrew tap. They have no longer any business being there.

@jacknagel
Copy link
Contributor

if someone would move all Formulas from homebrew-x11 that depend on either gtk+ or gtk+3 to the homebrew tap

Please don't PR this yet, though.

@jacknagel
Copy link
Contributor

Please remember that every time you push it starts a new CI job and does not kill those that are already queued. I understand this isn't ideal, but at this moment that's how it works, so let's try to ease up on the push frequency. Thanks!

@tschoonj
Copy link
Contributor Author

It looks like the CI jobs stopped. Can anyone check whats going on?

@MikeMcQuaid
Copy link
Member

They were probably manually killed and/or timed out.

@DomT4
Copy link
Member

DomT4 commented May 19, 2015

The last build failed on gnome-icon-theme, which had a fairly insistent failure during the compile. Not sure why the PR here says Merge build triggered still, doesn't seem to be in the queue.

@MikeMcQuaid
Copy link
Member

@BrewTestBot test this please

@tschoonj
Copy link
Contributor Author

Thanks, the gnome-icon-theme problem is related to gdk-pixbuf and librsvg: during the gnome-icon-theme build, the rsvg-convert utility is invoked to convert svg files. This utility depends on libgdk_pixbuf to load the svg files but fails because it cannot load the svg plugin for some reason.

My latest commits have bumped the revision number of both gdk-pixbuf and librsvg, hoping it may fix this problem.

Could anyone have a look at the audit --strict I am getting for fontforge?

 * python modules have explicit framework links
 These python extension modules were linked directly to a Python
 framework binary. They should be linked with -undefined dynamic_lookup
 instead of -lpython or -framework Python.
   /usr/local/Cellar/fontforge/20150430_1/lib/python2.7/site-packages/fontforge.so
     /usr/local/Cellar/fontforge/20150430_1/lib/python2.7/site-packages/psMat.so

Thanks

@DomT4
Copy link
Member

DomT4 commented May 19, 2015

Ignore the fontforge failure for now. I've already pinged Tim about it in an existing thread.

@tschoonj
Copy link
Contributor Author

Ok, will do.

Any reason to keep libgnomecanvas and libgnomecamvasmm? No other formulas appear to use them and they haven't seen an update in 5 years.

@MikeMcQuaid
Copy link
Member

Any reason to keep libgnomecanvas and libgnomecamvasmm? No other formulas appear to use them and they haven't seen an update in 5 years.

Seems a good enough reason to 💀 them to me.

@tschoonj
Copy link
Contributor Author

Ok, will delete them then.

I was also thinking to remove libglade but it turns out mdk really needs it...

@MikeMcQuaid
Copy link
Member

Also: if any of this stuff can be made to work with GTK+3 instead of 2 that'd be handy too.

@DomT4
Copy link
Member

DomT4 commented May 19, 2015

I'd also prefer we keep libglade. It's heavily useful for quite a few formulae, and outside of Homebrew too.

@MikeMcQuaid
Copy link
Member

@tschoonj Please don't push any more changes to this branch. I'll address these nits when I pull, thanks.

@tschoonj
Copy link
Contributor Author

@DomT4 pangox-compat is a really weird package. It appears to have been made just for gtkglext, and requires indeed X11. From the git repository of gtkglext it looks like they dropped the need for pangox-compat in their master branch but I doubt they will ever release an update to it.

@tschoonj
Copy link
Contributor Author

@MikeMcQuaid thanks Mike! After pulling in, can you restart the PRs in homebrew-x11 and homebrew-science?

@DomT4
Copy link
Member

DomT4 commented Jun 16, 2015

@tschoonj Thanks for the explanation on pangox-compat. Given the nature of the PR, just wanted to check wasn't a typo. Nice work here getting this done 👍.

@tschoonj
Copy link
Contributor Author

@DomT4 you're most welcome but we aren't done yet. As soon as this PR gets pulled in, after updating people will have runtime linking errors to their gtk-dependent homebrew-science and homebrew-x11 packages.

@tschoonj
Copy link
Contributor Author

@MikeMcQuaid by the way I still need to boneyard two formulas here: qiv and gtkglarea. Shall I already proceed with that?

@DomT4
Copy link
Member

DomT4 commented Jun 16, 2015

Once X11's PR is green I'll pull it there pretty quickly. I don't know how @MikeMcQuaid feels but potentially a lot of the GTK stuff can be moved back to the core now the X11 backend has been killed off.

@tschoonj
Copy link
Contributor Author

Well all packages in x11 and science need to be rebuilt against the new gtk+, which I guess will take a few hours.

And yes a lot of those packages really won't belong there anymore, including those that depend on gtk+3.

tschoonj added a commit to tschoonj/homebrew-boneyard that referenced this pull request Jun 16, 2015
victims of the transitioning to gtk-quartz-only: Homebrew/legacy-homebrew#39868
NOTE: these formulas will not be deleted with this PR, a separate will
be opened for that later...
tschoonj added a commit to tschoonj/homebrew that referenced this pull request Jun 16, 2015
victims of the transitioning to gtk-quartz-only: Homebrew#39868
a PR has been opened in the boneyard: Homebrew/homebrew-boneyard#54
@tschoonj
Copy link
Contributor Author

@DomT4 Just wondering about the migration to the boneyard: since I am not migrating out of the core tap, do I have to update the tap_migrations.rb in core with a separate PR? If not they will still be pointing in that file to the x11 tap instead of boneyard...

@tschoonj
Copy link
Contributor Author

@DomT4 sorry wrong PR, this refers of course to the X11 tap migration

@DomT4
Copy link
Member

DomT4 commented Jun 16, 2015

Theoretically if stuff has been migrated from homebrew/homebrew to homebrew/x11 and now to homebrew/boneyard we can change the lines in tap_migrations.rb in a separate PR in the core.

If they never existed in Homebrew/homebrew and aren't in the tap_migrations.rb list - Don't worry about it. Long-term my desire is to support tap_migrations from taps to other taps, or from taps to core, but for now there's no mechanism for that.

@tschoonj
Copy link
Contributor Author

I checked and all three formulas are indeed mentioned in tap_migrations.rb. Will file a PR in core when all this is over.

@tschoonj
Copy link
Contributor Author

@MikeMcQuaid thanks! I pushed my latest changes to homebrew-x11, the build has already started.

Can you manually restart the build in the homebrew-science PR?

@MikeMcQuaid
Copy link
Member

@tschoonj On it. The rebottling will take priority but I'll get on this ASAP.

DomT4 pushed a commit that referenced this pull request Jun 17, 2015
victims of the transitioning to gtk-quartz-only: #39868
a PR has been opened in the boneyard: Homebrew/homebrew-boneyard#54

Closes #40792.

Signed-off-by: Dominyk Tiller <dominyktiller@gmail.com>
@MikeMcQuaid
Copy link
Member

All PRs merged. Thanks for all your work here @tschoonj!

@tschoonj
Copy link
Contributor Author

you're most welcome. I expect people will file issues now and then when their formulas with optional gtk+ dependency fail to compile (like the vim one this morning).

If so ping me and I will try to fix it

juniorz added a commit to juniorz/go-gtk that referenced this pull request Sep 21, 2015
Homebrew has dropped support to X11 on every GTK+ related formula since
(Homebrew/legacy-homebrew#39868). This change reflects this
decision on go-gtk.

By default, builds without support to X11. X11 support can be enabled by using
the build flag `with-x11` for users containing a GTK+ installation with X11
support.
juniorz added a commit to juniorz/go-gtk that referenced this pull request Sep 21, 2015
Homebrew has dropped support to X11 on every GTK+ related formula since
(Homebrew/legacy-homebrew#39868). This change reflects this
decision on go-gtk.

By default, builds without support to X11. X11 support can be enabled by using
the build flag `with-x11` for users containing a GTK+ installation with X11
support.
@Homebrew Homebrew locked and limited conversation to collaborators Jul 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants