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-unsubscr...@tvm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
