Patchwork [PATCHv3] oe.terminal: add tmux classes

login
register
mail settings
Submitter Christopher Larson
Date April 5, 2013, 12:44 a.m.
Message ID <1365122683-17652-1-git-send-email-kergoth@gmail.com>
Download mbox | patch
Permalink /patch/47463/
State New
Headers show

Comments

Christopher Larson - April 5, 2013, 12:44 a.m.
From: Christopher Larson <chris_larson@mentor.com>

This adds two new Terminal classes. It's separated into two, so that opening
a split inside a tmux window is preferred to the other terminal types, but
opening a tmux session is prioritized only slightly higher than screen.

- tmuxrunning: Open a new pane in the current running tmux window. Requires
  that the TMUX variable be added to the env whitelist to use it.
- tmux: Open a new tmux session

Signed-off-by: Christopher Larson <chris_larson@mentor.com>
---
 meta/lib/oe/terminal.py | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)
Jason Wessel - April 5, 2013, 1:55 p.m.
On 04/04/2013 07:44 PM, Christopher Larson wrote:
> From: Christopher Larson <chris_larson@mentor.com>
>
> This adds two new Terminal classes. It's separated into two, so that opening
> a split inside a tmux window is preferred to the other terminal types, but
> opening a tmux session is prioritized only slightly higher than screen.
>
> - tmuxrunning: Open a new pane in the current running tmux window. Requires
>   that the TMUX variable be added to the env whitelist to use it.
> - tmux: Open a new tmux session
>
> Signed-off-by: Christopher Larson <chris_larson@mentor.com>
> ---
>  meta/lib/oe/terminal.py | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/meta/lib/oe/terminal.py b/meta/lib/oe/terminal.py
> index 2e23d59..e52863f 100644
> --- a/meta/lib/oe/terminal.py
> +++ b/meta/lib/oe/terminal.py
> @@ -142,6 +142,44 @@ class TmuxNewSession(Terminal):
>          else:
>              logger.warn(msg)
>  
> +class TmuxRunning(Terminal):
> +    """Open a new pane in the current running tmux window"""
> +    name = 'tmux-running'
> +    command = 'tmux split-window {command}'
> +    priority = 2.75
> +
> +    def __init__(self, sh_cmd, title=None, env=None, d=None):
> +        if not bb.utils.which(os.getenv('PATH'), 'tmux'):
> +            raise UnsupportedTerminal('tmux is not installed')
> +
> +        if not os.getenv('TMUX'):
> +            raise UnsupportedTerminal('tmux is not running')
> +
> +        Terminal.__init__(self, sh_cmd, title, env, d)


When we chatted yesterday I didn't fully understand how the Tmux Running piece worked.   Now I get it, you run tmux and then start a build with in that session and just join back to it when the TMUX var is set because that is how TMUX does internal communications setup.  I have to admit that is pretty slick. :-)

It seems to me we can just fold these two classes together and drive it off the TMUX variable.   This way we can just have one OE TERMINAL type, and if you don't want the behavior to engage of the "split" you just unset TMUX before you build.   I believe that this covers all the cases you care about, and would allow for the simplistic case I care about where I just set OE_TERMINAL=tmux to work for both cases. I would recommend we just patch up the bitbake.conf BBHASH pieces all in one shot with this patch so their is nothing special anyone has to do to make use of this.

Would you be ok with that?

Further work that I'll probably consider doing if no one else does it first is to add tmux-native so we always have it as the fall back for getting to patch failures and such on a remote build server.

Cheers,
Jason.



> +
> +class Tmux(Terminal):
> +    """Start a new tmux session and window"""
> +    command = 'tmux new -d -s devshell -n devshell {command}'
> +    priority = 0.75
> +
> +    def __init__(self, sh_cmd, title=None, env=None, d=None):
> +        if not bb.utils.which(os.getenv('PATH'), 'tmux'):
> +            raise UnsupportedTerminal('tmux is not installed')
> +
> +        # TODO: consider using a 'devshell' session shared amongst all
> +        # devshells, if it's already there, add a new window to it.
> +        window_name = 'devshell-%i' % os.getpid()
> +
> +        self.command = 'tmux new -d -s {0} -n {0} {{command}}'.format(window_name)
> +        Terminal.__init__(self, sh_cmd, title, env, d)
> +
> +        attach_cmd = 'tmux att -t {0}'.format(window_name)
> +        msg = 'Tmux started. Please connect in another terminal with `tmux att -t {0}`'.format(window_name)
> +        if d:
> +            bb.event.fire(bb.event.LogExecTTY(msg, attach_cmd, 0.5, 10), d)
> +        else:
> +            logger.warn(msg)
> +
>  class Custom(Terminal):
>      command = 'false' # This is a placeholder
>      priority = 3
Christopher Larson - April 5, 2013, 2:20 p.m.
On Fri, Apr 5, 2013 at 6:55 AM, Jason Wessel <jason.wessel@windriver.com>wrote:

