[PATCH RFC] docker: automatic dependencies for dockerfiles

John Snow posted 1 patch 4 years, 7 months ago
Test docker-clang@ubuntu passed
Test asan passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 failed
Test checkpatch passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190920001823.23279-1-jsnow@redhat.com
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, Fam Zheng <fam@euphon.net>
tests/docker/Makefile.include | 37 ++++++++-------------------------
tests/docker/deputil.py       | 39 +++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+), 28 deletions(-)
create mode 100755 tests/docker/deputil.py
[PATCH RFC] docker: automatic dependencies for dockerfiles
Posted by John Snow 4 years, 7 months ago
This is a demo for using makefile dependencies for image requisites.
Honestly, I don't like it -- Makefile sorcery is a bit beyond my
comprehension.

This is as near as I could stab, and it has the unfortunate requisite
that it will generate all of the *.d files at first run and not in an
on-demand way. Boo.

But, I wanted to raise the point that manually managing the variables
is not long-term viable -- we should manage them automatically if we
can.

As far as "partial" images vs "full" images, we should manage this
too; perhaps by subdirectory on the dockerfiles -- that way these
won't get out of date, either.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/docker/Makefile.include | 37 ++++++++-------------------------
 tests/docker/deputil.py       | 39 +++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 28 deletions(-)
 create mode 100755 tests/docker/deputil.py

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 50a400b573..266395d927 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -21,6 +21,7 @@ DOCKER_TOOLS := travis
 ENGINE := auto
 
 DOCKER_SCRIPT=$(SRC_PATH)/tests/docker/docker.py --engine $(ENGINE)
+DEPTOOL=$(SRC_PATH)/tests/docker/deputil.py
 
 TESTS ?= %
 IMAGES ?= %
@@ -47,12 +48,12 @@ docker-image: ${DOCKER_TARGETS}
 # invoked with SKIP_DOCKER_BUILD we still check the image is up to date
 # though
 ifdef SKIP_DOCKER_BUILD
-docker-image-%: $(DOCKER_FILES_DIR)/%.docker
+docker-image-%: $(DOCKER_FILES_DIR)/%.docker %.d
 	$(call quiet-command, \
 		$(DOCKER_SCRIPT) check --quiet qemu:$* $<, \
 		"CHECK", "$*")
 else
