ibus-engines.mozc-ut: init at 2.30.5544.102 by pineapplehunter · Pull Request #314248 · NixOS/nixpkgs

pineapplehunter · 2024-05-24T10:20:19Z

Description of changes

This is an attempt to package ibus-engines.mozc-ut which enables a new UT dictionaries option.

It copies a lot of the work in #296035 but changes every dictionary as an derivation. By making every dictionary a derivation each update to the dictionary will keep the IME up to date.

I am still unsure if this is the right way to implement it, so I will put this up as a draft for now.
Edit: changed the title so it only shows the mozc-ut

Things done

Add a 👍 reaction to pull requests you find important.

pluiedev · 2024-05-24T10:28:50Z

These packages all look extremely similar. Ever considered maybe writing a wrapper for them? (You can then expose them in merge-ut-dictionaries or somewhere else)

Thanks for checking these out so quickly!
I think it is better to create a wrapper too, but It is very convenient having the auto-update script running. Is there a way to have the auto-update happen without creating multiple derivations?

pluiedev · 2024-05-24T10:30:14Z

Why not just make these the default value of the dictionaries variable in the deriv, like you've done in merge-ut-dictionaries?

I'm unsure if it is a good design, but I wanted to show that dictionaries should be explicitly added and keep them out of the mozc derivation.
Further, I think it may be better if merge-ut-dictionaries were a function that takes some dictionaries and outputs a file. I wasn't quite sure where to put something like that in the by-name directory. Is there a place to put a file like that?

I am not sure if this is compelling but the current default value of dictionaries matches upstream's default.

mozcdic-ut-jawiki -> jawiki

mozcdic-ut-personal-names -> personal_names

mozcdic-ut-place-names -> place_names

mozcdic-ut-sudachidict -> sudachidict

pluiedev · 2024-05-24T10:31:03Z

Also the old PR title is just way too long haha, caught my attention immediately

stephen-huan

Tested against upstream dictionary (as of 2024-05-24). See instructions below.

git clone https://github.com/utuhiro78/merge-ut-dictionaries
cd merge-ut-dictionaries/src
# save below diff as enable_all.patch
git apply enable_all.patch
# ruby and wget should be on path, utf-8 locale
bash make.sh
sed -i "s|1843|1847|g" mozcdic-ut.txt
md5sum mozcdic-ut.txt

diff --git a/src/make.sh b/src/make.sh
index a5689a1..ef881ed 100644
--- a/src/make.sh
+++ b/src/make.sh
@@ -3,13 +3,13 @@
 # Author: UTUMI Hirosi (utuhiro78 at yahoo dot co dot jp)
 # License: Apache License, Version 2.0
 
-#alt_cannadic="true"
-#edict="true"
+alt_cannadic="true"
+edict="true"
 jawiki="true"
-#neologd="true"
+neologd="true"
 personal_names="true"
 place_names="true"
-#skk_jisyo="true"
+skk_jisyo="true"
 sudachidict="true"
 
 rm -f mozcdic-ut.txt

5a2d917e7185d8f196d4ea5fab9a0e65  mozcdic-ut.txt

5a2d917e7185d8f196d4ea5fab9a0e65  /nix/store/fpcb9rkppagaav45ry8ab5zqsfmlmch7-merge-ut-dictionaries-0-unstable-2024-05-18/mozcdic-ut.txt

Result of nixpkgs-review pr 314248 run on x86_64-linux 1

2 packages blacklisted:

nixos-install-tools
tests.nixos-functions.nixos-test

8 packages built:

emacsPackages.ac-mozc
emacsPackages.mozc
emacsPackages.mozc-cand-posframe
emacsPackages.mozc-im
emacsPackages.mozc-popup
emacsPackages.mozc-temp
ibus-engines.mozc
ibus-engines.mozc-ut

Did not test that ibus-engines.mozc-ut actually picked up the dictionary but in my experience if the dictionary is correct the IME should be fine.

stephen-huan · 2024-05-24T15:00:08Z

@pineapplehunter thanks for the PR! Was going to take a crack at it myself but didn't get around to it in time...

All of the dictionaries have build scripts, e.g. mozcdic-ut-place-name (except for mozcdic-ut-personal-names). Is there any merit in packaging the dictionaries "from source" rather than relying on the manually updated .txt.tar.bz2? I looked at some of the dictionary URLs and they seem not stable (updated in place) so there would need to be mirroring. If so, it'd probably be more trouble than it's worth since these dictionary repositories are already essentially mirrors.

Secondly, what concrete steps need to be taken to go from draft status to a mergeable state? Given that this PR is already functionally correct (in that it matches upstream + works with IME) and dictionaries are easily updated by nixpkgs-update the PR looks good to me. Given the 3-4 PRs I've seen, missing the ut dictionary has been a sore point for some time now and personally speaking, I'd like to stop compiling ibus every time I update my system ;).

pluiedev · 2024-06-12T07:20:20Z

