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.

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

Dynamically determine command for new pane? Make default_prog a function? #1776

Closed
swsnr opened this issue Mar 27, 2022 · 17 comments
Closed
Labels
enhancement New feature or request fixed-in-nightly This is (or is assumed to be) fixed in the nightly builds.

Comments

@swsnr
Copy link
Contributor

swsnr commented Mar 27, 2022

Describe the solution you'd like
I'd like to use a function to determine the exact command to use for a new pane. Currently default_prog is just a static table, so all new tabs and panes spawn the same program. However I'd like to start a slightly different program for each new pane or tab.

For this purpose I'd like default_prog to be a function, as an alternative to a fixed table. This function would receive a table with some information about the new tab/pane, and returns a table with the command to use.

Would you accept a pull request which also allows a function for default_prog?

See below for the why 🙂

Describe alternatives you've considered
Are there any other means to do this currently?

Additional context
I'd like to spawn every new shell into a dedicated systemd scope, to isolate tabs from each other and from the parent wezterm process.

This puts each pane under separate memory/CPU accounting, and makes it become a separate target for the userspace OOM killer. If some program running in wezterm consumes an excessive amount of system memory systemd will now take down just the offending pane instead of the entire wezterm process (which otherwise forms the parent scope).

In and by itself that's easy by wrapping systemd-run around my shell in in wezterm.lua:

return {
  default_prog = {
    '/usr/bin/systemd-run',
    '--user',
    '--scope',
    '--description=Shell started by wezterm',
    '--same-dir',
    '--collect',
    '/usr/bin/fish'
  },
}

However there's a minor nuisance: The scope name gets randomly generated by systemd-run itself and isn't exactly descriptive:

$ systemctl --user status | rg -B 3 fish
           │ ├─run-r38b287d9251e426cb0d3e6997da4310c.scope
           │ │ ├─65490 /usr/bin/fish
           │ │ ├─65967 nvim .
           │ │ └─66044 /usr/bin/wl-copy --foreground --type text/plain
           │ ├─run-ra630afc556034014ab80e7a49745a99a.scope
           │ │ ├─66086 /usr/bin/fish
           │ │ ├─66378 systemctl --user status
           │ │ └─66379 rg -B 3 fish

I'd prefer if it was something like wezterm-shell-$PID-$WINDOW-$TAB-$PANE-$RANDOM.scope where $PID is the PID of the current wezterm process, $RANDOM some random string for uniqueness, and the other variables unique identifiers for the Wezterm window, tab and pane running this shell.

I can pass a custom name for the new scope with the --unit option for systemd-run, but then systemd-run won't make that name unique. In other words I have to generate a unique name for each new shell.

But I can't, because default_prog is a static table.

@swsnr swsnr added the enhancement New feature or request label Mar 27, 2022
@wez
Copy link
Owner

wez commented Mar 27, 2022

This is a neat idea! There's some plumbing to facilitate this sort of thing; on Windows, we have a WslDomain that rewrites commands to spawn via wsl.exe that is conceptually similar to this request:

wezterm/mux/src/domain.rs

Lines 176 to 212 in 4cd8933

