| Summary: | Terminal (devshell, menuconfig) code broken in latest master | ||
|---|---|---|---|
| Product: | [Build System, Metadata & Runtime] OE-Core | Reporter: | Paul Eggleton <bluelightning> |
| Component: | core | Assignee: | Chen Qi <Qi.Chen> |
| Status: | VERIFIED FIXED | QA Contact: | |
| Severity: | major | ||
| Priority: | Medium+ | CC: | alexandrux.palalau, liezhi.yang, meta.mr.watcher, meta.watcher, Qi.Chen, richard.purdie, sgw, zhenfeng.zhao |
| Version: | 1.4 | ||
| Target Milestone: | 1.4 M1 | ||
| Hardware: | All | ||
| OS: | Multiple | ||
| Whiteboard: | A raw solution, waiting for advices and suggestions | ||
| OS type for building Yocto: | --- | Type of Regression: | --- |
| Verified: | Documentation change: | --- | |
|
Description
Paul Eggleton
2012-10-31 15:06:39 UTC
Oops, that second example should have been bitbake -c menuconfig virtual/kernel. %problem, cause, solution and a raw patch%
A rough analysis about this issue. Any suggestion would be welcome.
problem
-------
1) two terminals are launched instead of one
2) the environment is not correct, 'echo $PATH' for a verification
cause
-----
c76da87511d2668479745c2f18b8a9b8116c7489 in OE core changed the default behavior of terminal.bbclass
Problem 1 may be caused by the os.environ not correctly set, as that commit removed "os.environ[export] = str(value)".
Problem 2 is caused by passing wrong argument to the oe.terminal.spawn which invokes subprocess.Popen to do the actual job underneath.
- oe.terminal.spawn(terminal, command, title, None, d)
+ oe.terminal.spawn(terminal, command, title, env, d)
Changing the default fourth para from 'None' to env will override the environment in child process. The environment in child process in no longer the same with its parent which has the correct values for environment variables such as PATH.
solution
--------
Change all things back. The env argument is not needed, and what's more, it causes trouble.
patch
-----
diff --git a/meta/classes/terminal.bbclass b/meta/classes/terminal.bbclass
index 51846c1..906fa58 100644
--- a/meta/classes/terminal.bbclass
+++ b/meta/classes/terminal.bbclass
@@ -15,19 +15,17 @@ def oe_terminal(command, title, d):
import oe.data
import oe.terminal
- env = dict()
-
for export in oe.data.typed_value('OE_TERMINAL_EXPORTS', d):
value = d.getVar(export, True)
if value is not None:
- env[export] = str(value)
+ os.environ[export] = str(value)
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, env, d)
+ oe.terminal.spawn(terminal, command, title, None, d)
return
except oe.terminal.UnsupportedTerminal:
bb.warn('Unsupported terminal "%s", defaulting to "auto"' %
@@ -36,7 +34,7 @@ def oe_terminal(command, title, d):
bb.fatal('Unable to spawn terminal %s: %s' % (terminal, exc))
try:
- oe.terminal.spawn_preferred(command, title, env, d)
+ oe.terminal.spawn_preferred(command, title, None, d)
except oe.terminal.NoSupportedTerminals:
bb.fatal('No valid terminal found, unable to open devshell')
except oe.terminal.ExecutionError as exc:
An explanation of the 'env' para in subprocess.Popen: "If env is not None, it must be a mapping that defines the environment variables for the new process; these are used instead of inheriting the current process’ environment, which is the default behavior." From: http://docs.python.org/2/library/subprocess.html#subprocess.Popen I have to say I think we should just revert the patch in question. (In reply to comment #4) > I have to say I think we should just revert the patch in question. Ah... Yes. Reverting it would be better. Thanks. Fixed in master: http://git.yoctoproject.org/cgit.cgi/poky/commit/?id=05a1f4a5a32c6cc392fa4480732101d668cd6f77 http://git.yoctoproject.org/cgit.cgi/poky/commit/?id=ad9ad06056bb9ee1f9da54d60dfabb3240798018 Switching to verified |