You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/02/10 19:38:17 UTC

[GitHub] [tvm] driazati opened a new pull request #10214: [cleanup] Log compile errors for AOT tests

driazati opened a new pull request #10214:
URL: https://github.com/apache/tvm/pull/10214


   See #10213
   
   Thanks for contributing to TVM!   Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @ them in the pull request thread.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] lhutton1 commented on a change in pull request #10214: [cleanup] Log compile errors for AOT tests

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #10214:
URL: https://github.com/apache/tvm/pull/10214#discussion_r815032100



##########
File path: tests/python/relay/aot/aot_test_utils.py
##########
@@ -225,35 +225,43 @@ def parametrize_aot_options(test):
     )(test)
 
 
-def subprocess_log_output(cmd, cwd, logfile):
+def subprocess_check_log_output(cmd, cwd, logfile):
     """
     This method runs a process and logs the output to both a log file and stdout
     """
     _LOG.info("Execute (%s): %s", cwd, cmd)
     cmd_base = cmd[0] if isinstance(cmd, (list, tuple)) else cmd.split(" ", 1)[0]
     proc = subprocess.Popen(
-        cmd, cwd=cwd, shell=True, bufsize=0, stdout=subprocess.PIPE, stderr=subprocess.STDOUT
+        cmd,
+        cwd=cwd,
+        shell=True,
+        bufsize=0,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+        encoding="utf-8",
     )
-    with open(logfile, "ab") as f:
-        f.write(
-            bytes(
-                "\n"
-                + "-" * 80
-                + f"{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}: Execute ({cwd}): {cmd}\n"
-                + "-" * 80,
-                "utf-8",
-            )
+    with open(logfile, "a") as f:
+        msg = (
+            "\n"
+            + "-" * 80
+            + f"{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}: Execute ({cwd}): {cmd}\n"
+            + "-" * 80
         )
+        f.write(msg)
         while True:
             data = proc.stdout.readline()
-            _LOG.debug("%s: %s", cmd_base, str(data, "utf-8", "replace").rstrip("\n"))
+            _LOG.debug("%s: %s", cmd_base, data.rstrip("\n"))
             f.write(data)
 
             # process is done if there is no data and the result is valid
             if not data:  # EOF
                 break
 
-    return proc.wait()
+    proc.wait()
+    if proc.returncode != 0:
+        raise RuntimeError(
+            f"Subprocess failed: {cmd}\nstdout:\n{proc.stdout}\nstderr:\n{proc.stderr}"

Review comment:
       Apologies I didn't spot this earlier, currently the runtime message is `<_io.TextIOWrapper name=7 encoding='utf-8'>`. I tried to manufacture a compile error locally and test this out, and it seems as though attempting to fix this with something like `proc.stdout.read()` returns just an empty string, presumably because the lines are already read above. Any ideas other than just printing the contents of the log file?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] manupa-arm commented on pull request #10214: [cleanup] Log compile errors for AOT tests

Posted by GitBox <gi...@apache.org>.
manupa-arm commented on pull request #10214:
URL: https://github.com/apache/tvm/pull/10214#issuecomment-1056499894


   Thanks @driazati @ekalda @lhutton1 !
   
   Lets see if we can catch what's going on..


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] lhutton1 commented on a change in pull request #10214: [cleanup] Log compile errors for AOT tests

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #10214:
URL: https://github.com/apache/tvm/pull/10214#discussion_r814630538



##########
File path: tests/python/relay/aot/aot_test_utils.py
##########
@@ -225,35 +225,43 @@ def parametrize_aot_options(test):
     )(test)
 
 
-def subprocess_log_output(cmd, cwd, logfile):
+def subprocess_check_log_output(cmd, cwd, logfile):
     """
     This method runs a process and logs the output to both a log file and stdout
     """
     _LOG.info("Execute (%s): %s", cwd, cmd)
     cmd_base = cmd[0] if isinstance(cmd, (list, tuple)) else cmd.split(" ", 1)[0]
     proc = subprocess.Popen(
-        cmd, cwd=cwd, shell=True, bufsize=0, stdout=subprocess.PIPE, stderr=subprocess.STDOUT
+        cmd,
+        cwd=cwd,
+        shell=True,
+        bufsize=0,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+        encoding="utf-8",
     )
