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-bamboo: init at 0.6.6 #89176

Merged
merged 2 commits into from Sep 8, 2020
Merged

Conversation

SuperBo
Copy link
Contributor

@SuperBo SuperBo commented May 29, 2020

Add IBus Bamboo engine for Vietnamese

Motivation for this change

Currenty, NixOS doesn't have any support for typing Vietnamese. ibus-unikey and ibus-bamboo are two popular engines for Vietnamese input. However, ibus-bamboo is more updated than the old ibus-unikey.

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.

@SuperBo
Copy link
Contributor Author

SuperBo commented Jun 6, 2020

Any update on this?

@thongpv87
Copy link

@worldofpeace

@worldofpeace
Copy link
Contributor

I came up with the following diff to suggest as changes

diff --git a/pkgs/tools/inputmethods/ibus-engines/ibus-bamboo/default.nix b/pkgs/tools/inputmethods/ibus-engines/ibus-bamboo/default.nix
index 5992cbd7ca3..062274c99ee 100644
--- a/pkgs/tools/inputmethods/ibus-engines/ibus-bamboo/default.nix
+++ b/pkgs/tools/inputmethods/ibus-engines/ibus-bamboo/default.nix
@@ -1,5 +1,6 @@
 { stdenv
 , fetchFromGitHub
+, fetchpatch
 , gettext
 , xorg
 , pkgconfig
@@ -20,29 +21,35 @@ stdenv.mkDerivation rec {
     sha256 = "0v7n86bpyplfx617xiwwr4kkpjy3w55aqhd6fhljz6q69hgpvcmm";
   };
 
+  patches = [
+    # Add PREFIX to makefile
+    (fetchpatch {
+      url = "https://github.com/BambooEngine/ibus-bamboo/commit/01b9214cbe479e120de3af7d18df6bf575bead2b.patch";
+      sha256 = "1j66hlz008h1pxb9gngq1mldkynlikx5nldvpk3rpdx28ak1lvz9";
+    })
+  ];
+
   nativeBuildInputs = [
     gettext
     pkgconfig
     wrapGAppsHook
     go
-    xorg.libX11
-    xorg.libXtst
   ];
 
   buildInputs = [
-    ibus
-    gtk3
+    xorg.libX11
+    xorg.xorgproto
+    xorg.libXtst
+    xorg.libXi
   ];
 
   preConfigure = ''
     export GOCACHE="$TMPDIR/go-cache"
-    sed -i 's,/usr/lib,/lib,' Makefile
-    sed -i "s,/usr,$out," bamboo.xml
   '';
 
-  installPhase = ''
-    make DESTDIR="$out" engine_dir=/share/ibus-bamboo ibus_dir=/share/ibus install
-  '';
+  makeFlags = [
+    "PREFIX=${placeholder "out"}"
+  ];
 
   meta = with stdenv.lib; {
     isIbusEngine = true;

The main issue was the dependencies, what was in nativeBuildInputs for x11 dependencies were runtime dependencies. What is in buildinputs now is from looking at all the #include in source to make sure it's perfect.

The one thing I wasn't sure on was need to remove ibus and gtk3`. From looking at the source I don't see it need their headers, or are they needed in the wrapper somehow?

@SuperBo
Copy link
Contributor Author

SuperBo commented Sep 2, 2020

@worldofpeace: the patch have been included in v0.6.6. However, to make it work, we still need to edit bamboo.xml file. So I updated the package to v0.6.6. Please have check on it.

Thank you

Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

LGTM. Can you squash the update commit into the init commit?

@SuperBo SuperBo changed the title ibus-bamboo: init at 0.6.5 ibus-bamboo: init at 0.6.6 Sep 8, 2020
@SuperBo
Copy link
Contributor Author

SuperBo commented Sep 8, 2020

LGTM. Can you squash the update commit into the init commit?

@worldofpeace: I squash two commits. Please have a review.

Thank you

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

5 participants