> On 04/04/2013 07:44 PM, Christopher Larson wrote:
> > From: Christopher Larson <chris_larson@mentor.com>
> >
> > This adds two new Terminal classes. It's separated into two, so that
> opening
> > a split inside a tmux window is preferred to the other terminal types,
> but
> > opening a tmux session is prioritized only slightly higher than screen.
> >
> > - tmuxrunning: Open a new pane in the current running tmux window.
> Requires
> >   that the TMUX variable be added to the env whitelist to use it.
> > - tmux: Open a new tmux session
> >
> > Signed-off-by: Christopher Larson <chris_larson@mentor.com>
> > ---
> >  meta/lib/oe/terminal.py | 38 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> >
> > diff --git a/meta/lib/oe/terminal.py b/meta/lib/oe/terminal.py
> > index 2e23d59..e52863f 100644
> > --- a/meta/lib/oe/terminal.py
> > +++ b/meta/lib/oe/terminal.py
> > @@ -142,6 +142,44 @@ class TmuxNewSession(Terminal):
> >          else:
> >              logger.warn(msg)
> >
> > +class TmuxRunning(Terminal):
> > +    """Open a new pane in the current running tmux window"""
> > +    name = 'tmux-running'
> > +    command = 'tmux split-window {command}'
> > +    priority = 2.75
> > +
> > +    def __init__(self, sh_cmd, title=None, env=None, d=None):
> > +        if not bb.utils.which(os.getenv('PATH'), 'tmux'):
> > +            raise UnsupportedTerminal('tmux is not installed')
> > +
> > +        if not os.getenv('TMUX'):
> > +            raise UnsupportedTerminal('tmux is not running')
> > +
> > +        Terminal.__init__(self, sh_cmd, title, env, d)
>
>
> When we chatted yesterday I didn't fully understand how the Tmux Running
> piece worked.   Now I get it, you run tmux and then start a build with in
> that session and just join back to it when the TMUX var is set because that
> is how TMUX does internal communications setup.  I have to admit that is
> pretty slick. :-)
>
> It seems to me we can just fold these two classes together and drive it
> off the TMUX variable.   This way we can just have one OE TERMINAL type,
> and if you don't want the behavior to engage of the "split" you just unset
> TMUX before you build.   I believe that this covers all the cases you care
> about, and would allow for the simplistic case I care about where I just
> set OE_TERMINAL=tmux to work for both cases. I would recommend we just
> patch up the bitbake.conf BBHASH pieces all in one shot with this patch so
> their is nothing special anyone has to do to make use of this.
>
> Would you be ok with that?
>
> Further work that I'll probably consider doing if no one else does it
> first is to add tmux-native so we always have it as the fall back for
> getting to patch failures and such on a remote build server.


The problem with combining them is they'd be stuck at the same priority,
which makes 'auto' less useful. In the case where DISPLAY and TMUX are both
set, I think it makes more sense to prefer to interact with the running
tmux session rather than spawning a new xterm, yet spawning a new tmux
session should be kept at the same priority as screen to meet user
expectations.

