gemini-code-assist[bot] commented on code in PR #584:
URL: https://github.com/apache/tvm-ffi/pull/584#discussion_r3221133276
##########
python/tvm_ffi/cpp/extension.py:
##########
@@ -600,6 +603,12 @@ def build_ninja(build_dir: str) -> None:
raise RuntimeError("\n".join(msg))
+ if os.environ.get("TVM_FFI_LOG_BUILD") in ("1", "stdout"):
+ logger.info("ninja build stdout:\n%s", status.stdout.decode("utf-8"))
+
+ if os.environ.get("TVM_FFI_LOG_BUILD") in ("1", "stderr"):
+ logger.info("ninja build stderr:\n%s", status.stderr.decode("utf-8"))
Review Comment:
Hardcoding `utf-8` for decoding process output can lead to
`UnicodeDecodeError` on Windows, where the output typically uses the OEM code
page. This would cause the build process to fail even if the compilation was
successful. It is recommended to use the same encoding logic as the error path
and include `errors="replace"` for robustness. Additionally, caching the
environment variable and checking if the output is non-empty before logging
improves efficiency and log clarity.
```suggestion
log_build = os.environ.get("TVM_FFI_LOG_BUILD")
encoding = "oem" if IS_WINDOWS else "utf-8"
if log_build in ("1", "stdout") and status.stdout:
logger.info("ninja build stdout:\n%s",
status.stdout.decode(encoding, errors="replace"))
if log_build in ("1", "stderr") and status.stderr:
logger.info("ninja build stderr:\n%s",
status.stderr.decode(encoding, errors="replace"))
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
