[Qemu-devel] [PATCH v4 02/24] docker: add --include-files argument to 'build' command

Philippe Mathieu-Daudé posted 24 patches 8 years, 8 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v4 02/24] docker: add --include-files argument to 'build' command
Posted by Philippe Mathieu-Daudé 8 years, 8 months ago
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/docker/Makefile.include |  3 +++
 tests/docker/docker.py        | 11 ++++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 03eda37bf4..fe1a9a53ff 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -51,6 +51,7 @@ docker-image-%: $(DOCKER_FILES_DIR)/%.docker
 		$(SRC_PATH)/tests/docker/docker.py build qemu:$* $< \
 		$(if $V,,--quiet) $(if $(NOCACHE),--no-cache) \
 		$(if $(NOUSER),,--add-current-user) \
+		$(if $(EXTRA_FILES),--extra-files $(EXTRA_FILES))\
 		$(if $(EXECUTABLE),--include-executable=$(EXECUTABLE)),\
 		"BUILD","$*")
 
@@ -107,6 +108,8 @@ docker:
 	@echo '    NOUSER               Define to disable adding current user to containers passwd.'
 	@echo '    NOCACHE=1            Ignore cache when build images.'
 	@echo '    EXECUTABLE=<path>    Include executable in image.'
+	@echo '    EXTRA_FILES="<path> [... <path>]"'
+	@echo '                         Include extra files in image.'
 
 # This rule if for directly running against an arbitrary docker target.
 # It is called by the expanded docker targets (e.g. make
diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 6ddc6e4c2a..5401e58fce 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -237,6 +237,10 @@ class BuildCommand(SubCommand):
                             help="""Specify a binary that will be copied to the
                             container together with all its dependent
                             libraries""")
+        parser.add_argument("--extra-files", "-f", nargs='*',
+                            help="""Specify files that will be copied in the
+                            Docker image, fulfilling the ADD directive from the
+                            Dockerfile""")
         parser.add_argument("--add-current-user", "-u", dest="user",
                             action="store_true",
                             help="Add the current user to image's passwd")
@@ -270,10 +274,11 @@ class BuildCommand(SubCommand):
                     print "%s exited with code %d" % (docker_pre, rc)
                     return 1
 
-            # Do we include a extra binary?
+            # Include files used by ADD directives found within the Dockerfile.
             if args.include_executable:
-                _copy_binary_with_libs(args.include_executable,
-                                       docker_dir)
+                _copy_binary_with_libs(args.include_executable, docker_dir)
+            for filename in args.extra_files or []:
+                _copy_with_mkdir(filename, docker_dir)
 
             argv += ["--build-arg=" + k.lower() + "=" + v
                         for k, v in os.environ.iteritems()
-- 
2.11.0


Re: [Qemu-devel] [PATCH v4 02/24] docker: add --include-files argument to 'build' command
Posted by Alex Bennée 8 years, 8 months ago
Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/docker/Makefile.include |  3 +++
>  tests/docker/docker.py        | 11 ++++++++---
>  2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index 03eda37bf4..fe1a9a53ff 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -51,6 +51,7 @@ docker-image-%: $(DOCKER_FILES_DIR)/%.docker
>  		$(SRC_PATH)/tests/docker/docker.py build qemu:$* $< \
>  		$(if $V,,--quiet) $(if $(NOCACHE),--no-cache) \
>  		$(if $(NOUSER),,--add-current-user) \
> +		$(if $(EXTRA_FILES),--extra-files $(EXTRA_FILES))\
>  		$(if $(EXECUTABLE),--include-executable=$(EXECUTABLE)),\
>  		"BUILD","$*")
>
> @@ -107,6 +108,8 @@ docker:
>  	@echo '    NOUSER               Define to disable adding current user to containers passwd.'
>  	@echo '    NOCACHE=1            Ignore cache when build images.'
>  	@echo '    EXECUTABLE=<path>    Include executable in image.'
> +	@echo '    EXTRA_FILES="<path> [... <path>]"'
> +	@echo '                         Include extra files in image.'
>
>  # This rule if for directly running against an arbitrary docker target.
>  # It is called by the expanded docker targets (e.g. make
> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> index 6ddc6e4c2a..5401e58fce 100755
> --- a/tests/docker/docker.py
> +++ b/tests/docker/docker.py
> @@ -237,6 +237,10 @@ class BuildCommand(SubCommand):
>                              help="""Specify a binary that will be copied to the
>                              container together with all its dependent
>                              libraries""")
> +        parser.add_argument("--extra-files", "-f", nargs='*',
> +                            help="""Specify files that will be copied in the
> +                            Docker image, fulfilling the ADD directive from the
> +                            Dockerfile""")
>          parser.add_argument("--add-current-user", "-u", dest="user",
>                              action="store_true",
>                              help="Add the current user to image's passwd")
> @@ -270,10 +274,11 @@ class BuildCommand(SubCommand):
>                      print "%s exited with code %d" % (docker_pre, rc)
>                      return 1
>
> -            # Do we include a extra binary?
> +            # Include files used by ADD directives found within the
> Dockerfile.

