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

ibus-engines.table: fix after update of settings infrastructure #61978

Merged
merged 3 commits into from Aug 20, 2019

Conversation

laMudri
Copy link
Contributor

@laMudri laMudri commented May 23, 2019

Motivation for this change

Fixes #56621. Adds a wrapper to deal with gsettings, and includes a hack to make things actually work (postFixup).

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 nix-review --run "nix-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@laMudri
Copy link
Contributor Author

laMudri commented Jun 1, 2019

@jtojnar As probably the only person following this, can you comment?

@jtojnar
Copy link
Contributor

jtojnar commented Jun 1, 2019

Hmm, I would expect the standard constructor to work: https://github.com/ibus/ibus/blob/82a728de97308f6ce36c814e30a292f96e76cc13/bindings/pygobject/gi/overrides/IBus.py#L155-L174

Perhaps adding ibus to (python3.withPackages (pypkgs: with pypkgs; [ pygobject3 ])) might help.

@jtojnar
Copy link
Contributor

jtojnar commented Jun 1, 2019

One more thing I noticed, during the build:

/nix/store/mkbq9i2m8qva6s8wnw1c9yjg4zsb86p5-glib-2.60.1-dev/nix-support/setup-hook: line 4: [: /nix/store/airn74yfk0zbb3r9ym10w93inff80rwh-ibus-with-plugins-1.5.20/share/gsettings-schemas/ibus-1.5.20/glib-2.0/schemas: binary operator expected

Apparently, there are multiple schema directories:

$ file /nix/store/airn74yfk0zbb3r9ym10w93inff80rwh-ibus-with-plugins-1.5.20/share/gsettings-schemas/*/glib-2.0/schemas
/nix/store/airn74yfk0zbb3r9ym10w93inff80rwh-ibus-with-plugins-1.5.20/share/gsettings-schemas/ibus-1.5.20/glib-2.0/schemas:       directory
/nix/store/airn74yfk0zbb3r9ym10w93inff80rwh-ibus-with-plugins-1.5.20/share/gsettings-schemas/ibus-table-1.9.21/glib-2.0/schemas: directory

Which trips up glib setup hook:

if [ -d "$1"/share/gsettings-schemas/*/glib-2.0/schemas ]; then

@jtojnar
Copy link
Contributor

jtojnar commented Jun 1, 2019

Hmm, I am still getting #56621 (comment) when I click Input Method (IPA-X-SAMPA) Preferences in ibus-setup.

@jtojnar
Copy link
Contributor

jtojnar commented Jun 1, 2019

But yeah, at least it works:

image

@jtojnar
Copy link
Contributor

jtojnar commented Jun 1, 2019

Hmm, I would expect the standard constructor to work: ibus/ibus:bindings/pygobject/gi/overrides/IBus.py@82a728d#L155-L174

Perhaps adding ibus to (python3.withPackages (pypkgs: with pypkgs; [ pygobject3 ])) might help.

And that will not be enough since we are not actually building overrides in ibus.

@jtojnar
Copy link
Contributor

jtojnar commented Jun 1, 2019

The following should work

--- a/pkgs/tools/inputmethods/ibus/default.nix
+++ b/pkgs/tools/inputmethods/ibus/default.nix
@@ -130,6 +130,7 @@
     dconf
     gdk_pixbuf
     gobject-introspection
+    python3.pkgs.pygobject3 # for pygobject overrides
     gtk2
     gtk3
     isocodes
@@ -140,6 +141,11 @@
     wayland
   ];
 
+  makeFlags = [
+    # build does not make the path relative to prefix
+    "pyoverridesdir=${placeholder "out"}/${python3.sitePackages}/gi/overrides"
+  ];
+
   enableParallelBuilding = true;
 
   doCheck = false; # requires X11 daemon

see https://github.com/ibus/ibus/blob/82a728de97308f6ce36c814e30a292f96e76cc13/configure.ac#L432 and https://github.com/ibus/ibus/blob/82a728de97308f6ce36c814e30a292f96e76cc13/bindings/pygobject/Makefile.am#L35

but it does not:

 /nix/store/bv1lw6a2kw0mn2y3lxhi43180idx6sp9-coreutils-8.31/bin/mkdir -p '/nix/store/pa0s4y6dw95zlbqp3ysfbq2vfwjd470h-python3.7-pygobject-3.32.1/lib/python3.7/site-packages/gi/overrides'
 /nix/store/bv1lw6a2kw0mn2y3lxhi43180idx6sp9-coreutils-8.31/bin/install -c -m 644 gi/overrides/IBus.py '/nix/store/pa0s4y6dw95zlbqp3ysfbq2vfwjd470h-python3.7-pygobject-3.32.1/lib/python3.7/site-packages/gi/overrides'
/nix/store/bv1lw6a2kw0mn2y3lxhi43180idx6sp9-coreutils-8.31/bin/install: cannot create regular file '/nix/store/pa0s4y6dw95zlbqp3ysfbq2vfwjd470h-python3.7-pygobject-3.32.1/lib/python3.7/site-packages/gi/overrides/IBus.py': Permission denied

Edit: the issue is that we still build Python 2 library so we actually need to override py2overridesdir:

https://github.com/ibus/ibus/blob/82a728de97308f6ce36c814e30a292f96e76cc13/configure.ac#L436

https://github.com/ibus/ibus/blob/82a728de97308f6ce36c814e30a292f96e76cc13/bindings/pygobject/Makefile.am#L29

@jtojnar jtojnar mentioned this pull request Jun 1, 2019
10 tasks
@laMudri
Copy link
Contributor Author

laMudri commented Jun 1, 2019

Thanks for working on this! I'll test it all out and work out how to organise the commits. I guess it doesn't really matter between making a new commit for these changes vs modifying the old commit.

@laMudri
Copy link
Contributor Author

laMudri commented Jun 1, 2019

Can confirm the same behaviour. Input methods fundamentally work. Preferences for ibus-table via ibus-setup interface do not show. Preferences do, however, work by clicking the tray icon and choosing “Setup”.

@jtojnar
Copy link
Contributor

jtojnar commented Jun 1, 2019

Yeah, that probably has something to do with the /usr paths #56621 (comment)

Looking at https://github.com/mike-fabian/ibus-table/blob/7bf7a90d189364678a53aad1149246b5bff3a2e8/engine/ibus_table_location.py#L52, I wonder how does it even work when we are unsetting the env vars. I would expect a crash.

laMudri and others added 2 commits August 12, 2019 10:48
Fixes NixOS#56621. Adds a wrapper to deal with gsettings, and includes a hack to make
things actually work (postFixup).
Co-Authored-By: Jan Tojnar <jtojnar@gmail.com>
@laMudri
Copy link
Contributor Author

laMudri commented Aug 19, 2019

Sorry for leaving this for a while. After a quick test, I'm sufficiently convinced that we don't need the postFixup, so I have removed it.

@laMudri
Copy link
Contributor Author

laMudri commented Aug 20, 2019

This should be good to go now, right? Unless anyone else is wanting to test too

@jtojnar jtojnar merged commit 6aac40d into NixOS:master Aug 20, 2019
@jtojnar
Copy link
Contributor

jtojnar commented Aug 20, 2019

Thanks. If I recall correctly, there were still some issues with the preferences but this should at least fix the breakage that was preventing ibus-table from working at all.

@laMudri
Copy link
Contributor Author

laMudri commented Aug 20, 2019

I'll have another look at that, because I think everything's working fine on my machine.

@laMudri laMudri mentioned this pull request Aug 20, 2019
10 tasks
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.

ibus-table needs gsettings
3 participants