From f775ba7df83730870e71644da835a5660cc72be7 Mon Sep 17 00:00:00 2001
From: Steve Pham <82231385+spham-amzn@users.noreply.github.com>
Date: Fri, 11 Jun 2021 08:12:49 -0700
Subject: [PATCH] Provide more informative error messages on android related
 environment / device related issues (#1261)

- If gradle is installed, but JAVA_HOME is not set properly, no detail message is given. Bubble up the error message as part of the description
- When deploying a newer API Level APK (30) to an API Level 29 device, a python callstack is given without any detail of the error. Now it will report the actual error that is from the adb call so the user can act upon it
---
 .../Platform/Android/android_deployment.py     | 14 +++++++++-----
 cmake/Tools/common.py                          | 18 +++++++++++++-----
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/cmake/Tools/Platform/Android/android_deployment.py b/cmake/Tools/Platform/Android/android_deployment.py
index eb8361ec18..82d14a2746 100755
--- a/cmake/Tools/Platform/Android/android_deployment.py
+++ b/cmake/Tools/Platform/Android/android_deployment.py
@@ -184,11 +184,15 @@ class AndroidDeployment(object):
 
         call_arguments.extend(arg_list)
 
-        output = subprocess.check_output(call_arguments,
-                                         shell=True,
-                                         stderr=subprocess.DEVNULL).decode(common.DEFAULT_TEXT_READ_ENCODING,
-                                                                           common.ENCODING_ERROR_HANDLINGS)
-        return output
+        try:
+            output = subprocess.check_output(call_arguments,
+                                             shell=True,
+                                             stderr=subprocess.PIPE).decode(common.DEFAULT_TEXT_READ_ENCODING,
+                                                                               common.ENCODING_ERROR_HANDLINGS)
+            return output
+        except subprocess.CalledProcessError as err:
+            raise common.LmbrCmdError(err.stderr.decode(common.DEFAULT_TEXT_READ_ENCODING,
+                                                        common.ENCODING_ERROR_HANDLINGS))
 
     def adb_shell(self, command, device_id):
         """
diff --git a/cmake/Tools/common.py b/cmake/Tools/common.py
index 1990041c86..cf4631c227 100755
--- a/cmake/Tools/common.py
+++ b/cmake/Tools/common.py
@@ -274,6 +274,8 @@ def verify_tool(override_tool_path, tool_name, tool_filename, argument_name, too
     :return:    Tuple of the resolved tool version and the resolved override tool path if provided
     """
 
+    tool_source = tool_name
+
     try:
         # Use either the provided gradle override or the gradle in the path environment
         if override_tool_path:
@@ -306,18 +308,17 @@ def verify_tool(override_tool_path, tool_name, tool_filename, argument_name, too
             tool_desc = f"{tool_name} path provided in the command line argument '{argument_name}={override_tool_path}' "
         else:
             resolved_override_tool_path = None
-            tool_source = tool_name
             tool_desc = f"installed {tool_name} in the system path"
 
         # Extract the version and verify
         version_output = subprocess.check_output([tool_source, tool_version_argument],
-                                                 shell=True).decode(DEFAULT_TEXT_READ_ENCODING,
-                                                                    ENCODING_ERROR_HANDLINGS)
+                                                 shell=True,
+                                                 stderr=subprocess.PIPE).decode(DEFAULT_TEXT_READ_ENCODING,
+                                                                                ENCODING_ERROR_HANDLINGS)
         version_match = tool_version_regex.search(version_output)
         if not version_match:
             raise RuntimeError()
 
-
         # Since we are doing a compare, strip out any non-numeric and non . character from the version otherwise we will get a TypeError on the LooseVersion comparison
         result_version_str = re.sub(r"[^\.0-9]", "", str(version_match.group(1)).strip())
         result_version = LooseVersion(result_version_str)
@@ -331,7 +332,14 @@ def verify_tool(override_tool_path, tool_name, tool_filename, argument_name, too
 
         return result_version, resolved_override_tool_path
 
-    except (CalledProcessError, WindowsError, RuntimeError) as e:
+    except CalledProcessError as e:
+        error_msg = e.output.decode(DEFAULT_TEXT_READ_ENCODING,
+                                    ENCODING_ERROR_HANDLINGS)
+        raise LmbrCmdError(f"{tool_name} cannot be resolved or there was a problem determining its version number. "
+                           f"Either make sure its in the system path environment or a valid path is passed in "
+                           f"through the {argument_name} argument.\n{error_msg}",
+                           ERROR_CODE_ERROR_NOT_SUPPORTED)
+    except (WindowsError, RuntimeError) as e:
         logging.error(f"Call to '{tool_source}' resulted in error: {e}")
         raise LmbrCmdError(f"{tool_name} cannot be resolved or there was a problem determining its version number. "
                            f"Either make sure its in the system path environment or a valid path is passed in "