Patchwork [1/8] Rework how the devshell functions

login
register
mail settings
Submitter Otavio Salvador
Date July 8, 2011, 1:47 p.m.
Message ID <3e091c2f3f0f39d3557004dba908b650e9aea7fb.1310132765.git.otavio@ossystems.com.br>
Download mbox | patch
Permalink /patch/7237/
State New, archived
Headers show

Comments

Otavio Salvador - July 8, 2011, 1:47 p.m.
From: Chris Larson <chris_larson@mentor.com>

In the new implementation, each known terminal is defined as a class in
oe.terminal, as a subclass of bb.process.Popen.  terminal.bbclass wraps this
functionality, providing the metadata pieces.  It obeys the OE_TERMINAL
variable, which is a 'choice' typed variable.  This variable may be 'auto',
'none', or any of the names of the defined terminals.

When using 'auto', or requesting an unsupported terminal, we attempt to spawn
them in priority order until we get one that's available on this system (and
in the case of the X terminals, has DISPLAY defined).  The 'none' value is
used when we're doing things like automated builds, and want to ensure that no
terminal is *ever* spawned, under any circumstances.

Current available terminals:

    gnome
    konsole
    xterm
    rxvt
    screen

Signed-off-by: Chris Larson <chris_larson@mentor.com>
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
---
 meta/classes/devshell.bbclass |   26 ++++-------
 meta/classes/terminal.bbclass |   30 ++++++++++++
 meta/lib/oe/classutils.py     |    2 +
 meta/lib/oe/terminal.py       |  102 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 143 insertions(+), 17 deletions(-)
 create mode 100644 meta/classes/terminal.bbclass
 create mode 100644 meta/lib/oe/terminal.py
