Ensure we grab all of a process output.

When grabing the output of an external process, during build,
we need to make sure we get all of it since we rely on this
to parse errors to put error markers on files.

The current helper methods returned as soon as the process was
finished but this didn't mean that its output was completely read.

This could cause issue where a process would fail but no error
were read, putting no marker on any file.

The fix is twofold:
- forces the helper to blocks until all the output is read.
- put a marker on the project if the exec fails but nothing
  was output on stderr.

Change-Id: I8f0b3556ea9c4597a309627d6a14a2b0386e4e52

7 files changed, 108 insertions, 76 deletions

diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/AidlProcessor.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/AidlProcessor.java
index 8ed95b8..c6d3158 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/AidlProcessor.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/AidlProcessor.java
@@ -89,6 +89,7 @@ public class AidlProcessor extends SourceProcessor {
         return PROPERTY_COMPILE_AIDL;
     }
 
+    @SuppressWarnings("deprecation")
     @Override
     protected void doCompileFiles(List<IFile> sources, BaseBuilder builder,
             IProject project, IAndroidTarget projectTarget, int targetApi,
@@ -206,41 +207,51 @@ public class AidlProcessor extends SourceProcessor {
             Process p = Runtime.getRuntime().exec(command);
 
             // list to store each line of stderr
-            ArrayList<String> results = new ArrayList<String>();
+            ArrayList<String> stdErr = new ArrayList<String>();
 
             // get the output and return code from the process
-            int result = BuildHelper.grabProcessOutput(project, p, results);
-
-            // attempt to parse the error output
-            boolean error = parseAidlOutput(results, file);
-
-            // If the process failed and we couldn't parse the output
-            // we print a message, mark the project and exit
-            if (result != 0) {
-
-                if (error || verbose) {
-                    // display the message in the console.
-                    if (error) {
-                        AdtPlugin.printErrorToConsole(project, results.toArray());
-
-                        // mark the project
-                        BaseProjectHelper.markResource(project, AdtConstants.MARKER_AIDL,
-                                Messages.Unparsed_AIDL_Errors, IMarker.SEVERITY_ERROR);
-                    } else {
-                        AdtPlugin.printToConsole(project, results.toArray());
+            int returnCode = BuildHelper.grabProcessOutput(project, p, stdErr);
+
+            if (stdErr.size() > 0) {
+                // attempt to parse the error output
+                boolean parsingError = parseAidlOutput(stdErr, file);
+
+                // If the process failed and we couldn't parse the output
+                // we print a message, mark the project and exit
+                if (returnCode != 0) {
+
+                    if (parsingError || verbose) {
+                        // display the message in the console.
+                        if (parsingError) {
+                            AdtPlugin.printErrorToConsole(project, stdErr.toArray());
+
+                            // mark the project
+                            BaseProjectHelper.markResource(project, AdtConstants.MARKER_AIDL,
+                                    Messages.Unparsed_AIDL_Errors, IMarker.SEVERITY_ERROR);
+                        } else {
+                            AdtPlugin.printToConsole(project, stdErr.toArray());
+                        }
                     }
+                    return false;
                 }
+            } else if (returnCode != 0) {
+                // no stderr output but exec failed.
+                String msg = String.format(Messages.AIDL_Exec_Error_d, returnCode);
+
+                BaseProjectHelper.markResource(project, AdtConstants.MARKER_AIDL,
+                       msg, IMarker.SEVERITY_ERROR);
+
                 return false;
             }
         } catch (IOException e) {
             // mark the project and exit
-            String msg = String.format(Messages.AIDL_Exec_Error, command[0]);
+            String msg = String.format(Messages.AIDL_Exec_Error_s, command[0]);
             BaseProjectHelper.markResource(project, AdtConstants.MARKER_AIDL, msg,
                     IMarker.SEVERITY_ERROR);
             return false;
         } catch (InterruptedException e) {
             // mark the project and exit
-            String msg = String.format(Messages.AIDL_Exec_Error, command[0]);
+            String msg = String.format(Messages.AIDL_Exec_Error_s, command[0]);
             BaseProjectHelper.markResource(project, AdtConstants.MARKER_AIDL, msg,
                     IMarker.SEVERITY_ERROR);
             return false;
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/BuildHelper.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/BuildHelper.java
index 799cf0e..3ac4a8a 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/BuildHelper.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/BuildHelper.java
@@ -827,31 +827,30 @@ public class BuildHelper {
         }
 
         // launch
-        int execError = 1;
         try {
             // launch the command line process
             Process process = Runtime.getRuntime().exec(command);
 
             // list to store each line of stderr
-            ArrayList<String> results = new ArrayList<String>();
+            ArrayList<String> stdErr = new ArrayList<String>();
 
             // get the output and return code from the process
-            execError = grabProcessOutput(mProject, process, results);
+            int returnCode = grabProcessOutput(mProject, process, stdErr);
 
             if (mVerbose) {
-                for (String resultString : results) {
-                    mOutStream.println(resultString);
+                for (String stdErrString : stdErr) {
+                    mOutStream.println(stdErrString);
                 }
             }
-            if (execError != 0) {
-                throw new AaptResultException(execError,
-                        results.toArray(new String[results.size()]));
+            if (returnCode != 0) {
+                throw new AaptResultException(returnCode,
+                        stdErr.toArray(new String[stdErr.size()]));
             }
         } catch (IOException e) {
-            String msg = String.format(Messages.AAPT_Exec_Error, command[0]);
+            String msg = String.format(Messages.AAPT_Exec_Error_s, command[0]);
             throw new AaptExecException(msg, e);
         } catch (InterruptedException e) {
-            String msg = String.format(Messages.AAPT_Exec_Error, command[0]);
+            String msg = String.format(Messages.AAPT_Exec_Error_s, command[0]);
             throw new AaptExecException(msg, e);
         }
 
@@ -1048,20 +1047,20 @@ public class BuildHelper {
 
     /**
      * Get the stderr output of a process and return when the process is done.
-     * @param process The process to get the ouput from
-     * @param results The array to store the stderr output
+     * @param process The process to get the output from
+     * @param stderr The array to store the stderr output
      * @return the process return code.
      * @throws InterruptedException
      */
     public final static int grabProcessOutput(
             final IProject project,
             final Process process,
-            final ArrayList<String> results)
+            final ArrayList<String> stderr)
             throws InterruptedException {
 
         return GrabProcessOutput.grabProcessOutput(
                 process,
-                Wait.WAIT_FOR_PROCESS,
+                Wait.WAIT_FOR_READERS, // we really want to make sure we get all the output!
                 new IProcessOutput() {
 
                     @SuppressWarnings("unused")
@@ -1083,7 +1082,7 @@ public class BuildHelper {
                     @Override
                     public void err(@Nullable String line) {
                         if (line != null) {
-                            results.add(line);
+                            stderr.add(line);
                             if (BuildVerbosity.VERBOSE == AdtPrefs.getPrefs().getBuildVerbosity()) {
                                 AdtPlugin.printErrorToConsole(project, line);
                             }
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/Messages.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/Messages.java
index d1768c4..7a169f3 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/Messages.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/Messages.java
@@ -8,11 +8,15 @@ public class Messages extends NLS {
 
     public static String AAPT_Error;
 
-    public static String AAPT_Exec_Error;
+    public static String AAPT_Exec_Error_s;
+
+    public static String AAPT_Exec_Error_d;
 
     public static String Added_s_s_Needs_Updating;
 
-    public static String AIDL_Exec_Error;
+    public static String AIDL_Exec_Error_s;
+
+    public static String AIDL_Exec_Error_d;
 
     public static String AIDL_Java_Conflict;
 
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/RenderScriptProcessor.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/RenderScriptProcessor.java
index a400239..3fc7b07 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/RenderScriptProcessor.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/RenderScriptProcessor.java
@@ -245,32 +245,43 @@ public class RenderScriptProcessor extends SourceProcessor {
             Process p = Runtime.getRuntime().exec(command);
 
             // list to store each line of stderr
-            ArrayList<String> results = new ArrayList<String>();
+            ArrayList<String> stdErr = new ArrayList<String>();
 
             // get the output and return code from the process
-            int result = BuildHelper.grabProcessOutput(project, p, results);
-
-            // attempt to parse the error output
-            boolean error = parseLlvmOutput(results);
-
-            // If the process failed and we couldn't parse the output
-            // we print a message, mark the project and exit
-            if (result != 0) {
-
-                if (error || verbose) {
-                    // display the message in the console.
-                    if (error) {
-                        AdtPlugin.printErrorToConsole(project, results.toArray());
-
-                        // mark the project
-                        BaseProjectHelper.markResource(project,
-                                AdtConstants.MARKER_RENDERSCRIPT,
-                                "Unparsed Renderscript error! Check the console for output.",
-                                IMarker.SEVERITY_ERROR);
-                    } else {
-                        AdtPlugin.printToConsole(project, results.toArray());
+            int returnCode = BuildHelper.grabProcessOutput(project, p, stdErr);
+
+            if (stdErr.size() > 0) {
+                // attempt to parse the error output
+                boolean parsingError = parseLlvmOutput(stdErr);
+
+                // If the process failed and we couldn't parse the output
+                // we print a message, mark the project and exit
+                if (returnCode != 0) {
+
+                    if (parsingError || verbose) {
+                        // display the message in the console.
+                        if (parsingError) {
+                            AdtPlugin.printErrorToConsole(project, stdErr.toArray());
+
+                            // mark the project
+                            BaseProjectHelper.markResource(project,
+                                    AdtConstants.MARKER_RENDERSCRIPT,
+                                    "Unparsed Renderscript error! Check the console for output.",
+                                    IMarker.SEVERITY_ERROR);
+                        } else {
+                            AdtPlugin.printToConsole(project, stdErr.toArray());
+                        }
                     }
+                    return false;
                 }
+            } else if (returnCode != 0) {
+                // no stderr output but exec failed.
+                String msg = String.format("Error executing Renderscript: Return code %1$d",
+                        returnCode);
+
+                BaseProjectHelper.markResource(project, AdtConstants.MARKER_AIDL,
+                       msg, IMarker.SEVERITY_ERROR);
+
                 return false;
             }
         } catch (IOException e) {
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/build_messages.properties b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/build_messages.properties
index 28ceebd..70a3ab2 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/build_messages.properties
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/build_messages.properties
@@ -12,7 +12,8 @@ Output_Missing=Output folder missing\! Make sure your project is configured prop
 s_File_Missing=%1$s file missing\!
 Unparsed_AAPT_Errors=Unparsed aapt error(s)\! Check the console for output.
 Unparsed_AIDL_Errors=Unparsed aidl error\! Check the console for output.
-AAPT_Exec_Error=Error executing aapt. Please check aapt is present at %1$s
+AAPT_Exec_Error_s=Error executing aapt. Please check aapt is present at %1$s
+AAPT_Exec_Error_d=Error executing aapt: Return code %1$d
 Dalvik_Error_d=Conversion to Dalvik format failed with error %1$d
 DX_Jar_Error=Dx.jar is not found inside the plugin. Reinstall ADT\!
 Dalvik_Error_s=Conversion to Dalvik format failed: %1$s
@@ -34,7 +35,8 @@ Removing_Generated_Classes=Removing generated java classes.
 Delete_Obsolete_Error=Failed to delete obsolete %1$s, please delete it manually
 DexWrapper_Dex_Loader=Dex Loader
 AIDL_Java_Conflict=%1$s is in the way of %2$s, remove it or rename of one the files.
-AIDL_Exec_Error=Error executing aidl. Please check aidl is present at %1$s
+AIDL_Exec_Error_d=Error executing aidl: Return code %1$d
+AIDL_Exec_Error_s=Error executing aidl. Please check aidl is present at %1$s
 s_Removed_Recreating_s=%1$s was removed\! Recreating %1$s\!
 s_Modified_Manually_Recreating_s=%1$s was modified manually\! Reverting to generated version\!
 s_Modified_Recreating_s='%1$s' was modified.
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/builders/BaseBuilder.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/builders/BaseBuilder.java
index 1444f76..f58af56 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/builders/BaseBuilder.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/builders/BaseBuilder.java
@@ -233,13 +233,13 @@ public abstract class BaseBuilder extends IncrementalProjectBuilder {
     /**
      * Get the stderr output of a process and return when the process is done.
      * @param process The process to get the ouput from
-     * @param results The array to store the stderr output
+     * @param stdErr The array to store the stderr output
      * @return the process return code.
      * @throws InterruptedException
      */
     protected final int grabProcessOutput(final Process process,
-            final ArrayList<String> results) throws InterruptedException {
-        return BuildHelper.grabProcessOutput(getProject(), process, results);
+            final ArrayList<String> stdErr) throws InterruptedException {
+        return BuildHelper.grabProcessOutput(getProject(), process, stdErr);
     }
 
 
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/builders/PreCompilerBuilder.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/builders/PreCompilerBuilder.java
index 8a9364e..f9ea444 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/builders/PreCompilerBuilder.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/build/builders/PreCompilerBuilder.java
@@ -1045,38 +1045,43 @@ public class PreCompilerBuilder extends BaseBuilder {
         }
 
         // launch
-        int execError = 1;
         try {
             // launch the command line process
             Process process = Runtime.getRuntime().exec(
                     array.toArray(new String[array.size()]));
 
             // list to store each line of stderr
-            ArrayList<String> results = new ArrayList<String>();
+            ArrayList<String> stdErr = new ArrayList<String>();
 
             // get the output and return code from the process
-            execError = grabProcessOutput(process, results);
+            int returnCode = grabProcessOutput(process, stdErr);
 
             // attempt to parse the error output
-            boolean parsingError = AaptParser.parseOutput(results, project);
+            boolean parsingError = AaptParser.parseOutput(stdErr, project);
 
             // if we couldn't parse the output we display it in the console.
             if (parsingError) {
-                if (execError != 0) {
-                    AdtPlugin.printErrorToConsole(project, results.toArray());
+                if (returnCode != 0) {
+                    AdtPlugin.printErrorToConsole(project, stdErr.toArray());
                 } else {
                     AdtPlugin.printBuildToConsole(BuildVerbosity.NORMAL,
-                            project, results.toArray());
+                            project, stdErr.toArray());
                 }
             }
 
-            if (execError != 0) {
+            if (returnCode != 0) {
                 // if the exec failed, and we couldn't parse the error output
                 // (and therefore not all files that should have been marked,
                 // were marked), we put a generic marker on the project and abort.
                 if (parsingError) {
                     markProject(AdtConstants.MARKER_ADT,
                             Messages.Unparsed_AAPT_Errors, IMarker.SEVERITY_ERROR);
+                } else if (stdErr.size() == 0) {
+                    // no parsing error because sdterr was empty. We still need to put
+                    // a marker otherwise there's no user visible feedback.
+                    markProject(AdtConstants.MARKER_ADT,
+                            String.format(Messages.AAPT_Exec_Error_d, returnCode),
+                            IMarker.SEVERITY_ERROR);
                 }
 
                 AdtPlugin.printBuildToConsole(BuildVerbosity.VERBOSE, project,
@@ -1088,7 +1093,7 @@ public class PreCompilerBuilder extends BaseBuilder {
         } catch (IOException e1) {
             // something happen while executing the process,
             // mark the project and exit
-            String msg = String.format(Messages.AAPT_Exec_Error, array.get(0));
+            String msg = String.format(Messages.AAPT_Exec_Error_s, array.get(0));
             markProject(AdtConstants.MARKER_ADT, msg, IMarker.SEVERITY_ERROR);
 
             // Add workaround for the Linux problem described here:
@@ -1114,7 +1119,7 @@ public class PreCompilerBuilder extends BaseBuilder {
         } catch (InterruptedException e) {
             // we got interrupted waiting for the process to end...
             // mark the project and exit
-            String msg = String.format(Messages.AAPT_Exec_Error, array.get(0));
+            String msg = String.format(Messages.AAPT_Exec_Error_s, array.get(0));
             markProject(AdtConstants.MARKER_ADT, msg, IMarker.SEVERITY_ERROR);
 
             // This interrupts the build.