[BE] Add --clean option to install_requirements.py #5348
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/5348
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New Failure, 1 Unrelated FailureAs of commit a4d8fef with merge base 0186c7f ( NEW FAILURE - The following job has failed:
BROKEN TRUNK - The following job failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
|
@larryliu0820 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
install_requirements.py
Outdated
| elif arg == "--clean": | ||
| print(f"Cleaning build artifacts...") | ||
| if sys.platform == "win32": | ||
| subprocess.run(["rmdir", "/s", "cmake-out*/"], check=True) |
There was a problem hiding this comment.
Sounds like rmdir can't handle wildcards: https://superuser.com/a/764355
https://docs.python.org/3/library/shutil.html#shutil.rmtree is portable way to do this
install_requirements.py
Outdated
| subprocess.run(["rmdir", "/s", "pip-out/"], check=True) | ||
| else: | ||
| subprocess.run(["rm", "-rf", "cmake-out*/"], check=True) | ||
| subprocess.run(["rm", "-rf", "pip-out/"], check=True) |
There was a problem hiding this comment.
I suggest exiting at this point. "clean" commands typically clean-then-exit, not clean-then-build.
And maybe complain and exit non-zero if there are any commandline args other than "--clean", so that scripts don't say "install_requirements.sh --clean --pybind" and think that it successfully built.
Summary: as titled. remove `cmake-out/` and `pip-out/` before installing ExecuTorch. Android is creating directory name such as `cmake-out-armv8` so removing those as well. Test Plan: Tried on my machine Reviewers: Subscribers: Tasks: Tags:
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:
larryliu0820
force-pushed
the
add_clean
branch
from
ce150b6 to
a4d8fef
Compare
|
@larryliu0820 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
| print("Cleaning build artifacts...") | ||
| print("Cleaning pip-out/...") | ||
| shutil.rmtree("pip-out/", ignore_errors=True) | ||
| dirs = glob.glob("cmake-out*/") |
There was a problem hiding this comment.
Looks like the mediatek scripts create a directory called cmake-android-out. We should standardize them more. Or even put everything under a top-level out directory so we could just delete that.
|
@larryliu0820 merged this pull request in cb12061. |
|
@pytorchbot cherry-pick --onto release/0.4 -c fixnewfeature |
Summary: as titled. remove `cmake-out/` and `pip-out/` before installing ExecuTorch. Android is creating directory name such as `cmake-out-armv8` so removing those as well. Pull Request resolved: #5348 Test Plan: Tried on my machine ``` $ bash install_requirements.sh --clean Cleaning build artifacts... Cleaning pip-out/... Cleaning cmake-out-android-arm64-v8a/... Cleaning cmake-out-android-x86_64/... Done cleaning build artifacts. ``` Reviewed By: kimishpatel, dbort Differential Revision: D62648278 Pulled By: larryliu0820 fbshipit-source-id: 3699015fe4b960d8087556cb29388863df10285b (cherry picked from commit cb12061)
Cherry picking #5348The cherry pick PR is at #6137 and it is recommended to link a fixnewfeature cherry pick PR with an issue. The following tracker issues are updated: Details for Dev Infra teamRaised by workflow job |
Add --clean option to install_requirements.py (#5348) Summary: as titled. remove `cmake-out/` and `pip-out/` before installing ExecuTorch. Android is creating directory name such as `cmake-out-armv8` so removing those as well. Pull Request resolved: #5348 Test Plan: Tried on my machine ``` $ bash install_requirements.sh --clean Cleaning build artifacts... Cleaning pip-out/... Cleaning cmake-out-android-arm64-v8a/... Cleaning cmake-out-android-x86_64/... Done cleaning build artifacts. ``` Reviewed By: kimishpatel, dbort Differential Revision: D62648278 Pulled By: larryliu0820 fbshipit-source-id: 3699015fe4b960d8087556cb29388863df10285b (cherry picked from commit cb12061) Co-authored-by: Mengwei Liu <[email protected]>
Summary: as titled. remove
cmake-out/andpip-out/before installing ExecuTorch. Android is creating directory name such ascmake-out-armv8so removing those as well.Test Plan: Tried on my machine
Reviewers:
Subscribers:
Tasks:
Tags: