-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
qt5*: Enforce strict compatible version paths #65526
qt5*: Enforce strict compatible version paths #65526
Conversation
Possible improvements:
|
We should just make these fail at build-time to save everyone a lot of headaches. This is going to break the following use-case: the user has Qt applications installed in their user and system profiles, then they update the system but not user profile. The user profile applications will not be able to find input methods, themes, etc. anymore. This case works right now, as long as all the version numbers go in the right direction; it's admittedly fragile. Here are my controversial reasons we may not care about this breakage:
|
At least for the input method, I found out that Qt is able to resolve the one I use. I assume, though, it may not end up being true for all of them, and for all things.
I don't think it'd be enough; I think any file access would need to be checked. Isn't there more than
|
It looks like you are using I-Bus, right? I-Bus is built into Qt. I would be very impressed if you broke it 😀 I would not have a problem if we wanted to say "I-Bus is the only officially-supported input method for Qt," but we have others (at least fcitx) in Nixpkgs.
Qt only detects version mismatches in the shared object files. The mismatch only occurs because of the order that |
this PR plus my changes to wireshark (to use the new Qt mechanism) fixed my issue. |
Should we change this to go into |
7fade87
to
f61beba
Compare
f61beba
to
6cb99e7
Compare
(Oops, I tried to use the @ttuegel the commits are now rebased on the merge-base of staging/master; it can be switched as needed. |
Considering Qt says:
Why is this even an issue in the first place? |
Sorry for the totally unprofessional response, but ¯\_(ツ)_/¯ I think @ttuegel has a better handle on the explanation for that. It may be that specifics to NixOS and the way it load both Qt versions at the same time is the issue, if it loaded any one (and only one) of the patch-level versions at once, I guess it would be fine? Though, as I initially stated, I don't exactly know. I'm working off of evidence that (1) it's an issue with Qt as packaged withing Nixpkgs, (2) it demonstrably fixes the issue with this PR and (3) missing the wrapper decisively breaks instead of attempting to use a compatible other-patchlevel Qt from the environment. |
I'm not at all disputing that there is an issue - and this does seem to be an elegant way to fix it. I just can't help think that there's something we're missing. |
Oh, I wasn't aggravated or anything. I, too, am wondering the exact reasons this acts as it does. I would rather know the details to better understand the issue, than using only the observed effects. |
Although applications and plugins are compatible across patch-level versions, all of the parts of Qt itself need to be of the same version exactly. So, if an application compiled with Qt 5.12.3 tries to load a plugin compiled with Qt 5.12.0, its RUNPATH will be contaminated and it might load mis-matched Qt libraries. This isn't an issue with other distributions because they build the entire Qt installation at once and they have one canonical path ( Another way we could deal with this problem is to compile a monolithic Qt (so there is one canonical path where the Qt libraries are) and use |
I'm getting this problem in 2023, for when I use stable QT, but try to use |
Motivation for this change
Ensure we don't see the following error anymore:
This PR forces the path to use the minor version (5.major.minor) in the plugin paths, ensuring that it's improbable for Qt to look into an incompatible minor version's path for plugins. This builds upon discoveries made in #44047, see the end of the initial PR body.
This was tested on top of eb4e067, which has Qt 5.12.3. This was tested running on a 19.03 system which has 5.12.0-reliant software installed in the system.
This was verified using
multimc
(EDIT) With #65486 applied!This was verified by using
strace
and comparing what it tries to load, and validating it loads the equivalent right library for 5.12.3.(EDIT) Additionally, missing wrapQtAppsHook now fails more decisively at runtime, rather than using whatever traces could be found in the environment. This is both an advantage and a drawback. On one hand, it will force
wrapQtAppsHook
to happen to every Qt packages, ensuring they stay functional with mismatched versions. On the other hand, it will break a bunch of Qt apps at runtime, until every one of them is fixed. I think the advantage far outweighs the drawback.Good news, Qt is able to load the right 5.12.3 input method by name, as I thought while experimenting.
And since Qt now looks for
5.12.3
, instead of5.12
, in plugin paths, we don't have to care that much for the impure environment caused by PATH, as even if it's shock full of incompatible minor versions, Qt will not look for them.Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)ref #65399
cc @FRidh who reverted the 5.12.3 bump
cc @ttuegel