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

["Thomas Weißschuh" <linux@weissschuh.net>]

Hi,

thanks for the review!

On 2024-07-04 21:02:50+0000, Jan Alexander Steffens (heftig) wrote:
> On Thu, 2024-07-04 at 18:36 +0200, Thomas Weißschuh wrote:
> > diff --git a/scripts/package/PKGBUILD b/scripts/package/PKGBUILD
> > new file mode 100644
> > index 000000000000..29daf357edc1
> > --- /dev/null
> > +++ b/scripts/package/PKGBUILD
> > @@ -0,0 +1,72 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +# Contributor: Jan Alexander Steffens (heftig) <heftig@archlinux.org>
> > +# Maintainer: Thomas Weißschuh <linux@weissschuh.net>
> 
> Nitpick: Normally these lines are sorted newest to oldest, with the
> current maintainer(s) at the top.

Ack.

> > +
> > +pkgbase=linux-upstream
> > +pkgname=("$pkgbase" "$pkgbase-headers" "$pkgbase-api-headers")
> > +pkgver="${KERNELRELEASE//-/_}"
> > +pkgrel="$KBUILD_REVISION"
> > +pkgdesc='Linux'
> > +url='https://www.kernel.org/'
> > +arch=("$UTS_MACHINE")
> > +options=(!strip)
> 
> You should have !debug !strip here, otherwise makepkg can attempt (and
> will fail) to gather source files, creating an empty
> /usr/src/debug/$pkgbase.

Ack.

> Might also be worth considering !buildflags (to turn off injection of
> CFLAGS etc) and !makeflags (to turn off injection of MAKEFLAGS).

Ack.

!makeflags doesn't really help because the MAKEFLAGS inherited from
Kbuild are still overwritten, but with empty values.
Which is why the KBUILD_MAKEFLAGS variable is used.

But it's still better to have for safety.

> 
> > +license=(GPL-2.0-only)
> > +
> > +build() {
> > +  export MAKEFLAGS="${KBUILD_MAKEFLAGS}"
> 
> I think you can have this export at the top level instead of in each
> function.

That doesn't seem to work, MAKEFLAGS seem to be set before the function
is called.

> 
> > +  cd "$objtree"
> > +
> > +  ${MAKE} -f "${srctree}/Makefile"
> > +
> > +}
> > +
> > +package_linux-upstream() {
> > +  pkgdesc="The $pkgdesc kernel and modules"
> > +
> > +  export MAKEFLAGS="${KBUILD_MAKEFLAGS}"
> > +  cd "$objtree"
> > +  local modulesdir="$pkgdir/usr/$MODLIB"
> > +
> > +  echo "Installing boot image..."
> > +  # systemd expects to find the kernel here to allow hibernation
> > +  # https://github.com/systemd/systemd/commit/edda44605f06a41fb86b7ab8128dcf99161d2344
> > +  install -Dm644 "$(make -s image_name)" "$modulesdir/vmlinuz"
> 
> An invocation of make that could also use ${MAKE} for consistency.

Ack, Nathan also proposed this.

> > +
> > +  # Used by mkinitcpio to name the kernel
> > +  echo "$pkgbase" | install -Dm644 /dev/stdin "$modulesdir/pkgbase"
> > +
> > +  echo "Installing modules..."
> > +  ${MAKE} INSTALL_MOD_PATH="$pkgdir/usr" INSTALL_MOD_STRIP=1 \
> > +    DEPMOD=/doesnt/exist modules_install  # Suppress depmod
> > +
> > +  # remove build link
> > +  rm -f "$modulesdir/build"
> > +}
> > +
> > +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"
> 
> Should we be using this script upstream as well instead of our
> homegrown mess of install commands?

Maybe. It is missing a few things like resolve_btfids.
In the kernel repo if something is missing we can fix the script itself.
Downstream would still require custom logic.

> > +
> > +  echo "Adding symlink..."
> > +  mkdir -p "$pkgdir/usr/src"
> > +  ln -sr "$builddir" "$pkgdir/usr/src/$pkgbase"
> > +}
> > +
> > +package_linux-upstream-api-headers() {
> > +  pkgdesc="Kernel headers sanitized for use in userspace"
> > +  provides=(linux-api-headers)
> > +  conflicts=(linux-api-headers)
> > +
> > +  export MAKEFLAGS="${KBUILD_MAKEFLAGS}"
> > +  cd "$objtree"
> > +
> > +  ${MAKE} headers_install INSTALL_HDR_PATH="$pkgdir/usr"
> > +}
> > +
> > +# vim:set ts=8 sts=2 sw=2 et: