Re: [Qemu-devel] [PATCH v2 1/3] tests/docker/docker.py: docker_dir outside build

[Fam Zheng <famz@redhat.com>]

On Wed, 06/08 17:35, Alex Bennée wrote:
> Instead of letting the build_image create the temporary working dir we
> move the creation to the build command. This is preparation for the
> later patches where additional files can be added to the build context
> before the build step is run.
> 
> We also ensure we remove the build context after we are done (mkdtemp
> doesn't do this automatically for you).
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
> v2
>   - new for v2
> ---
>  tests/docker/docker.py | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> index 0151362..ae40bb3 100755
> --- a/tests/docker/docker.py
> +++ b/tests/docker/docker.py
> @@ -20,7 +20,7 @@ import atexit
>  import uuid
>  import argparse
>  import tempfile
> -from shutil import copy
> +from shutil import copy, rmtree
>  
>  def _text_checksum(text):
>      """Calculate a digest string unique to the text content"""
> @@ -87,20 +87,20 @@ class Docker(object):
>          labels = json.loads(resp)[0]["Config"].get("Labels", {})
>          return labels.get("com.qemu.dockerfile-checksum", "")
>  
> -    def build_image(self, tag, dockerfile, df_path, quiet=True, argv=None):
> +    def build_image(self, tag, docker_dir, dockerfile, quiet=True, argv=None):
>          if argv == None:
>              argv = []
> -        tmp_dir = tempfile.mkdtemp(prefix="docker_build")
>  
> -        tmp_df = tempfile.NamedTemporaryFile(dir=tmp_dir, suffix=".docker")
> +        tmp_df = tempfile.NamedTemporaryFile(dir=docker_dir, suffix=".docker")
>          tmp_df.write(dockerfile)
>  
>          tmp_df.write("\n")
>          tmp_df.write("LABEL com.qemu.dockerfile-checksum=%s" %
>                       _text_checksum(dockerfile))
>          tmp_df.flush()
> +
>          self._do(["build", "-t", tag, "-f", tmp_df.name] + argv + \
> -                 [tmp_dir],
> +                 [docker_dir],
>                   quiet=quiet)
>  
>      def image_matches_dockerfile(self, tag, dockerfile):
> @@ -164,10 +164,15 @@ class BuildCommand(SubCommand):
>          if dkr.image_matches_dockerfile(tag, dockerfile):
>              if not args.quiet:
>                  print "Image is up to date."
> -            return 0
> +        else:
> +            # Create a docker context directory for the build
> +            docker_dir = tempfile.mkdtemp(prefix="docker_build")
> +
> +            dkr.build_image(tag, docker_dir, dockerfile,
> +                            quiet=args.quiet, argv=argv)
> +
> +            rmtree(docker_dir)

Maybe we could use a "finally" here to make sure docker_dir is always removed,
but that is not a duty of this patch.

Reviewed-by: Fam Zheng <famz@redhat.com>

>  
> -        dkr.build_image(tag, dockerfile, args.dockerfile,
> -                        quiet=args.quiet, argv=argv)
>          return 0
>  
>  class CleanCommand(SubCommand):
> -- 
> 2.7.4
>