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

qt4: enable gtk2 by default #24861

Merged
merged 1 commit into from May 18, 2017
Merged

qt4: enable gtk2 by default #24861

merged 1 commit into from May 18, 2017

Conversation

abbradar
Copy link
Member

Motivation for this change

Analogous to Qt 5 currently. I split GNOME dependencies (we don't need them for most cases). Qt 4 applications look so much better now on XFCE!

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

@abbradar, thanks for your PR! By analyzing the history of the files in this pull request, we identified @vcunat, @edolstra and @urkud to be potential reviewers.

@vcunat
Copy link
Member

vcunat commented Apr 13, 2017

It's perhaps not clear whether to have qt4 depend on gtk2 by default (during runtime). EDIT: currently the added closure size would be ~33 MB.

BTW, gtk2 doesn't need schemas to display file dialogs, I think, so I expect we won't need any wrapping.

@vcunat
Copy link
Member

vcunat commented Apr 13, 2017

Perhaps /cc @ttuegel, too.

@abbradar
Copy link
Member Author

@vcunat Indeed, we don't need any wrapping. My point was that Qt 5 currently depends on GTK 3 for theming (sad but necessary), so we may as well have Qt 4 depend on GTK 2. I don't think there is any other way to make this work D:

@vcunat
Copy link
Member

vcunat commented Apr 13, 2017

Yes, I see no other easy and correct way. We don't yet have a good solution for plugin-like dependencies.

@abbradar
Copy link
Member Author

I'll merge this in few days unless anyone objects.

@abbradar abbradar merged commit 6315076 into NixOS:master May 18, 2017
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.

None yet

3 participants