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

skypeforlinux: Add GSettings schemas #44652

Merged
merged 5 commits into from Aug 19, 2018
Merged

skypeforlinux: Add GSettings schemas #44652

merged 5 commits into from Aug 19, 2018

Conversation

zgrannan
Copy link
Contributor

@zgrannan zgrannan commented Aug 8, 2018

Motivation for this change

Attempting to upload an attachment in Skype causes it to crash, with the error:

No GSettings schemas are installed on the system

These changes should resolve that issue.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Fits CONTRIBUTING.md.

Copy link
Contributor

@jtojnar jtojnar left a comment

Choose a reason for hiding this comment

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

Try using wrapGAppsHook instead.

@zgrannan
Copy link
Contributor Author

zgrannan commented Aug 9, 2018

@jtojnar I tried adding wrapGAppsHook to nativeBuildInputs but the error still remained. It looks like the underlying reason is that wrapGAppsHook operates in the fixup phase, but the skypeforlinux executable is moved into $out/bin in postFixup, so it is never wrapped.

There are a couple ways I could see this being resolved:

  1. Alllow passing an argument for extra executables to wrap in wrapGAppsHook
  2. Extract the wrapping code in wrap-gapps-hook.sh to a seperate script, and call that from postFixup in this derivation
  3. Ensure skypeforlinux is placed in $out/bin before the fixup phase (However I couldn't figure out a way to do this)

Or maybe there's some other simple way that I'm missing? I'd be happy to implement any option.

@jtojnar
Copy link
Contributor

jtojnar commented Aug 9, 2018

Yeah, I wanted to do 2. for a long time but never got to i wrap-gapps-hook.

You can still run wrapProgram $somefile "̈́''${gappsWrapperArgs[@]}" in postFixup manually, the gappsWrapperArgs array will be populated by then.

Alternately, you can move the symlink creation to installPhase, which I think is the right thing to do anyway.

@@ -68,6 +68,8 @@ in stdenv.mkDerivation {

inherit src;

nativeBuildInputs = [ gtk3 wrapGAppsHook ];
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, gtk3 should not be needed.

Copy link
Contributor Author

@zgrannan zgrannan Aug 11, 2018

Choose a reason for hiding this comment

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

I thought the same thing, but the error still appears on my machine if gtk3 is not in nativeBuildInputs.

Perhap's it's because I'm not using Gnome DE?

Copy link
Contributor

Choose a reason for hiding this comment

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

Weird. wrapGAppsHook should bring GTK already. Though if you need it here, move it to buildInputs. Just the hook is native.

buildInputs = [ dpkg makeWrapper ];
nativeBuildInputs = [ wrapGAppsHook ];

buildInputs = [ dpkg gtk3 makeWrapper ];
Copy link
Contributor

@jtojnar jtojnar Aug 12, 2018

Choose a reason for hiding this comment

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

Alright, you do not need gtk3 but glib’s setup hooks. Could you add it to nativeBuildInputs with a comment “for setup hook populating GSETTINGS_SCHEMAS_PATH”?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also makeWrapper is not used.

@zgrannan
Copy link
Contributor Author

@jtojnar Let me know if there is any other changes you think are necessary

@jtojnar jtojnar merged commit 8aeac6b into NixOS:release-18.03 Aug 19, 2018
@jtojnar
Copy link
Contributor

jtojnar commented Aug 19, 2018

Pushed to master as 3b2f3e5

jtojnar pushed a commit that referenced this pull request Aug 19, 2018
cherry-picked and squashed from #44652
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

3 participants