-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
podman: improve runtime patching #310182
podman: improve runtime patching #310182
Conversation
Change the approach used to integrate runtimes, in order to: - Better support macOS - Make obscure OCI runtimes optional - Work around a panic due to runtimes having no paths (see containers/podman#22561)
4356226
to
4499fca
Compare
@ofborg build podman.passthru.tests |
@heitorPB @kevincox @cyrillzadra @lunik1 Could you test this, please? |
@WxNzEMof how do I test this? Never reviewed/tested a nixpkgs PR before. |
I switched my input to your podman fix branch
updated my system with
Now executing |
@cyrillzadra Would you be so kind to share the json you got? I'd like to changing my inputs completely as to keep the rest of my system intact. Would an overlay to podman do the trick? |
|
Result of 3 packages built:
|
This fixes the issue for me. Is it ready to merge? |
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.
LGTM
Builds successfully and fixes the |
@WxNzEMof is there anything holding this back? It is a pretty major regression so I would like to merge it. |
No. I would have liked to add a test for this functionality, so it doesn't regress again, but I don't know how to reproduce it inside a NixOS test.
Be my guest, we can add the test separately later. |
Ok thanks. Unless there are objections soon I will merge later today. A test would be nice but I think that it can be a separate PR. I would rather it not block the fix. |
Thanks @WxNzEMof and everyone else who helped! |
Description of changes
Change the approach used to integrate runtimes, in order to:
Fixes #306398.
Context:
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.