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

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

[Alex Bennée]

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 <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()
 
 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:
-- 
2.20.1

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

[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 <alex.bennee@linaro.org>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/docker/docker.py | 27 ++++++++++++++-------------
>  1 file changed, 14 insertions(+), 13 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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

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

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

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

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@redhat.com> 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 <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.

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

>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

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