Fix bug with invalid process's "return code" when PlatformIO has internal error

Author: ivankravets

HISTORY.rst

@@ -31,6 +31,8 @@ Release History
 * Fixed compiling error if space is in user's folder (`issue #56 <https://github.com/ivankravets/platformio/issues/56>`_)
 * Fixed `AttributeError: 'module' object has no attribute 'disable_warnings'`
   when a version of `requests` package is less then 2.4.0
+* Fixed bug with invalid process's "return code" when PlatformIO has internal
+  error (`issue #81 <https://github.com/ivankravets/platformio/issues/81>`_)
 
 
 0.10.2 (2015-01-06)

platformio/__main__.py

@@ -8,8 +8,7 @@
 
 import click
 
-from platformio import __version__, maintenance
-from platformio.exception import PlatformioException, UnknownCLICommand
+from platformio import __version__, exception, maintenance
 from platformio.util import get_source_dir
 
 
@@ -31,7 +30,7 @@ def get_command(self, ctx, name):
             mod = __import__("platformio.commands." + name,
                              None, None, ["cli"])
         except ImportError:
-            raise UnknownCLICommand(name)
+            raise exception.UnknownCLICommand(name)
         return mod.cli
 
 
@@ -52,12 +51,13 @@ def main():
     try:
         cli(None)
     except Exception as e:  # pylint: disable=W0703
-        maintenance.on_platformio_exception(e)
-        if isinstance(e, PlatformioException):
-            click.echo("Error: " + str(e), err=True)
-            sys_exit(1)
-        else:
-            print format_exc()
+        if not isinstance(e, exception.ReturnErrorCode):
+            maintenance.on_platformio_exception(e)
+            if isinstance(e, exception.PlatformioException):
+                click.echo("Error: " + str(e), err=True)
+            else:
+                click.echo(format_exc(), err=True)
+        sys_exit(1)
 
 
 if __name__ == "__main__":

platformio/commands/run.py

@@ -36,6 +36,7 @@ def cli(ctx, environment, target, upload_port):
             getmtime(_pioenvs_dir)):
         rmtree(_pioenvs_dir)
 
+    found_error = False
     _first_done = False
     for section in config.sections():
         # skip main configuration section
@@ -56,9 +57,13 @@ def cli(ctx, environment, target, upload_port):
         if _first_done:
             click.echo()
 
-        process_environment(ctx, envname, options, target, upload_port)
+        if not process_environment(ctx, envname, options, target, upload_port):
+            found_error = True
         _first_done = True
 
+    if found_error:
+        raise exception.ReturnErrorCode()
+
 
 def process_environment(ctx, name, options, targets, upload_port):
     terminal_width, _ = click.get_terminal_size()
@@ -72,8 +77,8 @@ def process_environment(ctx, name, options, targets, upload_port):
     click.secho("-" * terminal_width, bold=True)
 
     result = _run_environment(ctx, name, options, targets, upload_port)
-    is_error = "error" in result['err'].lower()
 
+    is_error = result['returncode'] != 0
     summary_text = " Took %.2f seconds " % (time() - start_time)
     half_line = "=" * ((terminal_width - len(summary_text) - 10) / 2)
     click.echo("%s [%s]%s%s" % (
@@ -84,6 +89,8 @@ def process_environment(ctx, name, options, targets, upload_port):
         half_line
     ), err=is_error)
 
+    return not is_error
+
 
 def _run_environment(ctx, name, options, targets, upload_port):
     variables = ["PIOENV=" + name]

platformio/exception.py

@@ -13,6 +13,10 @@ def __str__(self):  # pragma: no cover
             return Exception.__str__(self)
 
 
+class ReturnErrorCode(PlatformioException):
+    pass
+
+
 class AbortedByUser(PlatformioException):
 
     MESSAGE = "Aborted by user"

platformio/platforms/base.py

@@ -1,6 +1,7 @@
 # Copyright (C) Ivan Kravets <[email protected]>
 # See LICENSE for details.
 
+import re
 from imp import load_source
 from os import listdir
 from os.path import isdir, isfile, join
@@ -76,6 +77,10 @@ def newPlatform(cls, name):
 class BasePlatform(object):
 
     PACKAGES = {}
+    LINE_ERROR_RE = re.compile(r"(\s+error|error[:\s]+)", re.I)
+
+    def __init__(self):
+        self._found_error = False
 
     def get_name(self):
         return self.__class__.__name__[:-8].lower()
@@ -214,6 +219,7 @@ def run(self, variables, targets):
             variables.append(
                 "PIOPACKAGE_%s=%s" % (options['alias'].upper(), name))
 
+        self._found_error = False
         try:
             result = util.exec_command(
                 [
@@ -227,9 +233,10 @@ def run(self, variables, targets):
         except OSError:
             raise exception.SConsNotInstalled()
 
-        return self.after_run(result)
+        assert "returncode" in result
+        if self._found_error:
+            result['returncode'] = 1
 
-    def after_run(self, result):  # pylint: disable=R0201
         return result
 
     def on_run_out(self, line):  # pylint: disable=R0201
@@ -239,5 +246,7 @@ def on_run_out(self, line):  # pylint: disable=R0201
         click.secho(line, fg=fg)
 
     def on_run_err(self, line):  # pylint: disable=R0201
-        click.secho(line, err=True,
-                    fg="red" if "error" in line.lower() else "yellow")
+        is_error = self.LINE_ERROR_RE.search(line) is not None
+        if is_error:
+            self._found_error = True
+        click.secho(line, err=True, fg="red" if is_error else "yellow")

platformio/util.py

@@ -117,7 +117,12 @@ def change_filemtime(path, time):
 
 
 def exec_command(*args, **kwargs):
-    result = {"out": None, "err": None}
+    result = {
+        "out": None,
+        "err": None,
+        "returncode": None
+    }
+
     default = dict(
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE,
@@ -129,6 +134,7 @@ def exec_command(*args, **kwargs):
     p = subprocess.Popen(*args, **kwargs)
     try:
         result['out'], result['err'] = p.communicate()
+        result['returncode'] = p.returncode
     except KeyboardInterrupt:
         for s in ("stdout", "stderr"):
             if isinstance(kwargs[s], AsyncPipe):
@@ -141,9 +147,8 @@ def exec_command(*args, **kwargs):
             result[s[3:]] = "\n".join(kwargs[s].get_buffer())
 
     for k, v in result.iteritems():
-        if not v:
-            continue
-        result[k].strip()
+        if v and isinstance(v, basestring):
+            result[k].strip()
 
     return result
 

tests/test_examples.py

@@ -30,9 +30,8 @@ def test_run(platformio_setup, pioproject_dir):
         ["platformio", "run"],
         cwd=pioproject_dir
     )
-    output = "%s\n%s" % (result['out'], result['err'])
-    if "error" in output.lower():
-        pytest.fail(output)
+    if result['returncode'] != 0:
+        pytest.fail(result)
 
     # check .elf file
     pioenvs_dir = join(pioproject_dir, ".pioenvs")