[PATCH 2/7] tests/docker: Simplify debian-all-test-cross

Richard Henderson posted 7 patches 4 years, 3 months ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Willian Rampazzo <willianr@redhat.com>, Wainer dos Santos Moschetta <wainersm@redhat.com>
[PATCH 2/7] tests/docker: Simplify debian-all-test-cross
Posted by Richard Henderson 4 years, 3 months ago
The base debian10 image contains enough to build qemu;
we do not need to repeat that within the new image.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tests/docker/dockerfiles/debian-all-test-cross.docker | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/tests/docker/dockerfiles/debian-all-test-cross.docker b/tests/docker/dockerfiles/debian-all-test-cross.docker
index dedcea58b4..b185b7c15a 100644
--- a/tests/docker/dockerfiles/debian-all-test-cross.docker
+++ b/tests/docker/dockerfiles/debian-all-test-cross.docker
@@ -8,11 +8,6 @@
 #
 FROM qemu/debian10
 
-# What we need to build QEMU itself
-RUN apt update && \
-    DEBIAN_FRONTEND=noninteractive eatmydata \
-    apt build-dep -yy qemu
-
 # Add the foreign architecture we want and install dependencies
 RUN DEBIAN_FRONTEND=noninteractive eatmydata \
         apt install -y --no-install-recommends \
-- 
2.25.1


Re: [PATCH 2/7] tests/docker: Simplify debian-all-test-cross
Posted by Alex Bennée 4 years, 3 months ago
Richard Henderson <richard.henderson@linaro.org> writes:

> The base debian10 image contains enough to build qemu;
> we do not need to repeat that within the new image.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tests/docker/dockerfiles/debian-all-test-cross.docker | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/tests/docker/dockerfiles/debian-all-test-cross.docker b/tests/docker/dockerfiles/debian-all-test-cross.docker
> index dedcea58b4..b185b7c15a 100644
> --- a/tests/docker/dockerfiles/debian-all-test-cross.docker
> +++ b/tests/docker/dockerfiles/debian-all-test-cross.docker
> @@ -8,11 +8,6 @@
>  #
>  FROM qemu/debian10
>  
> -# What we need to build QEMU itself
> -RUN apt update && \
> -    DEBIAN_FRONTEND=noninteractive eatmydata \
> -    apt build-dep -yy qemu
> -
>  # Add the foreign architecture we want and install dependencies
>  RUN DEBIAN_FRONTEND=noninteractive eatmydata \
>          apt install -y --no-install-recommends \

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

-- 
Alex Bennée

Re: [PATCH 2/7] tests/docker: Simplify debian-all-test-cross
Posted by Philippe Mathieu-Daudé 4 years, 3 months ago
On 10/18/21 12:54, Alex Bennée wrote:
> 
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> The base debian10 image contains enough to build qemu;
>> we do not need to repeat that within the new image.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  tests/docker/dockerfiles/debian-all-test-cross.docker | 5 -----
>>  1 file changed, 5 deletions(-)
>>
>> diff --git a/tests/docker/dockerfiles/debian-all-test-cross.docker b/tests/docker/dockerfiles/debian-all-test-cross.docker
>> index dedcea58b4..b185b7c15a 100644
>> --- a/tests/docker/dockerfiles/debian-all-test-cross.docker
>> +++ b/tests/docker/dockerfiles/debian-all-test-cross.docker
>> @@ -8,11 +8,6 @@
>>  #
>>  FROM qemu/debian10
>>  
>> -# What we need to build QEMU itself
>> -RUN apt update && \
>> -    DEBIAN_FRONTEND=noninteractive eatmydata \
>> -    apt build-dep -yy qemu
>> -
>>  # Add the foreign architecture we want and install dependencies
>>  RUN DEBIAN_FRONTEND=noninteractive eatmydata \
>>          apt install -y --no-install-recommends \
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

IIUC if we keep --arch-only in the previous step (previous patch),
this step is required here.