fn fixup_command(&self, cmd: &mut CommandBuilder) {
if let Some(wsl) = &self.wsl {
let mut args: Vec<OsString> = cmd.get_argv().clone();
if args.is_empty() {
if let Some(def_prog) = &wsl.default_prog {
for arg in def_prog {
args.push(arg.into());
}
}
}
let mut argv: Vec<OsString> = vec![
"wsl.exe".into(),
"--distribution".into(),
wsl.distribution
.as_deref()
.unwrap_or(wsl.name.as_str())
.into(),
];
if let Some(cwd) = cmd.get_cwd() {
argv.push("--cd".into());
argv.push(cwd.into());
}
if let Some(user) = &wsl.username {
argv.push("--user".into());
argv.push(user.into());
}
if !args.is_empty() {
argv.push("--exec".into());
for arg in args {
argv.push(arg);
}
}

When I wrote that, I had in mind the idea that we could extend it to also spawn into eg: docker containers, so it's not a huge stretch to consider also the container/isolation provided by systemd-run as part of this.

In terms of implementation, rather than making default_prog a function, I think we could add an event that is triggered by the fixup_command method and that is passed the command builder object. That event could then return either nil (meaning no change) or an alternative command builder object.

That event would be synchronous, which means that some of the more powerful asynchronous helpers provided in the wezterm lua module would not be callable, but I think that would be sufficient for these purposes.

Note that in that context the pane_id is part of the environment set in the command builder.

@wez
Copy link
Owner

wez commented Mar 27, 2022

Here's an example of triggering a synchronous lua event; it emits the mux-is-process-stateful event and processes the result:

let hook_result = config::run_immediate_with_lua_config(|lua| {
let lua = match lua {
Some(lua) => lua,
None => return Ok(None),
};
let v = config::lua::emit_sync_callback(
&*lua,
("mux-is-process-stateful".to_string(), (info.root.clone())),
)?;
match v {
mlua::Value::Nil => Ok(None),
mlua::Value::Boolean(v) => Ok(Some(v)),
_ => Ok(None),
}
});

If you wanted to try making a PR for what I suggested above, I think the steps would be:

#[derive(Default, Clone, Deserialize, Serialize, PartialEq, Eq)]
pub struct SpawnCommand {
/// Optional descriptive label
pub label: Option<String>,
/// The command line to use.
/// If omitted, the default command associated with the
/// domain will be used instead, which is typically the
/// shell for the user.
pub args: Option<Vec<String>>,
/// Specifies the current working directory for the command.
/// If omitted, a default will be used; typically that will
/// be the home directory of the user, but may also be the
/// current working directory of the wezterm process when
/// it was launched, or for some domains it may be some
/// other location appropriate to the domain.
pub cwd: Option<PathBuf>,
/// Specifies a map of environment variables that should be set.
/// Whether this is used depends on the domain.
#[serde(default)]
pub set_environment_variables: HashMap<String, String>,
#[serde(default)]
pub domain: SpawnTabDomain,
}

  • Dynamically construct the event name as mux-local-domain-NAME-fixup-command where NAME comes from LocalDomain::name. That would allow defining multiple domains with different hooks in the future (eg: one domain per docker container, or something like that). That would make the name something like mux-local-domain-local-fixup-command for the default LocalDomain of "local".
  • Emit the event using something like (event_name, (spawn_command_we_built_earlier)) as parameters
  • We expect the lua event handler to return either nil or a SpawnCommand
  • If we get a SpawnCommand, make a new command builder using logic similar to:

let cwd = if let Some(cwd) = spawn.cwd.as_ref() {
Some(cwd.to_str().map(|s| s.to_owned()).ok_or_else(|| {
anyhow!(
"Domain::spawn requires that the cwd be unicode in {:?}",
cwd
)
})?)
} else {
None
};
let cmd_builder = if let Some(args) = spawn.args {
let mut builder = CommandBuilder::from_argv(args.iter().map(Into::into).collect());
for (k, v) in spawn.set_environment_variables.iter() {
builder.env(k, v);
}
if let Some(cwd) = spawn.cwd {
builder.cwd(cwd);
}
Some(builder)

Then in your wezterm config, you might have something like:

wezterm.on("mux-local-domain-local-fixup-command", function(cmd)
   local args = {"systemd-run", table.unpack(cmd.args)}
   cmd.args = args
   return cmd
end)

@swsnr
Copy link
Contributor Author

swsnr commented Mar 27, 2022

@wez Oh that escalated quickly 😆 But a new event is a nice idea; I see if I can make a PR about this. Will take a while though, I'm pretty busy currently.

wez added a commit that referenced this issue Jul 7, 2022
An ExecDomain is a variation on WslDomain with the key difference
being that you can control how to map the command that would be
executed.

The idea is that the user can define eg: a domain for a docker
container, or a domain that chooses to run every command in its
own cgroup.

The example below shows a really crappy implementation as a
demonstration:

```
local wezterm = require 'wezterm'

return {
  exec_domains = {
    -- Commands executed in the woot domain have "WOOT" echoed
    -- first and are then run via bash.
    -- `cmd` is a SpawnCommand
    wezterm.exec_domain("woot", function(cmd)
      if cmd.args then
        cmd.args = {
          "bash",
          "-c",
          "echo WOOT && " .. wezterm.shell_join_args(cmd.args)
        }
      end
      -- you must return the SpawnCommand that will be run
      return cmd
    end),
  },
  default_domain = "woot",
}
```

This commit unfortunately does more than should go into a single
commit, but I'm a bit too lazy to wrangle splitting it up.

* Reverts the nil/null stuff from #2177 and makes the
  `ExtendSelectionToMouseCursor` parameter mandatory to dodge
  a whole load of urgh around nil in table values. That is
  necessary because SpawnCommand uses optional fields and the
  userdata proxy was making that a PITA.
* Adds some shell quoting helper functions
* Adds ExecDomain itself, which is really just a way to
  to run a callback to fixup the command that will be run.
  That command is converted to a SpawnCommand for the callback
  to process in lua and return an adjusted version of it,
  then converted back to a command builder for execution.

refs: #1776
@wez
Copy link
Owner

wez commented Jul 7, 2022

In main, you can now do something like this:

local wezterm = require 'wezterm'

return {
  exec_domains = {
    wezterm.exec_domain("woot", function(cmd)
      wezterm.log_info(cmd)
      if cmd.args then
        cmd.args = {
          "bash",
          "-c",
          "echo WOOT && " .. wezterm.shell_join_args(cmd.args)
        }
      end
      return cmd
    end),
  },
  default_domain = "woot",
}

You can define multiple exec_domains if you wish.

The callback is passed a SpawnCommand and the callback must return a SpawnCommand that encodes the actual command that will be run.

There are a couple of helper functions that can make some things more convenient:

@wez
Copy link
Owner

wez commented Jul 8, 2022

Here's a version of your original request:

local wezterm = require 'wezterm'

-- Equivalent to POSIX basename(3)
-- Given "/foo/bar" returns "bar"
-- Given "c:\\foo\\bar" returns "bar"
local function basename(s)
  return string.gsub(s, "(.*[/\\])(.*)", "%2")
end

return {
  exec_domains = {
    wezterm.exec_domain("scoped", function(cmd)
      wezterm.log_info(cmd)

      local env = cmd.set_environment_variables
      local ident = "wezterm-pane-" .. env.WEZTERM_PANE .. "-on-" .. basename(env.WEZTERM_UNIX_SOCKET)

      local wrapped = {
        '/usr/bin/systemd-run',
        '--user',
        '--scope',
        '--description=Shell started by wezterm',
        '--same-dir',
        '--collect',
        '--unit=' .. ident,
      }

      for _, arg in ipairs(cmd.args or {os.getenv("SHELL")}) do
        table.insert(wrapped, arg)
      end

      cmd.args = wrapped

      return cmd
    end),
  },
  default_domain = "scoped",
}
$ systemctl --user status | rg -B 3 wezterm
...
           │ ├─wezterm-pane-2-on-gui-sock-19112.scope
           │ │ ├─ 19121 /bin/zsh
           │ │ ├─ 19266 systemctl --user status
           │ │ └─ 19267 /bin/zsh

@swsnr
Copy link
Contributor Author

swsnr commented Jul 8, 2022

@wez Woa, that's really great. 🤩 🎉 Can't wait to try it; when will this make it into a release? 😇

@wez
Copy link
Owner

wez commented Jul 8, 2022

I want to get feedback on how well it works before I write up docs and then make a release; can you try a nightly build?

@swsnr
Copy link
Contributor Author

swsnr commented Jul 9, 2022

Will do next week! ❤️

wez added a commit that referenced this issue Jul 9, 2022
@wez
Copy link
Owner

wez commented Jul 9, 2022

Docs should show up at https://wezfurlong.org/wezterm/config/lua/ExecDomain.html shortly

@wez wez added the fixed-in-nightly This is (or is assumed to be) fixed in the nightly builds. label Jul 9, 2022
@swsnr
Copy link
Contributor Author

swsnr commented Jul 14, 2022

@wez I tested nightly, and it works well. I had to alter the identifier slightly, though: $WEZTERM_PANE doesn't seem to change for new tabs, so your code generated the same identifier for every new tab. However systemd scope identifiers must be unique, so this broke creating new tabs.

I used a random string in place of the $WEZTERM_UNIX_SOCKET part, and things worked well. Can't wait to see this in a release 🤩

@wez
Copy link
Owner

wez commented Jul 14, 2022

the pane id is unique for each tab created by a given wezterm process. If you are running multiple instances (processes!) of wezterm then it is important to include something that is derived from the wezterm pid in the identifier. The unix socket path I showed in the example above includes the pid.

@swsnr
Copy link
Contributor Author

swsnr commented Jul 15, 2022

@wez That didn't work for me; in my test $WEZTERM_PANE was the same for all tabs. Perhaps because I started the nightly app image from an existing wezterm window?

I've just tried to start it from another terminal, and now it doesn't start at all:

ERROR  wezterm_gui > calling ExecDomain scoped function: runtime error: [string "[…]"]:86: attempt to concatenate a nil value (field 'WEZTERM_PANE')

It looks as if $WEZTERM_PANE isn't defined at the time the "wrapper" function is invoked; the value of cmd.set_environment_variables.WEZTERM_PANE seems to be nil per above error. WEZTERM_PANE also doesn't appear in the set_environment_variables table when I log cmd as in your example above.

I'm starting the App image with a custom --config-file (so as not to break my real wezterm configuration); perhaps this is causing issues?

Edit: The nightly version is wezterm 20220714-232202-84842480, i.e. the latest one at this time.

wez added a commit that referenced this issue Jul 15, 2022
This way we can ensure that it is set in the environment
in time for the lua callback to see it.

refs: #1776
@wez
Copy link
Owner

wez commented Jul 15, 2022

Ah, I see; I pushed a fix for this!

@swsnr
Copy link
Contributor Author

swsnr commented Jul 15, 2022 via email

@wez
Copy link
Owner

wez commented Jul 15, 2022

the nightly builds for linux are all pushed!

@swsnr
Copy link
Contributor Author

swsnr commented Jul 15, 2022

@wez Tested again, now it works perfectly, thanks!

@wez wez closed this as completed Jul 17, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Feb 3, 2023

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request fixed-in-nightly This is (or is assumed to be) fixed in the nightly builds.
Projects
None yet
Development

No branches or pull requests

2 participants