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/07/06 17:52:05 UTC
[GitHub] [tvm] mehrdadh opened a new pull request, #12023: [microtvm][RVM] Refactor Arduino/Zephyr into one RVM
mehrdadh opened a new pull request, #12023:
URL: https://github.com/apache/tvm/pull/12023
This PR removes duplication of two RVMs for testing Arduino and Zephyr.
cc @areusch @guberti
--
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] areusch commented on a diff in pull request #12023: [microtvm][RVM] Refactor Arduino/Zephyr into one RVM
Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #12023:
URL: https://github.com/apache/tvm/pull/12023#discussion_r918131859
##########
apps/microtvm/reference-vm/base-box/base_box_provision.sh:
##########
@@ -20,12 +20,23 @@
# virtual machine similar to CI QEMU setup.
#
-set -e
Review Comment:
it's not terrible to do this explicitly in the script actually--the first line is only ran when you directly invoke it as a command rather than just via `bash path/to/script.sh`
--
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 diff in pull request #12023: [microtvm][RVM] Refactor Arduino/Zephyr into one RVM
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #12023:
URL: https://github.com/apache/tvm/pull/12023#discussion_r915323059
##########
apps/microtvm/reference-vm/README.md:
##########
@@ -108,7 +106,12 @@ $ ./base-box-tool.py --provider virtualbox build zephyr
4. If release tests pass, **release** the box:
```bash
-$ ./base-box-tool.py [--provider=PROVIDER] release --release-version=RELEASE_VER --platform-version=PLATFORM_VER PLATFORM
+$ ./base-box-tool.py [--provider=PROVIDER] release --release-version=RELEASE_VER
```
For that step be sure you've logged in to Vagrant Cloud using the `vagrant`
tool.
+
+## Versioning
+We use semantic versioning as it is recommended by [Vagrant](https://www.vagrantup.com/docs/boxes/versioning). We use `X.Y.Z` version where we maintain the same major version `X` it has minor changes and newer version is still compatible with older versions and we increase minor version `Y`. However, We increase the major version `X` when new RVM is not compatible with older onces. Updating Zephyr SDK is considered a major change and it requires incrementing major version `X`.
Review Comment:
added.
##########
apps/microtvm/reference-vm/base-box/base_box_test.sh:
##########
@@ -16,25 +16,27 @@
# specific language governing permissions and limitations
# under the License.
#
-# Usage: base_box_test.sh <ARDUINO_BOARD>
-# Execute microTVM Arduino tests.
-#
-set -e
set -x
if [ "$#" -lt 1 ]; then
- echo "Usage: base_box_test.sh <ARDUINO_BOARD>"
+ echo "Usage: base_box_test.sh <PLATFORM> <BOARD>"
exit -1
fi
+platform=$1
board=$1
Review Comment:
done.
##########
apps/microtvm/reference-vm/base-box-tool.py:
##########
@@ -34,7 +33,7 @@
_LOG = logging.getLogger(__name__)
-THIS_DIR = os.path.realpath(os.path.dirname(__file__) or ".")
+THIS_DIR = pathlib.Path(os.path.realpath(os.path.dirname(__file__) or "."))
Review Comment:
done.
##########
apps/microtvm/reference-vm/README.md:
##########
@@ -108,7 +106,12 @@ $ ./base-box-tool.py --provider virtualbox build zephyr
4. If release tests pass, **release** the box:
```bash
-$ ./base-box-tool.py [--provider=PROVIDER] release --release-version=RELEASE_VER --platform-version=PLATFORM_VER PLATFORM
+$ ./base-box-tool.py [--provider=PROVIDER] release --release-version=RELEASE_VER
```
For that step be sure you've logged in to Vagrant Cloud using the `vagrant`
tool.
+
+## Versioning
+We use semantic versioning as it is recommended by [Vagrant](https://www.vagrantup.com/docs/boxes/versioning). We use `X.Y.Z` version where we maintain the same major version `X` it has minor changes and newer version is still compatible with older versions and we increase minor version `Y`. However, We increase the major version `X` when new RVM is not compatible with older onces. Updating Zephyr SDK is considered a major change and it requires incrementing major version `X`.
+
+**Note**: We will release all microTVM RVM boxes under [microtvm](https://app.vagrantup.com/tlcpack/boxes/microtvm) and use box versioning in Vagrant file. Previous versions like `microtvm-zephyr`, `microtvm-arduino`, `microtvm-zephyr-2.5` and etc are not continued and will be removed in future.
Review Comment:
done
##########
apps/microtvm/reference-vm/README.md:
##########
@@ -108,7 +106,12 @@ $ ./base-box-tool.py --provider virtualbox build zephyr
4. If release tests pass, **release** the box:
```bash
-$ ./base-box-tool.py [--provider=PROVIDER] release --release-version=RELEASE_VER --platform-version=PLATFORM_VER PLATFORM
+$ ./base-box-tool.py [--provider=PROVIDER] release --release-version=RELEASE_VER
```
For that step be sure you've logged in to Vagrant Cloud using the `vagrant`
tool.
+
+## Versioning
+We use semantic versioning as it is recommended by [Vagrant](https://www.vagrantup.com/docs/boxes/versioning). We use `X.Y.Z` version where we maintain the same major version `X` it has minor changes and newer version is still compatible with older versions and we increase minor version `Y`. However, We increase the major version `X` when new RVM is not compatible with older onces. Updating Zephyr SDK is considered a major change and it requires incrementing major version `X`.
Review Comment:
done.
##########
apps/microtvm/reference-vm/base-box-tool.py:
##########
@@ -365,6 +355,11 @@ def do_build_release_test_vm(
if "config.vm.box_version" in line:
continue
m = VM_BOX_RE.match(line)
+ tvm_home_m = VM_TVM_HOME_RE.match(line)
+
+ if tvm_home_m:
+ f.write(f'{tvm_home_m.group(1)} = "../../../.."\n')
Review Comment:
added
##########
apps/microtvm/reference-vm/base-box/base_box_test.sh:
##########
@@ -16,25 +16,27 @@
# specific language governing permissions and limitations
# under the License.
#
-# Usage: base_box_test.sh <ARDUINO_BOARD>
-# Execute microTVM Arduino tests.
-#
-set -e
set -x
if [ "$#" -lt 1 ]; then
- echo "Usage: base_box_test.sh <ARDUINO_BOARD>"
+ echo "Usage: base_box_test.sh <PLATFORM> <BOARD>"
exit -1
fi
+platform=$1
board=$1
-pytest tests/micro/arduino/test_arduino_workflow.py --arduino-board=${board}
-
-if [ $board == "nano33ble" ]; then
- # https://github.com/apache/tvm/issues/8730
- echo "NOTE: skipped test_arduino_rpc_server.py on $board -- known failure"
-else
- pytest tests/micro/arduino/test_arduino_rpc_server.py --arduino-board=${board}
+if [ "${platform}" == "zephyr" ]; then
+ pytest tests/micro/zephyr --zephyr-board=${board}
fi
+
+if [ "${platform}" == "arduino" ]; then
+ pytest tests/micro/arduino/test_arduino_workflow.py --arduino-board=${board}
+ if [ $board == "nano33ble" ]; then
+ # https://github.com/apache/tvm/issues/8730
+ echo "NOTE: skipped test_arduino_rpc_server.py on $board -- known failure"
+ else
+ pytest tests/micro/arduino/test_arduino_rpc_server.py --arduino-board=${board}
+ fi
+fi
Review Comment:
done
##########
apps/microtvm/reference-vm/base-box/base_box_provision.sh:
##########
@@ -20,12 +20,23 @@
# virtual machine similar to CI QEMU setup.
#
-set -e
Review Comment:
because it is already added in first line
##########
apps/microtvm/reference-vm/base-box-tool.py:
##########
@@ -365,6 +355,11 @@ def do_build_release_test_vm(
if "config.vm.box_version" in line:
continue
m = VM_BOX_RE.match(line)
+ tvm_home_m = VM_TVM_HOME_RE.match(line)
+
+ if tvm_home_m:
+ f.write(f'{tvm_home_m.group(1)} = "../../../.."\n')
Review Comment:
it is for adjusting tvm home in testing step
--
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] guberti commented on pull request #12023: [microtvm][RVM] Refactor Arduino/Zephyr into one RVM
Posted by GitBox <gi...@apache.org>.
guberti commented on PR #12023:
URL: https://github.com/apache/tvm/pull/12023#issuecomment-1178016524
While testing this PR on physical hardware, me and Mehrdad ran into #12033. However, I'm confident that the test failure is unrelated to the changes in this PR, so I'm cool with this PR being merged.
--
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 merged pull request #12023: [microtvm][RVM] Refactor Arduino/Zephyr into one RVM
Posted by GitBox <gi...@apache.org>.
mehrdadh merged PR #12023:
URL: https://github.com/apache/tvm/pull/12023
--
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] guberti commented on a diff in pull request #12023: [microtvm][RVM] Refactor Arduino/Zephyr into one RVM
Posted by GitBox <gi...@apache.org>.
guberti commented on code in PR #12023:
URL: https://github.com/apache/tvm/pull/12023#discussion_r915313784
##########
apps/microtvm/reference-vm/README.md:
##########
@@ -29,26 +29,24 @@ For more information on how to use them, see the
## microTVM Developer Information
Each RTOS or platform (like Zephyr, Ardunio, etc) that integrates with microTVM
-can check-in a Reference VM in this directory to help the community collaborate.
-You should use the tools provided here to ensure a uniform release process
-across all platforms. Typically, releases need to be created by TVM committers.
+can check-in installation scripts in the Reference VM in this directory to help
+the community collaborate. You should use the tools provided here to ensure a
+uniform release process across all platforms. Typically, releases need to be
+created by TVM committers.
Generally speaking, it's expected that any integrated platform with a regression
test checked-in to the tvm repository should also define a reference VM. If you
want to integrate a new platform, please raise a discussion on
[the forum](https://discuss.tvm.ai).
-## Reference VMs Organization
+## Reference VM Organization
-Reference VMs are organized in this directory as follows:
+Reference VM is organized in this directory as follows:
Review Comment:
```suggestion
The Reference VM is organized in this directory as follows:
```
##########
apps/microtvm/reference-vm/README.md:
##########
@@ -108,7 +106,12 @@ $ ./base-box-tool.py --provider virtualbox build zephyr
4. If release tests pass, **release** the box:
```bash
-$ ./base-box-tool.py [--provider=PROVIDER] release --release-version=RELEASE_VER --platform-version=PLATFORM_VER PLATFORM
+$ ./base-box-tool.py [--provider=PROVIDER] release --release-version=RELEASE_VER
```
For that step be sure you've logged in to Vagrant Cloud using the `vagrant`
tool.
+
+## Versioning
+We use semantic versioning as it is recommended by [Vagrant](https://www.vagrantup.com/docs/boxes/versioning). We use `X.Y.Z` version where we maintain the same major version `X` it has minor changes and newer version is still compatible with older versions and we increase minor version `Y`. However, We increase the major version `X` when new RVM is not compatible with older onces. Updating Zephyr SDK is considered a major change and it requires incrementing major version `X`.
+
+**Note**: We will release all microTVM RVM boxes under [microtvm](https://app.vagrantup.com/tlcpack/boxes/microtvm) and use box versioning in Vagrant file. Previous versions like `microtvm-zephyr`, `microtvm-arduino`, `microtvm-zephyr-2.5` and etc are not continued and will be removed in future.
Review Comment:
```suggestion
**Note**: We will release all microTVM RVM boxes under [microtvm](https://app.vagrantup.com/tlcpack/boxes/microtvm) and use box versioning in Vagrant file. Previous versions like `microtvm-zephyr`, `microtvm-arduino`, `microtvm-zephyr-2.5`, etc. are deprecated and will be removed in the future.
```
##########
apps/microtvm/reference-vm/base-box-tool.py:
##########
@@ -34,7 +33,7 @@
_LOG = logging.getLogger(__name__)
-THIS_DIR = os.path.realpath(os.path.dirname(__file__) or ".")
+THIS_DIR = pathlib.Path(os.path.realpath(os.path.dirname(__file__) or "."))
Review Comment:
Can we just do:
```suggestion
THIS_DIR = pathlib.Path(__file__)
```
If `__file__` isn't defined for some reason, I think the previous code would throw an error anyway.
##########
apps/microtvm/reference-vm/README.md:
##########
@@ -108,7 +106,12 @@ $ ./base-box-tool.py --provider virtualbox build zephyr
4. If release tests pass, **release** the box:
```bash
-$ ./base-box-tool.py [--provider=PROVIDER] release --release-version=RELEASE_VER --platform-version=PLATFORM_VER PLATFORM
+$ ./base-box-tool.py [--provider=PROVIDER] release --release-version=RELEASE_VER
```
For that step be sure you've logged in to Vagrant Cloud using the `vagrant`
tool.
+
+## Versioning
+We use semantic versioning as it is recommended by [Vagrant](https://www.vagrantup.com/docs/boxes/versioning). We use `X.Y.Z` version where we maintain the same major version `X` it has minor changes and newer version is still compatible with older versions and we increase minor version `Y`. However, We increase the major version `X` when new RVM is not compatible with older onces. Updating Zephyr SDK is considered a major change and it requires incrementing major version `X`.
Review Comment:
Do we use `Z` in `X.Y.Z`? If we don't (as I don't think we do) we should mention that it is rarely used.
##########
apps/microtvm/reference-vm/base-box/base_box_provision.sh:
##########
@@ -20,12 +20,23 @@
# virtual machine similar to CI QEMU setup.
#
-set -e
Review Comment:
Why are we removing this?
##########
apps/microtvm/reference-vm/base-box/base_box_test.sh:
##########
@@ -16,25 +16,27 @@
# specific language governing permissions and limitations
# under the License.
#
-# Usage: base_box_test.sh <ARDUINO_BOARD>
-# Execute microTVM Arduino tests.
-#
-set -e
set -x
if [ "$#" -lt 1 ]; then
- echo "Usage: base_box_test.sh <ARDUINO_BOARD>"
+ echo "Usage: base_box_test.sh <PLATFORM> <BOARD>"
exit -1
fi
+platform=$1
board=$1
-pytest tests/micro/arduino/test_arduino_workflow.py --arduino-board=${board}
-
-if [ $board == "nano33ble" ]; then
- # https://github.com/apache/tvm/issues/8730
- echo "NOTE: skipped test_arduino_rpc_server.py on $board -- known failure"
-else
- pytest tests/micro/arduino/test_arduino_rpc_server.py --arduino-board=${board}
+if [ "${platform}" == "zephyr" ]; then
+ pytest tests/micro/zephyr --zephyr-board=${board}
fi
+
+if [ "${platform}" == "arduino" ]; then
+ pytest tests/micro/arduino/test_arduino_workflow.py --arduino-board=${board}
+ if [ $board == "nano33ble" ]; then
+ # https://github.com/apache/tvm/issues/8730
+ echo "NOTE: skipped test_arduino_rpc_server.py on $board -- known failure"
+ else
+ pytest tests/micro/arduino/test_arduino_rpc_server.py --arduino-board=${board}
+ fi
+fi
Review Comment:
newline
##########
apps/microtvm/reference-vm/base-box-tool.py:
##########
@@ -365,6 +355,11 @@ def do_build_release_test_vm(
if "config.vm.box_version" in line:
continue
m = VM_BOX_RE.match(line)
+ tvm_home_m = VM_TVM_HOME_RE.match(line)
+
+ if tvm_home_m:
+ f.write(f'{tvm_home_m.group(1)} = "../../../.."\n')
Review Comment:
What is this line doing? Can you add a comment?
##########
apps/microtvm/reference-vm/README.md:
##########
@@ -108,7 +106,12 @@ $ ./base-box-tool.py --provider virtualbox build zephyr
4. If release tests pass, **release** the box:
```bash
-$ ./base-box-tool.py [--provider=PROVIDER] release --release-version=RELEASE_VER --platform-version=PLATFORM_VER PLATFORM
+$ ./base-box-tool.py [--provider=PROVIDER] release --release-version=RELEASE_VER
```
For that step be sure you've logged in to Vagrant Cloud using the `vagrant`
tool.
+
+## Versioning
+We use semantic versioning as it is recommended by [Vagrant](https://www.vagrantup.com/docs/boxes/versioning). We use `X.Y.Z` version where we maintain the same major version `X` it has minor changes and newer version is still compatible with older versions and we increase minor version `Y`. However, We increase the major version `X` when new RVM is not compatible with older onces. Updating Zephyr SDK is considered a major change and it requires incrementing major version `X`.
Review Comment:
```suggestion
We use semantic versioning as it is recommended by [Vagrant](https://www.vagrantup.com/docs/boxes/versioning). We use `X.Y.Z` version where we maintain the same major version `X` it has minor changes and newer version is still compatible with older versions and we increase minor version `Y`. However, We increase the major version `X` when new RVM is not compatible with older onces. Updates to the Zephyr SDK or Arduino board SDKs are considered major changes and require incrementing major version `X`.
```
##########
apps/microtvm/reference-vm/base-box/base_box_test.sh:
##########
@@ -16,25 +16,27 @@
# specific language governing permissions and limitations
# under the License.
#
-# Usage: base_box_test.sh <ARDUINO_BOARD>
-# Execute microTVM Arduino tests.
-#
-set -e
set -x
if [ "$#" -lt 1 ]; then
- echo "Usage: base_box_test.sh <ARDUINO_BOARD>"
+ echo "Usage: base_box_test.sh <PLATFORM> <BOARD>"
exit -1
fi
+platform=$1
board=$1
Review Comment:
```suggestion
board=$2
```
?
--
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