Richard Purdie - July 8, 2011, 3:33 p.m.
On Fri, 2011-07-08 at 13:47 +0000, Otavio Salvador wrote:
> From: Chris Larson <chris_larson@mentor.com>
> 
> In the new implementation, each known terminal is defined as a class in
> oe.terminal, as a subclass of bb.process.Popen.  terminal.bbclass wraps this
> functionality, providing the metadata pieces.  It obeys the OE_TERMINAL
> variable, which is a 'choice' typed variable.  This variable may be 'auto',
> 'none', or any of the names of the defined terminals.
> 
> When using 'auto', or requesting an unsupported terminal, we attempt to spawn
> them in priority order until we get one that's available on this system (and
> in the case of the X terminals, has DISPLAY defined).  The 'none' value is
> used when we're doing things like automated builds, and want to ensure that no
> terminal is *ever* spawned, under any circumstances.
> 
> Current available terminals:
> 
>     gnome
>     konsole
>     xterm
>     rxvt
>     screen
> 
> Signed-off-by: Chris Larson <chris_larson@mentor.com>
> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> ---
>  meta/classes/devshell.bbclass |   26 ++++-------
>  meta/classes/terminal.bbclass |   30 ++++++++++++
>  meta/lib/oe/classutils.py     |    2 +
>  meta/lib/oe/terminal.py       |  102 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 143 insertions(+), 17 deletions(-)
>  create mode 100644 meta/classes/terminal.bbclass
>  create mode 100644 meta/lib/oe/terminal.py
> 
> diff --git a/meta/classes/devshell.bbclass b/meta/classes/devshell.bbclass
> index 5f262f4..7317d64 100644
> --- a/meta/classes/devshell.bbclass
> +++ b/meta/classes/devshell.bbclass
> @@ -1,22 +1,14 @@
> -do_devshell[dirs] = "${S}"
> -do_devshell[nostamp] = "1"
> +inherit terminal
>  
> -XAUTHORITY ?= "${HOME}/.Xauthority"
>  
> -devshell_do_devshell() {
> -	export DISPLAY='${DISPLAY}'
> -	export DBUS_SESSION_BUS_ADDRESS='${DBUS_SESSION_BUS_ADDRESS}'
> -	export XAUTHORITY='${XAUTHORITY}'
> -	export TERMWINDOWTITLE="Bitbake Developer Shell"
> -	export EXTRA_OEMAKE='${EXTRA_OEMAKE}'
> -	export SHELLCMDS="bash"
> -	${TERMCMDRUN}
> -	if [ $? -ne 0 ]; then
> -	    echo "Fatal: '${TERMCMD}' not found. Check TERMCMD variable."
> -	    exit 1
> -	fi

For reference, the reason I keep putting off merging this is that there
are a number of variables above that were added there for a reason (e.g.
DBUS_SESSION_BUS_ADDRESS) and this patch wipes them out.

The patch therefore makes someone's usecase work better and the expense
of breaking some others and to me that doesn't make a good patch.

I'm the first to agree that the bitbake environment handling sucks,
badly. It would be nice if there were some way we could save off the
original environment and then restore it under certain conditions which
would then make the need for gross code like the above go away. Likely
that would need changes at the bitbake level but I think we need to try
and fix this problem properly...

[I'm fine with the other changes that patch makes, I just worry about
the environment bits]

Cheers,

Richard

Patch

diff --git a/meta/classes/devshell.bbclass b/meta/classes/devshell.bbclass
index 5f262f4..7317d64 100644
--- a/meta/classes/devshell.bbclass
+++ b/meta/classes/devshell.bbclass
@@ -1,22 +1,14 @@ 
-do_devshell[dirs] = "${S}"
-do_devshell[nostamp] = "1"
+inherit terminal
 
-XAUTHORITY ?= "${HOME}/.Xauthority"
 
-devshell_do_devshell() {
-	export DISPLAY='${DISPLAY}'
-	export DBUS_SESSION_BUS_ADDRESS='${DBUS_SESSION_BUS_ADDRESS}'
-	export XAUTHORITY='${XAUTHORITY}'
-	export TERMWINDOWTITLE="Bitbake Developer Shell"
-	export EXTRA_OEMAKE='${EXTRA_OEMAKE}'
-	export SHELLCMDS="bash"
-	${TERMCMDRUN}
-	if [ $? -ne 0 ]; then
-	    echo "Fatal: '${TERMCMD}' not found. Check TERMCMD variable."
-	    exit 1
-	fi
+export XAUTHORITY ?= "${HOME}/.Xauthority"
+export SHELL ?= 'bash'
+
+python do_devshell () {
+    oe_terminal(d.getVar('SHELL', True), 'OpenEmbedded Developer Shell', d)
 }
-addtask devshell after do_patch
 
-EXPORT_FUNCTIONS do_devshell
+addtask devshell after do_patch
 
+do_devshell[dirs] = "${S}"
+do_devshell[nostamp] = "1"
diff --git a/meta/classes/terminal.bbclass b/meta/classes/terminal.bbclass
new file mode 100644
index 0000000..93646f7
--- /dev/null
+++ b/meta/classes/terminal.bbclass
@@ -0,0 +1,30 @@ 
+OE_TERMINAL ?= 'auto'
+OE_TERMINAL[type] = 'choice'
+OE_TERMINAL[choices] = 'auto none \
+                        ${@" ".join(o.name \
+                                    for o in oe.terminal.prioritized())}'
+
+
+def oe_terminal(command, title, d):
+    import oe.data
+    import oe.terminal
+
+    terminal = oe.data.typed_value('OE_TERMINAL', d).lower()
+    if terminal == 'none':
+        bb.fatal('Devshell usage disabled with OE_TERMINAL')
+    elif terminal != 'auto':
+        try:
+            oe.terminal.spawn(terminal, command, title)
+            return
+        except oe.terminal.UnsupportedTerminal:
+            bb.warn('Unsupported terminal "%s", defaulting to "auto"' %
+                    terminal)
+        except oe.terminal.ExecutionError as exc:
+            bb.fatal('Unable to spawn terminal %s: %s' % (terminal, exc))
+
+    try:
+        oe.terminal.spawn_preferred(command, title)
+    except oe.terminal.NoSupportedTerminals:
+        bb.fatal('No valid terminal found, unable to open devshell')
+    except oe.terminal.ExecutionError as exc:
+        bb.fatal('Unable to spawn terminal %s: %s' % (terminal, exc))
diff --git a/meta/lib/oe/classutils.py b/meta/lib/oe/classutils.py
index 58188fd..42b0f49 100644
--- a/meta/lib/oe/classutils.py
+++ b/meta/lib/oe/classutils.py
@@ -1,3 +1,5 @@ 
+__author__ = 'kergoth'
+
 class ClassRegistry(type):
     """Maintain a registry of classes, indexed by name.
 
diff --git a/meta/lib/oe/terminal.py b/meta/lib/oe/terminal.py
new file mode 100644
index 0000000..5336167
--- /dev/null
+++ b/meta/lib/oe/terminal.py
@@ -0,0 +1,102 @@ 
+import logging
+import os
+import oe.classutils
+import shlex
+from bb.process import Popen, ExecutionError
+
+logger = logging.getLogger('BitBake.OE.Terminal')
+
+
+class UnsupportedTerminal(StandardError):
+    pass
+
+class NoSupportedTerminals(StandardError):
+    pass
+
+
+class Registry(oe.classutils.ClassRegistry):
+    command = None
+
+    def __init__(cls, name, bases, attrs):
+        super(Registry, cls).__init__(name.lower(), bases, attrs)
+
+    @property
+    def implemented(cls):
+        return bool(cls.command)
+
+
+class Terminal(Popen):
+    __metaclass__ = Registry
+
+    def __init__(self, command, title=None):
+        self.format_command(command, title)
+        logger.debug(1, "%s: running %s", self.name, self.command)
+
+        try:
+            Popen.__init__(self, self.command, shell=False)
+        except OSError as exc:
+            import errno
+            if exc.errno == errno.ENOENT:
+                raise UnsupportedTerminal(self.name)
+            else:
+                raise
+
+    def format_command(self, command, title):
+        fmt = {'title': title or 'Terminal', 'command': command}
+        if isinstance(self.command, basestring):
+            self.command = shlex.split(self.command.format(**fmt))
+        else:
+            self.command = [element.format(**fmt) for element in self.command]
+
+class XTerminal(Terminal):
+    def __init__(self, command, title=None):
+        Terminal.__init__(self, command, title)
+        if not os.environ.get('DISPLAY'):
+            raise UnsupportedTerminal(self.name)
+
+class Gnome(XTerminal):
+    command = 'gnome-terminal --disable-factory -t "{title}" -x {command}'
+    priority = 2
+
+class Konsole(XTerminal):
+    command = 'konsole -T "{title}" -e {command}'
+    priority = 2
+
+class XTerm(XTerminal):
+    command = 'xterm -T "{title}" -e {command}'
+    priority = 1
+
+class Rxvt(XTerminal):
+    command = 'rxvt -T "{title}" -e {command}'
+    priority = 1
+
+class Screen(Terminal):
+    command = 'screen -D -m -t "{title}" {command}'
+
+
+def prioritized():
+    return Registry.prioritized()
+
+def spawn_preferred(command, title=None):
+    """Spawn the first supported terminal, by priority"""
+    for terminal in prioritized():
+        try:
+            spawn(terminal.name, command, title)
+            break
+        except UnsupportedTerminal:
+            continue
+    else:
+        raise NoSupportedTerminals()
+
+def spawn(name, command, title=None):
+    """Spawn the specified terminal, by name"""
+    logger.debug(1, 'Attempting to spawn terminal "%s"', name)
+    try:
+        terminal = Registry.registry[name]
+    except KeyError:
+        raise UnsupportedTerminal(name)
+
+    pipe = terminal(command, title)
+    output = pipe.communicate()[0]
+    if pipe.returncode != 0:
+        raise ExecutionError(pipe.command, pipe.returncode, output)