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/02 01:26:53 UTC
[GitHub] [tvm] mehrdadh opened a new pull request #10138: [microTVM][Zephyr] Update RVM to Zephyr 2.7
mehrdadh opened a new pull request #10138:
URL: https://github.com/apache/tvm/pull/10138
This PR:
- Updates Zephyr version on RVM to 2.7-branch
- Refactor duplication of zephyr branch and zephyr sdk version
- Fix tests for version upgrade
cc @Mousius @areusch @gromero
--
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] leandron commented on a change in pull request #10138: [microTVM][Zephyr] Update RVM to Zephyr 2.7
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #10138:
URL: https://github.com/apache/tvm/pull/10138#discussion_r797425670
##########
File path: docker/install/ubuntu_install_zephyr_sdk.sh
##########
@@ -0,0 +1,33 @@
+#!/bin/bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+set -e
+set -u
+set -o pipefail
+set -x
+
+INSTALLATION_PATH=$1
Review comment:
Can you set a default or exit in error in case this mandatory parameter is not provided?
##########
File path: docker/install/ubuntu_init_zephyr_project.sh
##########
@@ -29,20 +29,26 @@ set -x
DOWNLOAD_DIR=$1
shift
-ZEPHYR_BRANCH=$1
-shift
-commit_hash=
+if [ "$1" == "--zephyr-branch" ]; then
+ shift
+ ZEPHYR_BRANCH=$1
+else
+ ZEPHYR_BRANCH="v2.7-branch"
+ shift
Review comment:
I'm not sure this second `shift` here is needed, as in case `$1` is not `--zephyr-branch`, it will be incorrectly skipped?
--
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] leandron merged pull request #10138: [microTVM][Zephyr] Update RVM to Zephyr 2.7
Posted by GitBox <gi...@apache.org>.
leandron merged pull request #10138:
URL: https://github.com/apache/tvm/pull/10138
--
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] Mousius commented on a change in pull request #10138: [microTVM][Zephyr] Update RVM to Zephyr 2.7
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #10138:
URL: https://github.com/apache/tvm/pull/10138#discussion_r797443555
##########
File path: docker/install/ubuntu_install_zephyr.sh
##########
@@ -69,10 +68,5 @@ chmod o+rwx zephyr/.cache
#/opt/west/bin/pip3 install -r /opt/zephyrproject/zephyr/scripts/requirements.txt
pip3 install -r /opt/zephyrproject/zephyr/scripts/requirements.txt
-ZEPHYR_SDK_VERSION=0.13.2
-ZEPHYR_SDK_FILE=zephyr-sdk-linux-setup.run
-wget --no-verbose -O $ZEPHYR_SDK_FILE \
- https://github.com/zephyrproject-rtos/sdk-ng/releases/download/v${ZEPHYR_SDK_VERSION}/zephyr-sdk-${ZEPHYR_SDK_VERSION}-linux-x86_64-setup.run
-chmod +x $ZEPHYR_SDK_FILE
-"./$ZEPHYR_SDK_FILE" -- -d /opt/zephyr-sdk
-rm "$ZEPHYR_SDK_FILE"
+ZEPHYR_INSTALL_SDK_SCRIPT=$(find -name "ubuntu_install_zephyr_sdk.sh")
+bash ${ZEPHYR_INSTALL_SDK_SCRIPT} /opt/zephyr-sdk
Review comment:
I think you need to update ci_qemu to `ADD` this additional script, it'd also be better to explicitly `RUN` it rather than chaining them like this to make it clearer what gets run on build.
--
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] mehrdadh commented on pull request #10138: [microTVM][Zephyr] Update RVM to Zephyr 2.7 [Do not merge yet]
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on pull request #10138:
URL: https://github.com/apache/tvm/pull/10138#issuecomment-1028407688
Built the ci_qemu image after these changes and it passes microtvm tests.
--
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] mehrdadh commented on a change in pull request #10138: [microTVM][Zephyr] Update RVM to Zephyr 2.7 [Do not merge yet]
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on a change in pull request #10138:
URL: https://github.com/apache/tvm/pull/10138#discussion_r797822674
##########
File path: apps/microtvm/reference-vm/zephyr/Vagrantfile
##########
@@ -16,7 +16,7 @@
# under the License.
Vagrant.configure("2") do |config|
- config.vm.box = "tlcpack/microtvm-zephyr-2.5"
+ config.vm.box = "tlcpack/microtvm-zephyr-2.7-staging"
Review comment:
Name is temporary for testing. I will rename it before merging.
##########
File path: docker/install/ubuntu_install_zephyr_sdk.sh
##########
@@ -0,0 +1,33 @@
+#!/bin/bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+set -e
+set -u
+set -o pipefail
+set -x
+
+INSTALLATION_PATH=$1
Review comment:
done.
##########
File path: docker/install/ubuntu_init_zephyr_project.sh
##########
@@ -29,20 +29,26 @@ set -x
DOWNLOAD_DIR=$1
shift
-ZEPHYR_BRANCH=$1
-shift
-commit_hash=
+if [ "$1" == "--zephyr-branch" ]; then
+ shift
+ ZEPHYR_BRANCH=$1
+else
+ ZEPHYR_BRANCH="v2.7-branch"
+ shift
Review comment:
yes, thanks for catching. It should be fixed now.
##########
File path: docker/install/ubuntu_install_zephyr.sh
##########
@@ -69,10 +68,5 @@ chmod o+rwx zephyr/.cache
#/opt/west/bin/pip3 install -r /opt/zephyrproject/zephyr/scripts/requirements.txt
pip3 install -r /opt/zephyrproject/zephyr/scripts/requirements.txt
-ZEPHYR_SDK_VERSION=0.13.2
-ZEPHYR_SDK_FILE=zephyr-sdk-linux-setup.run
-wget --no-verbose -O $ZEPHYR_SDK_FILE \
- https://github.com/zephyrproject-rtos/sdk-ng/releases/download/v${ZEPHYR_SDK_VERSION}/zephyr-sdk-${ZEPHYR_SDK_VERSION}-linux-x86_64-setup.run
-chmod +x $ZEPHYR_SDK_FILE
-"./$ZEPHYR_SDK_FILE" -- -d /opt/zephyr-sdk
-rm "$ZEPHYR_SDK_FILE"
+ZEPHYR_INSTALL_SDK_SCRIPT=$(find -name "ubuntu_install_zephyr_sdk.sh")
+bash ${ZEPHYR_INSTALL_SDK_SCRIPT} /opt/zephyr-sdk
Review comment:
Added to ci_qemu.
I agree that's preferred. But there are extra steps in `ubuntu_install_zephyr.sh` which are tied to the order of the execution. I suggest we keep it this way for now and do a refactor later. We need a bigger refactor to remove other duplications in building ci_qemu and Zephyr RVM.
--
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] Mousius commented on a change in pull request #10138: [microTVM][Zephyr] Update RVM to Zephyr 2.7 [Do not merge yet]
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #10138:
URL: https://github.com/apache/tvm/pull/10138#discussion_r797833626
##########
File path: docker/install/ubuntu_install_zephyr.sh
##########
@@ -69,10 +68,5 @@ chmod o+rwx zephyr/.cache
#/opt/west/bin/pip3 install -r /opt/zephyrproject/zephyr/scripts/requirements.txt
pip3 install -r /opt/zephyrproject/zephyr/scripts/requirements.txt
-ZEPHYR_SDK_VERSION=0.13.2
-ZEPHYR_SDK_FILE=zephyr-sdk-linux-setup.run
-wget --no-verbose -O $ZEPHYR_SDK_FILE \
- https://github.com/zephyrproject-rtos/sdk-ng/releases/download/v${ZEPHYR_SDK_VERSION}/zephyr-sdk-${ZEPHYR_SDK_VERSION}-linux-x86_64-setup.run
-chmod +x $ZEPHYR_SDK_FILE
-"./$ZEPHYR_SDK_FILE" -- -d /opt/zephyr-sdk
-rm "$ZEPHYR_SDK_FILE"
+ZEPHYR_INSTALL_SDK_SCRIPT=$(find -name "ubuntu_install_zephyr_sdk.sh")
+bash ${ZEPHYR_INSTALL_SDK_SCRIPT} /opt/zephyr-sdk
Review comment:
Sounds good!
--
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] mehrdadh commented on pull request #10138: [microTVM][Zephyr] Update RVM to Zephyr 2.7
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on pull request #10138:
URL: https://github.com/apache/tvm/pull/10138#issuecomment-1028497923
The image is ready and passed the tests on hardware, we are good to merge this.
@leandron PTAL, thanks!
--
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] mehrdadh commented on pull request #10138: [microTVM][Zephyr] Update RVM to Zephyr 2.7
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on pull request #10138:
URL: https://github.com/apache/tvm/pull/10138#issuecomment-1028499219
link to image: https://app.vagrantup.com/tlcpack/boxes/microtvm-zephyr-2.7
--
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