-docker-image-%: $(DOCKER_FILES_DIR)/%.docker
+docker-image-%: $(DOCKER_FILES_DIR)/%.docker %.d
 	$(call quiet-command,\
 		$(DOCKER_SCRIPT) build qemu:$* $< \
 		$(if $V,,--quiet) $(if $(NOCACHE),--no-cache) \
@@ -88,23 +89,17 @@ docker-binfmt-image-debian-%: $(DOCKER_FILES_DIR)/debian-bootstrap.docker
 endif
 
 # Enforce dependencies for composite images
-docker-image-debian9-mxe: docker-image-debian9
+%.d: $(DOCKER_FILES_DIR)/%.docker
+	$(call quiet-command, $(DEPTOOL) $(DOCKER_FILES_DIR)/$*.docker > $@)
+
+DEPFILES := $(DOCKER_IMAGES:%=%.d)
+include $(DEPFILES)
+
 ifeq ($(ARCH),x86_64)
-docker-image-debian-amd64: docker-image-debian9
 DOCKER_PARTIAL_IMAGES += debian-amd64-cross
 else
-docker-image-debian-amd64-cross: docker-image-debian10
 DOCKER_PARTIAL_IMAGES += debian-amd64
 endif
-docker-image-debian-armel-cross: docker-image-debian9
-docker-image-debian-armhf-cross: docker-image-debian9
-docker-image-debian-mips-cross: docker-image-debian9
-docker-image-debian-mipsel-cross: docker-image-debian9
-docker-image-debian-mips64el-cross: docker-image-debian9
-docker-image-debian-ppc64el-cross: docker-image-debian9
-docker-image-debian-s390x-cross: docker-image-debian9
-docker-image-debian-win32-cross: docker-image-debian9-mxe
-docker-image-debian-win64-cross: docker-image-debian9-mxe
 
 # For non-x86 hosts not all cross-compilers have been packaged
 ifneq ($(ARCH),x86_64)
@@ -115,22 +110,8 @@ DOCKER_PARTIAL_IMAGES += debian-win32-cross debian-win64-cross
 DOCKER_PARTIAL_IMAGES += fedora travis
 endif
 
-docker-image-debian-alpha-cross: docker-image-debian10
-docker-image-debian-arm64-cross: docker-image-debian10
-docker-image-debian-hppa-cross: docker-image-debian10
-docker-image-debian-m68k-cross: docker-image-debian10
-docker-image-debian-mips64-cross: docker-image-debian10
-docker-image-debian-powerpc-cross: docker-image-debian10
-docker-image-debian-ppc64-cross: docker-image-debian10
-docker-image-debian-riscv64-cross: docker-image-debian10
-docker-image-debian-sh4-cross: docker-image-debian10
-docker-image-debian-sparc64-cross: docker-image-debian10
-
 docker-image-travis: NOUSER=1
 
-# Specialist build images, sometimes very limited tools
-docker-image-tricore-cross: docker-image-debian9
-
 # These images may be good enough for building tests but not for test builds
 DOCKER_PARTIAL_IMAGES += debian-alpha-cross
 DOCKER_PARTIAL_IMAGES += debian-hppa-cross
diff --git a/tests/docker/deputil.py b/tests/docker/deputil.py
new file mode 100755
index 0000000000..69711cf85e
--- /dev/null
+++ b/tests/docker/deputil.py
@@ -0,0 +1,39 @@
+#!/usr/bin/env python3
+import os
+import re
+import sys
+from typing import IO, Optional
+
+def get_dep(infile: IO[str]) -> Optional[str]:
+    """Get a dependency as a string from a dockerfile."""
+    for line in infile:
+        match = re.match(r'FROM (.+)', line)
+        if match:
+            return match[1]
+    return None
+
+def get_qemu_dep(infile: IO[str]) -> Optional[str]:
+    """Get a dependency on the qemu: namespace from a dockerfile."""
+    dep = get_dep(infile) or ''
+    match = re.match(r'qemu:(.+)', dep)
+    return match[1] if match else None
+
+def main() -> None:
+    filename = sys.argv[1]
+    basefile = os.path.basename(filename)
+    base = os.path.splitext(basefile)[0]
+    depfile = f"{base}.d"
+    deps = [filename]
+
+    print(f"{depfile}: {filename}")
+    with open(filename, "r") as infile:
+        match = get_qemu_dep(infile) or ''
+        if match:
+            deps.append(f"docker-image-{match}")
+    print("{}: {}".format(
+        f"docker-image-{base}",
+        " ".join(deps)
+    ))
+
+if __name__ == '__main__':
+    main()
-- 
2.21.0


Re: [PATCH RFC] docker: automatic dependencies for dockerfiles
Posted by no-reply@patchew.org 4 years, 7 months ago
Patchew URL: https://patchew.org/QEMU/20190920001823.23279-1-jsnow@redhat.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

libudev           no
default devices   yes

warning: Python 2 support is deprecated
warning: Python 3 will be required for building future versions of QEMU
cross containers  no

NOTE: guest cross-compilers enabled: cc


The full log is available at
http://patchew.org/logs/20190920001823.23279-1-jsnow@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [PATCH RFC] docker: automatic dependencies for dockerfiles
Posted by Alex Bennée 4 years, 6 months ago
John Snow <jsnow@redhat.com> writes:

> This is a demo for using makefile dependencies for image requisites.
> Honestly, I don't like it -- Makefile sorcery is a bit beyond my
> comprehension.
>
> This is as near as I could stab, and it has the unfortunate requisite
> that it will generate all of the *.d files at first run and not in an
> on-demand way. Boo.
>
> But, I wanted to raise the point that manually managing the variables
> is not long-term viable -- we should manage them automatically if we
> can.

I think this gets more complicated when we want to handle multiple host
architectures as well. We might for example have a final image that is
based of a native docker base in one case and a linux-user docker base
in another.

> As far as "partial" images vs "full" images, we should manage this
> too; perhaps by subdirectory on the dockerfiles -- that way these
> won't get out of date, either.

I'll have an experiment with different layouts and see.

>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/docker/Makefile.include | 37 ++++++++-------------------------
>  tests/docker/deputil.py       | 39 +++++++++++++++++++++++++++++++++++
>  2 files changed, 48 insertions(+), 28 deletions(-)
>  create mode 100755 tests/docker/deputil.py
>
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index 50a400b573..266395d927 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -21,6 +21,7 @@ DOCKER_TOOLS := travis
>  ENGINE := auto
>
>  DOCKER_SCRIPT=$(SRC_PATH)/tests/docker/docker.py --engine $(ENGINE)
> +DEPTOOL=$(SRC_PATH)/tests/docker/deputil.py
>
>  TESTS ?= %
>  IMAGES ?= %
> @@ -47,12 +48,12 @@ docker-image: ${DOCKER_TARGETS}
>  # invoked with SKIP_DOCKER_BUILD we still check the image is up to date
>  # though
>  ifdef SKIP_DOCKER_BUILD
> -docker-image-%: $(DOCKER_FILES_DIR)/%.docker
> +docker-image-%: $(DOCKER_FILES_DIR)/%.docker %.d
>  	$(call quiet-command, \
>  		$(DOCKER_SCRIPT) check --quiet qemu:$* $<, \
>  		"CHECK", "$*")
>  else
> -docker-image-%: $(DOCKER_FILES_DIR)/%.docker
> +docker-image-%: $(DOCKER_FILES_DIR)/%.docker %.d
>  	$(call quiet-command,\
>  		$(DOCKER_SCRIPT) build qemu:$* $< \
>  		$(if $V,,--quiet) $(if $(NOCACHE),--no-cache) \
> @@ -88,23 +89,17 @@ docker-binfmt-image-debian-%: $(DOCKER_FILES_DIR)/debian-bootstrap.docker
>  endif
>
>  # Enforce dependencies for composite images
> -docker-image-debian9-mxe: docker-image-debian9
> +%.d: $(DOCKER_FILES_DIR)/%.docker
> +	$(call quiet-command, $(DEPTOOL) $(DOCKER_FILES_DIR)/$*.docker > $@)
> +
> +DEPFILES := $(DOCKER_IMAGES:%=%.d)
> +include $(DEPFILES)
> +
>  ifeq ($(ARCH),x86_64)
> -docker-image-debian-amd64: docker-image-debian9
>  DOCKER_PARTIAL_IMAGES += debian-amd64-cross
>  else
> -docker-image-debian-amd64-cross: docker-image-debian10
>  DOCKER_PARTIAL_IMAGES += debian-amd64
>  endif
> -docker-image-debian-armel-cross: docker-image-debian9
> -docker-image-debian-armhf-cross: docker-image-debian9
> -docker-image-debian-mips-cross: docker-image-debian9
> -docker-image-debian-mipsel-cross: docker-image-debian9
> -docker-image-debian-mips64el-cross: docker-image-debian9
> -docker-image-debian-ppc64el-cross: docker-image-debian9
> -docker-image-debian-s390x-cross: docker-image-debian9
> -docker-image-debian-win32-cross: docker-image-debian9-mxe
> -docker-image-debian-win64-cross: docker-image-debian9-mxe
>
>  # For non-x86 hosts not all cross-compilers have been packaged
>  ifneq ($(ARCH),x86_64)
> @@ -115,22 +110,8 @@ DOCKER_PARTIAL_IMAGES += debian-win32-cross debian-win64-cross
>  DOCKER_PARTIAL_IMAGES += fedora travis
>  endif
>
> -docker-image-debian-alpha-cross: docker-image-debian10
> -docker-image-debian-arm64-cross: docker-image-debian10
> -docker-image-debian-hppa-cross: docker-image-debian10
> -docker-image-debian-m68k-cross: docker-image-debian10
> -docker-image-debian-mips64-cross: docker-image-debian10
> -docker-image-debian-powerpc-cross: docker-image-debian10
> -docker-image-debian-ppc64-cross: docker-image-debian10
> -docker-image-debian-riscv64-cross: docker-image-debian10
> -docker-image-debian-sh4-cross: docker-image-debian10
> -docker-image-debian-sparc64-cross: docker-image-debian10
> -
>  docker-image-travis: NOUSER=1
>
> -# Specialist build images, sometimes very limited tools
> -docker-image-tricore-cross: docker-image-debian9
> -
>  # These images may be good enough for building tests but not for test builds
>  DOCKER_PARTIAL_IMAGES += debian-alpha-cross
>  DOCKER_PARTIAL_IMAGES += debian-hppa-cross
> diff --git a/tests/docker/deputil.py b/tests/docker/deputil.py
> new file mode 100755
> index 0000000000..69711cf85e
> --- /dev/null
> +++ b/tests/docker/deputil.py
> @@ -0,0 +1,39 @@
> +#!/usr/bin/env python3
> +import os
> +import re
> +import sys
> +from typing import IO, Optional
> +
> +def get_dep(infile: IO[str]) -> Optional[str]:
> +    """Get a dependency as a string from a dockerfile."""
> +    for line in infile:
> +        match = re.match(r'FROM (.+)', line)
> +        if match:
> +            return match[1]
> +    return None
> +
> +def get_qemu_dep(infile: IO[str]) -> Optional[str]:
> +    """Get a dependency on the qemu: namespace from a dockerfile."""
> +    dep = get_dep(infile) or ''
> +    match = re.match(r'qemu:(.+)', dep)
> +    return match[1] if match else None
> +
> +def main() -> None:
> +    filename = sys.argv[1]
> +    basefile = os.path.basename(filename)
> +    base = os.path.splitext(basefile)[0]
> +    depfile = f"{base}.d"
> +    deps = [filename]
> +
> +    print(f"{depfile}: {filename}")
> +    with open(filename, "r") as infile:
> +        match = get_qemu_dep(infile) or ''
> +        if match:
> +            deps.append(f"docker-image-{match}")
> +    print("{}: {}".format(
> +        f"docker-image-{base}",
> +        " ".join(deps)
> +    ))
> +
> +if __name__ == '__main__':
> +    main()


--
Alex Bennée

Re: [PATCH RFC] docker: automatic dependencies for dockerfiles
Posted by John Snow 4 years, 6 months ago

On 10/7/19 12:12 PM, Alex Bennée wrote:
> 
> John Snow <jsnow@redhat.com> writes:
> 
>> This is a demo for using makefile dependencies for image requisites.
>> Honestly, I don't like it -- Makefile sorcery is a bit beyond my
>> comprehension.
>>
>> This is as near as I could stab, and it has the unfortunate requisite
>> that it will generate all of the *.d files at first run and not in an
>> on-demand way. Boo.
>>
>> But, I wanted to raise the point that manually managing the variables
>> is not long-term viable -- we should manage them automatically if we
>> can.
> 
> I think this gets more complicated when we want to handle multiple host
> architectures as well. We might for example have a final image that is
> based of a native docker base in one case and a linux-user docker base
> in another.
> 

Indeed!

>> As far as "partial" images vs "full" images, we should manage this
>> too; perhaps by subdirectory on the dockerfiles -- that way these
>> won't get out of date, either.
> 
> I'll have an experiment with different layouts and see.
> 

Yeah, no sweat -- this was just a quick experiment to see if I could do
it. I figured I would post my work, but I struggle with Makefile magic.

It left me wondering if we could start using Meson build systems in this
sub-tree; but maybe Meson is not well suited to this kind of "building".
I dunno. I'll experiment with this more another day when I have more
time to look at the test infrastructure.

Thanks for taking a look, anyway!

--js