[CI] Remove i386 and Hexagon from CI pipeline (1)#18737
Conversation
Summary of ChangesHello @mshr-h, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on reducing the maintenance burden and streamlining the CI infrastructure by completely removing support for the i386 and Hexagon architectures. This involves a comprehensive cleanup across configuration files, Jenkins pipeline definitions, Docker images, and test scripts, ensuring that the project's CI system is more focused and efficient. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes i386 and Hexagon from the CI pipeline. The changes are extensive, touching many configuration files, Jenkinsfiles, Dockerfiles, and test scripts. The removals appear to be consistent and correct for the most part. I've found a critical issue in docker/install/ubuntu_install_rust.sh where a variable is used after its definition has been removed, which will break the script. I also noticed a minor issue with a trailing space in the arguments of a script call within the generated Jenkinsfiles, which should be fixed in the template for cleanliness. Overall, good work on this large cleanup task.
| fi | ||
|
|
||
| # shellcheck disable=SC2086 # word splitting is intentional here | ||
| curl -s -S -L https://sh.rustup.rs -sSf | sh -s -- -y --no-modify-path --profile minimal --default-toolchain stable $HOST_ARG |
There was a problem hiding this comment.
The HOST_ARG variable is used here, but it's no longer defined since the logic for setting it for i386 builds has been removed. This will cause the script to fail. Please remove $HOST_ARG from this command.
| curl -s -S -L https://sh.rustup.rs -sSf | sh -s -- -y --no-modify-path --profile minimal --default-toolchain stable $HOST_ARG | |
| curl -s -S -L https://sh.rustup.rs -sSf | sh -s -- -y --no-modify-path --profile minimal --default-toolchain stable |
| if (env.DETERMINE_DOCKER_IMAGES == 'yes') { | ||
| sh( | ||
| script: "./${jenkins_scripts_root}/determine_docker_images.py ci_arm ci_cpu ci_gpu ci_hexagon ci_i386 ci_lint ci_wasm ", | ||
| script: "./${jenkins_scripts_root}/determine_docker_images.py ci_arm ci_cpu ci_gpu ci_lint ci_wasm ", |
There was a problem hiding this comment.
There's a trailing space in the script arguments. While it might not cause issues, it's better to remove it for cleanliness. This likely needs to be fixed in the template ci/jenkins/templates/utils/base.groovy.j2 and will affect all generated Jenkinsfiles.
script: "./${jenkins_scripts_root}/determine_docker_images.py ci_arm ci_cpu ci_gpu ci_lint ci_wasm",
mshr-h
force-pushed
the
remove-i386-hexagon-ci
branch
from
38be339 to
432f112
Compare
|
note that because main requires the ci to pass, we should first have a PR to .asf.yaml to removes dep on the required tests then remove |
|
Thanks! That’s exactly what I was wondering about 🙏 |
mshr-h
force-pushed
the
remove-i386-hexagon-ci
branch
from
432f112 to
2718a78
Compare
mshr-h
force-pushed
the
remove-i386-hexagon-ci
branch
from
2718a78 to
839da1a
Compare
mshr-h
force-pushed
the
remove-i386-hexagon-ci
branch
from
7df1830 to
458b544
Compare
mshr-h
changed the title
This is a follow-up pr of #18906 - **Removed "Unity" branding**: Replace "Apache TVM Unity" with "Apache TVM" across all docs (Unity branch has been merged into main) - **Fixed stale Python APIs**: `tvm.instrument` → `tvm.ir.instrument`, `tvm.transform` → `tvm.ir.transform`, `tvm.module.Module` → `tvm.runtime.Module`, `tvm.convert` → `tvm.runtime.convert`, `tvm.runtime.load` → `tvm.runtime.load_module` - **Fixed S-TIR migration**: `tvm.tir.transform.DefaultGPUSchedule` → `tvm.s_tir.transform.DefaultGPUSchedule`; added missing `s_tir/transform` API doc page - **Removed references to deleted features**: AutoTVM/AutoScheduler (removed), FewShotTuning (phased out in #18864), i386 CI (removed in #18737), VTA (removed), TensorFlow frontend (not in Relax), MXNet/Gluon (archived) - **Fixed stale C++ references**: `NodeRef` → `ObjectRef`, `make_node` → `ffi::make_object`, removed unused `using tvm::runtime::Registry`, `src/relax/transforms/` → `src/relax/transform/` - **Updated CI docs**: Added GitHub Actions lint workflow info to `ci.rst` - **Fixed broken links**: dead tutorial links in `pass_infra.rst`, `gallery/` → `docs/how_to/tutorials/`, `how-to/index.rst` → `how_to/dev/index.rst`, `discuss.tvm.ai` → `discuss.tvm.apache.org` - **Updated overview**: TensorFlow → ONNX in supported framework list, AutoTVM → RPC in security docs
2 participants
Part of #18682