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
Add ibus-engines.rime #87735
Add ibus-engines.rime #87735
Conversation
@pmeiyu please solve the merge conflict. |
ec006c0
to
16928ea
Compare
@SuperSandro2000 I have rebased the code onto master branch. You can merge now. Thanks. |
@SuperSandro2000 Are you a merge conflict detection bot? This repository is full of useless bots. |
@SuperSandro2000 Bad bot. Waste my time. Please notify me only when you are actually going to review the code. |
Result of 1 package blacklisted:
1 package built:
|
d3fa25f
to
d90b4a5
Compare
substituteInPlace Makefile \ | ||
--replace 'cmake' 'cmake -DRIME_DATA_DIR=${brise}/share/rime-data' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to specify this in cmakeFlags
, see https://github.com/NixOS/nixpkgs/pull/77877/files#diff-22b83f2a09917d6f17fa4fb028cd36fd499c7d046298d46202d60a2fea4dde9fR39-R43
in combination with #77877 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ibus-rime project used a weird mixture of Makefile and cmake. I choose to use the make build system here. Therefore I have to patch Makefile. The other PR used cmake build system and that caused other dirt hacks. I think using Makefile is cleaner here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue with this is that it steps around the flags we pass to CMake like cmakeBuildType
. I have opened rime/ibus-rime#112 to allow us to use CMake for everything, including correct paths in the files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great. So this will be resolved in next ibus-rime release.
Result of 1 package blacklisted:
1 package built:
|
Motivation for this change
This pull request adds ibus-engines.rime.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)