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

lxqt: fix themes and translations #109324

Merged
merged 1 commit into from Jan 14, 2021
Merged

Conversation

tpwrules
Copy link
Contributor

@tpwrules tpwrules commented Jan 13, 2021

Motivation for this change

LXQt binaries look for their themes and translations based on the name of the binary, which is changed by the wrapper script. This patches liblxqt to recover the original name from the wrapped binary name. This is intended to fix #70780.

I don't know if this is the best or most general solution to the problem. The root cause is that QCoreApplication::applicationFilePath() reads /proc/self/exe to determine the binary name, which points to the renamed wrapped binary, instead of using argv[0], which points to the originally named wrapper script.

I'm not familiar with Qt, so I'm not sure what applications generally expect to use the path for. If it's mostly for locating resources relative to the binary, then I wonder if it would be better to patch Qt to read argv[0]. Consider that because of wrapQtAppsHook, /proc/self/exe will never be the original name of the binary.

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.

@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch)
If you find some bugs or got suggestions for further things to search or run please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 109324 run on x86_64-linux 1

14 packages built:
  • lxqt.liblxqt
  • lxqt.lxqt-about
  • lxqt.lxqt-admin
  • lxqt.lxqt-config
  • lxqt.lxqt-globalkeys
  • lxqt.lxqt-notificationd
  • lxqt.lxqt-openssh-askpass
  • lxqt.lxqt-panel
  • lxqt.lxqt-policykit
  • lxqt.lxqt-powermanagement
  • lxqt.lxqt-runner
  • lxqt.lxqt-session
  • lxqt.lxqt-sudo
  • lxqt.qps

@romildo
Copy link
Contributor

romildo commented Jan 14, 2021

Currently running LXQt with this fix applied. It works well for me.

LXQt binaries look for their themes and translations based on the name of the binary, which is changed by the wrapper script. This patches liblxqt to recover the original name from the wrapped binary name.
@ofborg ofborg bot removed the 6.topic: qt/kde label Jan 14, 2021
@ofborg ofborg bot requested a review from romildo January 14, 2021 17:31
@Mic92 Mic92 merged commit e1ac6eb into NixOS:master Jan 14, 2021
@Mic92
Copy link
Member

Mic92 commented Jan 14, 2021

Result of nixpkgs-review pr 109324 run on x86_64-linux 1

14 packages built:
  • lxqt.liblxqt
  • lxqt.lxqt-about
  • lxqt.lxqt-admin
  • lxqt.lxqt-config
  • lxqt.lxqt-globalkeys
  • lxqt.lxqt-notificationd
  • lxqt.lxqt-openssh-askpass
  • lxqt.lxqt-panel
  • lxqt.lxqt-policykit
  • lxqt.lxqt-powermanagement
  • lxqt.lxqt-runner
  • lxqt.lxqt-session
  • lxqt.lxqt-sudo
  • lxqt.qps

@tpwrules tpwrules deleted the lxqt-fix-locations branch January 14, 2021 18:29
@wamserma
Copy link
Member

+1 for a backport to 20.09

@Mic92
Copy link
Member

Mic92 commented Jan 15, 2021

backported.

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.

LXQt has wrong/invalid defaults
6 participants