On Tue, 2019-02-05 at 17:53 +0000, Daniel P. Berrangé wrote: [...]
- flattened = [] + pkgs = [] for item in temp: - if temp[item] is not None and temp[item] not in flattened: - flattened += [temp[item]] + pkgname = temp[item] + if pkgname is None: + continue + if pkgname not in pkgs: + pkgs.append(pkgname)
Nothing against refactoring the code this way, but such a change belongs in its own patch. [...]
- sys.stdout.write("ENV PACKAGES ") - sys.stdout.write(" \\\n ".join(sorted(flattened))) - + varmap = {} + varmap["pkgs"] = "".join([" \\\n " + pkgname + for pkgname in sorted(pkgs)])
Unlike the original, this pointlessly loops through sorted(pkgs) one additional time and adds a phantom element to the beginning of the list. So, once you put everything together...
if package_format == "deb": sys.stdout.write(textwrap.dedent(""" RUN 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)s && \\ apt-get autoremove -y && \\ apt-get autoclean -y \\ )
... the result looks like apt-get install --no-install-recommends -y \ augeas-tools \ autoconf \ automake \ ... Notice the double space between '-y' and '\' as well as the weird indentation. A better solution would be to do varmap["pkgs"] = " \\\n ".join(sorted(pkgs)) followed by sys.stdout.write(textwrap.dedent(""" ... apt-get install --no-install-recommends -y \\ %(pkgs)s && \\ apt-get autoremove -y && \\ ... which is clearer and results in the more pleasant apt-get install --no-install-recommends -y \ augeas-tools \ autoconf \ automake \ ...
- """)) + """) % varmap )
Again, I don't have a problem with using this syntax for string formatting (and here it's actually much clearer than the one we are currently using), but I don't want the script to become inconsistent so we have to stick to a single style. -- Andrea Bolognani / Red Hat / Virtualization