-    with open(logfile, "ab") as f:
-        f.write(
-            bytes(
-                "\n"
-                + "-" * 80
-                + f"{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}: Execute ({cwd}): {cmd}\n"
-                + "-" * 80,
-                "utf-8",
-            )
+    with open(logfile, "a") as f:
+        msg = (
+            "\n"
+            + "-" * 80
+            + f"{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}: Execute ({cwd}): {cmd}\n"
+            + "-" * 80
         )
+        f.write(msg.encode())

Review comment:
       Thanks for persisting with this @driazati, I was able to reproduce the failures in CI locally, the following should fix the issues:
   ```suggestion
           f.write(msg)
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] lhutton1 commented on a change in pull request #10214: [cleanup] Log compile errors for AOT tests

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #10214:
URL: https://github.com/apache/tvm/pull/10214#discussion_r814630538



##########
File path: tests/python/relay/aot/aot_test_utils.py
##########
@@ -225,35 +225,43 @@ def parametrize_aot_options(test):
     )(test)
 
 
-def subprocess_log_output(cmd, cwd, logfile):
+def subprocess_check_log_output(cmd, cwd, logfile):
     """
     This method runs a process and logs the output to both a log file and stdout
     """
     _LOG.info("Execute (%s): %s", cwd, cmd)
     cmd_base = cmd[0] if isinstance(cmd, (list, tuple)) else cmd.split(" ", 1)[0]
     proc = subprocess.Popen(
-        cmd, cwd=cwd, shell=True, bufsize=0, stdout=subprocess.PIPE, stderr=subprocess.STDOUT
+        cmd,
+        cwd=cwd,
+        shell=True,
+        bufsize=0,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+        encoding="utf-8",
     )
-    with open(logfile, "ab") as f:
-        f.write(
-            bytes(
-                "\n"
-                + "-" * 80
-                + f"{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}: Execute ({cwd}): {cmd}\n"
-                + "-" * 80,
-                "utf-8",
-            )
+    with open(logfile, "a") as f:
+        msg = (
+            "\n"
+            + "-" * 80
+            + f"{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}: Execute ({cwd}): {cmd}\n"
+            + "-" * 80
         )
+        f.write(msg.encode())

Review comment:
       Thanks for persisting with this @driazati, I was able to reproduce the failures ~in CI~ with this PR locally, the following should fix the issues:
   ```suggestion
           f.write(msg)
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] ekalda commented on a change in pull request #10214: [cleanup] Log compile errors for AOT tests

Posted by GitBox <gi...@apache.org>.
ekalda commented on a change in pull request #10214:
URL: https://github.com/apache/tvm/pull/10214#discussion_r811034393



##########
File path: tests/python/relay/aot/aot_test_utils.py
##########
@@ -244,14 +244,20 @@ def parametrize_aot_options(test):
     )(test)
 
 
-def subprocess_log_output(cmd, cwd, logfile):
+def subprocess_check_log_output(cmd, cwd, logfile):
     """
     This method runs a process and logs the output to both a log file and stdout
     """
     _LOG.info("Execute (%s): %s", cwd, cmd)
     cmd_base = cmd[0] if isinstance(cmd, (list, tuple)) else cmd.split(" ", 1)[0]
     proc = subprocess.Popen(
-        cmd, cwd=cwd, shell=True, bufsize=0, stdout=subprocess.PIPE, stderr=subprocess.STDOUT
+        cmd,
+        cwd=cwd,
+        shell=True,
+        bufsize=0,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+        encoding="utf-8",

Review comment:
       Nit: maybe add a variable for this `"utf-8"` and the other `"utf-8"`s below?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] lhutton1 commented on a change in pull request #10214: [cleanup] Log compile errors for AOT tests

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #10214:
URL: https://github.com/apache/tvm/pull/10214#discussion_r815032100



##########
File path: tests/python/relay/aot/aot_test_utils.py
##########
@@ -225,35 +225,43 @@ def parametrize_aot_options(test):
     )(test)
 
 
