| Message ID | 20241105160738.3795497-5-sjg@chromium.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | CI: Set up for an arm64 runner | expand |
On Tue, Nov 05, 2024 at 09:07:33AM -0700, Simon Glass wrote: > The package names are slightly different for arm64 and we don't seem to > have a linux-image-kvm package. Provide a different set for arm64 > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > Changes in v2: > - Swap order so that amd64 is the exception > > tools/docker/Dockerfile | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile > index 540718df062..84661b036c6 100644 > --- a/tools/docker/Dockerfile > +++ b/tools/docker/Dockerfile > @@ -44,7 +44,12 @@ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_ > RUN wget -O - https://github.com/foss-xtensa/toolchain/releases/download/2020.07/x86_64-2020.07-xtensa-dc233c-elf.tar.gz | tar -C /opt -xz Oh no, here's a big problem ^^^^^ You didn't switch to downloading arm64 toolchains, no wonder everything else blows up. > # Update and install things from apt now > -RUN apt-get update && apt-get install -y \ > +RUN if [ "$TARGETPLATFORM" = "linux/amd64" ]; then \ > + EXTRA_PACKAGES="grub-efi-amd64-bin grub-efi-ia32-bin libc6-i386 linux-image-kvm"; \ > + else \ > + EXTRA_PACKAGES="grub-efi linux-image-generic"; \ > + fi; \ Again, grub-efi should be used for everyone and linux/amd64 adds grub-efi-ia32-bin. And linux-image-generic should work for everyone, too.
Hi Tom, On Tue, 5 Nov 2024 at 09:11, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Nov 05, 2024 at 09:07:33AM -0700, Simon Glass wrote: > > The package names are slightly different for arm64 and we don't seem to > > have a linux-image-kvm package. Provide a different set for arm64 > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > Changes in v2: > > - Swap order so that amd64 is the exception > > > > tools/docker/Dockerfile | 12 +++++++----- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile > > index 540718df062..84661b036c6 100644 > > --- a/tools/docker/Dockerfile > > +++ b/tools/docker/Dockerfile > > @@ -44,7 +44,12 @@ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_ > > RUN wget -O - https://github.com/foss-xtensa/toolchain/releases/download/2020.07/x86_64-2020.07-xtensa-dc233c-elf.tar.gz | tar -C /opt -xz > > Oh no, here's a big problem ^^^^^ > You didn't switch to downloading arm64 toolchains, no wonder everything > else blows up. Er, yes I know that...I would have to put 'if' statements on each one, but I was hoping we could come up with a better way of handling this. Same with all the variables being set for grub. > > > # Update and install things from apt now > > -RUN apt-get update && apt-get install -y \ > > +RUN if [ "$TARGETPLATFORM" = "linux/amd64" ]; then \ > > + EXTRA_PACKAGES="grub-efi-amd64-bin grub-efi-ia32-bin libc6-i386 linux-image-kvm"; \ > > + else \ > > + EXTRA_PACKAGES="grub-efi linux-image-generic"; \ > > + fi; \ > > Again, grub-efi should be used for everyone and linux/amd64 adds > grub-efi-ia32-bin. And linux-image-generic should work for everyone, > too. So what about this commit?[1] Regards, Simon [1] f9abaa53ec8 tools: docker: Install a readable kernel for libguestfs-tools
On Tue, Nov 05, 2024 at 09:26:10AM -0700, Simon Glass wrote: > Hi Tom, > > On Tue, 5 Nov 2024 at 09:11, Tom Rini <trini@konsulko.com> wrote: > > > > On Tue, Nov 05, 2024 at 09:07:33AM -0700, Simon Glass wrote: > > > The package names are slightly different for arm64 and we don't seem to > > > have a linux-image-kvm package. Provide a different set for arm64 > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > --- > > > > > > Changes in v2: > > > - Swap order so that amd64 is the exception > > > > > > tools/docker/Dockerfile | 12 +++++++----- > > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > > > diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile > > > index 540718df062..84661b036c6 100644 > > > --- a/tools/docker/Dockerfile > > > +++ b/tools/docker/Dockerfile > > > @@ -44,7 +44,12 @@ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_ > > > RUN wget -O - https://github.com/foss-xtensa/toolchain/releases/download/2020.07/x86_64-2020.07-xtensa-dc233c-elf.tar.gz | tar -C /opt -xz > > > > Oh no, here's a big problem ^^^^^ > > You didn't switch to downloading arm64 toolchains, no wonder everything > > else blows up. > > Er, yes I know that...I would have to put 'if' statements on each one, > but I was hoping we could come up with a better way of handling this. Yes, hopefully we can do something with $TARGETPLATFORM to set another variable that's exposed for wider use. > Same with all the variables being set for grub. > > > > > > # Update and install things from apt now > > > -RUN apt-get update && apt-get install -y \ > > > +RUN if [ "$TARGETPLATFORM" = "linux/amd64" ]; then \ > > > + EXTRA_PACKAGES="grub-efi-amd64-bin grub-efi-ia32-bin libc6-i386 linux-image-kvm"; \ > > > + else \ > > > + EXTRA_PACKAGES="grub-efi linux-image-generic"; \ > > > + fi; \ > > > > Again, grub-efi should be used for everyone and linux/amd64 adds > > grub-efi-ia32-bin. And linux-image-generic should work for everyone, > > too. > > So what about this commit?[1] > > Regards, > Simon > > [1] f9abaa53ec8 tools: docker: Install a readable kernel for libguestfs-tools Yes, linux-image-generic should also provide a useful image, and you would need to check what CI says when using this image.
HI Tom, On Tue, 5 Nov 2024 at 09:45, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Nov 05, 2024 at 09:26:10AM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Tue, 5 Nov 2024 at 09:11, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Tue, Nov 05, 2024 at 09:07:33AM -0700, Simon Glass wrote: > > > > The package names are slightly different for arm64 and we don't seem to > > > > have a linux-image-kvm package. Provide a different set for arm64 > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > --- > > > > > > > > Changes in v2: > > > > - Swap order so that amd64 is the exception > > > > > > > > tools/docker/Dockerfile | 12 +++++++----- > > > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile > > > > index 540718df062..84661b036c6 100644 > > > > --- a/tools/docker/Dockerfile > > > > +++ b/tools/docker/Dockerfile > > > > @@ -44,7 +44,12 @@ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_ > > > > RUN wget -O - https://github.com/foss-xtensa/toolchain/releases/download/2020.07/x86_64-2020.07-xtensa-dc233c-elf.tar.gz | tar -C /opt -xz > > > > > > Oh no, here's a big problem ^^^^^ > > > You didn't switch to downloading arm64 toolchains, no wonder everything > > > else blows up. > > > > Er, yes I know that...I would have to put 'if' statements on each one, > > but I was hoping we could come up with a better way of handling this. > > Yes, hopefully we can do something with $TARGETPLATFORM to set another > variable that's exposed for wider use. I recall that docker doesn't let variables set in one phase affect subsequent ones, but perhaps something can be done in the ENV section > > > Same with all the variables being set for grub. > > > > > > > > > # Update and install things from apt now > > > > -RUN apt-get update && apt-get install -y \ > > > > +RUN if [ "$TARGETPLATFORM" = "linux/amd64" ]; then \ > > > > + EXTRA_PACKAGES="grub-efi-amd64-bin grub-efi-ia32-bin libc6-i386 linux-image-kvm"; \ > > > > + else \ > > > > + EXTRA_PACKAGES="grub-efi linux-image-generic"; \ > > > > + fi; \ > > > > > > Again, grub-efi should be used for everyone and linux/amd64 adds > > > grub-efi-ia32-bin. And linux-image-generic should work for everyone, > > > too. > > > > So what about this commit?[1] > > > > Regards, > > Simon > > > > [1] f9abaa53ec8 tools: docker: Install a readable kernel for libguestfs-tools > > Yes, linux-image-generic should also provide a useful image, and you > would need to check what CI says when using this image. OK, so we don't actually need the kvm one, it seems? Anyway, the upshot of all of this is that you want arm64 to run the full suite of tests, including kvm and world builds. But presumably not in addition to the x86_64 ones...? I suppose I am just unclear as to what you are looking for. Are you trying to test that QEMU works properly, or that U-Boot's code works? My expectation was that we just want sandbox testing on arm64...I don't see much point in anything else. What am I missing? Regards, Simon
On Tue, Nov 05, 2024 at 12:20:51PM -0700, Simon Glass wrote: > HI Tom, > > On Tue, 5 Nov 2024 at 09:45, Tom Rini <trini@konsulko.com> wrote: > > > > On Tue, Nov 05, 2024 at 09:26:10AM -0700, Simon Glass wrote: > > > Hi Tom, > > > > > > On Tue, 5 Nov 2024 at 09:11, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Tue, Nov 05, 2024 at 09:07:33AM -0700, Simon Glass wrote: > > > > > The package names are slightly different for arm64 and we don't seem to > > > > > have a linux-image-kvm package. Provide a different set for arm64 > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > --- > > > > > > > > > > Changes in v2: > > > > > - Swap order so that amd64 is the exception > > > > > > > > > > tools/docker/Dockerfile | 12 +++++++----- > > > > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > > > > > > > diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile > > > > > index 540718df062..84661b036c6 100644 > > > > > --- a/tools/docker/Dockerfile > > > > > +++ b/tools/docker/Dockerfile > > > > > @@ -44,7 +44,12 @@ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_ > > > > > RUN wget -O - https://github.com/foss-xtensa/toolchain/releases/download/2020.07/x86_64-2020.07-xtensa-dc233c-elf.tar.gz | tar -C /opt -xz > > > > > > > > Oh no, here's a big problem ^^^^^ > > > > You didn't switch to downloading arm64 toolchains, no wonder everything > > > > else blows up. > > > > > > Er, yes I know that...I would have to put 'if' statements on each one, > > > but I was hoping we could come up with a better way of handling this. > > > > Yes, hopefully we can do something with $TARGETPLATFORM to set another > > variable that's exposed for wider use. > > I recall that docker doesn't let variables set in one phase affect > subsequent ones, but perhaps something can be done in the ENV section Yes, my hope is we could set something in the ENV section. > > > Same with all the variables being set for grub. > > > > > > > > > > > > # Update and install things from apt now > > > > > -RUN apt-get update && apt-get install -y \ > > > > > +RUN if [ "$TARGETPLATFORM" = "linux/amd64" ]; then \ > > > > > + EXTRA_PACKAGES="grub-efi-amd64-bin grub-efi-ia32-bin libc6-i386 linux-image-kvm"; \ > > > > > + else \ > > > > > + EXTRA_PACKAGES="grub-efi linux-image-generic"; \ > > > > > + fi; \ > > > > > > > > Again, grub-efi should be used for everyone and linux/amd64 adds > > > > grub-efi-ia32-bin. And linux-image-generic should work for everyone, > > > > too. > > > > > > So what about this commit?[1] > > > > > > Regards, > > > Simon > > > > > > [1] f9abaa53ec8 tools: docker: Install a readable kernel for libguestfs-tools > > > > Yes, linux-image-generic should also provide a useful image, and you > > would need to check what CI says when using this image. > > OK, so we don't actually need the kvm one, it seems? No, that was just a reasonable at the time choice for a kernel image file. > Anyway, the upshot of all of this is that you want arm64 to run the > full suite of tests, including kvm and world builds. But presumably > not in addition to the x86_64 ones...? I suppose I am just unclear as > to what you are looking for. Are you trying to test that QEMU works > properly, or that U-Boot's code works? > > My expectation was that we just want sandbox testing on arm64...I > don't see much point in anything else. What am I missing? I would like, especially if we get additional compute resources donated, to duplicate much of the testing we do _also_ on arm64 as a host, at least on tagged releases. This is a thing more and more people do, and it would be good to be able to point at CI when something like: https://source.denx.de/u-boot/u-boot/-/issues/44 is filed.
Hi Tom, On Tue, 5 Nov 2024 at 13:55, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Nov 05, 2024 at 12:20:51PM -0700, Simon Glass wrote: > > HI Tom, > > > > On Tue, 5 Nov 2024 at 09:45, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Tue, Nov 05, 2024 at 09:26:10AM -0700, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Tue, 5 Nov 2024 at 09:11, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > On Tue, Nov 05, 2024 at 09:07:33AM -0700, Simon Glass wrote: > > > > > > The package names are slightly different for arm64 and we don't seem to > > > > > > have a linux-image-kvm package. Provide a different set for arm64 > > > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > > --- > > > > > > > > > > > > Changes in v2: > > > > > > - Swap order so that amd64 is the exception > > > > > > > > > > > > tools/docker/Dockerfile | 12 +++++++----- > > > > > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > > > > > > > > > diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile > > > > > > index 540718df062..84661b036c6 100644 > > > > > > --- a/tools/docker/Dockerfile > > > > > > +++ b/tools/docker/Dockerfile > > > > > > @@ -44,7 +44,12 @@ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_ > > > > > > RUN wget -O - https://github.com/foss-xtensa/toolchain/releases/download/2020.07/x86_64-2020.07-xtensa-dc233c-elf.tar.gz | tar -C /opt -xz > > > > > > > > > > Oh no, here's a big problem ^^^^^ > > > > > You didn't switch to downloading arm64 toolchains, no wonder everything > > > > > else blows up. > > > > > > > > Er, yes I know that...I would have to put 'if' statements on each one, > > > > but I was hoping we could come up with a better way of handling this. > > > > > > Yes, hopefully we can do something with $TARGETPLATFORM to set another > > > variable that's exposed for wider use. > > > > I recall that docker doesn't let variables set in one phase affect > > subsequent ones, but perhaps something can be done in the ENV section > > Yes, my hope is we could set something in the ENV section. > > > > > Same with all the variables being set for grub. > > > > > > > > > > > > > > > # Update and install things from apt now > > > > > > -RUN apt-get update && apt-get install -y \ > > > > > > +RUN if [ "$TARGETPLATFORM" = "linux/amd64" ]; then \ > > > > > > + EXTRA_PACKAGES="grub-efi-amd64-bin grub-efi-ia32-bin libc6-i386 linux-image-kvm"; \ > > > > > > + else \ > > > > > > + EXTRA_PACKAGES="grub-efi linux-image-generic"; \ > > > > > > + fi; \ > > > > > > > > > > Again, grub-efi should be used for everyone and linux/amd64 adds > > > > > grub-efi-ia32-bin. And linux-image-generic should work for everyone, > > > > > too. > > > > > > > > So what about this commit?[1] > > > > > > > > Regards, > > > > Simon > > > > > > > > [1] f9abaa53ec8 tools: docker: Install a readable kernel for libguestfs-tools > > > > > > Yes, linux-image-generic should also provide a useful image, and you > > > would need to check what CI says when using this image. > > > > OK, so we don't actually need the kvm one, it seems? > > No, that was just a reasonable at the time choice for a kernel image > file. OK > > > Anyway, the upshot of all of this is that you want arm64 to run the > > full suite of tests, including kvm and world builds. But presumably > > not in addition to the x86_64 ones...? I suppose I am just unclear as > > to what you are looking for. Are you trying to test that QEMU works > > properly, or that U-Boot's code works? > > > > My expectation was that we just want sandbox testing on arm64...I > > don't see much point in anything else. What am I missing? > > I would like, especially if we get additional compute resources donated, > to duplicate much of the testing we do _also_ on arm64 as a host, at > least on tagged releases. This is a thing more and more people do, and > it would be good to be able to point at CI when something like: > https://source.denx.de/u-boot/u-boot/-/issues/44 > is filed. Well, that issue is a mystery to me too. Anyway, this is mostly a difference in approach. I like to do things incrementally, rather than trying to do everything at once. Regards, Simon
diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile index 540718df062..84661b036c6 100644 --- a/tools/docker/Dockerfile +++ b/tools/docker/Dockerfile @@ -44,7 +44,12 @@ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_ RUN wget -O - https://github.com/foss-xtensa/toolchain/releases/download/2020.07/x86_64-2020.07-xtensa-dc233c-elf.tar.gz | tar -C /opt -xz # Update and install things from apt now -RUN apt-get update && apt-get install -y \ +RUN if [ "$TARGETPLATFORM" = "linux/amd64" ]; then \ + EXTRA_PACKAGES="grub-efi-amd64-bin grub-efi-ia32-bin libc6-i386 linux-image-kvm"; \ + else \ + EXTRA_PACKAGES="grub-efi linux-image-generic"; \ + fi; \ + apt-get update && apt-get install -y \ automake \ autopoint \ bc \ @@ -70,13 +75,10 @@ RUN apt-get update && apt-get install -y \ gnu-efi \ gnutls-dev \ graphviz \ - grub-efi-amd64-bin \ - grub-efi-ia32-bin \ help2man \ iasl \ imagemagick \ iputils-ping \ - libc6-i386 \ libconfuse-dev \ libgit2-dev \ libjson-glib-dev \ @@ -94,7 +96,6 @@ RUN apt-get update && apt-get install -y \ libtool \ libudev-dev \ libusb-1.0-0-dev \ - linux-image-kvm \ lzma-alone \ lzop \ mount \ @@ -131,6 +132,7 @@ RUN apt-get update && apt-get install -y \ xilinx-bootgen \ xxd \ zip \ + ${EXTRA_PACKAGES} \ && rm -rf /var/lib/apt/lists/* # Make kernels readable for libguestfs tools to work correctly
The package names are slightly different for arm64 and we don't seem to have a linux-image-kvm package. Provide a different set for arm64 Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v2: - Swap order so that amd64 is the exception tools/docker/Dockerfile | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)