-
-
Notifications
You must be signed in to change notification settings - Fork 103
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
fix(linux): Don't start ibus-daemon multiple times 🚌 #6283
Conversation
User Test ResultsTest specification and instructions
Test Artifacts |
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.
lgtm
This change adds a 1s sleep after restarting ibus before checking that it actually runs. Hopefully this will solve the problem. Since this bug is hard to reproduce, we also log a critical error for the other code path this could happen. Fixes #6237.
c440852
to
e30c007
Compare
@@ -130,6 +132,7 @@ def restart_ibus(bus=None): | |||
except Exception as e: | |||
logging.warning("Failed to restart IBus") | |||
logging.warning(e) | |||
time.sleep(1) # 1s |
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.
Can you add a comment explaining this delay?
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.
done
@bharanidharanj Could I ask you to re-run the tests with the new instructions. Thanks! I added some debug output, so hopefully this will allow me to figure out what's going wrong. In my local testing I'm not able to reproduce the problem after having the additional debug statements in place. @keymanapp-test-bot retest all |
GROUP_BIONIC: Ubuntu 18.04 Bionic with Gnome Shell and X11
|
- output error if more than one ibus-daemon is running - log Keyman version after starting km-config
ca2d59e
to
e6cf032
Compare
@bharanidharanj Can you please test again? Don't forget to install the new build (linked in the "User Test Results" comment). Please test on all platforms, starting with Bionic, until you get a failure. Thanks! @keymanapp-test-bot retest GROUP_BIONIC all |
@ermshiperete Sure. I will test it and will post my comment. |
GROUP_BIONIC: Ubuntu 18.04 Bionic with Gnome Shell and X11
|
@bharanidharanj That doesn't look like it's running the version from this PR. Can you show me the output of |
@ermshiperete km-config version 16.0.2-alpha |
GROUP_IMPISH_X11: Ubuntu 21.10 Impish with Gnome Shell and X11
GROUP_IMPISH_WAYLAND: Ubuntu 21.10 Impish with Gnome Shell and Wayland
GROUP_WASTA: Wasta 20.04 with Cinnamon
GROUP_FOCAL: Ubuntu 20.04 Focal with Gnome Shell and X11
|
@bharanidharanj That should be 15.0.210-beta. Please install the package built for this PR by following the steps in the wiki. Please let me know if the instructions in the wiki are not clear enough. Thanks! |
@ermshiperete As per our discussion, I have attached the Screenshot as well as the log file for your reference. |
Instead of trying to restart ibus by passing True to bus.exit() we now exit ibus and then let verify_ibus_daemon() start it again. Also added additional checks to verify that we're not running multiple ibus-daemons at the start of km-config and before restarting.
@bharanidharanj Please try again. I made some preliminary changes - so you can stop testing after the first failure. @keymanapp-test-bot retest all |
Test ResultsGROUP_BIONIC: Ubuntu 18.04 Bionic with Gnome Shell and X11
I've uninstalled and removed everything related to Keyman using
and
then after restarting the machine, the instructions at https://github.com/keymanapp/keyman/wiki/How-to-test-artifacts-from-pull-requests-for-Keyman-for-Linux were followed, but the second line of commands show some suspicious errors, see below:
|
@MakaraSok The errors you get in the @keymanapp-test-bot retest GROUP_BIONIC TEST_KB_INSTALL1 |
GROUP_BIONIC: Ubuntu 18.04 Bionic with Gnome Shell and X11
|
GROUP_WASTA: Wasta 20.04 with Cinnamon
|
GROUP_FOCAL: Ubuntu 20.04 Focal with Gnome Shell and X11
|
GROUP_BIONIC: Ubuntu 18.04 Bionic with Gnome Shell and X11
|
GROUP_IMPISH_X11: Ubuntu 21.10 Impish with Gnome Shell and X11
GROUP_IMPISH_WAYLAND: Ubuntu 21.10 Impish with Gnome Shell and Wayland
|
Changes in this pull request will be available for download in Keyman version 15.0.213-beta |
This change adds a 1s sleep after restarting ibus before checking that it actually runs. Hopefully this will solve the problem. Since
this bug is hard to reproduce, we also log a critical error for the other code path this could happen.
Fixes #6237.
User Testing
Preparations
Since this bug is hard to reliably reproduce, the tests should be run on all Linux platforms:
Install build artifacts of this PR
Reboot
Tests
TEST_KB_INSTALL1: Download and install a keyboard
ibus-daemon
/tmp/km-config.log
or paste the content of/tmp/km-config.log
TEST_KB_INSTALL2: Download and install a second keyboard
ibus-daemon
/tmp/km-config2.log
or paste the content of/tmp/km-config2.log