-def subprocess_log_output(cmd, cwd, logfile):
+def subprocess_check_log_output(cmd, cwd, logfile):
     """
     This method runs a process and logs the output to both a log file and stdout
     """
     _LOG.info("Execute (%s): %s", cwd, cmd)
     cmd_base = cmd[0] if isinstance(cmd, (list, tuple)) else cmd.split(" ", 1)[0]
     proc = subprocess.Popen(
-        cmd, cwd=cwd, shell=True, bufsize=0, stdout=subprocess.PIPE, stderr=subprocess.STDOUT
+        cmd,
+        cwd=cwd,
+        shell=True,
+        bufsize=0,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+        encoding="utf-8",
     )
-    with open(logfile, "ab") as f:
-        f.write(
-            bytes(
-                "\n"
-                + "-" * 80
-                + f"{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}: Execute ({cwd}): {cmd}\n"
-                + "-" * 80,
-                "utf-8",
-            )
+    with open(logfile, "a") as f:
+        msg = (
+            "\n"
+            + "-" * 80
+            + f"{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}: Execute ({cwd}): {cmd}\n"
+            + "-" * 80
         )
+        f.write(msg)
         while True:
             data = proc.stdout.readline()
-            _LOG.debug("%s: %s", cmd_base, str(data, "utf-8", "replace").rstrip("\n"))
+            _LOG.debug("%s: %s", cmd_base, data.rstrip("\n"))
             f.write(data)
 
             # process is done if there is no data and the result is valid
             if not data:  # EOF
                 break
 
-    return proc.wait()
+    proc.wait()
+    if proc.returncode != 0:
+        raise RuntimeError(
+            f"Subprocess failed: {cmd}\nstdout:\n{proc.stdout}\nstderr:\n{proc.stderr}"

Review comment:
       The last run on CI luckily hit a flaky test, although the Runtime Error doesn't really give many details currently (other than the command that failed): 
   ```
   RuntimeError: Subprocess failed: make -f /workspace/tests/python/relay/aot/corstone300.mk build_dir=/tmp/tmpqlaqvgf3/test/build CFLAGS='-DTVM_RUNTIME_ALLOC_ALIGNMENT_BYTES=8 ' TVM_ROOT=/workspace/tests/python/relay/aot/../../../.. AOT_TEST_ROOT=/workspace/tests/python/relay/aot CODEGEN_ROOT=/tmp/tmpqlaqvgf3/test/codegen STANDALONE_CRT_DIR=/workspace/build/standalone_crt FVP_DIR=/opt/arm/FVP_Corstone_SSE-300/models/Linux64_GCC-6.4/ run
   E           stdout:
   E           <_io.TextIOWrapper name=7 encoding='utf-8'>
   E           stderr:
   E           None
   ```
   
   I tried to change `proc.stdout` to something like `proc.stdout.read()` but this just returns an empty string, presumably because the lines are already read above. There will also be no stderr since this is passed to stdout in the subprocess declaration.
   
   We could make the error visibly by adding the contents of the log file to the error message, although this isn't very elegant. Any other ideas?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] manupa-arm merged pull request #10214: [cleanup] Log compile errors for AOT tests

Posted by GitBox <gi...@apache.org>.
manupa-arm merged pull request #10214:
URL: https://github.com/apache/tvm/pull/10214


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] lhutton1 commented on a change in pull request #10214: [cleanup] Log compile errors for AOT tests

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #10214:
URL: https://github.com/apache/tvm/pull/10214#discussion_r815032100



##########
File path: tests/python/relay/aot/aot_test_utils.py
##########
@@ -225,35 +225,43 @@ def parametrize_aot_options(test):
     )(test)
 
 
-def subprocess_log_output(cmd, cwd, logfile):
+def subprocess_check_log_output(cmd, cwd, logfile):
     """
     This method runs a process and logs the output to both a log file and stdout
     """
     _LOG.info("Execute (%s): %s", cwd, cmd)
     cmd_base = cmd[0] if isinstance(cmd, (list, tuple)) else cmd.split(" ", 1)[0]
     proc = subprocess.Popen(
-        cmd, cwd=cwd, shell=True, bufsize=0, stdout=subprocess.PIPE, stderr=subprocess.STDOUT
+        cmd,
+        cwd=cwd,
+        shell=True,
+        bufsize=0,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+        encoding="utf-8",
     )
