search

brndnmtthws / conky

Light-weight system monitor for X, Wayland (sort of), and other things, too
https://conky.cc
GNU General Public License v3.0
7.32k stars 620 forks source link

Implement `-U` for Linux #2049

Closed g0mb4 closed 3 weeks ago

g0mb4 commented 1 month ago

If this argument is provided Conky won't start if another Conky process is already running.

Conky starts multiple times on my system at startup, and it is bothering me. I'm too lazy to check all of my config files, so I added this mechanism to prevent this behavior.

netlify[bot] commented 1 month ago

Deploy Preview for conkyweb canceled.

Name Link
Latest commit 6cef51f45c2a44c71dc69482364d021a6451dda3
Latest deploy log https://app.netlify.com/sites/conkyweb/deploys/672882b14e86b400080e37a4
brndnmtthws commented 1 month ago

A couple issues: /proc is generally Linux-only (see https://en.wikipedia.org/wiki/Procfs), so we can't assume it's there on BSDs and macOS.

Another problem, getting the process name from /proc/[pid]/cmdline isn't a great idea because the command could have a symlink or some other strangeness. Better to use either /proc/[pid]/stat or follow the /proc[pid]/exe symlink.

g0mb4 commented 1 month ago

Thanks! It's the typical case of "it works on my machine"... I'll try to figure out a POSIX solution (maybe using ps).

brndnmtthws commented 1 month ago

I know I'm a bit late on getting back to you on this, and I apologize for that.

I am not a big fan of shelling out to ps in this way, it also feels like something that is likely to break and probably not consistent across platforms. I'm more inclined to take the original patch you submitted, and just refactor it so that it's Linux-only, and if people want to port it to the BSDs/macOS then so be it.

And to clarify, when I say "original patch" I mean reading from /proc, except rather than parsing /proc/[pid]/cmdline we can pull the name from /proc/[pid]/stat.

g0mb4 commented 1 month ago

No problem at all.

I tested the ps on FreeBSD and on Linux and the output was the same. I read that the Mac implementation should work as expected (based on some man pages).

It feels a "good enough" soultion for me as well, but this was the most POSIX-way I can came up with.

g0mb4 commented 1 month ago

I can do the /proc/[pid]/stat method in a couple of days if it is okay.

Caellian commented 4 weeks ago

I just added ifdef for Linux around everything and note in the manual, to avoid issue reports for it not working on non-Linux systems.

LinuxOnTheDesktop commented 3 weeks ago

This option seems like a useful addition. For, one can find a few reports on the web of people ending up with weird-looking conkies that turn out to be one conky superimposed upon another. I've encountered that myself. So there's even a case for making this option the default.

g0mb4 commented 3 weeks ago

Thanks! I can try to implement this feature on some BSDs if @brndnmtthws finds it as useful as you.

Caellian commented 3 weeks ago

So there's even a case for making this option the default.

After triaging issues I'd say changing the defaults will likely cause more confusion than having the option be opt-in. A lot of setups also spawn several conky instances to show information in different corners of the screen for example.

I can try to implement this feature on some BSDs if [Brandon] finds it as useful as you.

It would be a nice feature-parity addition, but it's up to you. I opened #2072, so you can use it as a guide on which platforms need to be supported if you decide to add support any other. I will also redirect people there if they open new issues about the feature not being supported for one of those platforms.

LinuxOnTheDesktop commented 2 weeks ago

@g0mb4

Question: will conky -U run Conky if another user is running a Conky? For, consider so-called fast-user switching.

g0mb4 commented 2 weeks ago

@LinuxOnTheDesktop

It depends on how /proc was mounted. If it allows the current user to see the processes of the other user, then conky won't start.