| Message ID | 20170508221759.15616-3-f4bug@amsat.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 05/08 19:17, Philippe Mathieu-Daudé wrote: > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > tests/docker/Makefile.include | 3 ++- > tests/docker/docker.py | 5 +++++ > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include > index 03eda37bf4..c99ce702ea 100644 > --- a/tests/docker/Makefile.include > +++ b/tests/docker/Makefile.include > @@ -51,7 +51,8 @@ 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 $(EXECUTABLE),--include-executable=$(EXECUTABLE)),\ > + $(if $(EXECUTABLE),--include-executable=$(EXECUTABLE))\ > + $(if $(EXTRA_FILE),--include-file=$(EXTRA_FILE)),\ > "BUILD","$*") > > # Enforce dependancies for composite images > diff --git a/tests/docker/docker.py b/tests/docker/docker.py > index 6ddc6e4c2a..90520e4bca 100755 > --- a/tests/docker/docker.py > +++ b/tests/docker/docker.py > @@ -237,6 +237,9 @@ class BuildCommand(SubCommand): > help="""Specify a binary that will be copied to the > container together with all its dependent > libraries""") > + parser.add_argument("--include-file", "-f", > + help="""Specify a file that will be copied to the > + container""") Is it more useful if it's '--include-files' that accepts multiple files? Or alternatively allow this option to be use multiple times: -f foo -f bar -f ... Similarly, should EXTRA_FILE be EXTRA_FILES or something? We are already lacking this with --include-executable's documentation, but please also document "where" is it copied to in the container's filesystem. Fam > parser.add_argument("--add-current-user", "-u", dest="user", > action="store_true", > help="Add the current user to image's passwd") > @@ -274,6 +277,8 @@ class BuildCommand(SubCommand): > if args.include_executable: > _copy_binary_with_libs(args.include_executable, > docker_dir) > + if args.include_file: > + _copy_with_mkdir(args.include_file, docker_dir) > > argv += ["--build-arg=" + k.lower() + "=" + v > for k, v in os.environ.iteritems() > -- > 2.11.0 >
On 05/09/2017 01:01 AM, Fam Zheng wrote: > On Mon, 05/08 19:17, Philippe Mathieu-Daudé wrote: >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> tests/docker/Makefile.include | 3 ++- >> tests/docker/docker.py | 5 +++++ >> 2 files changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include >> index 03eda37bf4..c99ce702ea 100644 >> --- a/tests/docker/Makefile.include >> +++ b/tests/docker/Makefile.include >> @@ -51,7 +51,8 @@ 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 $(EXECUTABLE),--include-executable=$(EXECUTABLE)),\ >> + $(if $(EXECUTABLE),--include-executable=$(EXECUTABLE))\ >> + $(if $(EXTRA_FILE),--include-file=$(EXTRA_FILE)),\ >> "BUILD","$*") >> >> # Enforce dependancies for composite images >> diff --git a/tests/docker/docker.py b/tests/docker/docker.py >> index 6ddc6e4c2a..90520e4bca 100755 >> --- a/tests/docker/docker.py >> +++ b/tests/docker/docker.py >> @@ -237,6 +237,9 @@ class BuildCommand(SubCommand): >> help="""Specify a binary that will be copied to the >> container together with all its dependent >> libraries""") >> + parser.add_argument("--include-file", "-f", >> + help="""Specify a file that will be copied to the >> + container""") > > Is it more useful if it's '--include-files' that accepts multiple files? Or > alternatively allow this option to be use multiple times: > > -f foo -f bar -f ... I agree, since I'm not confident with Python so I tried to touch as little as possible. One iter I'm thinking about do a consistent rename with the "dockerfiles/%.docker -> docker.d/%/Dockerfile" pattern and adding git symlinks to required EXTRA_FILES in each dir, to avoid the call to docker.py and be able to build/push images on hub.docker.com directly. > Similarly, should EXTRA_FILE be EXTRA_FILES or something? Yes. > We are already lacking this with --include-executable's documentation, but > please also document "where" is it copied to in the container's filesystem. Ok. Thank for the review, Phil. > Fam > >> parser.add_argument("--add-current-user", "-u", dest="user", >> action="store_true", >> help="Add the current user to image's passwd") >> @@ -274,6 +277,8 @@ class BuildCommand(SubCommand): >> if args.include_executable: >> _copy_binary_with_libs(args.include_executable, >> docker_dir) >> + if args.include_file: >> + _copy_with_mkdir(args.include_file, docker_dir) >> >> argv += ["--build-arg=" + k.lower() + "=" + v >> for k, v in os.environ.iteritems() >> -- >> 2.11.0 >> >
On Tue, 05/09 09:14, Philippe Mathieu-Daudé wrote: > > > > Is it more useful if it's '--include-files' that accepts multiple files? Or > > alternatively allow this option to be use multiple times: > > > > -f foo -f bar -f ... > > I agree, since I'm not confident with Python so I tried to touch as little > as possible. > > One iter I'm thinking about do a consistent rename with the > "dockerfiles/%.docker -> docker.d/%/Dockerfile" pattern and adding git > symlinks to required EXTRA_FILES in each dir, to avoid the call to docker.py > and be able to build/push images on hub.docker.com directly. Originally I thought about "$QEMU/tests/docker/dockerfiles/$IMAGE/Dockerfile" but that sounded too fussy. "$QEMU/tests/docker/docker.d/$IMAGE/Dockerfile" is a bit better. But I'm not sure doing that can allow us to remove the necessity of docker.py - it does more such as calling .pre and passing in env vars etc.. Fam
diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index 03eda37bf4..c99ce702ea 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -51,7 +51,8 @@ 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 $(EXECUTABLE),--include-executable=$(EXECUTABLE)),\ + $(if $(EXECUTABLE),--include-executable=$(EXECUTABLE))\ + $(if $(EXTRA_FILE),--include-file=$(EXTRA_FILE)),\ "BUILD","$*") # Enforce dependancies for composite images diff --git a/tests/docker/docker.py b/tests/docker/docker.py index 6ddc6e4c2a..90520e4bca 100755 --- a/tests/docker/docker.py +++ b/tests/docker/docker.py @@ -237,6 +237,9 @@ class BuildCommand(SubCommand): help="""Specify a binary that will be copied to the container together with all its dependent libraries""") + parser.add_argument("--include-file", "-f", + help="""Specify a file that will be copied to the + container""") parser.add_argument("--add-current-user", "-u", dest="user", action="store_true", help="Add the current user to image's passwd") @@ -274,6 +277,8 @@ class BuildCommand(SubCommand): if args.include_executable: _copy_binary_with_libs(args.include_executable, docker_dir) + if args.include_file: + _copy_with_mkdir(args.include_file, docker_dir) argv += ["--build-arg=" + k.lower() + "=" + v for k, v in os.environ.iteritems()
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- tests/docker/Makefile.include | 3 ++- tests/docker/docker.py | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-)