-
Notifications
You must be signed in to change notification settings - Fork 537
Fix the broken cmake commands in sdk integration tutorial #3391
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/3391
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 0849660 with merge base fac1ae6 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
@Olivia-liu has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@Olivia-liu has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@Olivia-liu merged this pull request in 130d7e8. |
# cd .. | ||
# cmake --build cmake-out -j8 -t sdk_example_runner | ||
# ./cmake-out/examples/sdk/sdk_example_runner --bundled_program_path <bundled_program> | ||
# rm -rf cmake-out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The old instructions only needed a few lines, but this is too complicated to copy-and-paste; please add a script.
I put together a first pass in https://gist.github.com/dbort/55cb1aee2c5673aab4f61087c80363b4.
Though when I run it it fails during the second cmake with
CMake Error at /Users/dbort/local/executorch/build/Codegen.cmake:132 (add_library):
add_library cannot create target "portable_ops_lib" because an imported
target with the same name already exists.
Call Stack (most recent call first):
CMakeLists.txt:57 (gen_operators_lib)
If you do keep this as pasteable instructions, you should remove the local
keyword, since it's not useful in this context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I'll cherry-pick this PR first to unbreak the tutorial, then work on moving it to a script.
@pytorchbot cherry-pick --onto release/0.2 -c docs |
Cherry picking #3391The cherry pick PR is at #3432 Details for Dev Infra teamRaised by workflow job |
) Summary: Pull Request resolved: #3391 Reviewed By: tarun292, Jack-Khuu Differential Revision: D56692344 Pulled By: Olivia-liu fbshipit-source-id: 8d83de4a689ab7642e8cfca3e912694b254a5a12 (cherry picked from commit 130d7e8) Co-authored-by: Peixuan Liu <[email protected]>
Summary
The added cmake commands are copied from a .sh recently updated in #3298. The removed commands in this tutorial.py are old and don't work.
I also realized that #3298 was never cherry-picked to 0.2. Whatever we will do to this PR (whether it ends up in 0.2.0, 0.2.1 or something other branch), let's the do same for that one too.
Test Plan
I followed the tutorial through and it worked.