All of the dictionaries have build scripts, e.g. mozcdic-ut-place-name (except for mozcdic-ut-personal-names). Is there any merit in packaging the dictionaries "from source" rather than relying on the manually updated .txt.tar.bz2?

It's more a principle thing, I think — aside from some core bootstrapping libraries, Nixpkgs tries to compile things from source as much as possible to ensure that the compilation process is reproducible. Unwrapping and repackaging prebuilt files should only be done if there's no way to build the package correctly and/or reproducibly

stephen-huan · 2024-06-12T12:12:03Z

I see. I'll try to take a shot at it at some point. Either way, I don't think it's a blocker for this PR since a "from source" derivation can transparently replace any of the "repacked" derivations (which are their own derivations already).

musjj · 2024-09-11T15:31:58Z

Could this be turned into a new separate package instead (e.g. mozc/default.nix)? It looks like that this package could also be reused for fcitx5-mozc, so I don't think it should be an ibus-specific package.

I am not quite sure about fcitx5-mozc package. I may checkout later.

See #251706 which might try to coordinate with this PR.

To be clear, I don't think you need to make any fcitx5-specific changes. Simply moving the package to its own separate namespace would probably be enough.

@musjj I'll try moving this package to pkgs/by-name/mo/mozc/package.nix.
Let me know if this causes any issues with the fcitx5 implementation.

h7x4 · 2024-09-19T07:15:40Z

@pineapplehunter Is there anything keeping this PR from being undrafted? @musjj would you be okay with merging this PR first, and rebasing your work onto it in #251706? I'd be happy if we could get the new mozc stuff in before 24.11

musjj · 2024-09-19T07:48:53Z

would you be okay with merging this PR first, and rebasing your work onto it

Yeah, I'd be okay with that.

pineapplehunter · 2024-09-19T11:27:48Z

@h7x4 @musjj Thanks for the comments, and sorry for the late reply.
I originally started this PR as a PoC to see if making all the dictionaries as packages would work. I wasn't quite sure if this was the right way to go.
But I'm happy to undrafted and merge this.
Feel free to let me know if there's anything I should fix.

h7x4 · 2024-09-19T11:57:31Z

+          --replace-fail "urllib.request.urlopen" "open" \
+          --replace-fail "read().decode()" "read()"
+
+        for dir in ${builtins.toString dictionaries}; do


Not a blocker, but I think the use of toString for lists like this is a bit of an antipattern, maybe apart from debugging purposes. It's better to be explicit.

Suggested change

for dir in ${builtins.toString dictionaries}; do

for dir in ${lib.concatStringsSep " " dictionaries}; do

This is how I did it in my PR:

'' for name in ${lib.escapeShellArgs dictionaries}; do ... done ''

This ensures that everything is escaped correctly.

h7x4 · 2024-09-19T12:02:54Z

-    rev = "119c888a38032a92e139c52cd26f45bb495c4d54";
-    hash = "sha256-uyAL2TcFJsYZACFDAxIQ4LE40Hi4PVrQRnJl5O5+RmU=";
-  };
+  ut-dictionary = merge-ut-dictionaries.override { inherit dictionaries; };


Will this make the overriden merge-ut-dictionaries throw an error by default, considering the dictionaries ? [ ] and the non-empty assertion in merge-ut-dictionaries? Maybe it would be better to do something along the lines of this instead?

Suggested change

ut-dictionary = merge-ut-dictionaries.override { inherit dictionaries; };

ut-dictionary = merge-ut-dictionaries.override (lib.optionalAttrs (dictionaries != [ ]) { inherit dictionaries; });

Will this make the overriden merge-ut-dictionaries throw an error by default,

No, It is only evaluated when any dictionary is in the list, on line 78.
I made it so you could build mozc with or without the dictionaries.
But I can see this being kind of confusing. I'm happy to change it.

pineapplehunter · 2024-09-19T13:14:53Z

I just remembered I have to double-check the licenses on the dictionaries. I remember each of them having slightly different licenses.

pineapplehunter · 2024-09-19T13:40:34Z

All licenses have been checked and should now be good to go.

pineapplehunter · 2024-09-19T13:51:09Z

Missed the license for jp-zip-codes

h7x4

This looks good to me. Merging in a few days unless someone says otherwise :)

h7x4 · 2024-09-21T18:18:42Z

Thank you all for the great work!

pluiedev suggested changes May 24, 2024

View reviewed changes

ofborg Bot added the 8.has: package (new) This PR adds a new package label May 24, 2024

ofborg Bot requested a review from gebner May 24, 2024 11:00

pineapplehunter force-pushed the mozc-updates branch from 40e81b4 to 28559e3 Compare May 24, 2024 11:41

pineapplehunter mentioned this pull request May 24, 2024

mozc: init at unstable-2024-02-09, fcitx5-mozc: 2.26.4220.102 -> unstable-2024-02-09 #251706

Closed

12 tasks

stephen-huan reviewed May 24, 2024

View reviewed changes

Comment thread pkgs/by-name/ja/jawiki-all-titles-in-ns0/package.nix Outdated

