wlh320 / rime-ls

A language server for Rime input method engine 通过 LSP 代码补全使用 Rime 输入法
BSD 3-Clause "New" or "Revised" License
181 stars 11 forks source link

test_get_candidates failed on non-FHS system (NixOS) #32

Closed definfo closed 2 weeks ago

definfo commented 3 weeks ago

I tried to build it on NixOS. Compilation passed, but test_get_candidates failed. Then I found the related code in src/rime.rs.

#[test]
fn test_get_candidates() {
    let shared_data_dir = "/usr/share/rime-data/";

This assumes a fixed path for shared data. However, NixOS does not provide such path since it's non-FHS.

definfo commented 3 weeks ago

Though I can bypass check phase by setting doCheck = false, editor/IDE configuration still relies on this path 😇

wlh320 commented 3 weeks ago

Thank you for pointing that out. Actually, not just NixOS, but most systems should not be able to pass this test, because this hard-coded path for the Rime shared data seems to only work on Linux.

On NixOS, where is the shared data for Rime typically stored? Maybe it can be improved by conditional compilation or environment variable?

definfo commented 3 weeks ago

On NixOS, where is the shared data for Rime typically stored?

In convention, /nix/store/<hash>-rime-xxx-<version>/share/rime-data/ Note that /nix/store is read-only to ensure reproducibility.

A common practice is to add rime data during package build, see https://github.com/NixOS/nixpkgs/blob/nixos-unstable/pkgs/tools/inputmethods/fcitx5/fcitx5-rime.nix#L42 TLDR: add rime-data as input, and move it into the final package during postInstall phase.

wlh320 commented 3 weeks ago

I tried to fix it with a helper function, which reads environment variable RIME_DATA_DIR at compile time, if not set, it uses known default shared data dir on different platforms. It works not only for cargo test, but also for default LSP initialization options.

wlh320 commented 3 weeks ago

I just noticed that there have already been an environment variable in Makefile of librime for rime shared data, so I changed the name to keep the same as it. See 5037bb6

definfo commented 2 weeks ago

Thanks, now all tests pass. My package definition is here https://github.com/definfo/nur-packages/blob/master/pkgs/rime-ls/default.nix . Maybe I can open a PR to nixpkgs right after the next release. As for editor/IDE configuration, I will try adding some script for assistance, since shared_data_dir on NixOS will change on package upgrade.

By the way, could you add Cargo.lock to your repo? Lockfile is crucial for reproducible build, and Nix package definition relies on it. I work around by generating it manually through cargo fetch and patch into the repo directory, but I cannot ensure my lockfile always works...

wlh320 commented 2 weeks ago

Thanks. I used to directly use .gitignore generated by Cargo, but it was only after your reminder that I realized it recommends removing Cargo.lock from gitignore if creating an executable.