ibus-kkc engine support #34416

vanzef · 2018-01-30T12:19:25Z

Motivation for this change

This PR adds support for ibus-kkc IME, based on libkkc.

Things done

Added packages pythonPackages.marisa (python bindings, generated by swig), libkkc, libkkc-data and ibus-kkc.

Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
Fits CONTRIBUTING.md.

dotlambda · 2018-01-30T12:27:51Z

pkgs/development/python-modules/marisa-python/default.nix

+  version = "1.3.40";
+  name = "${pname}-${version}";
+
+  src = fetchgit {


Better use fetchFromGitHub. This will download a tarball instead of cloning the repo.

dotlambda · 2018-01-30T12:27:54Z

pkgs/development/python-modules/marisa-python/default.nix

+buildPythonPackage rec {
+  pname = "marisa-python";
+  version = "1.3.40";
+  name = "${pname}-${version}";


Sorry, I don't get it. Why don't you like 6th line where name is defined?

There's no need for name. It is generated from pname and version by buildPythonPackage.

Got it. Thanks for advice.

dotlambda · 2018-01-30T12:28:53Z

pkgs/development/python-modules/marisa-python/default.nix

+  buildinputs = [ marisa ];
+
+  preBuild = ''
+    cd bindings/python


Better use sourceRoot.

dotlambda · 2018-01-30T12:30:04Z

pkgs/development/python-modules/marisa-python/default.nix

+{ stdenv, buildPythonPackage, fetchgit, marisa, swig }:
+
+buildPythonPackage rec {
+  pname = "marisa-python";


Is there another package repo that calls this marisa-python or why did you choose that name?

At first I thought that it's a good idea since there is a project named marisa-tie which is a handwritten(?) python bindings to C++ marisa library. And this one is the bindings generated by swig.

Now I think that you are right and it's better to rename it to marisa especially since the same package called python2-marisa in Arch Linux repos and we have pythonPackages namespace.

FRidh · 2018-02-11T07:47:47Z

pkgs/data/misc/libkkc-data/default.nix

+  pname = "libkkc-data";
+  version = "0.2.7";
+  name = "${pname}-${version}";
+  libkkc_version = "0.3.5";


This attribute can go in a let ... in expression

Do you mean only libkkc_version? If so -- fixed.

FRidh · 2018-02-11T07:50:04Z

pkgs/development/python-modules/marisa/default.nix

+  sourceRoot = "${src.name}/bindings/python";
+
+  meta = with stdenv.lib; {
+    description = "Python binding for marisa package (do not confuse with marisa-trie python bindings)";


yet the homepage does point to marisa-trie ?

Well, as I mentioned it previously (#34416 (comment)) there are two different bindings for marisa-trie C++ library: one are generated by swig (this package), so the homepage of bindings is the homepage of C++ library, other bindings (also called marisa-trie) are from the different package (you can find them here https://github.com/pytries/marisa-trie/). However, they are incompatible.

At first I packaged the bindings by pytries and called the package marisa-trie, but then I'd found out that I need bindings from s-yata's repo, so I'd decided to give latter the name as for the C++ package.

I agree that the description is pretty misleading, do you have any suggestions for improvement?

FRidh · 2018-02-11T07:51:47Z

pkgs/tools/inputmethods/libkkc/default.nix

@@ -0,0 +1,40 @@
+{ stdenv, fetchurl
+, vala, gobjectIntrospection, intltool, pythonPackages, python, glib


either pythonPackages or python

vanzef · 2018-02-20T09:05:46Z

Should I fix something else?

cc @FRidh

abbradar · 2018-03-10T12:04:10Z

pkgs/tools/inputmethods/ibus-engines/ibus-kkc/default.nix

+
+  configureFlags = [ "--disable-static" ];
+
+  buildInputs = [ libkkc ibus skk-dicts gnome3.gtk3 ];


Don't use gnome3 as an argument; instead just use gtk3.

abbradar · 2018-03-10T12:05:16Z

pkgs/data/misc/libkkc-data/default.nix

+{ stdenv, fetchurl, pythonPackages }:
+
+let
+  libkkc_version = "0.3.5";


Given that you need to keep those versions synchronized with libkkc and it's the only user why not move this directly into libkkc instead?

On another hand you could just use libkkc as an argument and use libkkc.version. Don't worry about circular dependencies -- libkkc.version can be used without instantiating libkkc derivation itself.

abbradar · 2018-03-10T12:05:29Z

pkgs/tools/inputmethods/ibus-engines/ibus-kkc/default.nix

+    vala intltool pkgconfig
+  ];
+
+  configureFlags = [ "--disable-static" ];


Shouldn't be needed.

abbradar · 2018-03-10T12:05:48Z

pkgs/tools/inputmethods/libkkc/default.nix

+
+  propagatedBuildInputs = [ gnome3.libgee json_glib ];
+
+  configureFlags = [ "--disable-static --disable-silent-rules" ];


Shouldn't be needed too; static libraries are not built by default (tested by removing those and building).

abbradar · 2018-03-10T12:06:59Z

pkgs/tools/inputmethods/libkkc/default.nix

+
+  buildInputs = [ marisa libkkc-data ];
+
+  propagatedBuildInputs = [ gnome3.libgee json_glib ];


Just use libgee and inherit (gnome3) libgee; in all-packages.nix instead; we try to avoid using whole (sub-)package sets as arguments.

abbradar

Built this locally but didn't verify that it works; ready to merge as soon as remaining issues are addressed. Sorry you had to wait for a long time; we are a bit short on manpower ^_^".

abbradar · 2018-03-10T12:14:04Z

pkgs/data/misc/libkkc-data/default.nix

@@ -0,0 +1,26 @@
+{ stdenv, fetchurl, pythonPackages }:


Don't use pythonPackages; as with libgee use inherit instead.

abbradar · 2018-03-10T13:49:47Z

@GrahamcOfBorg build ibus-engines.kkc

GrahamcOfBorg · 2018-03-10T14:02:22Z

Success on x86_64-linux (full log)

Partial log (click to expand)

glibPreFixupPhase
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/b7phz9kx42spwbwm0lsasymhnan0bzp6-ibus-kkc-1.5.22
shrinking /nix/store/b7phz9kx42spwbwm0lsasymhnan0bzp6-ibus-kkc-1.5.22/libexec/ibus-engine-kkc
shrinking /nix/store/b7phz9kx42spwbwm0lsasymhnan0bzp6-ibus-kkc-1.5.22/libexec/ibus-setup-kkc
strip is /nix/store/b0zlxla7dmy1iwc3g459rjznx59797xy-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/b7phz9kx42spwbwm0lsasymhnan0bzp6-ibus-kkc-1.5.22/libexec 
patching script interpreter paths in /nix/store/b7phz9kx42spwbwm0lsasymhnan0bzp6-ibus-kkc-1.5.22
checking for references to /tmp/nix-build-ibus-kkc-1.5.22.drv-0 in /nix/store/b7phz9kx42spwbwm0lsasymhnan0bzp6-ibus-kkc-1.5.22...
/nix/store/b7phz9kx42spwbwm0lsasymhnan0bzp6-ibus-kkc-1.5.22

GrahamcOfBorg · 2018-03-10T14:08:25Z

Success on aarch64-linux (full log)

Partial log (click to expand)

glibPreFixupPhase
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/bgzda7ihx8zpb02gaygxy86fic85k7wa-ibus-kkc-1.5.22
shrinking /nix/store/bgzda7ihx8zpb02gaygxy86fic85k7wa-ibus-kkc-1.5.22/libexec/ibus-setup-kkc
shrinking /nix/store/bgzda7ihx8zpb02gaygxy86fic85k7wa-ibus-kkc-1.5.22/libexec/ibus-engine-kkc
strip is /nix/store/lvx1acn1ig1j2km8jds5x3ggh3f2wa8v-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/bgzda7ihx8zpb02gaygxy86fic85k7wa-ibus-kkc-1.5.22/libexec
patching script interpreter paths in /nix/store/bgzda7ihx8zpb02gaygxy86fic85k7wa-ibus-kkc-1.5.22
checking for references to /build in /nix/store/bgzda7ihx8zpb02gaygxy86fic85k7wa-ibus-kkc-1.5.22...
/nix/store/bgzda7ihx8zpb02gaygxy86fic85k7wa-ibus-kkc-1.5.22

vanzef requested a review from FRidh as a code owner January 30, 2018 12:19

dotlambda reviewed Jan 30, 2018

View reviewed changes

GrahamcOfBorg added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Jan 30, 2018

vanzef force-pushed the kkc branch 2 times, most recently from 5be2fcf to d360232 Compare January 30, 2018 18:59

marisa-python: init at 1.3.40

dc856dc

vanzef force-pushed the kkc branch from d360232 to 74e2f9e Compare February 1, 2018 15:27

FRidh requested changes Feb 11, 2018

View reviewed changes

vanzef force-pushed the kkc branch from 74e2f9e to e9b97bd Compare February 11, 2018 09:15

GrahamcOfBorg added 6.topic: python 8.has: package (new) labels Feb 11, 2018

abbradar reviewed Mar 10, 2018

View reviewed changes

vanzef force-pushed the kkc branch 2 times, most recently from 717cab5 to e9fdd35 Compare March 10, 2018 13:12

vanzef added 3 commits March 10, 2018 16:16

libkkc-data: init at 0.2.7

8a094b3

libkkc: init at 0.3.5

5e95787

ibus-kkc: init at 1.5.22

c40e0af

vanzef force-pushed the kkc branch from e9fdd35 to c40e0af Compare March 10, 2018 13:17

abbradar merged commit 504b911 into NixOS:master Mar 10, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ibus-kkc engine support #34416

ibus-kkc engine support #34416

vanzef commented Jan 30, 2018 •

edited

dotlambda Jan 30, 2018

vanzef Jan 30, 2018

dotlambda Jan 30, 2018

vanzef Jan 30, 2018

dotlambda Jan 30, 2018

vanzef Feb 1, 2018

dotlambda Jan 30, 2018

vanzef Jan 30, 2018

dotlambda Jan 30, 2018

vanzef Jan 30, 2018

FRidh Feb 11, 2018

vanzef Feb 11, 2018

FRidh Feb 11, 2018

vanzef Feb 11, 2018

FRidh Feb 11, 2018

vanzef Feb 11, 2018

vanzef commented Feb 20, 2018

abbradar Mar 10, 2018

vanzef Mar 10, 2018

abbradar Mar 10, 2018

abbradar Mar 10, 2018

abbradar Mar 10, 2018

vanzef Mar 10, 2018

abbradar Mar 10, 2018

vanzef Mar 10, 2018

abbradar Mar 10, 2018

vanzef Mar 10, 2018

abbradar left a comment

abbradar Mar 10, 2018

vanzef Mar 10, 2018

abbradar commented Mar 10, 2018

GrahamcOfBorg commented Mar 10, 2018

GrahamcOfBorg commented Mar 10, 2018

		@@ -0,0 +1,40 @@
		{ stdenv, fetchurl
		, vala, gobjectIntrospection, intltool, pythonPackages, python, glib


		configureFlags = [ "--disable-static" ];

		buildInputs = [ libkkc ibus skk-dicts gnome3.gtk3 ];


		propagatedBuildInputs = [ gnome3.libgee json_glib ];

		configureFlags = [ "--disable-static --disable-silent-rules" ];


		buildInputs = [ marisa libkkc-data ];

		propagatedBuildInputs = [ gnome3.libgee json_glib ];

ibus-kkc engine support #34416

ibus-kkc engine support #34416

Conversation

vanzef commented Jan 30, 2018 • edited

Motivation for this change

Things done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vanzef commented Feb 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abbradar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abbradar commented Mar 10, 2018

GrahamcOfBorg commented Mar 10, 2018

GrahamcOfBorg commented Mar 10, 2018

vanzef commented Jan 30, 2018 •

edited