From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: "Alex Bennée" <alex.bennee@linaro.org>, qemu-devel@nongnu.org
Cc: "Fam Zheng" <fam@euphon.net>,
"Marc-André Lureau" <marcandre.lureau@redhat.com>,
berrange@redhat.com, f4bug@amsat.org
Subject: Re: [Qemu-devel] [PATCH] tests/docker: upgrade docker.py to python3
Date: Thu, 29 Aug 2019 17:28:04 +0200 [thread overview]
Message-ID: <9e98702a-6582-3d38-c9a4-93f0279033bf@redhat.com> (raw)
In-Reply-To: <20190829144120.12701-1-alex.bennee@linaro.org>
On 8/29/19 4:41 PM, Alex Bennée wrote:
> The recent podman changes (9459f754134bb) imported enum which is part
> of the python3 standard library but only available as an external
> library for python2. This causes problems on the fairly restricted
> environment such as shippable. Lets bite the bullet and make the
> script a fully python3 one. To that end:
>
> - drop the from __future__ import (we are there now ;-)
> - avoid the StringIO import hack
> - be consistent with the mode we read/write dockerfiles
> - s/iteritems/items/
> - ensure check_output returns strings for processing
Many boring changes at once...
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> tests/docker/docker.py | 27 ++++++++++++++-------------
> 1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> index ac5baab4cad..4bba29e104e 100755
> --- a/tests/docker/docker.py
> +++ b/tests/docker/docker.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/env python2
> +#!/usr/bin/env python3
> #
> # Docker controlling module
> #
> @@ -11,7 +11,6 @@
> # or (at your option) any later version. See the COPYING file in
> # the top-level directory.
>
> -from __future__ import print_function
> import os
> import sys
> import subprocess
> @@ -25,10 +24,7 @@ import tempfile
> import re
> import signal
> from tarfile import TarFile, TarInfo
> -try:
> - from StringIO import StringIO
> -except ImportError:
> - from io import StringIO
> +from io import StringIO
> from shutil import copy, rmtree
> from pwd import getpwuid
> from datetime import datetime, timedelta
> @@ -62,11 +58,13 @@ USE_ENGINE = EngineEnum.AUTO
>
> def _text_checksum(text):
> """Calculate a digest string unique to the text content"""
> - return hashlib.sha1(text).hexdigest()
> + return hashlib.sha1(text.encode('utf-8')).hexdigest()
>
> +def _read_dockerfile(path):
> + return open(path, 'rt', encoding='utf-8').read()
TIL it's cleaner to explicit the 't' mode.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> def _file_checksum(filename):
> - return _text_checksum(open(filename, 'rb').read())
> + return _text_checksum(_read_dockerfile(filename))
>
>
> def _guess_engine_command():
> @@ -192,7 +190,7 @@ def _read_qemu_dockerfile(img_name):
>
> df = os.path.join(os.path.dirname(__file__), "dockerfiles",
> img_name + ".docker")
> - return open(df, "r").read()
> + return _read_dockerfile(df)
>
>
> def _dockerfile_preprocess(df):
> @@ -262,6 +260,7 @@ class Docker(object):
> def _output(self, cmd, **kwargs):
> return subprocess.check_output(self._command + cmd,
> stderr=subprocess.STDOUT,
> + encoding='utf-8',
> **kwargs)
>
> def inspect_tag(self, tag):
> @@ -283,7 +282,9 @@ class Docker(object):
> if argv is None:
> argv = []
>
> - tmp_df = tempfile.NamedTemporaryFile(dir=docker_dir, suffix=".docker")
> + tmp_df = tempfile.NamedTemporaryFile(mode="w+t",
> + encoding='utf-8',
> + dir=docker_dir, suffix=".docker")
> tmp_df.write(dockerfile)
>
> if user:
> @@ -396,7 +397,7 @@ class BuildCommand(SubCommand):
> help="Dockerfile name")
>
> def run(self, args, argv):
> - dockerfile = open(args.dockerfile, "rb").read()
> + dockerfile = _read_dockerfile(args.dockerfile)
> tag = args.tag
>
> dkr = Docker()
> @@ -442,7 +443,7 @@ class BuildCommand(SubCommand):
> cksum += [(filename, _file_checksum(filename))]
>
> argv += ["--build-arg=" + k.lower() + "=" + v
> - for k, v in os.environ.iteritems()
> + for k, v in os.environ.items()
> if k.lower() in FILTERED_ENV_NAMES]
> dkr.build_image(tag, docker_dir, dockerfile,
> quiet=args.quiet, user=args.user, argv=argv,
> @@ -611,7 +612,7 @@ class CheckCommand(SubCommand):
> print("Need a dockerfile for tag:%s" % (tag))
> return 1
>
> - dockerfile = open(args.dockerfile, "rb").read()
> + dockerfile = _read_dockerfile(args.dockerfile)
>
> if dkr.image_matches_dockerfile(tag, dockerfile):
> if not args.quiet:
>
next prev parent reply other threads:[~2019-08-29 15:29 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-29 14:41 [Qemu-devel] [PATCH] tests/docker: upgrade docker.py to python3 Alex Bennée
2019-08-29 14:44 ` Daniel P. Berrangé
2019-08-29 15:28 ` Philippe Mathieu-Daudé [this message]
2019-08-29 15:46 ` Alex Bennée
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9e98702a-6582-3d38-c9a4-93f0279033bf@redhat.com \
--to=philmd@redhat.com \
--cc=alex.bennee@linaro.org \
--cc=berrange@redhat.com \
--cc=f4bug@amsat.org \
--cc=fam@euphon.net \
--cc=marcandre.lureau@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).