Re: [PATCH 2/7] tests/docker: Simplify debian-all-test-cross
Posted by Alex Bennée 4 years, 3 months ago
Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> On 10/18/21 12:54, Alex Bennée wrote:
>> 
>> Richard Henderson <richard.henderson@linaro.org> writes:
>> 
>>> The base debian10 image contains enough to build qemu;
>>> we do not need to repeat that within the new image.
>>>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>>  tests/docker/dockerfiles/debian-all-test-cross.docker | 5 -----
>>>  1 file changed, 5 deletions(-)
>>>
>>> diff --git a/tests/docker/dockerfiles/debian-all-test-cross.docker b/tests/docker/dockerfiles/debian-all-test-cross.docker
>>> index dedcea58b4..b185b7c15a 100644
>>> --- a/tests/docker/dockerfiles/debian-all-test-cross.docker
>>> +++ b/tests/docker/dockerfiles/debian-all-test-cross.docker
>>> @@ -8,11 +8,6 @@
>>>  #
>>>  FROM qemu/debian10
>>>  
>>> -# What we need to build QEMU itself
>>> -RUN apt update && \
>>> -    DEBIAN_FRONTEND=noninteractive eatmydata \
>>> -    apt build-dep -yy qemu
>>> -
>>>  # Add the foreign architecture we want and install dependencies
>>>  RUN DEBIAN_FRONTEND=noninteractive eatmydata \
>>>          apt install -y --no-install-recommends \
>> 
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>
> IIUC if we keep --arch-only in the previous step (previous patch),
> this step is required here.

What for? The --arch-only prevents the attempt to install cross
compilers that don't exist on a given host but we install all the cross
compilers we need for tests explicitly.

-- 
Alex Bennée

Re: [PATCH 2/7] tests/docker: Simplify debian-all-test-cross
Posted by Alex Bennée 4 years, 3 months ago
Alex Bennée <alex.bennee@linaro.org> writes:

> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
>
>> On 10/18/21 12:54, Alex Bennée wrote:
>>> 
>>> Richard Henderson <richard.henderson@linaro.org> writes:
>>> 
>>>> The base debian10 image contains enough to build qemu;
>>>> we do not need to repeat that within the new image.
>>>>
>>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>>> ---
>>>>  tests/docker/dockerfiles/debian-all-test-cross.docker | 5 -----
>>>>  1 file changed, 5 deletions(-)
>>>>
>>>> diff --git a/tests/docker/dockerfiles/debian-all-test-cross.docker b/tests/docker/dockerfiles/debian-all-test-cross.docker
>>>> index dedcea58b4..b185b7c15a 100644
>>>> --- a/tests/docker/dockerfiles/debian-all-test-cross.docker
>>>> +++ b/tests/docker/dockerfiles/debian-all-test-cross.docker
>>>> @@ -8,11 +8,6 @@
>>>>  #
>>>>  FROM qemu/debian10
>>>>  
>>>> -# What we need to build QEMU itself
>>>> -RUN apt update && \
>>>> -    DEBIAN_FRONTEND=noninteractive eatmydata \
>>>> -    apt build-dep -yy qemu
>>>> -
>>>>  # Add the foreign architecture we want and install dependencies
>>>>  RUN DEBIAN_FRONTEND=noninteractive eatmydata \
>>>>          apt install -y --no-install-recommends \
>>> 
>>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>>
>> IIUC if we keep --arch-only in the previous step (previous patch),
>> this step is required here.
>
> What for? The --arch-only prevents the attempt to install cross
> compilers that don't exist on a given host but we install all the cross
> compilers we need for tests explicitly.

Well patches 1 & 2 have been a cause of hair tearing the last two days. If
we replace the previous:

 $(apt-get -s build-dep --arch-only qemu | egrep ^Inst | fgrep '[all]' | cut -d\  -f2)

which only installs (all) packages with:

  apt build-dep -yy --arch-only qemu

