Re: [PATCH v2] kbuild: add script and target to generate pacman package

[Thomas Weißschuh]

Hi Nathan,

On 2024-07-07 22:50:46+0000, Nathan Chancellor wrote:
> On Sat, Jul 06, 2024 at 09:33:46AM +0200, Thomas Weißschuh wrote:
> > pacman is the package manager used by Arch Linux and its derivates.
> > Creating native packages from the kernel tree has multiple advantages:
> > 
> > * The package triggers the correct hooks for initramfs generation and
> >   bootloader configuration
> > * Uninstallation is complete and also invokes the relevant hooks
> > * New UAPI headers can be installed without any manual bookkeeping
> > 
> > The PKGBUILD file is a simplified version of the one used for the
> > downstream Arch Linux "linux" package.
> > Extra steps that should not be necessary for a development kernel have
> > been removed and an UAPI header package has been added.
> > 
> > Signed-off-by: Thomas Weißschuh <linux@xxxxxxxxxxxxxx>
> 
> Thanks a lot for addressing my comments. From a PKGBUILD perspective,
> this looks good to me (I have a couple more comments below). I am not as
> familiar with the Kbuild packaging infrastructure, so Masahiro might
> have more comments on that, but it works for me in my basic testing so
> consider it:
> 
> Reviewed-by: Nathan Chancellor <nathan@xxxxxxxxxx>
> Tested-by: Nathan Chancellor <nathan@xxxxxxxxxx>

Thanks!

> 
> > ---
> > Changes in v2:
> > - Replace ${MAKE} with $MAKE for consistency with other variables
> > - Use $MAKE for "-s image_name"
> > - Avoid permission warnings from build directory
> > - Clarify reason for /build symlink removal
> > - Install System.map and config
> > - Install dtbs where available
> > - Allow cross-build through arch=any
> > - Sort Contributor/Maintainer chronologically
> > - Disable some unneeded makepkg options
> > - Use DEPMOD=true for consistency with rpm-package
> > - Link to v1: https://lore.kernel.org/r/20240704-kbuild-pacman-pkg-v1-1-ac2f63f5fa7b@xxxxxxxxxxxxxx
> > ---
> >  .gitignore               |  6 ++++
> >  scripts/Makefile.package | 15 +++++++++
> >  scripts/package/PKGBUILD | 83 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 104 insertions(+)

<snip>

> > diff --git a/scripts/package/PKGBUILD b/scripts/package/PKGBUILD
> > new file mode 100644
> > index 000000000000..fe899c77a976
> > --- /dev/null
> > +++ b/scripts/package/PKGBUILD
> > @@ -0,0 +1,83 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +# Maintainer: Thomas Weißschuh <linux@xxxxxxxxxxxxxx>
> > +# Contributor: Jan Alexander Steffens (heftig) <heftig@xxxxxxxxxxxxx>
> > +
> > +pkgbase=linux-upstream
> > +pkgname=("$pkgbase" "$pkgbase-headers" "$pkgbase-api-headers")
> > +pkgver="${KERNELRELEASE//-/_}"
> > +pkgrel="$KBUILD_REVISION"
> > +pkgdesc='Linux'
> > +url='https://www.kernel.org/'
> > +arch=(any)
> 
> I see why you went this way but this feels a little dangerous because
> this means the package will be installable on architectures other than
> the one that it is built for. I think a better solution for this problem
> would be moving arch back to $UTS_MACHINE but setting CARCH to that same
> value in scripts/Makefile.package above. This diff works for me,
> allowing me to build an aarch64 package on x86_64:

This is what I used in v1 of the patch.
But I felt that this only works by pure chance.
IMO users of this feature should know what they are doing.

That said, if anybody has strong opinions on this, I'll be happy to change it.

> diff --git a/scripts/Makefile.package b/scripts/Makefile.package
> index 8c0c80f8bec0..a5b5b899d90c 100644
> --- a/scripts/Makefile.package
> +++ b/scripts/Makefile.package
> @@ -151,6 +151,7 @@ pacman-pkg:
>  		srctree="$(realpath $(srctree))" \
>  		objtree="$(realpath $(objtree))" \
>  		BUILDDIR="$(realpath $(objtree))/pacman" \
> +		CARCH="$(UTS_MACHINE)" \
>  		KBUILD_MAKEFLAGS="$(MAKEFLAGS)" \
>  		KBUILD_REVISION="$(shell $(srctree)/init/build-version)" \
>  		makepkg
> diff --git a/scripts/package/PKGBUILD b/scripts/package/PKGBUILD
> index fe899c77a976..7f1a4588c3d3 100644
> --- a/scripts/package/PKGBUILD
> +++ b/scripts/package/PKGBUILD
> @@ -8,7 +8,7 @@ pkgver="${KERNELRELEASE//-/_}"
>  pkgrel="$KBUILD_REVISION"
>  pkgdesc='Linux'
>  url='https://www.kernel.org/'
> -arch=(any)
> +arch=($UTS_MACHINE)
>  options=(!debug !strip !buildflags !makeflags)
>  license=(GPL-2.0-only)
>  
> 
> > +options=(!debug !strip !buildflags !makeflags)
> > +license=(GPL-2.0-only)
> > +

<snip>

> > +
> > +package_linux-upstream-headers() {
> > +  pkgdesc="Headers and scripts for building modules for the $pkgdesc kernel"
> > +
> > +  export MAKEFLAGS="${KBUILD_MAKEFLAGS}"
> > +  cd "$objtree"
> > +  local builddir="$pkgdir/usr/$MODLIB/build"
> > +
> > +  echo "Installing build files..."
> > +  "$srctree/scripts/package/install-extmod-build" "$builddir"
> > +
> > +  echo "Installing System.map and config..."
> > +  cp System.map "$builddir/System.map"
> > +  cp .config "$builddir/.config"
> 
> Remove the dot on the installation location so that it is more visible.

This is the location used by the downstream linux-headers package.
I can add a symlink for better visibility, though.

> > +  echo "Adding symlink..."
> > +  mkdir -p "$pkgdir/usr/src"
> > +  ln -sr "$builddir" "$pkgdir/usr/src/$pkgbase"
> > +}
> > +

<snip>