-    with open(logfile, "ab") as f:
-        f.write(
-            bytes(
-                "\n"
-                + "-" * 80
-                + f"{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}: Execute ({cwd}): {cmd}\n"
-                + "-" * 80,
-                "utf-8",
-            )
+    with open(logfile, "a") as f:
+        msg = (
+            "\n"
+            + "-" * 80
+            + f"{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}: Execute ({cwd}): {cmd}\n"
+            + "-" * 80
         )
+        f.write(msg)
         while True:
             data = proc.stdout.readline()
-            _LOG.debug("%s: %s", cmd_base, str(data, "utf-8", "replace").rstrip("\n"))
+            _LOG.debug("%s: %s", cmd_base, data.rstrip("\n"))
             f.write(data)
 
             # process is done if there is no data and the result is valid
             if not data:  # EOF
                 break
 
-    return proc.wait()
+    proc.wait()
+    if proc.returncode != 0:
+        raise RuntimeError(
+            f"Subprocess failed: {cmd}\nstdout:\n{proc.stdout}\nstderr:\n{proc.stderr}"

Review comment:
       Apologies I didn't spot this earlier, currently the runtime error message is `<_io.TextIOWrapper name=7 encoding='utf-8'>`. I tried to manufacture a compile error locally and test this out, and it seems as though attempting to fix this with something like `proc.stdout.read()` returns just an empty string, presumably because the lines are already read above. Any ideas other than just printing the contents of the log file?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] driazati commented on a change in pull request #10214: [cleanup] Log compile errors for AOT tests

Posted by GitBox <gi...@apache.org>.
driazati commented on a change in pull request #10214:
URL: https://github.com/apache/tvm/pull/10214#discussion_r816385970



##########
File path: tests/python/relay/aot/aot_test_utils.py
##########
@@ -225,35 +225,43 @@ def parametrize_aot_options(test):
     )(test)
 
 
-def subprocess_log_output(cmd, cwd, logfile):
+def subprocess_check_log_output(cmd, cwd, logfile):
     """
     This method runs a process and logs the output to both a log file and stdout
     """
     _LOG.info("Execute (%s): %s", cwd, cmd)
     cmd_base = cmd[0] if isinstance(cmd, (list, tuple)) else cmd.split(" ", 1)[0]
     proc = subprocess.Popen(
-        cmd, cwd=cwd, shell=True, bufsize=0, stdout=subprocess.PIPE, stderr=subprocess.STDOUT
+        cmd,
+        cwd=cwd,
+        shell=True,
+        bufsize=0,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+        encoding="utf-8",
     )
-    with open(logfile, "ab") as f:
-        f.write(
-            bytes(
-                "\n"
-                + "-" * 80
-                + f"{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}: Execute ({cwd}): {cmd}\n"
-                + "-" * 80,
-                "utf-8",
-            )
+    with open(logfile, "a") as f:
+        msg = (
+            "\n"
+            + "-" * 80
+            + f"{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}: Execute ({cwd}): {cmd}\n"
+            + "-" * 80
         )
+        f.write(msg)
         while True:
             data = proc.stdout.readline()
-            _LOG.debug("%s: %s", cmd_base, str(data, "utf-8", "replace").rstrip("\n"))
+            _LOG.debug("%s: %s", cmd_base, data.rstrip("\n"))
             f.write(data)
 
             # process is done if there is no data and the result is valid
             if not data:  # EOF
                 break
 
-    return proc.wait()
+    proc.wait()
+    if proc.returncode != 0:
+        raise RuntimeError(
+            f"Subprocess failed: {cmd}\nstdout:\n{proc.stdout}\nstderr:\n{proc.stderr}"

Review comment:
       added a side channel variable to capture the same info, seems to work locally




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] lhutton1 commented on a change in pull request #10214: [cleanup] Log compile errors for AOT tests

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #10214:
URL: https://github.com/apache/tvm/pull/10214#discussion_r815032100



##########
File path: tests/python/relay/aot/aot_test_utils.py
##########
@@ -225,35 +225,43 @@ def parametrize_aot_options(test):
     )(test)
 
 
-def subprocess_log_output(cmd, cwd, logfile):
+def subprocess_check_log_output(cmd, cwd, logfile):
     """
     This method runs a process and logs the output to both a log file and stdout
     """
     _LOG.info("Execute (%s): %s", cwd, cmd)
     cmd_base = cmd[0] if isinstance(cmd, (list, tuple)) else cmd.split(" ", 1)[0]
     proc = subprocess.Popen(
-        cmd, cwd=cwd, shell=True, bufsize=0, stdout=subprocess.PIPE, stderr=subprocess.STDOUT
+        cmd,
+        cwd=cwd,
+        shell=True,
+        bufsize=0,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+        encoding="utf-8",
     )
-    with open(logfile, "ab") as f:
-        f.write(
-            bytes(
-                "\n"
-                + "-" * 80
-                + f"{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}: Execute ({cwd}): {cmd}\n"
-                + "-" * 80,
-                "utf-8",
-            )
+    with open(logfile, "a") as f:
+        msg = (
+            "\n"
+            + "-" * 80
+            + f"{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}: Execute ({cwd}): {cmd}\n"
+            + "-" * 80
         )
+        f.write(msg)
         while True:
             data = proc.stdout.readline()
-            _LOG.debug("%s: %s", cmd_base, str(data, "utf-8", "replace").rstrip("\n"))
+            _LOG.debug("%s: %s", cmd_base, data.rstrip("\n"))
             f.write(data)
 
             # process is done if there is no data and the result is valid
             if not data:  # EOF
                 break
 
-    return proc.wait()
+    proc.wait()
+    if proc.returncode != 0:
+        raise RuntimeError(
+            f"Subprocess failed: {cmd}\nstdout:\n{proc.stdout}\nstderr:\n{proc.stderr}"

Review comment:
       Apologies I didn't spot this earlier, currently, the runtime error message is `<_io.TextIOWrapper name=7 encoding='utf-8'>`. I tried to manufacture a compile error locally and test this out, and it seems as though attempting to fix this with something like `proc.stdout.read()` returns just an empty string, presumably because the lines are already read above. Any ideas other than just printing the contents of the log file?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] lhutton1 commented on a change in pull request #10214: [cleanup] Log compile errors for AOT tests

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #10214:
URL: https://github.com/apache/tvm/pull/10214#discussion_r815032100



##########
File path: tests/python/relay/aot/aot_test_utils.py
##########
@@ -225,35 +225,43 @@ def parametrize_aot_options(test):
     )(test)
 
 
-def subprocess_log_output(cmd, cwd, logfile):
+def subprocess_check_log_output(cmd, cwd, logfile):
     """
     This method runs a process and logs the output to both a log file and stdout
     """
     _LOG.info("Execute (%s): %s", cwd, cmd)
     cmd_base = cmd[0] if isinstance(cmd, (list, tuple)) else cmd.split(" ", 1)[0]
     proc = subprocess.Popen(
-        cmd, cwd=cwd, shell=True, bufsize=0, stdout=subprocess.PIPE, stderr=subprocess.STDOUT
+        cmd,
+        cwd=cwd,
+        shell=True,
+        bufsize=0,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+        encoding="utf-8",
     )
-    with open(logfile, "ab") as f:
-        f.write(
-            bytes(
-                "\n"
-                + "-" * 80
-                + f"{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}: Execute ({cwd}): {cmd}\n"
-                + "-" * 80,
-                "utf-8",
-            )
+    with open(logfile, "a") as f:
+        msg = (
+            "\n"
+            + "-" * 80
+            + f"{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}: Execute ({cwd}): {cmd}\n"
+            + "-" * 80
         )
+        f.write(msg)
         while True:
             data = proc.stdout.readline()
-            _LOG.debug("%s: %s", cmd_base, str(data, "utf-8", "replace").rstrip("\n"))
+            _LOG.debug("%s: %s", cmd_base, data.rstrip("\n"))
             f.write(data)
 
             # process is done if there is no data and the result is valid
             if not data:  # EOF
                 break
 
-    return proc.wait()
+    proc.wait()
+    if proc.returncode != 0:
+        raise RuntimeError(
+            f"Subprocess failed: {cmd}\nstdout:\n{proc.stdout}\nstderr:\n{proc.stderr}"

Review comment:
       The last run on CI luckily hit a flaky test, although the Runtime Error doesn't really give many details currently (other than the command that failed): 
   ```
   RuntimeError: Subprocess failed: make -f /workspace/tests/python/relay/aot/corstone300.mk build_dir=/tmp/tmpqlaqvgf3/test/build CFLAGS='-DTVM_RUNTIME_ALLOC_ALIGNMENT_BYTES=8 ' TVM_ROOT=/workspace/tests/python/relay/aot/../../../.. AOT_TEST_ROOT=/workspace/tests/python/relay/aot CODEGEN_ROOT=/tmp/tmpqlaqvgf3/test/codegen STANDALONE_CRT_DIR=/workspace/build/standalone_crt FVP_DIR=/opt/arm/FVP_Corstone_SSE-300/models/Linux64_GCC-6.4/ run
   E           stdout:
   E           <_io.TextIOWrapper name=7 encoding='utf-8'>
   E           stderr:
   E           None
   ```
   
   I tried to change `proc.stdout` to something like `proc.stdout.read()` but this just returns an empty string, presumably because the lines are already read above. There will also be no stderr since this is passed to stdout in the subprocess declaration.
   
   We could make the error visible by adding the contents of the log file to the error message, although I don't think this is very elegant. Any other ideas?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] lhutton1 commented on a change in pull request #10214: [cleanup] Log compile errors for AOT tests

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #10214:
URL: https://github.com/apache/tvm/pull/10214#discussion_r815032100



##########
File path: tests/python/relay/aot/aot_test_utils.py
##########
@@ -225,35 +225,43 @@ def parametrize_aot_options(test):
     )(test)
 
 
-def subprocess_log_output(cmd, cwd, logfile):
+def subprocess_check_log_output(cmd, cwd, logfile):
     """
     This method runs a process and logs the output to both a log file and stdout
     """
     _LOG.info("Execute (%s): %s", cwd, cmd)
     cmd_base = cmd[0] if isinstance(cmd, (list, tuple)) else cmd.split(" ", 1)[0]
     proc = subprocess.Popen(
-        cmd, cwd=cwd, shell=True, bufsize=0, stdout=subprocess.PIPE, stderr=subprocess.STDOUT
+        cmd,
+        cwd=cwd,
+        shell=True,
+        bufsize=0,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+        encoding="utf-8",
     )
-    with open(logfile, "ab") as f:
-        f.write(
-            bytes(
-                "\n"
-                + "-" * 80
-                + f"{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}: Execute ({cwd}): {cmd}\n"
-                + "-" * 80,
-                "utf-8",
-            )
+    with open(logfile, "a") as f:
+        msg = (
+            "\n"
+            + "-" * 80
+            + f"{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}: Execute ({cwd}): {cmd}\n"
+            + "-" * 80
         )
+        f.write(msg)
         while True:
             data = proc.stdout.readline()
-            _LOG.debug("%s: %s", cmd_base, str(data, "utf-8", "replace").rstrip("\n"))
+            _LOG.debug("%s: %s", cmd_base, data.rstrip("\n"))
             f.write(data)
 
             # process is done if there is no data and the result is valid
             if not data:  # EOF
                 break
 
-    return proc.wait()
+    proc.wait()
+    if proc.returncode != 0:
+        raise RuntimeError(
+            f"Subprocess failed: {cmd}\nstdout:\n{proc.stdout}\nstderr:\n{proc.stderr}"

Review comment:
       The last run on CI luckily hit a flaky test, although the Runtime Error doesn't really give many details currently: 
   ```
   RuntimeError: Subprocess failed: make -f /workspace/tests/python/relay/aot/corstone300.mk build_dir=/tmp/tmpqlaqvgf3/test/build CFLAGS='-DTVM_RUNTIME_ALLOC_ALIGNMENT_BYTES=8 ' TVM_ROOT=/workspace/tests/python/relay/aot/../../../.. AOT_TEST_ROOT=/workspace/tests/python/relay/aot CODEGEN_ROOT=/tmp/tmpqlaqvgf3/test/codegen STANDALONE_CRT_DIR=/workspace/build/standalone_crt FVP_DIR=/opt/arm/FVP_Corstone_SSE-300/models/Linux64_GCC-6.4/ run
   E           stdout:
   E           <_io.TextIOWrapper name=7 encoding='utf-8'>
   E           stderr:
   E           None
   ```
   
   I tried to change `proc.stdout` to something like `proc.stdout.read()` but this just returns an empty string, presumably because the lines are already read above. There will also be no stderr since this is passed to stdout in the subprocess declaration.
   
   We could make the error visibly by adding the contents of the log file to the error message, although this isn't very elegant. Any other ideas?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org