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

fcitx: fix fcitx-qt5 attribute path #23411

Merged
merged 1 commit into from Mar 4, 2017
Merged

Conversation

ericsagnes
Copy link
Contributor

Motivation for this change

Fixes #23395

qt5.fcitx-qt5 path changed to libsForQt5.fcitx-qt5. This reflects this change in the fcitx wrapper.

Tested in a i3 nixos vm, with mozc and chewing engines on Qt5 applications.

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

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

@sifmelcara
Copy link
Member

sifmelcara commented Mar 3, 2017

I tested this pr and can confirm this solves #23395

@ttuegel
Copy link
Member

ttuegel commented Mar 3, 2017

Does fcitx-qt5 provide any shared objects besides a plugin? If not, it should be moved out of libsForQt5 into the top level because it only needs to be built with the latest Qt version. (Qt plugin ABIs are backward-compatible.)

@ericsagnes
Copy link
Contributor Author

@ttuegel Thanks for looking into this!

Does fcitx-qt5 provide any shared objects besides a plugin?

As far as I know it is only a plugin.
But, I remember there was an incompatibility with Qt 5.5 applications as pointed in #15569 (comment) that motivated the move of fcitx-qt5 in qt5LibsFun.

If not, it should be moved out of libsForQt5 into the top level because it only needs to be built with the latest Qt version.

I have not followed the recent changes of the Qt structure in nixpkgs, but will that ensure that #15569 like incompatibilities will not happen anymore?

@ttuegel
Copy link
Member

ttuegel commented Mar 4, 2017

But, I remember there was an incompatibility with Qt 5.5 applications as pointed in #15569 (comment) that motivated the move of fcitx-qt5 in qt5LibsFun.

Qt plugins are backward ABI-compatible, i.e. a plugin built with Qt 5.8 will work with any version of Qt 5 <= 5.8. The only reason this would fail is if the plugin uses private Qt headers, and I recall that is the case here. Putting it in libsForQt5 doesn't really solve the problem because a user can only have one version installed at a time: either their Qt 5.5 applications work, or their Qt 5.6 applications, but never both at the same time. The only real solution here is to file a bug upstream: the package is broken by design. Until there is a better resolution, though, I think you're right to keep it in libsForQt5.

@ttuegel ttuegel merged commit ca8edb7 into NixOS:master Mar 4, 2017
@ericsagnes
Copy link
Contributor Author

Thanks for merging and the explanations.
I will look at the upstream package and try to submit a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants