Every statement in a dockerfile results in a new layer in the
image. There is no need for an env var to store the package list
when it can be included inline. This avoids the env variable being
later exposed to the container at runtime.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 guests/lcitool | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/guests/lcitool b/guests/lcitool
index 0978c40..8252dc2 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -539,34 +539,34 @@ class Application:
 
         print("FROM {}".format(facts["docker_base"]))
 
-        sys.stdout.write("ENV PACKAGES ")
-        sys.stdout.write(" \\\n             ".join(sorted(pkgs)))
-
+        varmap = {}
+        varmap["pkgs"] = " \\\n            ".join(sorted(pkgs))
         if package_format == "deb":
             sys.stdout.write(textwrap.dedent("""
                 RUN export DEBIAN_FRONTEND=noninteractive && \\
                     apt-get update && \\
                     apt-get dist-upgrade -y && \\
-                    apt-get install --no-install-recommends -y ${PACKAGES} && \\
+                    apt-get install --no-install-recommends -y \\
+                            {pkgs} && \\
                     apt-get autoremove -y && \\
                     apt-get autoclean -y
-            """))
+            """).format(**varmap))
         elif package_format == "rpm":
             if os_name == "Fedora" and os_version == "Rawhide":
                 sys.stdout.write(textwrap.dedent("""
                     RUN yum update -y --nogpgcheck fedora-gpg-keys && \\
                         yum update -y && \\
-                        yum install -y ${PACKAGES} && \\
+                        yum install -y %(pkgs)s && \\
                         yum autoremove -y && \\
                         yum clean all -y
-                """))
+                """).format(**varmap))
             else:
                 sys.stdout.write(textwrap.dedent("""
                     RUN yum update -y && \\
-                        yum install -y ${PACKAGES} && \\
+                        yum install -y %(pkgs)s && \\
                         yum autoremove -y && \\
                         yum clean all -y
-                """))
+                """).format(**varmap))
 
     def run(self):
         cmdline = self._parser.parse_args()
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Wed, 2019-02-13 at 19:03 +0000, Daniel P. Berrangé wrote:
[...]
>          elif package_format == "rpm":
>              if os_name == "Fedora" and os_version == "Rawhide":
>                  sys.stdout.write(textwrap.dedent("""
>                      RUN yum update -y --nogpgcheck fedora-gpg-keys && \\
>                          yum update -y && \\
> -                        yum install -y ${PACKAGES} && \\
> +                        yum install -y %(pkgs)s && \\

You need to use '{pkgs}' instead of '%(pkgs)s' here, or substitution
will not be performed.

It should also be indented like

  yum install -y \\
          {pkgs} && \\

to avoid misalignment in the output file.

There's a bit of room for improvement (ideally the packages would be
aligned with "install" as they are in Debian) but we can deal with
that later.

>                          yum autoremove -y && \\
>                          yum clean all -y
> -                """))
> +                """).format(**varmap))
>              else:
>                  sys.stdout.write(textwrap.dedent("""
>                      RUN yum update -y && \\
> -                        yum install -y ${PACKAGES} && \\
> +                        yum install -y %(pkgs)s && \\

Same here.

With all of the above taken care of,

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

Can you please push the series up until this point right away? The
last patch needs some more discussion, but with these changes in I
can already refresh the existing native Dockerfiles and trigger a
build, which I'm very keen on doing sooner rather than later as the
existing images are fairly outdated by now.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list