Skip to content
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: 4.9.3 -> 5.0.1 #301553

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

the-sun-will-rise-tomorrow
Copy link
Contributor

@the-sun-will-rise-tomorrow the-sun-will-rise-tomorrow commented Apr 4, 2024

Description of changes

Depends on #265409

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@bachp
Copy link
Member

bachp commented Apr 8, 2024

@WxNzEMof #265409 is merged. Not sure if it needs to be update tough

@the-sun-will-rise-tomorrow
Copy link
Contributor Author

Updated!

@the-sun-will-rise-tomorrow the-sun-will-rise-tomorrow marked this pull request as ready for review April 9, 2024 08:00
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rootless continues to work on WSL

};

patches = [
(substituteAll {
src = ./hardcode-paths.patch;
inherit crun runc gvisor youki conmon;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would we want to limit this - to reduce the number of dependencies?

But, if runc is obsolete, why would upstream still continue supporting it?

Since upstream continues supporting it, wouldn't that imply that it is valuable to have it as an option?

Also, if we don't want to support runc, shouldn't we delete it from nixpkgs?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would we want to limit this - to reduce the number of dependencies?

Yeah, we increased the dependencies with this update a lot.

But, if runc is obsolete, why would upstream still continue supporting it?

Maybe there are certain usecases when it is useful which are not relevant to nixpkgs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to make these dependencies (runc, gvisor, youki) optional?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think so. I think we could right now overwrite them with null, but that is a bit dirty.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#310182 does this. I left runc in by default on Linux, because nixpkgs actually has a test that podman works with that runtime. But, it can now be disabled.

@SuperSandro2000 SuperSandro2000 merged commit f9be31d into NixOS:master Apr 17, 2024
25 of 26 checks passed
@natsukium
Copy link
Member

This PR broke darwin support.

error: Package ‘conmon-2.1.10’ in /nix/store/wmcbdvplkli110vmdn1ci96n8zmzh7yn-source/pkgs/applications/virtualization/conmon/default.nix:45 is not available on the requested hostPlatform:
         hostPlatform.config = "aarch64-apple-darwin"

@mlyxshi
Copy link
Contributor

mlyxshi commented Apr 20, 2024

gvisor doesn't support aarch64-linux now

error: Package ‘gvisor-20240401.0’ in /nix/store/v4pcs3nzx54m5bmxd39win0rgl2d2hbx-source/pkgs/by-name/gv/gvisor/package.nix:46 is not available on the requested hostPlatform:
         hostPlatform.config = "aarch64-unknown-linux-gnu"
         package.meta.platforms = [
           "x86_64-linux"
         ]

@the-sun-will-rise-tomorrow
Copy link
Contributor Author

the-sun-will-rise-tomorrow commented Apr 20, 2024

@natsukium Sorry about that, if you're willing to try making a patch, I think this just needs the Linux-only dependencies to be made optional (e.g. with // optionalAttrs) in substituteAll.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants