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.

Sign up for GitHub

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

Jump to bottom

fcitx-configtool: Point exec_prefix to installed location of fcitx-remote #60285

Closed
wants to merge 2 commits into from
Closed

fcitx-configtool: Point exec_prefix to installed location of fcitx-remote #60285

wants to merge 2 commits into from

Conversation

telotortium
Copy link
Contributor

@telotortium telotortium commented Apr 26, 2019 •
edited

Motivation for this change

Fixes errors like this printed to the console when changing settings using fcitx-config-gtk3:

(fcitx-config-gtk3:17980): GLib-WARNING **: 12:24:47.394: GError set over the top of a previous GError or uninitialized memory.
This indicates a bug in someone's code. You must ensure an error is NULL before it's set.
The overwriting error message was: Failed to execute child process “/nix/store/02i4q909l3jk6080zb57jmd23ra4244x-fcitx-configtool-0.4.10/bin/fcitx-remote” (No such file or directory)
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option 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 nix-review --run "nix-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@telotortium
Point exec_prefix to installed location of fcitx-remote
15a225f 
Fixes errors like this printed to the console when changing settings using `fcitx-config-gtk3`:

```
(fcitx-config-gtk3:17980): GLib-WARNING **: 12:24:47.394: GError set over the top of a previous GError or uninitialized memory.
This indicates a bug in someone's code. You must ensure an error is NULL before it's set.
The overwriting error message was: Failed to execute child process “/nix/store/02i4q909l3jk6080zb57jmd23ra4244x-fcitx-configtool-0.4.10/bin/fcitx-remote” (No such file or directory)
```
@telotortium telotortium changed the title Point exec_prefix to installed location of fcitx-remote fcitx-configtool: Point exec_prefix to installed location of fcitx-remote Apr 26, 2019
teto
teto reviewed Apr 27, 2019
View reviewed changes
pkgs/tools/inputmethods/fcitx/fcitx-configtool.nix
substituteInPlace config.h.in \
--subst-var-by exec_prefix ${fcitx}
'';

Copy link
Member

Choose a reason for hiding this comment

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

shoulnd't this be fixed upstream instead ? it seems to be looking for fcitx find_package(Fcitx 4.2.8 REQUIRED) so it could use the correct path no ? https://gitlab.com/fcitx/fcitx-configtool/blob/master/CMakeLists.txt#L43

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've decided to patch CMakeLists.txt to point exec_prefix to the right place. I'll try to submit this patch upstream, but I don't know how to navigate their merge request workflow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@teto please take a look

Copy link
Member

Choose a reason for hiding this comment

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

It works like github you an fork from https://gitlab.com/fcitx/fcitx-configtool and generate a PR there. The fix doesn't look nixos specific so best upstream though we can carry it until then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opened https://gitlab.com/fcitx/fcitx-configtool/merge_requests/17 upstream.

Copy link
Member

Choose a reason for hiding this comment

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

thanks I subscribed to the PR. Let's see how it goes

@teto teto self-assigned this May 14, 2019
@telotortium
Copy link
Contributor Author

I think this has been addressed by commit 05c94e3.

@telotortium telotortium closed this Apr 9, 2020
@telotortium telotortium deleted the patch-2 branch April 9, 2020 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@teto teto teto left review comments

Assignees

@teto teto

Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@telotortium @teto