Optionally move setup.py/pip cmake-out to repo_root/cmake-out #7720
Conversation
|
Stack from ghstack (oldest at bottom): |
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/7720
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New FailureAs of commit 1071d8e with merge base 57a09f4 ( NEW FAILURE - The following job has failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
Our setup.py/pip runs currently do CMake builds under the setup.py build_temp directory, which results in the build directory pip-out/temp.something/cmake-out. This does not share built artifacts with any of the other things that might do a CMake build, such as CI scripts, developers focused on C++, or users following the directions for an example or backend. I am not aware of any particular cost to just using the same cmake-out directory as everything else (please chime in if you are aware!) and there's a clear benefit, so let's try this? ghstack-source-id: 5a46e016a9f420fa3c9dd455c4342d6ed0a41285 ghstack-comment-id: 2597548020 Pull Request resolved: #7720
facebook-github-bot
added
the
CLA Signed
zingo
requested review from
Erik-Lundell
and removed request for
Erik-Lundell
|
Note: CI seems bad, No PULL jobs had been running |
swolchok
added
the
release notes: build
looks like they quit running yesterday: https://hud.pytorch.org/hud/pytorch/executorch/main/1?per_page=50&name_filter=pull&mergeLF=true |
Our setup.py/pip runs currently do CMake builds under the setup.py build_temp directory, which results in the build directory pip-out/temp.something/cmake-out. This does not share built artifacts with any of the other things that might do a CMake build, such as CI scripts, developers focused on C++, or users following the directions for an example or backend. I am not aware of any particular cost to just using the same cmake-out directory as everything else (please chime in if you are aware!) and there's a clear benefit, so let's try this? ghstack-source-id: b5a35882ec38cc6512e68947beb5c6a5d9395c15 ghstack-comment-id: 2597548020 Pull Request resolved: #7720
swolchok
requested review from
dbort,
larryliu0820 and
mergennachin
and removed request for
dbort
|
test-llava-runner-linux finished in 50 minutes on this PR, whereas typical times in main are around 1-1.1 hours. Promising, though we should confirm on main once this ships. |
There was a problem hiding this comment.
I separated these intentionally to keep the pip build hermetic. setup.py sets specific build flags to create the prebuilt artifacts that go into the wheel. Users might significantly change the set of cmake flags for their core cmake-out build, including doing cross-compilation or disabling things like pybindings (which would break assumptions in the pip artifacts).
I could see some cases where it might be helpful to share these two directories, but I see a bunch of cases where they really shouldn't be shared.
setup.py deletes the CMake cache and reruns CMake always. I don't see how the user doing other stuff in that directory could cause problems given that behavior. @dbort can you elaborate? |
|
Partly it's out of paranoia, knowing how hard it is to keep make-based systems coherent. Even though the cache file is deleted, I think it might re-use artifacts between builds? And if that's true, then it might try to re-use a cross-compiled artifact. If you've played around with this new setup and it seems bulletproof to you then go for it. But I was trading off time-to-rebuild as a worthwhile cost for avoiding painful debugging sessions when accidentally using incorrect artifacts. |
I'll try switching back and forth between Android and host while letting CMake default to make. FWIW I recommend |
|
Ninja is great but we currently still need to support make; at least, all of our docs tell people to use make. |
|
we should change the docs :) |
swolchok
changed the title
|
changing this to be optional, so that only people (and CI jobs, where the pip build happens first!) that have opted in take the risk of painful debugging sessions. |
hmm, affected CI jobs just shouldn't be double-building ExecuTorch. need to think about this more. |
Our setup.py/pip runs currently do CMake builds under the setup.py build_temp directory, which results in the build directory pip-out/temp.something/cmake-out. This does not share built artifacts with any of the other things that might do a CMake build, such as CI scripts, developers focused on C++, or users following the directions for an example or backend. I am not aware of any particular cost to just using the same cmake-out directory as everything else (please chime in if you are aware!) and there's a clear benefit, so let's try this?