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

Jump to bottom

birdtray: fix qttranslations path #90585

Merged
merged 1 commit into from Jul 13, 2020
Merged

birdtray: fix qttranslations path #90585

merged 1 commit into from Jul 13, 2020

Conversation

ymarkus
Copy link
Contributor

@ymarkus ymarkus commented Jun 16, 2020

Motivation for this change
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 nixpkgs-review --run "nixpkgs-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.

@ymarkus
birdtray: fix qttranslations path
fd474c0 
birdtray crashes not finding the translations without this fix
@ofborg ofborg bot requested a review from Flakebi June 16, 2020 07:06
@Flakebi
Copy link
Member

Flakebi commented Jun 16, 2020

Thanks, builds and runs.
I just tried the version without the fix (/nix/store/q85idskz8qizkyxxrghcf66fy5nija0w-birdtray-1.8.1) and env LC_ALL=de birdtray correctly works and shows translations too?

@ymarkus
Copy link
Contributor Author

ymarkus commented Jun 16, 2020

I just tested on 20.09pre228622.029a5de0839 (Nightingale):

Without env:

[nix-shell:~]$ birdtray 
Using Wayland-EGL
Segmentation fault (core dumped)

With:

[nix-shell:~]$ env LC_ALL=de birdtray
Using Wayland-EGL
Segmentation fault (core dumped)

This is what the log shows (run with option -l):

2020-06-16 10:33:31 Birdtray version 1.8.1 started 
2020-06-16 10:33:31 Failed to load translation for de_DE

The log shows the same message for any locale.

According to the issue I linked, this is the expected behavior without the patch.

@Flakebi
Copy link
Member

Flakebi commented Jun 16, 2020

It doesn’t work on wayland anyway, neither with, nor without the patch :)
I always test with env QT_QPA_PLATFORM= WAYLAND_DISPLAY= birdtray there.

Ah, I didn’t know about -l. I also get Failed to load translation for de_DE but it still displays everything in german (without the patch), so I guess it fails to load translations that are used somewhere else?

@ymarkus
Copy link
Contributor Author

ymarkus commented Jun 16, 2020

I just found out it does launch on wayland!
Just prefix QT_QPA_PLATFORM="xcb" when running, then it works!

Yeah, I'm not sure which translations are the problem, but this patch removes the error, so I guess it is needed?

Flakebi
Flakebi approved these changes Jun 16, 2020
View reviewed changes
Copy link
Member

@Flakebi Flakebi left a comment

Choose a reason for hiding this comment

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

Yeah, I'm not sure which translations are the problem, but this patch removes the error, so I guess it is needed?

I guess it could fix some translations then, so thanks again!

@ofborg ofborg bot requested a review from Flakebi June 17, 2020 06:53
@Mic92
Copy link
Member

Mic92 commented Jun 17, 2020

I just found out it does launch on wayland!
Just prefix QT_QPA_PLATFORM="xcb" when running, then it works!

Yeah, I'm not sure which translations are the problem, but this patch removes the error, so I guess it is needed?

This likely enforces xwayland.

@ymarkus
Copy link
Contributor Author

ymarkus commented Jun 17, 2020

Sorry everyone, I forgot to checkout a new branch. I'm still a bit new to this and learning! Rebased my commits and now it looks clean again...
Hope I didn't cause any trouble.

Mic92
Mic92 reviewed Jun 17, 2020
View reviewed changes
pkgs/applications/misc/birdtray/fix-qttranslations-path.diff
QLocale locale = QLocale::system();
bool success = loadTranslation(
- qtTranslator, locale, "qt", {QLibraryInfo::location(QLibraryInfo::TranslationsPath)});
+ qtTranslator, locale, "qt", {QLatin1String("@qttranslations@/translations")});
Copy link
Member

Choose a reason for hiding this comment

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

This works, however I was wondering if there was a more general way for qt?

cc @ttuegel @schmittlauch

Copy link
Member

Choose a reason for hiding this comment

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

Not really, this is an unresolved issue.

ttuegel
ttuegel approved these changes Jul 13, 2020
View reviewed changes
pkgs/applications/misc/birdtray/fix-qttranslations-path.diff
QLocale locale = QLocale::system();
bool success = loadTranslation(
- qtTranslator, locale, "qt", {QLibraryInfo::location(QLibraryInfo::TranslationsPath)});
+ qtTranslator, locale, "qt", {QLatin1String("@qttranslations@/translations")});
Copy link
Member

Choose a reason for hiding this comment

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

Not really, this is an unresolved issue.

@ttuegel ttuegel merged commit 64b395e into NixOS:master Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mic92 Mic92 Mic92 left review comments

@ttuegel ttuegel ttuegel approved these changes

@Flakebi Flakebi Awaiting requested review from Flakebi

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ymarkus @Flakebi @Mic92 @ttuegel @Janik-Haag