-
Notifications
You must be signed in to change notification settings - Fork 32
action: permit skipping dependency installation #30
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
base: main
Are you sure you want to change the base?
action: permit skipping dependency installation #30
Conversation
fabiobaltieri
left a comment
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.
Uhm, ok but then what's left, just west init and update?
I was gonna ask the same, but I'm sure I'm missing something. |
c144b22 to
650bc53
Compare
That and the pip dependency installation with caching is all that is left, yeah. The reasoning for this change is that we're using this action in CI, and some CI jobs run within the Zephyr project docker container. setting a flag like this is just an optimization since those jobs only need the pip dependencies and west init steps. If you don't think this is useful I'm happy to close it- just wanted to reuse this workflow as much as possible within our CI and avoid repeating parts of it when we were running in a docker container |
|
I think it's fine, clearly you have a use case for it and it's backward compatible, just a bit worried by the amount of flags and conditions growing but I guess that's just how this is going to go. |
That's totally fair- another option here would be to instead check if the Zephyr SDK is present before we run setup, right? Rerunning the APT install command shouldn't have that much impact in a docker container, the main thing is the SDK install. If you'd prefer I can look at writing a detection method for that |
650bc53 to
15c2c60
Compare
Yeah but then it's more logic, this at least is trivial, I think we can go with it. @kartben @henrikbrixandersen any opinion? |
| required: false | ||
| default: https://github.com/zephyrproject-rtos/sdk-ng/releases/download | ||
|
|
||
| skip-dependencies: |
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.
i think 'skip-dependencies' is a bit too generic here, what dependencies exactly? we are talking about packages here, so maybe it should be explicit.
Maybe we could use package-group which can be set to none, but can also take other values, i.e. minimal, full, etc. with minimal including what we have now, full with more packages for special ops, like if we want coverage tools and such.
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.
What defining package-groups, an array which can take values like so:
python: installs pip dependencies only (like whatskip-dependenciesdoes now)sdk: installs the Zephyr SDK onlybuild: installs build dependencies
The default value would then be [python, sdk, build] to install the same set as we have now
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.
@nashif what are your thoughts on this suggestion? Would that make sense to get this moved forwards?
|
@nashif ping |
Support skipping os dependencies within the action. This is useful when running in a container where these dependencies are present. Signed-off-by: Daniel DeGrasse <[email protected]>
Add example of how to configure action to skip installing OS level dependencies and Zephyr SDK Signed-off-by: Daniel DeGrasse <[email protected]>
15c2c60 to
444fa0e
Compare
|
@nashif can you take a look? Haven't updated the action tag in a while, no pressure I can do it an any time but there's a bit of a backlog think I'll update the current HEAD tomorrow regardless |
|
@danieldegrasse - this could use a rebase |
Allow skipping the dependency installation within the setup process. This is useful for cases where the setup-zephyr action is run within a docker container that already has OS level dependencies and the Zephyr SDK installed