On 2018-09-22 14:13 +0200, Helmut Grohne wrote:
> Hi Wookey,
>
> I think you didn't explicitly ask for a review of your ilp32 work, but I
> think doing it anyway is useful for speeding up the work. Here we go:
Cheers - that is indeed useful.
> https://salsa.debian.org/wookey/rebootstrap.git
> b0bf57cde2019f1b1a..059c993dfb2895c6
>
> > @@ -980,36 +995,31 @@ EOF
> > echo "debian_patches += arm64-ilp32-default" | drop_privs tee -a debian/rules.patch >/dev/null
> > fi
> > drop_privs patch -p1 <<'EOF'
> > ---- a/debian/patches/gcc-multiarch.diff
> > -+++ b/debian/patches/gcc-multiarch.diff
>
> This one needs an new upstreaming reference. The old bug was closed a
> while ago.
Yes. As soon as I have run something and thus believe that what I am
building actually works, I'll file remaining toolchain patches
upstream. Although debian-specific multiarch path changes presumably
remain as debian patches for now.
> > + echo "patching kernel to add ilp32 support"
> > + drop_privs patch -p1 < ${REBOOTSTRAPDIR}/kernel-add-debian-config-for-arm64ilp32.patch
>
> Do you really need a separate kernel configuration? Why can't it run a
> standard arm64 kernel? That'd allow you to just build the headers.
That patch is by way of completeness. I think the kernel is actually
configured for arm64, due to the "arm64ilp32) kernel_arch=arm64;" line
mentioned below. It may be that that config doesn't actually make any
sense, but if it is possible to build such a thing, then the configure
foo should be present.
> > + drop_privs patch -p1 < ${REBOOTSTRAPDIR}/debian-kernel-4.18.6-ilp32.patch
>
> Upstreaming reference?
Well, there is a git tree, but not a bug as it's stuck in staging for
now (and may be forever). Is that sufficient?
> > + drop_privs dch --nmu -m "Add arm64ilp32 support"
> > + # install kernel-wedge unconditionally because we've changed control headers
> > + # whatever arch gets built, even Stage1 linux-libc-dev will need this
> > + apt_get_install kernel-wedge
>
> This already happens a few lines later. Why do you duplicate it?
Good question. My notes do not record why I had to do this, but
something about the conditional in the existing code "if test -n
"$kernel_arch"" meant that it didn't always happen when I wanted it. I
can't see why that should happen with the current code. It was
probably something to do with my 'restarting' use of rebootstrap.
It may not longer be an issue so I'll try dropping it.
> > + comment="just building headers initially"
>
> Duplicate assignment. Did you mean to change the previous assignment?
It was presumably a correction of that assignment, yes.
"just building headers yet" is not correct English.
I presume it should say
"just building headers now"
or
"just building headers initially"
unless that comment is used as part of some larger sentence?
Obviously the assignment should be corrected in one place, not redone on another line.
> > case "$HOST_ARCH" in
> > arm|ia64|nios2)
> > kernel_arch=$HOST_ARCH
> > @@ -1240,6 +1258,7 @@ patch_linux() {
> > mipsr6|mipsr6el|mipsn32r6|mipsn32r6el|mips64r6|mips64r6el)
> > kernel_arch=defines-only
> > ;;
> > + arm64ilp32) kernel_arch=arm64; ;;
>
> Duplicate case.
So it is.
> > powerpcel) kernel_arch=powerpc; ;;
> > riscv64) kernel_arch=riscv; ;;
> > *-linux-*)
> > @@ -1411,6 +1430,100 @@ fi
> >
> > # libc
> > patch_glibc() {
> > + echo "patching glibc to add ilp32 support (#874587)"
> > + drop_privs patch -p1 < ${REBOOTSTRAPDIR}/glibc-ilp32-debian-clean.patch
> > + #Debian packaging patch
> > + drop_privs patch -p1 <<'EOF'
> > +--- glibc-2.27/debian/control.orig 2018-09-02 03:48:06.011622729 +0100
> > ++++ glibc-2.27/debian/control 2018-09-03 03:12:25.116778131 +0100
>
> This patch is different from the one in #874587.
Yes - it got updated for current glibc (although is functionally identical). I'll update the bug.
> > +--- glibc-2.27/debian/rules.d/control.mk.orig 2018-09-02 03:48:06.011622729 +0100
> > ++++ glibc-2.27/debian/rules.d/control.mk 2018-09-03 03:13:47.506965172 +0100
> > +@@ -1,7 +1,7 @@
> > + libc_packages := libc6 libc6.1 libc0.1 libc0.3
> > + libc0_1_archs := kfreebsd-amd64 kfreebsd-i386
> > + libc0_3_archs := hurd-i386
> > +-libc6_archs := amd64 arm64 armel armhf hppa i386 m68k mips mipsel mipsn32 mipsn32el mips64 mips64el mipsr6 mipsr6el \
> > ++libc6_archs := amd64 arm64 arm64ilp32 armel armhf hppa i386 m68k mips mipsel mipsn32 mipsn32el mips64 mips64el mipsr6 mipsr6el \
>
> In theory, is already handled by patch_glibc around lines 1447 "adding
> $HOST_ARCH to libc6_archs". Why does that not work for you?
Maybe it does. I didn't try it without my glibc packaging patch (whch
I'm pretty sure _was_ needed a year ago), but I'll try without it now.
> > + echo "patching glibc to select the correct packages in stage1"
>
> This is #881454 and was fixed in glibc/2.25-0experimental4. The patch is
> no longer necessary. Do you see any problems after dropping it?
No. I've already tested this, and you are right that that patch is no longer needed.
> > +#manually install the stuff that cross-build essential should ensure
> > +DEB_HOST_GNU_TYPE_NORM=$(dpkg-architecture -aarm64ilp32 -q DEB_HOST_GNU_TYPE | sed s/_/-/)
> > +apt_get_install binutils-$DEB_HOST_GNU_TYPE_NORM gcc-$GCC_VER-$DEB_HOST_GNU_TYPE_NORM g++-$GCC_VER-$DEB_HOST_GNU_TYPE_NORM dpkg-dev
> > +
>
> These should be installed automatically already. Do you see any problems
> after removing the hunk?
Yes. I still needed to add this, otherwise the cross-compilers are
missing on the first package build after, so configure fails.
I've not looked in detail as to why. I recall originally that this was
an issue of re-running bootstrap.sh. Perhaps it still is, and would
work on the initial run without this?
> > + echo "Add arm64ilp32 support to gmp"
>
> Upstreaming reference?
>
> Still having arm64ilp32 work on unstable is quite an achievement!
>
> Hope this helps.
Yes, very helpful.
> I think you can significantly reduce the diff with no
> loss of functionality. That in turn should make merging with my tree a
> lot easier.
Yes. Looks like there are quite a few things to tidy up. I'll do that
and test (after I get back to UK on Monday)
Wookey
--
Principal hats: Linaro, Debian, Wookware, ARM
http://wookware.org/
Attachment:
signature.asc
Description: PGP signature