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

zoom-us: unset Qt env variables to fix dialog boxes #116355

Merged
merged 3 commits into from Mar 15, 2021
Merged

Conversation

Yarny0
Copy link
Contributor

@Yarny0 Yarny0 commented Mar 14, 2021

Motivation for this change

Zoom wouldn't show the "Participants" dialog box when used in a plasma environment; precisely, the dialog box failed to show its content. The problem doesn't exist in other environments like Gnome or Xfce. Experiments have shown that clearing the environment variable QML2_IMPORT_PATH before calling Zoom fixes the issue.

I suspect the reason to be as follows: While the zoom build recipe is called with libsForQt5xx.callPackage, putting qttools.dev in zoom's PATH is the only connection to nixpkgs' Qt ecosystem. Zoom brings its own Qt libraries. Hence it seems to be a good idea to shield zoom from access to nixpkgs' Qt files to avoid problems from version mismatch or similar troubles. So the commit at hand expands zoom's wrapper script to clear the Qt-related enviornemt variables QML2_IMPORT_PATH and QT_PLUGIN_PATH.

Original issue report, with some discussion: #107495 (comment)

Notify maintainers: @danbst @tadfisher @doronbehar
Notify reporter: @miniBill

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.

More:

  • Tested meeting with video on current nixos-unstable channel, in Xfce and in Plasma (inside VirtualBox): participants lists shows up correctly
  • Tested meeting with video and audio on NixOS 20.09, in Xfce and in Plasma (Zoom package build with all dependencies from nixos-unstable, surrounding system NixOS 20.09): participants lists shows up correctly

Zoom wouldn't show the "Participants"
dialog box when used in a plasma environment;
precisely, the dialog box failed to show its content.
The problem doesn't exist in other
environments like Gnome or Xfce.
Experiments have shown that clearing the environment variable
`QML2_IMPORT_PATH` before calling Zoom fixes the issue.

I suspect the reason to be as follows:
While the zoom build recipe is called with
`libsForQt5xx.callPackage`, putting `qttools.dev` in zoom's
`PATH` is the only connection to nixpkgs' Qt ecosystem.
Zoom brings its own Qt libraries.
Hence it seems to be a good idea to shield
zoom from access to nixpkgs' Qt files to avoid
problems from version mismatch or similar troubles.
So the commit at hand expands zoom's wrapper script
to clear the Qt-related enviornemt variables
`QML2_IMPORT_PATH` and `QT_PLUGIN_PATH`.

Original issue report, with some discussion:
NixOS#107495 (comment)
Copy link
Contributor

@doronbehar doronbehar left a comment

Choose a reason for hiding this comment

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

I suspect the reason to be as follows: While the zoom build recipe is called with libsForQt5xx.callPackage, putting qttools.dev in zoom's PATH is the only connection to nixpkgs' Qt ecosystem. Zoom brings its own Qt libraries. Hence it seems to be a good idea to shield zoom from access to nixpkgs' Qt files to avoid problems from version mismatch or similar troubles. So the commit at hand expands zoom's wrapper script to clear the Qt-related enviornemt variables QML2_IMPORT_PATH and QT_PLUGIN_PATH.

Makes sense to me. I'd like to add though, that this issue is sourced from the fact that in general, the environment of any Plasma user is impure by default in terms of environment variables, and they leak in this case in a manner that's harmful for this application's functionality.

Speaking of qttools, It's possible it can be removed and we can resolve to using normal callPackage - it was added to the $PATH of the wrapper in 2018 in #43426 .

this is a left-over when we used nixpkgs's qt instead of the bundled version
@Mic92 Mic92 merged commit dc4f0a1 into NixOS:master Mar 15, 2021
@Mic92
Copy link
Member

Mic92 commented Mar 15, 2021

I think this is safe for backporting. Please ping me again when this has hit the nixpkgs unstable channels.

@miniBill
Copy link
Contributor

Thank you @Yarny0, I really appreciate you taking the time to investigate and create a fix

@Yarny0
Copy link
Contributor Author

Yarny0 commented Mar 18, 2021

@Mic92

Thanks for merging!

Mic92:

I think this is safe for backporting. Please ping me again when this has hit the nixpkgs unstable channels.

This is now part of nixos-unstable channel https://hydra.nixos.org/build/139121498 .

Re backporting: I tested this pull request's changes in NixOS 20.09 in Plasma5 desktop environment, without problems (participants list shows up, audio and video working)

@Mic92
Copy link
Member

Mic92 commented Mar 19, 2021

@Yarny0 there you go: #116821

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.

None yet

4 participants