Re: [PATCH v3 29/33] docker: remove 'deprecated' image definitions

[Alex Bennée]

Philippe Mathieu-Daudé <address@hidden> writes:

> Hi Alex,
>
> On 9/26/19 1:34 AM, Alex Bennée wrote:
>> Philippe Mathieu-Daudé <address@hidden> writes:
>>> On 9/24/19 11:01 PM, Alex Bennée wrote:
>>>> From: John Snow <address@hidden>
>>>>
>>>> There isn't a debian.dockerfile anymore,
>>>> so perform some ghost-busting.
>>>
>>> Won't we deprecate other images in the future?
>>
>> Sure but we can just drop them from dockerfiles. It's not like we
>> allowed people to use them as we filtered them out.
>
> This patch isn't about removing a deprecated image, but about removing
> the handy DOCKER_DEPRECATED_IMAGES variable used to start a deprecation
> process.

But it wasn't really doing anything. Because adding the image to
DOCKER_DEPRECATED_IMAGES had the same effect as using
DOCKER_PARTIAL_IMAGES. You couldn't invoke the test from make and it was
dropped out from the dependencies for the do-X-on-all image rules. As it
was also hidden from the help you might as well just delete the thing
all together.

> Fam remembered once we should respect the QEMU deprecation policy even
> with docker images, because there might be users relying on them, so we
> want to give them time to adapt. I can not find a thread on the list, so
> we might have discussed that over IRC. The related commits are:
>
> $ git show bcaf457786c
>
>     docker: do not display deprecated images in 'make docker' help
>
>     the 'debian' base image is deprecated since 3e11974988d8
>
> $ git show 3e11974988d8
>
>     docker: warn users to use newer debian8/debian9 base image
>
>     to stay backward incompatible.
>
> I'd rather keep the DOCKER_DEPRECATED_IMAGES variable empty, maybe with
> a comment describing why it exists. What do you think?

I think deprecations are better handled by a noisy warning in the
dockerfile than the silent suppression of their existence by the make
system.

>
> Thanks,
>
> Phil.
>
>>>> Signed-off-by: John Snow <address@hidden>
>>>> Message-Id: <address@hidden>
>>>> Signed-off-by: Alex Bennée <address@hidden>
>>>> ---
>>>>  tests/docker/Makefile.include | 7 +++----
>>>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
>>>> index 82d5a8a5393..fd6f470fbf8 100644
>>>> --- a/tests/docker/Makefile.include
>>>> +++ b/tests/docker/Makefile.include
>>>> @@ -4,11 +4,10 @@
>>>>
>>>>  DOCKER_SUFFIX := .docker
>>>>  DOCKER_FILES_DIR := $(SRC_PATH)/tests/docker/dockerfiles
>>>> -DOCKER_DEPRECATED_IMAGES := debian
>>>>  # we don't run tests on intermediate images (used as base by another 
>>>> image)
>>>> -DOCKER_PARTIAL_IMAGES := debian debian9 debian10 debian-sid
>>>> +DOCKER_PARTIAL_IMAGES := debian9 debian10 debian-sid
>>>>  DOCKER_PARTIAL_IMAGES += debian9-mxe debian-ports debian-bootstrap
>>>> -DOCKER_IMAGES := $(filter-out $(DOCKER_DEPRECATED_IMAGES),$(sort $(notdir 
>>>> $(basename $(wildcard $(DOCKER_FILES_DIR)/*.docker)))))
>>>> +DOCKER_IMAGES := $(sort $(notdir $(basename $(wildcard 
>>>> $(DOCKER_FILES_DIR)/*.docker))))
>>>>  DOCKER_TARGETS := $(patsubst %,docker-image-%,$(DOCKER_IMAGES))
>>>>  # Use a global constant ccache directory to speed up repetitive builds
>>>>  DOCKER_CCACHE_DIR := $$HOME/.cache/qemu-docker-ccache
>>>> @@ -160,7 +159,7 @@ docker-image-debian-powerpc-user-cross: 
>>>> docker-binfmt-image-debian-powerpc-user
>>>>  DOCKER_USER_IMAGES += debian-powerpc-user
>>>>
>>>>  # Expand all the pre-requistes for each docker image and test combination
>>>> -$(foreach i,$(filter-out $(DOCKER_PARTIAL_IMAGES),$(DOCKER_IMAGES) 
>>>> $(DOCKER_DEPRECATED_IMAGES)), \
>>>> +$(foreach i,$(filter-out $(DOCKER_PARTIAL_IMAGES),$(DOCKER_IMAGES)), \
>>>>    $(foreach t,$(DOCKER_TESTS) $(DOCKER_TOOLS), \
>>>>            $(eval .PHONY: docker-$t@$i) \
>>>>            $(eval docker-$t@$i: docker-image-$i docker-run-$t@$i) \
>>>>
>>
>>
>> --
>> Alex Bennée
>>


--
Alex Bennée