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

Add restart command #904

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@sambazley
Copy link
Contributor

sambazley commented Jan 19, 2019

Add ability to restart the window manager, without logging out to the display manager. Can be used, for example, to restart after an update.

Show resolved Hide resolved src/bspwm.c Outdated
@baskerville

This comment has been minimized.

Copy link
Owner

baskerville commented Jan 20, 2019

I guess I will remove examples/loop after merging this.

@sambazley sambazley force-pushed the sambazley:master branch from 2212bbf to 275980d Jan 20, 2019

}

if (restart) {
FILE *f = fopen(restart_state_path, "w");

This comment has been minimized.

@MyNameIsCosmo

MyNameIsCosmo Jan 25, 2019

Contributor

Maybe if RESTART_STATE_ENV_VAR is not set, the temporary file should be cleaned up?

@MyNameIsCosmo

This comment has been minimized.

Copy link
Contributor

MyNameIsCosmo commented Jan 25, 2019

It seems that upon bspc wm -r, this will remove the socket at /tmp/bspwm_0_0-socket (or similar), causing issues with other programs subscribed to the socket.

Maybe the sock_fd could be passed from the "original" process to the "new" bspwm process, and sock cleanup can be skipped on restart?
I'm unsure how to handle this, but maybe this will be a good reference.

@baskerville

This comment has been minimized.

Copy link
Owner

baskerville commented Jan 25, 2019

@MyNameIsCosmo Indeed, it would be great.

@baskerville

This comment has been minimized.

Copy link
Owner

baskerville commented Jan 25, 2019

Here's an excerpt from man execve:

File descriptors open in the calling process image remain open in the new process image, except for those for which the close-on-exec flag is set.

@baskerville

This comment has been minimized.

Copy link
Owner

baskerville commented Jan 25, 2019

I think it might be best to pass sock_fd as an environment variable to the new process.

Then, query_tree needs to be extended to output the built-in rules. Once this is done, we can avoid calling run_config when -s is set.

We also need to restore the list of subscribers and the pending rules.

@sambazley

This comment has been minimized.

Copy link
Contributor Author

sambazley commented Jan 25, 2019

Would it be better if we removed the -s option? Instead, we could load the state file if it exists using RESTART_STATE_ENV_VAR, then remove it when we're done with it.

@baskerville

This comment has been minimized.

Copy link
Owner

baskerville commented Jan 25, 2019

@sambazley It's fine with me.

@sambazley

This comment has been minimized.

Copy link
Contributor Author

sambazley commented Jan 25, 2019

@baskerville Sorry, to clarify are you saying it's fine the way it currently is, or the change is fine with you?

@baskerville

This comment has been minimized.

Copy link
Owner

baskerville commented Jan 25, 2019

You can remove the -s flag.

@sambazley sambazley force-pushed the sambazley:master branch from 861aba9 to 29ca001 Jan 25, 2019

@sambazley sambazley force-pushed the sambazley:master branch from 29ca001 to 19a47ac Jan 25, 2019

@MyNameIsCosmo

This comment has been minimized.

Copy link
Contributor

MyNameIsCosmo commented Jan 30, 2019

I agree on the sock_fd environment variable.

I added a rule query to query_tree in #916

How should restoring the list of subscribers and the pending rules occur? Should there be a query to print the subscribers and pending rules, similar to the state? Should they be included in the state query? Does it matter?

@baskerville

This comment has been minimized.

Copy link
Owner

baskerville commented Jan 30, 2019

@MyNameIsCosmo I'm working on finishing this in a local branch.

@MyNameIsCosmo

This comment has been minimized.

Copy link
Contributor

MyNameIsCosmo commented Jan 30, 2019

Neat! I'll test this out later today.

@MyNameIsCosmo MyNameIsCosmo referenced this pull request Feb 17, 2019

Closed

bspc wm -r #928

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment