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

Don't run in background by default #182

Open
WhyNotHugo opened this issue Nov 6, 2023 · 8 comments · May be fixed by #230
Open

Don't run in background by default #182

WhyNotHugo opened this issue Nov 6, 2023 · 8 comments · May be fixed by #230

Comments

@WhyNotHugo
Copy link
Contributor

By default, swww forks and runs as a background process. Every time somebody runs swww init, the output of the command is not visible, and killing or restarting the command isn't as simple as Ctrl+c. One needs to inspect running processes and figure out which is the right one. There is no obvious indicator of what's going on, and this is especially worse on the first usage of swww, where it seems like it's exiting immediately and not really working properly.

This runs contrary to what the gran majority of other commands do: simply run in foreground. If somebody needs to run it in background, popular shells (including bash, zsh, and ksh) allow simply using swww init & to run in background.

Service managers and supervisors also always expect services to run in foreground; demonising like this makes the job of service supervisors a lot harder, since tracking the lifetime of the child doesn't track the lifetime of the service. Eventually the solution is to always use swww init --no-daemon.

I'd like to propose simply running in foreground like any other command and dropping the extra code for daemonisation. For any unique situation where running in background is preferible, using swww init & (or maybe nohup swww init &) is a trivial workaround.
@LGFae
Copy link
Owner

LGFae commented Nov 6, 2023

Interesting proposal. It seems to me like a big breaking change, considering the way we've done things since the beginning. I'll think about it, but I feel like this is a good idea overall.

@Nemo157
Copy link

Nemo157 commented Dec 24, 2023

A semi-related question: is swww-daemon public API? For my systemd service I run that directly instead of going through swww init --no-daemon to avoid the extra layering.

@LGFae
Copy link
Owner

LGFae commented Dec 24, 2023

Running the daemon directly is currently exactly equivalent to running init --no-daemon. I cannot guarantee this will be the case forever, though.

Nevertheless, it would be a breaking change, should it ever change, with big bold letters in the release page.

WhyNotHugo added a commit to WhyNotHugo/swww that referenced this issue Mar 9, 2024
@WhyNotHugo
Drop daemonizing logic
accb564 
Run in foreground, which is what service managers expect. Usages like
running `exec swww` in sway's config continue to work fine. User session
start-up scripts can simply run `swww &`.

This is a first iteration simply removing the interface. A following
patch shall start converging the two modules in this codebase into one.

See: LGFae#182
WhyNotHugo added a commit to WhyNotHugo/swww that referenced this issue Mar 9, 2024
@WhyNotHugo
Drop daemonizing logic
96bd3a8 
Run in foreground, which is what service managers expect. Usages like
running `exec swww` in sway's config continue to work fine. User session
start-up scripts can simply run `swww &`.

See: LGFae#182
WhyNotHugo added a commit to WhyNotHugo/swww that referenced this issue Mar 9, 2024
@WhyNotHugo
Drop daemonizing logic
4d5100e 
Run in foreground, which is what service managers expect. Usages like
running `exec swww` in sway's config continue to work fine. User session
start-up scripts can simply run `swww &`.

Fixes: LGFae#182
@WhyNotHugo WhyNotHugo linked a pull request Mar 9, 2024 that will close this issue
Open
@WhyNotHugo
Copy link
Contributor Author

Having swww init an swww-daemon do the same thing is kind of ambiguous.
What do you think of merging both commands into one? Or maybe drop swww init?

@KAGEYAM4
Copy link

Having swww init an swww-daemon do the same thing is kind of ambiguous. What do you think of merging both commands into one? Or maybe drop swww init?

man swww-daemon says this is for debug pupose, and swww init should be preffered as it waits for the daemon to be ready.

But i agree the, name and functionality is ambigous unless one looks at the man-page.

@LGFae
Copy link
Owner

LGFae commented Mar 11, 2024

man swww-daemon says this is for debug pupose, and swww init should be preffered as it waits for the daemon to be ready.

Ah, this is out of date now. @musjj implemented the necessary logic to always wait for the daemon to be ready for all requests.

Since this is already a breaking change, I feel like we should just drop swww init. The advantage is that swww init will exit immediately without an error. If we keep swww init but change its behavior, depending on how the compositor loads its configuration, swww init could end up deadlocking the loading process (since it won't exit).

@KAGEYAM4
Copy link

KAGEYAM4 commented Mar 12, 2024 •
edited

Since this is already a breaking change, I feel like we should just drop swww init. The advantage is that swww init will exit immediately without an error.

How about sending a message to stderr stating that sww init is deprecated and then later down the road, init it can be totally removed from the argument list.

@LGFae
Copy link
Owner

LGFae commented Mar 13, 2024

How about sending a message to stderr stating that swww init is deprecated and then later down the road, init it can be totally removed from the argument list.

The problem is that because swww initwill now by default lock the calling parent, we might deadlock certain compositor's initialization processes by accident, depending on how they were implemented.

If we just deprecate swww init, then I'd rather do it without changing its behavior. That way we won't interfere with anyone's setup and give them some time to update their configs. Otherwise, if we change how swww init works, then I believe it should fail loudly.

@LGFae LGFae mentioned this issue Mar 23, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Drop daemonizing logic WhyNotHugo/swww
4 participants
@Nemo157 @WhyNotHugo @LGFae @KAGEYAM4