Re: [PATCH] tests/docker: Kludge <linux/swab.h> breakage by pinning linux-libc-dev

[Philippe Mathieu-Daudé]

On 5/13/20 5:15 PM, Alex Bennée wrote:
> Philippe Mathieu-Daudé <address@hidden> writes:
>
> > Linux kernel commit d5767057c9a [1] aimed to fix an issue with the
> > swab() declaration, but doing so it introduced the BITS_PER_LONG
> > definition, without using the kernel __ prefix, leading to odd
> > failures in userland code using kernel uapi headers, such:
> >
> >        CC      block/file-posix.o
> >      In file included from include/qemu/timer.h:4,
> >                       from include/qemu/timed-average.h:29,
> >                       from include/block/accounting.h:28,
> >                       from include/block/block_int.h:27,
> >                       from block/file-posix.c:30:
> >      /usr/include/linux/swab.h: In function `__swab':
> >      include/qemu/bitops.h:20:34: error: "sizeof" is not defined, evaluates to 
> > 0 [-Werror=undef]
> >         20 | #define BITS_PER_LONG           (sizeof (unsigned long) * 
> > BITS_PER_BYTE)
> >            |                                  ^~~~~~
> >      include/qemu/bitops.h:20:41: error: missing binary operator before token 
> > "("
> >         20 | #define BITS_PER_LONG           (sizeof (unsigned long) * 
> > BITS_PER_BYTE)
> >            |                                         ^
> >      cc1: all warnings being treated as errors
> >      make: *** [rules.mak:69: block/file-posix.o] Error 1
> >
> > The issue has been fixed in Linux kernel commit 467d12f5c78 [2].
> > Some distributions already backported the first commit, but still
> > lack the second one.
> >
> > This is the case for Debian Buster.
> > The linux-libc-dev package contains the Linux kernel headers.
> > Kernel commit d5767057c9a has been backported in package
> > commit 1fb0eb7956 "Update to 4.19.116" [3], see debian/changelog:
> >
> >    +linux (4.19.116-1) UNRELEASED; urgency=medium
> >    ...
> >    +    - uapi: rename ext2_swab() to swab() and share globally in swab.h
> >
> > The previous released update before it is debian/4.19.98-1,
> > released as debian/4.19.98-1+deb10u1.
> >
> > We can find this package in the Debian snapshot archives,
> > luckily archived on 2020-04-28 21:20:54 (see [4]).
> >
> > QEMU use Debian based Docker images for most of its cross-builds,
> > which are tested by our Shippable CI jobs. The current broken
> > package makes most of our CI red. We can kludge this by using the
> > latest package released before the breakage. Do so by pinning
> > the package version (apt hold), and using the snapshot archives,
> > similar to commit b4048a7cd1.
> > We'll revert once the fix is backported on Debian.
>
> May as well mention:
>
>    https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=960271

Sure (I remembered you talked about it on IRC, but was lazy to dig).

> > Reference to commits:
> > [1] 
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d5767057c9a
> > [2] 
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=467d12f5c78
> > [3] https://salsa.debian.org/kernel-team/linux/-/commit/1fb0eb7956
> > [4] 
> > http://snapshot.debian.org/package/linux/4.19.98-1%2Bdeb10u1/#linux-libc-dev_4.19.98-1:2b:deb10u1
> >
> > Cc: Salvatore Bonaccorso <address@hidden>
> > Cc: Christian Borntraeger <address@hidden>
> > Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> > ---
> > Shippable green again:
> > https://app.shippable.com/github/philmd/qemu/runs/759/summary/console
> > ---
> >   tests/docker/dockerfiles/debian10.docker | 16 ++++++++++++++++
> >   1 file changed, 16 insertions(+)
> >
> > diff --git a/tests/docker/dockerfiles/debian10.docker 
> > b/tests/docker/dockerfiles/debian10.docker
> > index 0769700a41..d7084ac283 100644
> > --- a/tests/docker/dockerfiles/debian10.docker
> > +++ b/tests/docker/dockerfiles/debian10.docker
> > @@ -9,6 +9,15 @@
> >   #
> >   FROM debian:buster-slim
> >   
> > + # Use a snapshot known to work (see http://snapshot.debian.org/#Usage)
> > +ENV DEBIAN_SNAPSHOT_DATE "20200428T212054Z"
> > +RUN sed -i \
> > +    "s%^deb \(https\?://\)deb.debian.org/debian/\? \(.*\)%deb 
> > [check-valid-until=no] \1snapshot.debian.org/archive/debian/${DEBIAN_SNAPSHOT_DATE} 
> > \2%" \
> > +    /etc/apt/sources.list
> > +RUN sed -i \
> > +    "s%^deb \(https\?://\)security.debian.org/debian-security/\? \(.*\)%deb 
> > [check-valid-until=no] 
> > \1snapshot.debian.org/archive/debian-security/${DEBIAN_SNAPSHOT_DATE} \2%" \
> > +    /etc/apt/sources.list
> > +
> >   # Duplicate deb line as deb-src
> >   RUN cat /etc/apt/sources.list | sed "s/^deb\ /deb-src /" >> 
> > /etc/apt/sources.list
> >   
> > @@ -35,4 +44,11 @@ RUN apt update && \
> >           texinfo \
> >           $(apt-get -s build-dep qemu | egrep ^Inst | fgrep '[all]' | cut -d\  
> > -f2)
> >   
> > +# Install common build utilities
> > +RUN apt update && \
> > +    DEBIAN_FRONTEND=noninteractive eatmydata \
> > +    apt install -y --allow-downgrades \
> > +        linux-libc-dev=4.19.98-1+deb10u1 && \
> > +    apt-mark hold linux-libc-dev
> > +
>
> Do we need this bit? Surely if we have a snapshot URL we don't need to
> hold the package as it will never get updated?

You are right, it is a remnant from the first approach (pin it without 
using snapshot) which didn't work. I'll remove this line and the comment 
in description.

> >   ENV FEATURES docs