Minor nit, as we are not actually parsing the Dockerfile here maybe it
should be re-worded to something like:

  "Copy any extra files into the Docker context. These can be included by the use
    of the ADD directive in the Dockerfile."?

Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


>              if args.include_executable:
> -                _copy_binary_with_libs(args.include_executable,
> -                                       docker_dir)
> +                _copy_binary_with_libs(args.include_executable, docker_dir)
> +            for filename in args.extra_files or []:
> +                _copy_with_mkdir(filename, docker_dir)
>
>              argv += ["--build-arg=" + k.lower() + "=" + v
>                          for k, v in os.environ.iteritems()


--
Alex Bennée

Re: [Qemu-devel] [PATCH v4 02/24] docker: add --include-files argument to 'build' command
Posted by Fam Zheng 8 years, 8 months ago
On Wed, 05/31 17:31, Alex Bennée wrote:
> 
> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
> 
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
> >  tests/docker/Makefile.include |  3 +++
> >  tests/docker/docker.py        | 11 ++++++++---
> >  2 files changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> > index 03eda37bf4..fe1a9a53ff 100644
> > --- a/tests/docker/Makefile.include
> > +++ b/tests/docker/Makefile.include
> > @@ -51,6 +51,7 @@ docker-image-%: $(DOCKER_FILES_DIR)/%.docker
> >  		$(SRC_PATH)/tests/docker/docker.py build qemu:$* $< \
> >  		$(if $V,,--quiet) $(if $(NOCACHE),--no-cache) \
> >  		$(if $(NOUSER),,--add-current-user) \
> > +		$(if $(EXTRA_FILES),--extra-files $(EXTRA_FILES))\
> >  		$(if $(EXECUTABLE),--include-executable=$(EXECUTABLE)),\
> >  		"BUILD","$*")
> >
> > @@ -107,6 +108,8 @@ docker:
> >  	@echo '    NOUSER               Define to disable adding current user to containers passwd.'
> >  	@echo '    NOCACHE=1            Ignore cache when build images.'
> >  	@echo '    EXECUTABLE=<path>    Include executable in image.'
> > +	@echo '    EXTRA_FILES="<path> [... <path>]"'
> > +	@echo '                         Include extra files in image.'
> >
> >  # This rule if for directly running against an arbitrary docker target.
> >  # It is called by the expanded docker targets (e.g. make
> > diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> > index 6ddc6e4c2a..5401e58fce 100755
> > --- a/tests/docker/docker.py
> > +++ b/tests/docker/docker.py
> > @@ -237,6 +237,10 @@ class BuildCommand(SubCommand):
> >                              help="""Specify a binary that will be copied to the
> >                              container together with all its dependent
> >                              libraries""")
> > +        parser.add_argument("--extra-files", "-f", nargs='*',
> > +                            help="""Specify files that will be copied in the
> > +                            Docker image, fulfilling the ADD directive from the
> > +                            Dockerfile""")
> >          parser.add_argument("--add-current-user", "-u", dest="user",
> >                              action="store_true",
> >                              help="Add the current user to image's passwd")
> > @@ -270,10 +274,11 @@ class BuildCommand(SubCommand):
> >                      print "%s exited with code %d" % (docker_pre, rc)
> >                      return 1
> >
> > -            # Do we include a extra binary?
> > +            # Include files used by ADD directives found within the
> > Dockerfile.
> 
> Minor nit, as we are not actually parsing the Dockerfile here maybe it
> should be re-worded to something like:
> 
>   "Copy any extra files into the Docker context. These can be included by the use
>     of the ADD directive in the Dockerfile."?

Yes, I prefer it this way as well.

Fam