| Message ID | 20211014224435.2539547-3-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | tests: docker images for hexagon, nios2, microblaze | expand |
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
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.
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
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
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.
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 \
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(-) -- 2.25.1