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

wxGTK: use GTK 3 by default #73145

Open
wants to merge 20 commits into
base: master
from
Open

Conversation

@lopsided98
Copy link
Contributor

lopsided98 commented Nov 9, 2019

Motivation for this change

wxGTK should switch to using GTK 3 where possible. Currently this PR implements the basic change for wxGTK30 and wxPython 3.0. There are a lot of packages that may be affected by this change, as shown in the following nix-review report:

24 packages failed to build:
89 packages were built:
  • aegisub
  • bossa
  • cl
  • couchdb
  • couchdb2
  • cubicsdr
  • curaByDagoma
  • cura_stable
  • cuter
  • digikam
  • displaycal
  • dvdstyler
  • ejabberd
  • elixir
  • elixir_1_5
  • elixir_1_6
  • elixir_1_7
  • elixir_1_8
  • erlang
  • erlangR18
  • erlangR19
  • erlangR20
  • erlangR21
  • erlang_javac
  • erlang_odbc
  • erlang_odbc_javac
  • far2l
  • flamerobin
  • gnss-sdr
  • gnuradio
  • gr-ais
  • gr-gsm
  • gr-limesdr
  • gr-nacl
  • gr-osmosdr
  • gr-rds
  • gnuradio-with-packages
  • golly
  • gqrx
  • hex2nix
  • hugin
  • inspectrum
  • kicad
  • kicad-with-packages3d
  • lenmus
  • limesuite
  • loxodo
  • lutris
  • lutris-free
  • mercury
  • metamorphose2
  • mmex
  • notmuch-bower
  • odamex
  • perl528Packages.AlienWxWidgets
  • perl530Packages.AlienWxWidgets
  • playonlinux
  • plover.stable
  • printrun
  • python27Packages.pyspread
  • python27Packages.robotframework-ride
  • python27Packages.runsnakerun
  • python27Packages.wxPython
  • qradiolink
  • rabbitmq-server
  • radiotray-ng
  • rebar
  • rebar3
  • relxExe
  • rtl_433
  • saga
  • scyther
  • sdrangel
  • soapysdr-with-plugins
  • torchat
  • truecrypt
  • tsung
  • urh
  • welle-io
  • wings
  • winpdb
  • winusb
  • wxGTK30
  • wxSVG
  • wxmaxima
  • wxsqlite3
  • xylib
  • yaws
  • zeroad

Some of these failures may be prexisting, but many are caused by the switch to GTK 3.

I would like to drop support for GTK 2 altogether in wxGTK, but I'm not sure if this will be possible. I also need to test wxGTK 3.1 with GTK 3.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@ofborg ofborg bot requested a review from pSub Nov 10, 2019
@lopsided98 lopsided98 force-pushed the lopsided98:wxwidgets-gtk3 branch from 366d73a to 44bbeef Nov 10, 2019
@ofborg ofborg bot requested a review from kragniz Nov 10, 2019
@Evils-Devils

This comment has been minimized.

Copy link
Contributor

Evils-Devils commented Nov 10, 2019

