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 2021/05/28 21:26:12 UTC
[GitHub] [tvm] areusch commented on a change in pull request #8156: [QEMU] Add number of cores, target list for build
areusch commented on a change in pull request #8156:
URL: https://github.com/apache/tvm/pull/8156#discussion_r641821198
##########
File path: docker/install/ubuntu_install_qemu.sh
##########
@@ -16,22 +16,46 @@
# specific language governing permissions and limitations
# under the License.
+#
+# Install QEMU on Ubuntu.
+#
+# Usage: ubuntu_install_qemu.sh [--target-list target0,target1,...]
+# --target-list is list of target for QEMU comma seperated. e.g. aarch64-softmmu,arm-softmmu,...
+#
+
+set -e
+set -o pipefail
+
+QEMU_DIR=qemu-5.1.0
+
+# Get number of cores for build
if [[ "${CI_NUM_CORE}" ]]; then
Review comment:
let's pick either CORE or CORES and stick with it. Prefer NUM_CORES since it's more natural
##########
File path: docker/install/ubuntu_install_qemu.sh
##########
@@ -16,22 +16,46 @@
# specific language governing permissions and limitations
# under the License.
+#
+# Install QEMU on Ubuntu.
+#
+# Usage: ubuntu_install_qemu.sh [--target-list target0,target1,...]
+# --target-list is list of target for QEMU comma seperated. e.g. aarch64-softmmu,arm-softmmu,...
+#
+
+set -e
+set -o pipefail
+
+QEMU_DIR=qemu-5.1.0
+
+# Get number of cores for build
if [[ "${CI_NUM_CORE}" ]]; then
- NUM_CORE=${CI_NUM_CORE}
+ num_cores=${CI_NUM_CORE}
else
- NUM_CORE=2
+ num_cores=2
fi
-set -e
-set -u
-set -o pipefail
+# Set target list for QEMU
+if [ "$1" == "--target-list" ]; then
+ shift
+ target_list=$1
+else
+ target_list="aarch64-softmmu,arm-softmmu,i386-softmmu,riscv32-softmmu,riscv64-softmmu,x86_64-softmmu"
Review comment:
comment where this list comes from
##########
File path: docker/install/ubuntu_install_qemu.sh
##########
@@ -16,22 +16,46 @@
# specific language governing permissions and limitations
# under the License.
+#
+# Install QEMU on Ubuntu.
+#
+# Usage: ubuntu_install_qemu.sh [--target-list target0,target1,...]
+# --target-list is list of target for QEMU comma seperated. e.g. aarch64-softmmu,arm-softmmu,...
+#
+
+set -e
+set -o pipefail
+
+QEMU_DIR=qemu-5.1.0
+
+# Get number of cores for build
if [[ "${CI_NUM_CORE}" ]]; then
- NUM_CORE=${CI_NUM_CORE}
+ num_cores=${CI_NUM_CORE}
else
- NUM_CORE=2
+ num_cores=2
fi
-set -e
-set -u
-set -o pipefail
+# Set target list for QEMU
+if [ "$1" == "--target-list" ]; then
+ shift
+ target_list=$1
+else
+ target_list="aarch64-softmmu,arm-softmmu,i386-softmmu,riscv32-softmmu,riscv64-softmmu,x86_64-softmmu"
+fi
-sudo sed -i '/deb-src/s/^# //' /etc/apt/sources.list
-apt update
-apt-get -y build-dep qemu
+# Check if QEMU already built
+rebuild=0
Review comment:
why do we need this logic in this file?
##########
File path: docker/install/ubuntu_install_qemu.sh
##########
@@ -45,14 +69,16 @@ p5ez/+2k4VAIwIQoP5DoO06waLBffvLIAdPPKYsx71K67OoGG2svc7duC/+5qf1x
=hCS7
-----END PGP ARMORED FILE-----
EOF
-curl -OLs https://download.qemu.org/qemu-5.1.0.tar.xz
-gpg --verify qemu-5.1.0.tar.xz.sig
+ curl -OLs https://download.qemu.org/qemu-5.1.0.tar.xz
+ gpg --verify qemu-5.1.0.tar.xz.sig
+
+ tar -xf qemu-5.1.0.tar.xz
+fi
-tar -xf qemu-5.1.0.tar.xz
cd qemu-5.1.0
-./configure --target-list=aarch64-softmmu,arm-softmmu,i386-softmmu,riscv32-softmmu,riscv64-softmmu,x86_64-softmmu
-make -j${NUM_CORE}
+./configure --target-list=${target_list}
+make -j${num_cores}
sudo make install
-# For debugging with qemu
+# # For debugging with qemu
Review comment:
nit: revert this change
##########
File path: docker/install/ubuntu_install_qemu.sh
##########
@@ -16,22 +16,46 @@
# specific language governing permissions and limitations
# under the License.
+#
+# Install QEMU on Ubuntu.
+#
+# Usage: ubuntu_install_qemu.sh [--target-list target0,target1,...]
+# --target-list is list of target for QEMU comma seperated. e.g. aarch64-softmmu,arm-softmmu,...
+#
+
+set -e
+set -o pipefail
+
+QEMU_DIR=qemu-5.1.0
+
+# Get number of cores for build
if [[ "${CI_NUM_CORE}" ]]; then
- NUM_CORE=${CI_NUM_CORE}
+ num_cores=${CI_NUM_CORE}
else
- NUM_CORE=2
+ num_cores=2
fi
-set -e
-set -u
-set -o pipefail
+# Set target list for QEMU
+if [ "$1" == "--target-list" ]; then
+ shift
+ target_list=$1
+else
+ target_list="aarch64-softmmu,arm-softmmu,i386-softmmu,riscv32-softmmu,riscv64-softmmu,x86_64-softmmu"
+fi
-sudo sed -i '/deb-src/s/^# //' /etc/apt/sources.list
-apt update
-apt-get -y build-dep qemu
+# Check if QEMU already built
+rebuild=0
+if [ -d ${QEMU_DIR} ]; then
+ rebuild=1
+fi
+
+if [ $rebuild -eq 0 ]; then
+ sudo sed -i '/deb-src/s/^# //' /etc/apt/sources.list
Review comment:
i think you should at least filter by the particular deb-src line you want to re-enable? also comment why this is needed
##########
File path: docker/install/ubuntu_install_qemu.sh
##########
@@ -45,14 +69,16 @@ p5ez/+2k4VAIwIQoP5DoO06waLBffvLIAdPPKYsx71K67OoGG2svc7duC/+5qf1x
=hCS7
-----END PGP ARMORED FILE-----
EOF
-curl -OLs https://download.qemu.org/qemu-5.1.0.tar.xz
-gpg --verify qemu-5.1.0.tar.xz.sig
+ curl -OLs https://download.qemu.org/qemu-5.1.0.tar.xz
+ gpg --verify qemu-5.1.0.tar.xz.sig
+
+ tar -xf qemu-5.1.0.tar.xz
+fi
-tar -xf qemu-5.1.0.tar.xz
cd qemu-5.1.0
-./configure --target-list=aarch64-softmmu,arm-softmmu,i386-softmmu,riscv32-softmmu,riscv64-softmmu,x86_64-softmmu
-make -j${NUM_CORE}
+./configure --target-list=${target_list}
Review comment:
seems like if rebuild should also gate these? should it be renamed?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org