Subject: Re: bug#52174: [PATCH] gnu: Add podman

From: Ludovic Courtès <[email-masked]>
Date: Sat, 18 Dec 2021 18:57:20 +0100

Hi Timmy,

Timmy Douglas <[email-masked]> skribis:

> * gnu/packages/containers.scm (crun, conmon, libslirp, slirp4netns,
> cni-plugins, podman): Add podman and dependencies.
> ---
>
> I was going to try running some docker containers on my Guix system
> today, but I noticed docker was an old version. I decided to try
> packaging podman as the daemonless aspect seems appealing and was able
> to run a basic alpine image (rootless) after a couple of hours of
> putting this together. This is one of my first packages for Guix, so
> please give me some feedback--I'm also interested if anyone else would
> like to work together on this, because I'm fairly new to Guix and I
> haven't used podman before either. This probably needs some more
> testing since I only tried a basic scenario.
>
> For podman to work, I needed to run this:
> `sudo mount -t cgroup2 none /sys/fs/cgroup`

Woow, that’s a great start, and it’ll certainly be useful to many.

Overall it LGTM.  Here are some minor issues that would be nice
addressing:

> gnu/packages/containers.scm | 304 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 304 insertions(+)
>  create mode 100644 gnu/packages/containers.scm

We usually have one patch per new package; IWBN if you could split this
patch accordingly.

Make sure to add ‘containers.scm’ to ‘gnu/local.mk’ too.

> +    (arguments
> +     '(#:tests? #f
> +                #:configure-flags '("--disable-systemd")
> +                #:phases

Please align keywords, as in:

  '(#:tests? #f
    #:configure-flags …
    #:phases …)

> +                          (display (string-append
> +                                    "/* autogenerated.  */\n#ifndef GIT_VERSION\n# define GIT_VERSION \""
> +                                    "8e5757a4e68590326dafe8a8b1b4a584b10a1370" ; refactor this

You can write “,commit” (read: “unquote commit”), assuming there’s a
‘commit’ local variable above.

> +    (inputs
> +     `(("libcap" ,libcap)
> +       ("libseccomp" ,libseccomp)
> +       ("libyajl" ,libyajl)))
> +    (native-inputs
> +     `(("automake" ,automake)
> +       ("autoreconf" ,autoconf)
> +       ("git" ,git)
> +       ("libtool" ,libtool)
> +       ("pkg-config" ,pkg-config)
> +       ("python-3" ,python-3)))

IWBN if you could run packages through ‘guix style’:

  https://guix.gnu.org/manual/devel/en/html_node/Invoking-guix-style.html

> +    (name "conmon")
> +    (version "v2.0.30")

Please remove “v” from the version number (here and elsewhere), and…

> +       (uri (git-reference
> +             (url "https://github.com/containers/conmon")
> +             (commit version)))
                        ^
… add it here, with (string-append "v" version).

> +    (home-page "https://gitlab.freedesktop.org/slirp/libslirp")
> +    (synopsis "User-mode networking library")
> +    (description
> +     "libslirp is a user-mode networking library used by virtual machines,
> +containers or various tools.")
> +    (license license:non-copyleft))) ;fixme what is this?

It’s ‘license:bsd-3’, per
<https://gitlab.freedesktop.org/slirp/libslirp/-/blob/master/COPYRIGHT>.

That’s it.

Could you send updated patches?

Thank you, and apologies for the delay!

Ludo’.