gemini-code-assist[bot] commented on code in PR #19700: URL: https://github.com/apache/tvm/pull/19700#discussion_r3383049804
########## tests/scripts/release/test_release_package.sh: ########## @@ -65,37 +65,42 @@ wget -c https://github.com/apache/tvm/releases/download/${version_rc}/apache-tvm md5sum ./* > ./md5sum.txt cd - -echo "[3/9] Check difference between github.com and apache.org ..." +echo "[3/10] Check difference between github.com and apache.org ..." diff github/md5sum.txt ./apache/md5sum.txt -echo "[4/9] Checking asc ..." +echo "[4/10] Checking asc ..." cd github gpg --verify ./apache-tvm-src-${version_rc}.tar.gz.asc ./apache-tvm-src-${version_rc}.tar.gz -echo "[5/9] Checking sha512 ..." +echo "[5/10] Checking sha512 ..." sha512sum -c ./apache-tvm-src-${version_rc}.tar.gz.sha512 -echo "[6/9] Unzip ..." +echo "[6/10] Unzip ..." tar -zxf apache-tvm-src-${version_rc}.tar.gz -echo "[7/9] Checking whether binary in source code ..." -output=`find apache-tvm-src-${version} -type f -exec file {} + | grep -w "ELF\|shared object"` +echo "[7/10] Checking whether binary in source code ..." +output=`find apache-tvm-src-${version} -type f -exec file {} + | grep -w "ELF\|shared object" || true` if [[ -n "$output" ]]; then echo "Error: ELF or shared object files found:" echo "$output" exit 1 fi -echo "[8/9] Compile and Python Import on Linux ..." +echo "[8/10] Compile and Python Import on Linux ..." cd apache-tvm-src-${version} Review Comment:  Similar to the issue in the binary check step, this should use `${version_rc}` instead of `${version}` to correctly change into the extracted directory when verifying a release candidate. ```suggestion cd apache-tvm-src-${version_rc} ``` ########## tests/scripts/release/test_release_package.sh: ########## @@ -65,37 +65,42 @@ wget -c https://github.com/apache/tvm/releases/download/${version_rc}/apache-tvm md5sum ./* > ./md5sum.txt cd - -echo "[3/9] Check difference between github.com and apache.org ..." +echo "[3/10] Check difference between github.com and apache.org ..." diff github/md5sum.txt ./apache/md5sum.txt -echo "[4/9] Checking asc ..." +echo "[4/10] Checking asc ..." cd github gpg --verify ./apache-tvm-src-${version_rc}.tar.gz.asc ./apache-tvm-src-${version_rc}.tar.gz -echo "[5/9] Checking sha512 ..." +echo "[5/10] Checking sha512 ..." sha512sum -c ./apache-tvm-src-${version_rc}.tar.gz.sha512 -echo "[6/9] Unzip ..." +echo "[6/10] Unzip ..." tar -zxf apache-tvm-src-${version_rc}.tar.gz -echo "[7/9] Checking whether binary in source code ..." -output=`find apache-tvm-src-${version} -type f -exec file {} + | grep -w "ELF\|shared object"` +echo "[7/10] Checking whether binary in source code ..." +output=`find apache-tvm-src-${version} -type f -exec file {} + | grep -w "ELF\|shared object" || true` Review Comment:  Under `set -u` (which is enabled at the top of the script), using `${version}` here instead of `${version_rc}` will cause the script to fail when verifying a release candidate (where `rc` is not empty, e.g., `rc0`), because the extracted directory name contains the release candidate suffix (e.g., `apache-tvm-src-v0.16.0.rc0` instead of `apache-tvm-src-v0.16.0`).\n\nAdditionally, using `$(...)` is preferred over backticks for command substitution for better readability and nesting. ```suggestion output=$(find apache-tvm-src-${version_rc} -type f -exec file {} + | grep -w "ELF\|shared object" || true) ``` ########## tests/scripts/release/test_release_package.sh: ########## @@ -65,37 +65,42 @@ wget -c https://github.com/apache/tvm/releases/download/${version_rc}/apache-tvm md5sum ./* > ./md5sum.txt cd - -echo "[3/9] Check difference between github.com and apache.org ..." +echo "[3/10] Check difference between github.com and apache.org ..." diff github/md5sum.txt ./apache/md5sum.txt -echo "[4/9] Checking asc ..." +echo "[4/10] Checking asc ..." cd github gpg --verify ./apache-tvm-src-${version_rc}.tar.gz.asc ./apache-tvm-src-${version_rc}.tar.gz -echo "[5/9] Checking sha512 ..." +echo "[5/10] Checking sha512 ..." sha512sum -c ./apache-tvm-src-${version_rc}.tar.gz.sha512 -echo "[6/9] Unzip ..." +echo "[6/10] Unzip ..." tar -zxf apache-tvm-src-${version_rc}.tar.gz -echo "[7/9] Checking whether binary in source code ..." -output=`find apache-tvm-src-${version} -type f -exec file {} + | grep -w "ELF\|shared object"` +echo "[7/10] Checking whether binary in source code ..." +output=`find apache-tvm-src-${version} -type f -exec file {} + | grep -w "ELF\|shared object" || true` if [[ -n "$output" ]]; then echo "Error: ELF or shared object files found:" echo "$output" exit 1 fi -echo "[8/9] Compile and Python Import on Linux ..." +echo "[8/10] Compile and Python Import on Linux ..." cd apache-tvm-src-${version} mkdir build cd build cp ../cmake/config.cmake . cmake .. -make -j4 +make -j16 Review Comment:  Hardcoding `-j16` can cause Out-Of-Memory (OOM) issues or CPU starvation on resource-constrained environments (such as standard GitHub Actions runners, which typically have only 2 cores and 7 GB of RAM). TVM compilation is highly memory-intensive and can easily exceed available memory when spawning 16 parallel compiler processes.\n\nConsider using `nproc` (with a fallback if not available) to scale the build parallelism dynamically based on the host's available CPU cores. ```suggestion make -j$(nproc 2>/dev/null || echo 2) ``` ########## tests/scripts/release/test_release_package.sh: ########## @@ -65,37 +65,42 @@ wget -c https://github.com/apache/tvm/releases/download/${version_rc}/apache-tvm md5sum ./* > ./md5sum.txt cd - -echo "[3/9] Check difference between github.com and apache.org ..." +echo "[3/10] Check difference between github.com and apache.org ..." diff github/md5sum.txt ./apache/md5sum.txt -echo "[4/9] Checking asc ..." +echo "[4/10] Checking asc ..." cd github gpg --verify ./apache-tvm-src-${version_rc}.tar.gz.asc ./apache-tvm-src-${version_rc}.tar.gz -echo "[5/9] Checking sha512 ..." +echo "[5/10] Checking sha512 ..." sha512sum -c ./apache-tvm-src-${version_rc}.tar.gz.sha512 -echo "[6/9] Unzip ..." +echo "[6/10] Unzip ..." tar -zxf apache-tvm-src-${version_rc}.tar.gz -echo "[7/9] Checking whether binary in source code ..." -output=`find apache-tvm-src-${version} -type f -exec file {} + | grep -w "ELF\|shared object"` +echo "[7/10] Checking whether binary in source code ..." +output=`find apache-tvm-src-${version} -type f -exec file {} + | grep -w "ELF\|shared object" || true` if [[ -n "$output" ]]; then echo "Error: ELF or shared object files found:" echo "$output" exit 1 fi -echo "[8/9] Compile and Python Import on Linux ..." +echo "[8/10] Compile and Python Import on Linux ..." cd apache-tvm-src-${version} mkdir build cd build cp ../cmake/config.cmake . cmake .. -make -j4 +make -j16 cd .. -echo "[9/9] Import TVM and print path ..." +echo "[9/10] Install Python runtime dependencies ..." +# TVM imports the tvm_ffi package (and other runtime deps) at startup; install the +# dependencies declared in pyproject.toml so the import check below can run. +python3 -m pip install "apache-tvm-ffi>=0.1.11" ml_dtypes numpy psutil typing_extensions + +echo "[10/10] Import TVM and print path ..." export TVM_HOME=$(pwd) export PYTHONPATH=$TVM_HOME/python:${PYTHONPATH} Review Comment:  Since `set -u` (nounset) is enabled, referencing `${PYTHONPATH}` when it is not already set in the environment will cause the script to terminate immediately with an `unbound variable` error.\n\nUsing the `${PYTHONPATH:-}` parameter expansion syntax safely defaults to an empty string if the variable is unset. ```suggestion export PYTHONPATH=$TVM_HOME/python:${PYTHONPATH:-} ``` -- 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]
