-
Notifications
You must be signed in to change notification settings - Fork 61
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
fix: Small fixes resolving naming and nil-pointers #1552
fix: Small fixes resolving naming and nil-pointers #1552
Conversation
bbcdd5f
to
0da37c6
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.
Hi @nderjung. Thanks for this. kraft pkg
works as expected now.
One question about the runtime
flag: is the following setup meant to work?
Kraftfile
spec: v0.6
rootfs: ./Dockerfile
cmd: ["/server"]
then:
kraft run --runtime unikraft.org/base:latest .
I get the following errors:
E could not determine how to run provided input: first positional argument is a directory: .
first positional argument is a directory: /home/kero/unikraft-work/catalog/examples/http-go1.21
cannot run project build without unikraft
cannot run project without runtime directive
arguments represent a file or directory
arguments represent a file or directory
Perhaps the runtime flag should also be injected elsewhere?
This makes it consistent with how it is presented externally, i.e. via the `runtime` element in the `Kraftfile`. Signed-off-by: Alexander Jung <alex@unikraft.io>
GitHub-Fixes: unikraft#1551 Signed-off-by: Alexander Jung <alex@unikraft.io>
973d0eb
to
62bb264
Compare
The `--runtime` flag for `kraft run` was implicitly registered by the `runtime` package. In this commit we remove this implicit registration and declare it properly as `RunOptions` attribute. As a result, we explicitly use it also in relevant runners which employ a runtime name. Signed-off-by: Alexander Jung <alex@unikraft.io>
62bb264
to
2f326fa
Compare
Thanks @LucaSeri, I've broken up the commits to be a bit more explicit about the (re)introduction of the |
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.
Thanks! Works well.
Prerequisite checklist
make fmt
on your commit series before opening this PR;Description of changes
GitHub-Fixes: #1551