-
Notifications
You must be signed in to change notification settings - Fork 76
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
Improvements to run list and run build #639
Conversation
# Build HoloHub sample apps | ||
list_desc() { | ||
echo "" | ||
echo "Display the list of applications to run." | ||
echo "Display the list of packages, applications, and operators to build." | ||
echo "Usage: ./run list" | ||
echo "" | ||
} | ||
|
||
list() { |
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.
looks like the autocompletion_list
below could also be updated to use some of the logic changed here... though it might need more love than that, since I was confused why apps where even listed, or what some of the options in there related to (cpp, python).
Happy to update it if I get some confirmation on the expected behavior and on the parts that don't currently seem correct in there.
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.
Going to defer to @jjomier as I haven't touched the autocomplete implementation before. I'm OK with moving forward without in this PR and updating as necessary.
d884c75
to
41be94f
Compare
ba23d27
to
6dc6e81
Compare
a058db0
to
724cf40
Compare
Signed-off-by: Alexis Girault <[email protected]>
Also: - verify input validity using the improve listing from the last commit - allow passing a path to the app/operator dir, for improve UX by leveraging terminal autocompletion Signed-off-by: Alexis Girault <[email protected]>
Signed-off-by: Alexis Girault <[email protected]>
Signed-off-by: Alexis Girault <[email protected]>
Signed-off-by: Alexis Girault <[email protected]>
724cf40
to
733bb52
Compare
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.
Update to include pkg
looks good. LGTM, merge away
Co-authored-by: Tom Birdsong <[email protected]> Signed-off-by: Alexis Girault <[email protected]>
Regression introduced in #639 was only looking for cmake targets, did not account for pure python targets Signed-off-by: Alexis Girault <[email protected]>
Regression introduced in #639 was only looking for cmake targets, did not account for pure python targets Signed-off-by: Alexis Girault <[email protected]>
* Fix run list for non-cmake targets Regression introduced in #639 was only looking for cmake targets, did not account for pure python targets Signed-off-by: Alexis Girault <[email protected]> * Ignore templated pathname in run list Ignore {{cookiecutter.project_slug}} Signed-off-by: Alexis Girault <[email protected]> --------- Signed-off-by: Alexis Girault <[email protected]>
* Improve ./run list results Signed-off-by: Alexis Girault <[email protected]> * Allow to build operators with run script Also: - verify input validity using the improve listing from the last commit - allow passing a path to the app/operator dir, for improve UX by leveraging terminal autocompletion Signed-off-by: Alexis Girault <[email protected]> * Allow to build packages with run script Signed-off-by: Alexis Girault <[email protected]> * Style improvement to remaining _desc() Signed-off-by: Alexis Girault <[email protected]> * Update README for build instructions Signed-off-by: Alexis Girault <[email protected]> * Update README.md Co-authored-by: Tom Birdsong <[email protected]> Signed-off-by: Alexis Girault <[email protected]> --------- Signed-off-by: Alexis Girault <[email protected]> Signed-off-by: Alexis Girault <[email protected]> Co-authored-by: Tom Birdsong <[email protected]>
* Fix run list for non-cmake targets Regression introduced in nvidia-holoscan#639 was only looking for cmake targets, did not account for pure python targets Signed-off-by: Alexis Girault <[email protected]> * Ignore templated pathname in run list Ignore {{cookiecutter.project_slug}} Signed-off-by: Alexis Girault <[email protected]> --------- Signed-off-by: Alexis Girault <[email protected]>
* Improve ./run list results Signed-off-by: Alexis Girault <[email protected]> * Allow to build operators with run script Also: - verify input validity using the improve listing from the last commit - allow passing a path to the app/operator dir, for improve UX by leveraging terminal autocompletion Signed-off-by: Alexis Girault <[email protected]> * Allow to build packages with run script Signed-off-by: Alexis Girault <[email protected]> * Style improvement to remaining _desc() Signed-off-by: Alexis Girault <[email protected]> * Update README for build instructions Signed-off-by: Alexis Girault <[email protected]> * Update README.md Co-authored-by: Tom Birdsong <[email protected]> Signed-off-by: Alexis Girault <[email protected]> --------- Signed-off-by: Alexis Girault <[email protected]> Signed-off-by: Alexis Girault <[email protected]> Co-authored-by: Tom Birdsong <[email protected]>
Context
While preparing support for generating packages from the
run
anddev_container
scripts, having a goodlist
mechanism seemed like it would impact existing infrastructure. Instead of a large MR that mixes infra improvements and packaging support, this MR is scoped for infra improvements that the packaging support will then be able to leverage.Changes
List improvements
Build improvements
terminal autocompletion
Results
List improvements
Build improvements
$ ./run build nein ERROR: Unknown application or operator: nein You can run ./run list for a full list of options.