stephen-huan approved these changes May 24, 2024

View reviewed changes

pineapplehunter force-pushed the mozc-updates branch from 28559e3 to 74e3631 Compare August 23, 2024 00:22

ofborg Bot added 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. and removed 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Aug 23, 2024

pineapplehunter force-pushed the mozc-updates branch from 74e3631 to ccfc7ad Compare August 23, 2024 15:18

pineapplehunter changed the title ~~ibus-engines.mozc-ut: init at 2.29.5374.102~~ ibus-engines.mozc-ut: init at 2.30.5544.102 Aug 30, 2024

pineapplehunter force-pushed the mozc-updates branch from 40643e6 to e3ab57b Compare August 30, 2024 14:26

musjj reviewed Sep 11, 2024

View reviewed changes

pineapplehunter added 4 commits September 13, 2024 13:41

jp-zip-codes: init at 0-unstable-2024-08-01

633055a

jawiki-all-titles-in-ns0: init at 0-unstable-2024-09-11

392ec6a

mozcdic-ut-alt-cannadic: init at 0-unstable-2024-07-28

78b3a6b

mozcdic-ut-edict2: init at 0-unstable-2024-07-28

d148df3

pineapplehunter added 8 commits September 13, 2024 13:43

mozcdic-ut-jawiki: init at 0-unstable-2024-08-23

6d47131

mozcdic-ut-neologd: init at 0-unstable-2024-07-28

57f2fc9

mozcdic-ut-personal-names: init at 0-unstable-2024-08-06

ee06c04

mozcdic-ut-place-names: init at 0-unstable-2024-08-06

6201a27

mozcdic-ut-skk-jisyo: init at 0-unstable-2024-07-27

58a342c

mozcdic-ut-sudachidict: init at 0-unstable-2024-07-28

4441c63

merge-ut-dictionaries: init at 0-unstable-2024-08-28

bbb9b4b

ibus-engines.mozc-ut: init at 2.30.5544.102

c99947b

pineapplehunter force-pushed the mozc-updates branch from 1655f85 to c99947b Compare September 13, 2024 04:49

pineapplehunter marked this pull request as ready for review September 19, 2024 11:27

h7x4 reviewed Sep 19, 2024

View reviewed changes

pineapplehunter added 2 commits September 19, 2024 21:57

fixup! merge-ut-dictionaries: init at 0-unstable-2024-08-28

47bf9f1

fixup! jp-zip-codes: init at 0-unstable-2024-08-01

867f358

fixup! mozcdic-ut-skk-jisyo: init at 0-unstable-2024-07-27

c1a8c57

h7x4 reviewed Sep 19, 2024

View reviewed changes

Comment thread pkgs/by-name/me/merge-ut-dictionaries/package.nix Outdated

pineapplehunter added 2 commits September 19, 2024 22:45

fixup! fixup! merge-ut-dictionaries: init at 0-unstable-2024-08-28

3049c67

fixup! jp-zip-codes: init at 0-unstable-2024-08-01

dda629d

fixup! ibus-engines.mozc-ut: init at 2.30.5544.102

5566c5f

h7x4 requested a review from pluiedev September 19, 2024 14:08

h7x4 approved these changes Sep 19, 2024

View reviewed changes

h7x4 merged commit 9a0f009 into NixOS:master Sep 21, 2024

pineapplehunter mentioned this pull request Sep 22, 2024

ibus-engines.mozc: 2.29.5374.102 -> 2.30.5544.102 #338454

Closed

13 tasks

pineapplehunter deleted the mozc-updates branch March 31, 2025 02:49

	for dir in ${builtins.toString dictionaries}; do
	for dir in ${lib.concatStringsSep " " dictionaries}; do

	ut-dictionary = merge-ut-dictionaries.override { inherit dictionaries; };
	ut-dictionary = merge-ut-dictionaries.override (lib.optionalAttrs (dictionaries != [ ]) { inherit dictionaries; });

Uh oh!

Conversation

pineapplehunter commented May 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes

Things done

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pluiedev commented May 24, 2024

Uh oh!

Uh oh!

stephen-huan left a comment

Choose a reason for hiding this comment

Uh oh!

stephen-huan commented May 24, 2024

Uh oh!

pluiedev commented Jun 12, 2024

Uh oh!

stephen-huan commented Jun 12, 2024

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

h7x4 commented Sep 19, 2024

Uh oh!

musjj commented Sep 19, 2024

Uh oh!

pineapplehunter commented Sep 19, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pineapplehunter Sep 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pineapplehunter commented Sep 19, 2024

Uh oh!

pineapplehunter commented Sep 19, 2024

Uh oh!

Uh oh!

pineapplehunter commented Sep 19, 2024

Uh oh!

h7x4 left a comment

Choose a reason for hiding this comment

Uh oh!

h7x4 commented Sep 21, 2024

Uh oh!

Reviewers

Assignees

pineapplehunter commented May 24, 2024 •

edited

Loading

pineapplehunter Sep 19, 2024 •

edited

Loading