rebootstrap/ilp32 review

[Helmut Grohne <helmut@subdivi.de>]

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:

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.

> +	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.

> +	drop_privs patch -p1 < ${REBOOTSTRAPDIR}/debian-kernel-4.18.6-ilp32.patch

Upstreaming reference?

> +	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?

> +	comment="just building headers initially"

Duplicate assignment. Did you mean to change the previous assignment?

>  	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.

>  		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.

> +--- 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?

> +	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?

> +#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?

> +	echo "Add arm64ilp32 support to gmp"

Upstreaming reference?

Still having arm64ilp32 work on unstable is quite an achievement!

Hope this helps. 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.

Helmut