-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
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
qt: include qttranslations properly #246022
Conversation
Also something like this will also need to be done for Qt6, I just started looking into 5 because the original reported issue was that Plasma isn't handling RTL correctly. |
Drafting for now to clean up the workarounds all over the tree. |
Verified all the affected packages build and run except stellarium (still building qtwebengine-6) and valentina (builds, but crashes on startup, same on master). |
Fixed qt6.qtwebengine and Stellarium. |
Can we skip the bootstrapping like just build qttranslations without qtbase? |
No, it needs qttools to actually generate the final files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy with the changes. Waiting for others' opinions.
502d9fb
to
6b7235e
Compare
@@ -15,6 +15,7 @@ | |||
# optional dependencies | |||
, cups ? null, postgresql ? null | |||
, withGtk3 ? false, dconf, gtk3 | |||
, qttranslations ? null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this ? null needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it will break baseScope but I moved things around a bit since the last time I tried. I'll have to double check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't tested but the diff seems fine. I don't know that much about the scopes part, but presumably it's now tested and at least fixes some of the LTR/RTL issues
@ofborg build qt6.qtwebengine |
Qt loads its own translations from a hardcoded path, and those are used (among other things) for determining RTL layout preferences in applications, so they are definitely something we want to have. This adds a qtbase/qttools rebuild to the chain, but it's fast enough that it's probably fine. Another approach would be to load translation paths from the environment, and inject it in wrapQtAppsHook, but that seems like more complexity for very questionable build time savings.
There are two kinds of changes here: - removing explicit qttranslations path hardcoding from applications that were patched to do it - replacing qttranslations in buildInputs with qttools for packages that really depend on the latter After this, qttranslation is never used outside Qt itself, as it should.
I'm just going to merge this before we accumulate more treewide horkage. |
Since this change, it's not possible to use qt5.qttranslations, or anything that depends on it, in
This worked before this change, and is orthogonal to cross compiling qt itself — |
Description of changes
Qt loads its own translations from a hardcoded path, and those are used (among other things) for determining RTL layout preferences in applications, so they are definitely something we want to have.
This adds a qtbase/qttools rebuild to the chain, but it's fast enough that it's probably fine.
Another approach would be to load translation paths from the environment, and inject it in wrapQtAppsHook, but that seems like more complexity for very questionable build time savings.
Fixes #86054. Fixes #240101.
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)