A decent compromise, to make the explicit OE_TERMINAL=tmux case more
useful, would be to merge the tmuxrunning bits into tmux, but still keep
the independent tmuxrunning class for the 'auto' case. Thoughts on that?
Jason Wessel - April 5, 2013, 10:10 p.m.
On 04/05/2013 09:20 AM, Chris Larson wrote:
>
> On Fri, Apr 5, 2013 at 6:55 AM, Jason Wessel <jason.wessel@windriver.com <mailto:jason.wessel@windriver.com>> wrote:
>
>     On 04/04/2013 07:44 PM, Christopher Larson wrote:
>     > From: Christopher Larson <chris_larson@mentor.com <mailto:chris_larson@mentor.com>>
>     >
>     > This adds two new Terminal classes. It's separated into two, so that opening
>     > a split inside a tmux window is preferred to the other terminal types, but
>     > opening a tmux session is prioritized only slightly higher than screen.
>     >
>     > - tmuxrunning: Open a new pane in the current running tmux window. Requires
>     >   that the TMUX variable be added to the env whitelist to use it.
>     > - tmux: Open a new tmux session
>     >
>     > Signed-off-by: Christopher Larson <chris_larson@mentor.com <mailto:chris_larson@mentor.com>>
>     > ---
>     >  meta/lib/oe/terminal.py | 38 ++++++++++++++++++++++++++++++++++++++
>     >  1 file changed, 38 insertions(+)
>     >
>     > diff --git a/meta/lib/oe/terminal.py b/meta/lib/oe/terminal.py
>     > index 2e23d59..e52863f 100644
>     > --- a/meta/lib/oe/terminal.py
>     > +++ b/meta/lib/oe/terminal.py
>     > @@ -142,6 +142,44 @@ class TmuxNewSession(Terminal):
>     >          else:
>     >              logger.warn(msg)
>     >
>     > +class TmuxRunning(Terminal):
>     > +    """Open a new pane in the current running tmux window"""
>     > +    name = 'tmux-running'
>     > +    command = 'tmux split-window {command}'
>     > +    priority = 2.75
>     > +
>     > +    def __init__(self, sh_cmd, title=None, env=None, d=None):
>     > +        if not bb.utils.which(os.getenv('PATH'), 'tmux'):
>     > +            raise UnsupportedTerminal('tmux is not installed')
>     > +
>     > +        if not os.getenv('TMUX'):
>     > +            raise UnsupportedTerminal('tmux is not running')
>     > +
>     > +        Terminal.__init__(self, sh_cmd, title, env, d)
>
>
>     When we chatted yesterday I didn't fully understand how the Tmux Running piece worked.   Now I get it, you run tmux and then start a build with in that session and just join back to it when the TMUX var is set because that is how TMUX does internal communications setup.  I have to admit that is pretty slick. :-)
>
>     It seems to me we can just fold these two classes together and drive it off the TMUX variable.   This way we can just have one OE TERMINAL type, and if you don't want the behavior to engage of the "split" you just unset TMUX before you build.   I believe that this covers all the cases you care about, and would allow for the simplistic case I care about where I just set OE_TERMINAL=tmux to work for both cases. I would recommend we just patch up the bitbake.conf BBHASH pieces all in one shot with this patch so their is nothing special anyone has to do to make use of this.
>
>     Would you be ok with that?
>
>     Further work that I'll probably consider doing if no one else does it first is to add tmux-native so we always have it as the fall back for getting to patch failures and such on a remote build server.
>
>
> The problem with combining them is they'd be stuck at the same priority, which makes 'auto' less useful. In the case where DISPLAY and TMUX are both set, I think it makes more sense to prefer to interact with the running tmux session rather than spawning a new xterm, yet spawning a new tmux session should be kept at the same priority as screen to meet user expectations.
>
> A decent compromise, to make the explicit OE_TERMINAL=tmux case more useful, would be to merge the tmuxrunning bits into tmux, but still keep the independent tmuxrunning class for the 'auto' case. Thoughts on that?


Based on how the auto list processing works your latest proposed solution seems like the most reasonable choice.

Thanks,
Jason.

Patch

diff --git a/meta/lib/oe/terminal.py b/meta/lib/oe/terminal.py
index 2e23d59..e52863f 100644
--- a/meta/lib/oe/terminal.py
+++ b/meta/lib/oe/terminal.py
@@ -142,6 +142,44 @@  class TmuxNewSession(Terminal):
         else:
             logger.warn(msg)
 
+class TmuxRunning(Terminal):
+    """Open a new pane in the current running tmux window"""
+    name = 'tmux-running'
+    command = 'tmux split-window {command}'
+    priority = 2.75
+
+    def __init__(self, sh_cmd, title=None, env=None, d=None):
+        if not bb.utils.which(os.getenv('PATH'), 'tmux'):
+            raise UnsupportedTerminal('tmux is not installed')
+
+        if not os.getenv('TMUX'):
+            raise UnsupportedTerminal('tmux is not running')
+
+        Terminal.__init__(self, sh_cmd, title, env, d)
+
+class Tmux(Terminal):
+    """Start a new tmux session and window"""
+    command = 'tmux new -d -s devshell -n devshell {command}'
+    priority = 0.75
+
+    def __init__(self, sh_cmd, title=None, env=None, d=None):
+        if not bb.utils.which(os.getenv('PATH'), 'tmux'):
+            raise UnsupportedTerminal('tmux is not installed')
+
+        # TODO: consider using a 'devshell' session shared amongst all
+        # devshells, if it's already there, add a new window to it.
+        window_name = 'devshell-%i' % os.getpid()
+
+        self.command = 'tmux new -d -s {0} -n {0} {{command}}'.format(window_name)
+        Terminal.__init__(self, sh_cmd, title, env, d)
+
+        attach_cmd = 'tmux att -t {0}'.format(window_name)
+        msg = 'Tmux started. Please connect in another terminal with `tmux att -t {0}`'.format(window_name)
+        if d:
+            bb.event.fire(bb.event.LogExecTTY(msg, attach_cmd, 0.5, 10), d)
+        else:
+            logger.warn(msg)
+
 class Custom(Terminal):
     command = 'false' # This is a placeholder
     priority = 3