Re: [Qemu-devel] [PATCH] tests/docker: upgrade docker.py to python3

2019-08-29 Thread Alex Bennée


Philippe Mathieu-Daudé  writes:

> 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 
>> Cc: Marc-André Lureau 
>> ---
>>  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.

Yeah it is the default but I favoured being explicit and bringing it all
together in one place...

>
> Reviewed-by: Philippe Mathieu-Daudé 

Thanks

>
>>  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:
>>


--
Alex Bennée



Re: [Qemu-devel] [PATCH] tests/docker: upgrade docker.py to python3

2019-08-29 Thread Philippe Mathieu-Daudé
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 
> Cc: Marc-André Lureau 
> ---
>  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é 

>  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:
> 



Re: [Qemu-devel] [PATCH] tests/docker: upgrade docker.py to python3

2019-08-29 Thread Daniel P . Berrangé
On Thu, Aug 29, 2019 at 03:41:20PM +0100, 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
> 
> Signed-off-by: Alex Bennée 
> Cc: Marc-André Lureau 
> ---
>  tests/docker/docker.py | 27 ++-
>  1 file changed, 14 insertions(+), 13 deletions(-)

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|