i've addressed some of the failures here
i set withGtk2 = true; where it seemed appropriate (where the source doesn't mention gtk3),
though maybe using wxGTK29 is more appropriate for those?

i also defaulted wxGTK31 to gtk3, nothing was negatively affected.

i'm still seeing these failures

  • dolphinEmu; @delroth seems involved in that, perhaps they can offer clarification?
  • prusa-slicer; wxWidgets not found
  • slade, sladeUnstable; should support gtk3 but can't find it
  • wxsqliteplus; is not fixed by setting withGtk2 = true;

and these which are also broken without this PR

  • electricsheep
  • pcsx2
  • salut_a_toi
  • sasview
@delroth

This comment has been minimized.

Copy link
Contributor

delroth commented Nov 10, 2019

Re: dolphinEmu, the UI was migrated from WX to Qt a few years ago. Unfortunately I don't think any stable release has been tagged since that migration to Qt. I would either keep wxGTK2 or drop dolphinEmu stable from nixpkgs in favor of dolphinEmuMaster.

@ofborg ofborg bot requested a review from domenkozar Nov 10, 2019
@lopsided98

This comment has been minimized.

Copy link
Contributor Author

lopsided98 commented Nov 10, 2019

rigsofrods has a similar problem; master supports GTK 3, but there has not been an official release since 2016. It seems like upstream considered 4.8rc5 to be a stable release. I tried to update it, but ran into a bunch of runtime issues (crashes, missing textures), so I think I'll just have it use GTK2 for now.

I don't know how aggressive we should be in switching to GTK3. Should I apply the patches Debian and Fedora have written to add GTK3 support to the packages that don't officially support it?

@worldofpeace

This comment has been minimized.

Copy link
Member

worldofpeace commented Nov 10, 2019

I don't know how aggressive we should be in switching to GTK3. Should I apply the patches Debian and Fedora have written to add GTK3 support to the packages that don't officially support it?

@lopsided98 Absolutely, as a gtk maintainer in nixpkgs I'm always glad to come upon these patches where needed 👍 Though if the package appears completely unused and unmaintained, feel free to remove it in a separate PR.

@Evils-Devils Evils-Devils mentioned this pull request Nov 11, 2019
2 of 10 tasks complete
@Evils-Devils

This comment has been minimized.

Copy link
Contributor

Evils-Devils commented Nov 11, 2019

i've resolved the remaining build failures, still here.

i removed slade-git since it's been surpassed by slade stable, a separate PR may be in order, but i'd rather not introduce a failing build with this one and fixing this just to remove it seem silly.

i also added wrapGAppsHook to most since they otherwise aren't usable without a gnome DE

remaining failures (also fail on master)
electricsheep pcsx2 salut_a_toi sasview

and 110 packages were build aegisub audacity boinc bossa cl couchdb couchdb2 cubicsdr curaByDagoma cura_stable cuter diff-pdf digikam displaycal dvdstyler ejabberd elixir elixir_1_5 elixir_1_6 elixir_1_7 elixir_1_8 erlang erlangR18 erlangR19 erlangR20 erlangR21 erlang_javac erlang_odbc erlang_odbc_javac far2l filezilla fityk flamerobin gnss-sdr gnuradio gr-ais gr-gsm gr-limesdr gr-nacl gr-osmosdr gr-rds gnuradio-with-packages golly golly-beta gqrx grass hex2nix hugin inspectrum kicad kicad-with-packages3d lenmus limesuite loxodo lutris lutris-free mercury metamorphose2 mmex notmuch-bower odamex opencpn perl528Packages.AlienWxWidgets perl528Packages.Wx perl528Packages.WxGLCanvas perl530Packages.AlienWxWidgets perl530Packages.Wx perl530Packages.WxGLCanvas playonlinux plover.stable poedit printrun prusa-slicer pwsafe python27Packages.pyspread python27Packages.robotframework-ride python27Packages.runsnakerun python27Packages.wxPython qradiolink rabbitmq-server radiotray-ng rebar rebar3 relxExe rtl_433 saga scyther sdrangel slade slic3r soapysdr-with-plugins torchat treesheets truecrypt tsung urh welle-io wings winpdb winusb wxGTK30 wxGTK31 wxSVG wxhexeditor wxmaxima wxsqlite3 wxsqliteplus xylib yaws zeroad
@c0bw3b c0bw3b mentioned this pull request Nov 12, 2019
7 of 10 tasks complete
@c0bw3b

This comment has been minimized.

Copy link
Contributor

c0bw3b commented Nov 13, 2019

@lopsided98 you can drop audacity commit since it was done in #73055

@lopsided98 lopsided98 force-pushed the lopsided98:wxwidgets-gtk3 branch from d074ede to 390dc49 Nov 14, 2019
@ofborg ofborg bot added the 8.has: clean-up label Nov 14, 2019
@lopsided98 lopsided98 force-pushed the lopsided98:wxwidgets-gtk3 branch from 390dc49 to 96c695d Nov 14, 2019
@ofborg ofborg bot requested a review from sikmir Nov 14, 2019
lopsided98 and others added 6 commits Nov 10, 2019
also fix the file chooser

remaining issues
in nix-shell --pure
  fails with a "Switching prusaSlicer to language en_US failed" popup
  and "dbind-WARNING AT-SPI: error retrieving accessibility bus address"
@lopsided98 lopsided98 force-pushed the lopsided98:wxwidgets-gtk3 branch from 96c695d to ba1b37b Nov 14, 2019
@lopsided98

This comment has been minimized.

Copy link
Contributor Author

lopsided98 commented Nov 14, 2019

I think that is everything except for golly-beta. The only packages left using wxGTK with GTK 2 are dolphinEmu and rigsofrods. Both will be fixed next time there is a new upstream release.

In the case of rigsofrods, I am experimenting with packaging 0.4.8rc5, which supports GTK 3 and seems to be considered stable by the upstream project. They appear to be planning to switch to a date based versioning scheme and make more frequent releases (RigsOfRods/rigs-of-rods#2415), although I'm not a fan of their choice to automatically set the version to the build date (RigsOfRods/rigs-of-rods#2422). The version currently packaged has some visual bugs (ground textures not rendering) that make many maps almost unplayable, and the problems are even worse with the latest version. These bugs are due to some kind of build issue, but I haven't figured it out yet. If I solve this problem, I will make a separate PR.

I modified some of @Evils-Devils commits to take into account @worldofpeace's comment here. I don't know much about GNOME packaging, and adding different GNOME packages as dependencies does not seem to have much effect on the non-NixOS system I am using for testing, so I would like someone to review them. I did notice that using wrapGAppsHook causes the applications to not respect my theme choice.

I would like to remove golly-beta if possible, but I don't know there is a reason to keep it. cc @7c6f434c, who added it.

I am also ccing the maintainers of all packages I touched who were not automatically requested for review by ofborg:
prusa-slicer: @thorstenweber83
slade-git: @ertes
rigsofrods: @7c6f434c
poedit: @bignaux

@lopsided98 lopsided98 marked this pull request as ready for review Nov 14, 2019
@lopsided98 lopsided98 requested review from FRidh and jonringer as code owners Nov 14, 2019
@worldofpeace

This comment has been minimized.

Copy link
Member

worldofpeace commented Nov 14, 2019

I modified some of @Evils-Devils commits to take into account @worldofpeace's comment here. I don't know much about GNOME packaging, and adding different GNOME packages as dependencies does not seem to have much effect on the non-NixOS system I am using for testing, so I would like someone to review them. I did notice that using wrapGAppsHook causes the applications to not respect my theme choice.

As I just happen to be a Gnome maintainer I'd be happy to do this 👍

Copy link
Member

worldofpeace left a comment

You should probably add wrapGAppsHook to all the applications.

pkgs/development/libraries/wxsqliteplus/default.nix Outdated Show resolved Hide resolved
@7c6f434c

This comment has been minimized.

Copy link
Member

7c6f434c commented Nov 14, 2019

OK to drop golly-beta now that it is way behind the stable release.
OK whatever happens with rigsofrods, it never was packaged in a fully stable way, so if it builds and starts, it's not a big regression.

Evils-Devils and others added 4 commits Nov 11, 2019
build reports as gtk1, source doesn't mention gtk3

wrap to get file chooser

remaining nitpicks from nix-shell --pure
  dbind-WARNING: AT-SPI: Error retrieving accessibility bus address
  dconf-WARNING: failed to commit changes to dconf
this seems to have be done in anticipation of the 3.1.2 release
which has come and gone
  stable is at 3.1.7 and master is only 6 commits ahead
@lopsided98 lopsided98 force-pushed the lopsided98:wxwidgets-gtk3 branch from ba1b37b to 634d282 Nov 14, 2019
@lopsided98

This comment has been minimized.

Copy link
Contributor Author

lopsided98 commented Nov 14, 2019

Is wrapGAppsHook only needed for GTK 3 applications? If we don't apply it to all applications that use wxGTK, will this PR cause regressions for the applications that used to use GTK 2? It seems infeasible to fix all the applications affected by this PR right now, since there are over 100 of them.

@Evils-Devils

This comment has been minimized.

Copy link
Contributor

Evils-Devils commented Nov 19, 2019

i switched my branch to lopsided's, rebased on nixos-unstable and tried to execute the rebuild packages via nix-review

109 rebuild aegisub audacity boinc bossa cl couchdb couchdb2 cubicsdr curaByDagoma cura_stable cuter diff-pdf digikam displaycal dvdstyler ejabberd elixir elixir_1_5 elixir_1_6 elixir_1_7 elixir_1_8 erlang erlangR18 erlangR19 erlangR20 erlangR21 erlang_javac erlang_odbc erlang_odbc_javac far2l filezilla fityk flamerobin gnss-sdr gnuradio gr-ais gr-gsm gr-limesdr gr-nacl gr-osmosdr gr-rds gnuradio-with-packages golly gqrx grass hugin inspectrum kicad kicad-with-packages3d lenmus limesuite loxodo lutris lutris-free mercury metamorphose2 mmex notmuch-bower odamex opencpn perl528Packages.AlienWxWidgets perl528Packages.Wx perl528Packages.WxGLCanvas perl530Packages.AlienWxWidgets perl530Packages.Wx perl530Packages.WxGLCanvas playonlinux plover.stable poedit printrun prusa-slicer pwsafe python27Packages.pyspread python27Packages.robotframework-ride python27Packages.runsnakerun python27Packages.wxPython qradiolink rabbitmq-server radiotray-ng rebar rebar3 relxExe rtl_433 saga scyther sdrangel slade slic3r soapysdr-with-plugins torchat treesheets truecrypt tsung urh welle-io wings winpdb winusb wxGTK30 wxGTK31 wxSVG wxhexeditor wxmaxima wxsqlite3 wxsqliteplus xchm xylib yaws zeroad
28 with file chooser / GSettings-schema crash aegisub audacity bossa displaycal dvdstyler filezilla fityk golly hugin kicad lenmus limesuite loxodo metamorphose2 mmex opencpn plover pwsafe python27Packages.runsnakerun saga scyther slic3r treesheets winpdb wxmaxima xchm xylib zeroad
4 with the gtk2 file chooser? digikam qradiolink sdrangel urh
5 fail with "no module" no module named wx:
curaByDagorna playonlinux torchat
no module named gtk:
gnuradio-companion
no module named _multiarray_umath:
cura
5 seem to work far2l lutris poedit slade wxsqliteplus
3 seem to work due to not using/needing a file chooser boinc welle-io radiotray-ng
3 seem messed up without this PR as well notmuch-bower odamex flamerobin
4 unchanged build failures unrelated to this PR electricsheep pcsx2 salut_a_toi sasview
the rest i couldn't test on their own couchdb couchdb2 cubicsdr cuter diff-pdf ejabberd elixir* erlang* gnss-sdr gr-* gqrx grass inspectrum mercury perl5* printrun prusa-slicer python27* rabbitmq-server rebar rebar3 relxExe rtl_433 soapysdr-with-plugins truecrypt tsung wings winusb wxGTK3* wxSVG wxhexeditor wxsqlite3 yaws

@worldofpeace since most of the packages using wxGTK30 with gtk3 seem to be affected, is there a more systemic way to treat this rather than to patch each of them?

@sikmir
sikmir approved these changes Nov 19, 2019
@worldofpeace

This comment has been minimized.

Copy link
Member

worldofpeace commented Nov 19, 2019

when you get
No GSettings schemas are installed on the system

see https://nixos.org/nixpkgs/manual/#ssec-gnome-common-issues and the rest of the GNOME docs.

It needs wrapGAppsHook in nativeBuildInputs and some other package that provides the gsettings schema. I'm guessing it's probably the filechooser so you'll want wxGTK.gtk.
(also note glib is needed for its hook too)

@lopsided98 The gtk2 version of the apps maybe used GConf, so yes, doing I've instructed in the previous comment is the only way to land this PR without regressions.

It also seems @Evils-Devils has done a great job of testing it as well.

Copy link
Member

worldofpeace left a comment

See above comment.

@worldofpeace

This comment has been minimized.

Copy link
Member

worldofpeace commented Nov 19, 2019

is there a more systemic way to treat this rather than to patch each of them

No, that's simply how packaging gtk3 apps works in nixpkgs. It all revolves around that setup hook.

@Evils-Devils Evils-Devils mentioned this pull request Nov 26, 2019
18 of 26 tasks complete
@worldofpeace worldofpeace mentioned this pull request Dec 5, 2019
1 of 10 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.