Skip to content

Conversation

@patrick96
Copy link
Member

@patrick96 patrick96 commented Oct 15, 2021

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update
  • Other: Replace this with a description of the type of this PR

Description

Using a single fifo to accept IPC messages does not work. It fails when multiple people want to send concurrently or when messages arrive close to eachother.
That's why polybar should use a socket for IPC.

In addition /run/user/UID/ is a safer place to put the socket.

This deprecates using the fifo.

TODOs:

  • Make polybar-msg use the socket
  • Use an unambiguous format for the socket messages
  • Make polybar-msg action more user-friendly by letting it accept 2 or 3 arguments. (2 for module name + action and 3 when it includes data).
  • Deprecate hook ipc messages, they are superseeded by module actions (see Ipc module action #2528). Both polybar and polybar-msg should print a warning when it is used. polybar-msg should convert each such call to an action messages (that way we can completely remove hooks from the socket message format).
  • Now that we use sockets, polybar can write back success or error messages as a response to ipc messages.
  • Print info log with path to IPC socket.
  • Print notice log with IPC init success message + PID

Related Issues & Documents

Fixes #2532
Closes #2465
Fixes #2504

Documentation (check all applicable)

  • This PR requires changes to the Wiki documentation (describe the changes)
  • This PR requires changes to the documentation inside the git repo (please add them to the PR).
  • Does not require documentation changes

This PR also moves the IPC documentation to the repo. The wiki sidebar and all other links to the IPC page should be updated to point to RTD (on stable). In addition, the IPC wiki page should have at least a banner at the top, linking to RTD and maybe the rest of the page should be removed as well.

This also adds a small manpage for polybar-msg

@codecov
Copy link

codecov bot commented Oct 16, 2021

Codecov Report

Merging #2539 (968e18d) into master (e549527) will increase coverage by 0.59%.
The diff coverage is 14.80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2539      +/-   ##
==========================================
+ Coverage   10.83%   11.43%   +0.59%     
==========================================
  Files         149      153       +4     
  Lines       10779    11127     +348     
==========================================
+ Hits         1168     1272     +104     
- Misses       9611     9855     +244     
Flag Coverage Δ
unittests 11.43% <14.80%> (+0.59%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
include/common.hpp 50.00% <0.00%> (-50.00%) ⬇️
include/components/bar.hpp 100.00% <ø> (ø)
include/components/controller.hpp 0.00% <0.00%> (ø)
include/components/eventloop.hpp 0.00% <0.00%> (ø)
include/drawtypes/animation.hpp 0.00% <ø> (ø)
include/drawtypes/label.hpp 100.00% <ø> (ø)
include/drawtypes/ramp.hpp 50.00% <ø> (ø)
include/events/signal.hpp 0.00% <0.00%> (ø)
include/ipc/ipc.hpp 0.00% <0.00%> (ø)
include/utils/mixins.hpp 50.00% <ø> (-50.00%) ⬇️
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e549527...968e18d. Read the comment docs.

This only happens for commands and empty actions.
Non-empty actions are not immediately executed, but deferred until the
next loop iteration.
The socket file is not deleted after socket.close() is called, only
after libuv executes the close callback.
So we can't just call rmdir because it will probably always fail.
@patrick96 patrick96 changed the title Feat/sock Use socket for IPC Jan 22, 2022
@patrick96 patrick96 marked this pull request as ready for review January 22, 2022 17:20
@patrick96 patrick96 merged commit 3356188 into polybar:master Jan 22, 2022
@patrick96 patrick96 deleted the feat/sock branch January 22, 2022 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Improve IPC hook protocol Bug: race condition when triggering multiple IPC hooks simultaneously Socket should be in /run/user/XXXX/polybar

1 participant