From 44fe0370f8fc1ad34db859d67f86661caf7718fa Mon Sep 17 00:00:00 2001
From: Brendan Jackman <brendan.jackman@arm.com>
Date: Wed, 25 Jan 2017 12:06:38 +0000
Subject: [PATCH] android: Handle variation in ADB behaviour

On older combinations of ADB/Android versions, the adb host command always
exits with 0 if it was able to run the command on the target, even if the
command failed (https://code.google.com/p/android/issues/detail?id=3254).

When we need the target's exit code (check_exit_code=True), we currently work
around this behaviour by echoing the target's exit code after the command and
parsing it out of the output.

The ADB behaviour is now "fixed" on newer versions with newer Androids (It's not
clear which versions these are and it appears that different builds of ADB with
the same version number differ in this respect). For those version combinations
adb_shell will currently raise a CalledProcessError when check_exit_code=False
and the target command fails.

So lets now use the "echo $?" trick whether or not we need the exit code.

Fixes https://github.com/ARM-software/devlib/issues/85
---
 devlib/utils/android.py | 45 ++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/devlib/utils/android.py b/devlib/utils/android.py
index 1a195a6..b3479a8 100644
--- a/devlib/utils/android.py
+++ b/devlib/utils/android.py
@@ -335,29 +335,33 @@ def adb_shell(device, command, timeout=None, check_exit_code=False,
     if as_root:
         command = 'echo \'{}\' | su'.format(escape_single_quotes(command))
     device_part = ['-s', device] if device else []
-    device_string = ' {} {}'.format(*device_part) if device_part else ''
-    full_command = 'adb{} shell "{}"'.format(device_string,
-                                             escape_double_quotes(command))
-    logger.debug(full_command)
-    if check_exit_code:
-        adb_shell_command = '({}); echo \"\n$?\"'.format(command)
-        actual_command = ['adb'] + device_part + ['shell', adb_shell_command]
-        raw_output, error = check_output(actual_command, timeout, shell=False)
-        if raw_output:
-            try:
-                output, exit_code, _ = raw_output.rsplit(newline_separator, 2)
-            except ValueError:
-                exit_code, _ = raw_output.rsplit(newline_separator, 1)
-                output = ''
-        else:  # raw_output is empty
-            exit_code = '969696'  # just because
-            output = ''
 
+    # On older combinations of ADB/Android versions, the adb host command always
+    # exits with 0 if it was able to run the command on the target, even if the
+    # command failed (https://code.google.com/p/android/issues/detail?id=3254).
+    # Homogenise this behaviour by running the command then echoing the exit
+    # code.
+    adb_shell_command = '({}); echo \"\n$?\"'.format(command)
+    actual_command = ['adb'] + device_part + ['shell', adb_shell_command]
+    logger.debug('adb {} shell {}'.format(' '.join(device_part), command))
+    raw_output, error = check_output(actual_command, timeout, shell=False)
+    if raw_output:
+        try:
+            output, exit_code, _ = raw_output.rsplit(newline_separator, 2)
+        except ValueError:
+            exit_code, _ = raw_output.rsplit(newline_separator, 1)
+            output = ''
+    else:  # raw_output is empty
+        exit_code = '969696'  # just because
+        output = ''
+
+    if check_exit_code:
         exit_code = exit_code.strip()
         if exit_code.isdigit():
             if int(exit_code):
-                message = 'Got exit code {}\nfrom: {}\nSTDOUT: {}\nSTDERR: {}'
-                raise TargetError(message.format(exit_code, full_command, output, error))
+                message = ('Got exit code {}\nfrom target command: {}\n'
+                           'STDOUT: {}\nSTDERR: {}')
+                raise TargetError(message.format(exit_code, command, output, error))
             elif AM_START_ERROR.findall(output):
                 message = 'Could not start activity; got the following:'
                 message += '\n{}'.format(AM_START_ERROR.findall(output)[0])
@@ -370,8 +374,7 @@ def adb_shell(device, command, timeout=None, check_exit_code=False,
                 message = 'adb has returned early; did not get an exit code. '\
                           'Was kill-server invoked?'
                 raise TargetError(message)
-    else:  # do not check exit code
-        output, _ = check_output(full_command, timeout, shell=True)
+
     return output