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

[Philippe Mathieu-Daudé]

On 9/26/19 6:04 PM, Alex Bennée wrote:
> 
> 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.

Yes, fair enough. Thanks for keeping explaining ;)

Acked-by: Philippe Mathieu-Daudé <address@hidden>

>>>>> 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
>