it breaks all the cross compiler images that are based on on debian10
because suddenly their:

  apt build-dep -yy -a $ARCH --arch-only qemu

will fail to build. However this is somewhat hidden by the fact that
most people pushing to gitlab would have that effect masked as the
images will be based on the older ones in the registry.

Of course I ran into the opposite problem trying to test things as we
aggressively used the upstream registry. As a result of the dropped the
build-dep in the all-cross but without being based on the new image we
end up without enough deps to build stuff.

In short I need to drop the first two patches.

-- 
Alex Bennée

Re: [PATCH 2/7] tests/docker: Simplify debian-all-test-cross
Posted by Philippe Mathieu-Daudé 4 years, 3 months ago
On 11/2/21 13:24, Alex Bennée wrote:
> Alex Bennée <alex.bennee@linaro.org> writes:
>> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
>>> On 10/18/21 12:54, Alex Bennée wrote:
>>>> Richard Henderson <richard.henderson@linaro.org> writes:
>>>>
>>>>> The base debian10 image contains enough to build qemu;
>>>>> we do not need to repeat that within the new image.
>>>>>
>>>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>>>> ---
>>>>>  tests/docker/dockerfiles/debian-all-test-cross.docker | 5 -----
>>>>>  1 file changed, 5 deletions(-)
>>>>>
>>>>> diff --git a/tests/docker/dockerfiles/debian-all-test-cross.docker b/tests/docker/dockerfiles/debian-all-test-cross.docker
>>>>> index dedcea58b4..b185b7c15a 100644
>>>>> --- a/tests/docker/dockerfiles/debian-all-test-cross.docker
>>>>> +++ b/tests/docker/dockerfiles/debian-all-test-cross.docker
>>>>> @@ -8,11 +8,6 @@
>>>>>  #
>>>>>  FROM qemu/debian10
>>>>>  
>>>>> -# What we need to build QEMU itself
>>>>> -RUN apt update && \
>>>>> -    DEBIAN_FRONTEND=noninteractive eatmydata \
>>>>> -    apt build-dep -yy qemu
>>>>> -
>>>>>  # Add the foreign architecture we want and install dependencies
>>>>>  RUN DEBIAN_FRONTEND=noninteractive eatmydata \
>>>>>          apt install -y --no-install-recommends \
>>>>
>>>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>>>
>>> IIUC if we keep --arch-only in the previous step (previous patch),
>>> this step is required here.
>>
>> What for? The --arch-only prevents the attempt to install cross
>> compilers that don't exist on a given host but we install all the cross
>> compilers we need for tests explicitly.
> 
> Well patches 1 & 2 have been a cause of hair tearing the last two days. If
> we replace the previous:
> 
>  $(apt-get -s build-dep --arch-only qemu | egrep ^Inst | fgrep '[all]' | cut -d\  -f2)
> 
> which only installs (all) packages with:
> 
>   apt build-dep -yy --arch-only qemu
> 
> it breaks all the cross compiler images that are based on on debian10
> because suddenly their:
> 
>   apt build-dep -yy -a $ARCH --arch-only qemu
> 
> will fail to build. However this is somewhat hidden by the fact that
> most people pushing to gitlab would have that effect masked as the
> images will be based on the older ones in the registry.
> 
> Of course I ran into the opposite problem trying to test things as we
> aggressively used the upstream registry. As a result of the dropped the
> build-dep in the all-cross but without being based on the new image we
> end up without enough deps to build stuff.
> 
> In short I need to drop the first two patches.

I remember I had to do it this ugly way to simplify the
rest, and later I came back to it because Peter said it
was confusing, but I couldn't find a easier way to remove
this command.

Today I think we shouldn't bother to include intermediate
Dockerfiles. I'd simply use one plain file and let the
Docker daemon worry about caching/reusing.

But the whole machinery needs some thought since we want
to be able to use non-x86_64 hosts; and on the CI side
we want pulled images to be stable.