Message-ID: <221289d6-790f-4b26-aa88-3c502566cf8a@t-8ch.de>
Date: Mon, 15 Jul 2024 09:38:38 +0200
From: Thomas Weißschuh <linux@...ssschuh.net>
To: Masahiro Yamada <masahiroy@...nel.org>
Cc: Nathan Chancellor <nathan@...nel.org>, 
	Nicolas Schier <nicolas@...sle.eu>, "Jan Alexander Steffens (heftig)" <heftig@...hlinux.org>, 
	linux-kernel@...r.kernel.org, linux-kbuild@...r.kernel.org
Subject: Re: [PATCH v4] kbuild: add script and target to generate pacman
 package

On 2024-07-14 17:57:39+0000, Masahiro Yamada wrote:
> On Thu, Jul 11, 2024 at 4:32 AM Thomas Weißschuh <linux@...ssschuh.net> 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.
> >
> > Reviewed-by: Nathan Chancellor <nathan@...nel.org>
> > Tested-by: Nathan Chancellor <nathan@...nel.org>
> > Signed-off-by: Thomas Weißschuh <linux@...ssschuh.net>
> > ---
> > Changes in v4:
> > - Update MRPROPER_FILES
> > - Unify shell variable syntax
> > - Link to v3: https://lore.kernel.org/r/20240708-kbuild-pacman-pkg-v3-1-885df3cbc740@weissschuh.net
> >
> > Changes in v3:
> > - Enforce matching architectures for installation
> > - Add Reviewed-by and Tested-by from Nathan
> > - Link to v2: https://lore.kernel.org/r/20240706-kbuild-pacman-pkg-v2-1-613422a03a7a@weissschuh.net
> >
> > Changes in v2:
> > - Replace ${MAKE} with $MAKE for consistency with other variables
> 
> 
> Personally, I prefer the brace form ${MAKE}, but
> indeed, there is no treewide consistency.

Ack, I'll change it for all 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@weissschuh.net
> > ---
> >  .gitignore               |  6 ++++
> >  Makefile                 |  2 +-
> >  scripts/Makefile.package | 16 ++++++++++
> >  scripts/package/PKGBUILD | 83 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 106 insertions(+), 1 deletion(-)
> >
> > diff --git a/.gitignore b/.gitignore
> > index c59dc60ba62e..7902adf4f7f1 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -92,6 +92,12 @@ modules.order
> >  #
> >  /tar-install/
> >
> > +#
> > +# pacman files (make pacman-pkg)
> > +#
> > +/PKGBUILD
> > +/pacman/
> > +
> >  #
> >  # We don't want to ignore the following even if they are dot-files
> >  #
> > diff --git a/Makefile b/Makefile
> > index b25b5b44af10..79e8dcec6be9 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1497,7 +1497,7 @@ CLEAN_FILES += vmlinux.symvers modules-only.symvers \
> >  # Directories & files removed with 'make mrproper'
> >  MRPROPER_FILES += include/config include/generated          \
> >                   arch/$(SRCARCH)/include/generated .objdiff \
> > -                 debian snap tar-install \
> > +                 debian snap tar-install PKGBUILD pacman \
> >                   .config .config.old .version \
> >                   Module.symvers \
> >                   certs/signing_key.pem \
> > diff --git a/scripts/Makefile.package b/scripts/Makefile.package
> > index bf016af8bf8a..a5b5b899d90c 100644
> > --- a/scripts/Makefile.package
> > +++ b/scripts/Makefile.package
> > @@ -141,6 +141,21 @@ snap-pkg:
> >         cd $(objtree)/snap && \
> >         snapcraft --target-arch=$(UTS_MACHINE)
> >
> > +# pacman-pkg
> > +# ---------------------------------------------------------------------------
> > +
> > +PHONY += pacman-pkg
> > +pacman-pkg:
> > +       @ln -srf $(srctree)/scripts/package/PKGBUILD $(objtree)/PKGBUILD
> 
> 
> 
> I really hoped to avoid the symlink and
> 
>    makepkg -p $(srctree)/scripts/package/PKGBUILD
> 
> would work instead.
> 
> 
> However, I got this error.
> 
>   makepkg -p /home/masahiro/ref/linux/scripts/package/PKGBUILD
>   ==> ERROR: /home/masahiro/ref/linux/scripts/package/PKGBUILD must be
> in the current working directory.
> 
> So, the -p option does not seem to be useful here.
> 
> I hope somebody in the Arch Linux community would consider
> supporting PKGBUILD located in a different directory.

This seems intentional as it's mentioned in the error message and the
manpage.
Other buildsystems seem to have the same restriction.

> > +       cd $(objtree) && \
> 
> 
> objtree is always '.'
> 
> 
> You are doing "cd . && makepkg"
> 
> It is meaningless.

Ack.

> 
> 
> 
> > +               srctree="$(realpath $(srctree))" \
> 
> Is this necessary?

It was at some point, not anymore. Thanks.

> 
> > +               objtree="$(realpath $(objtree))" \
> > +               BUILDDIR="$(realpath $(objtree))/pacman" \
> 
> 
>      BUILDDIR=pacman

Ack.

> > +               CARCH="$(UTS_MACHINE)" \
> > +               KBUILD_MAKEFLAGS="$(MAKEFLAGS)" \
> 
> 
> Please explain why this is necessary.
> 
> 
> With '!makeflags' dropped from options,
> it worked for me without MAKEFLAGS/KBUILD_MAKEFLAGS
> conversion.

For me I have to drop both !makeflags and !buildflags for the MAKEFLAGS
from kbuild to be passed through.
Furthermore, as soon as MAKEFLAGS is set in /etc/makepkg.conf then that
will override MAKEFLAGS from Kbuild.

> > +               KBUILD_REVISION="$(shell $(srctree)/init/build-version)" \
> 
> 
> You are inventing more and more new env variables.
> 
> How about this?
> 
>    pkgrel="$(${srctree}/init/build-version)"

The PKBUILD is evaluated multiple times. So if init/build-version is
executed from there, the versions will have gaps and be inconsistent.

I can add some explanation. But the Makefile doesn't seem to be the
right place. Is the PKGBUILD fine for it?

> > +               makepkg
> > +
> >  # dir-pkg tar*-pkg - tarball targets
> >  # ---------------------------------------------------------------------------
> >
> > @@ -221,6 +236,7 @@ help:
> >         @echo '  bindeb-pkg          - Build only the binary kernel deb package'
> >         @echo '  snap-pkg            - Build only the binary kernel snap package'
> >         @echo '                        (will connect to external hosts)'
> > +       @echo '  pacman-pkg          - Build only the binary kernel pacman package'
> >         @echo '  dir-pkg             - Build the kernel as a plain directory structure'
> >         @echo '  tar-pkg             - Build the kernel as an uncompressed tarball'
> >         @echo '  targz-pkg           - Build the kernel as a gzip compressed tarball'
> > diff --git a/scripts/package/PKGBUILD b/scripts/package/PKGBUILD
> > new file mode 100644
> > index 000000000000..b0b133ac28eb
> > --- /dev/null
> > +++ b/scripts/package/PKGBUILD
> > @@ -0,0 +1,83 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +# Maintainer: Thomas Weißschuh <linux@...ssschuh.net>
> > +# Contributor: Jan Alexander Steffens (heftig) <heftig@...hlinux.org>
> > +
> > +pkgbase=linux-upstream
> > +pkgname=("$pkgbase" "$pkgbase-headers" "$pkgbase-api-headers")
> > +pkgver="${KERNELRELEASE//-/_}"
> > +pkgrel="$KBUILD_REVISION"
> > +pkgdesc='Linux'
> > +url='https://www.kernel.org/'
> > +arch=($CARCH)
> > +options=(!debug !strip !buildflags !makeflags)
> > +license=(GPL-2.0-only)
> 
> 
> makedepends=(
>      ...
> )
> 
> is missing. Is it intentional?

Yes. This also depends on the chosen configuration.
If you prefer I can add it back.

> > +build() {
> > +  export MAKEFLAGS="$KBUILD_MAKEFLAGS"
> > +  cd "$objtree"
> > +
> > +  # makepkg does a "chmod a-srw", triggering warnings during kbuild
> > +  chmod 0755 "$pkgdirbase" || true
> > +
> > +  $MAKE -f "$srctree/Makefile"
> 
> The -f option is unneeded.
> 
> Just simply:
> 
>       ${MAKE}

Ack.

> > +}
> > +
> > +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"
> > +
> > +  # 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=true modules_install
> > +
> > +  if $MAKE run-command KBUILD_RUN_COMMAND='test -d $srctree/arch/$SRCARCH/boot/dts' 2>/dev/null; then
> > +    echo "Installing dtbs..."
> > +    $MAKE INSTALL_DTBS_PATH="$modulesdir/dtb" dtbs_install
> > +  fi
> > +
> > +  # remove build link, will be part of -headers package
> > +  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"
> > +
> > +  echo "Installing System.map and config..."
> > +  cp System.map "$builddir/System.map"
> > +  cp .config "$builddir/.config"
> > +
> > +  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:
> 
> 
> 
> Documentation/process/coding-style.rst
> discourages this.
> 
> 
>  """
>         /* vim:set sw=8 noet */
> 
>  Do not include any of these in source files.
>  """
> 
> If you need to specify this in .editorconfig
> 
> I do not know the style guide of PKGCONFIG.
> Does it conventionally use 2-space indentation?

Ack.

I don't think it's important.
PKGBUILD(5) even uses 8 spaces.
I'll align it to kernel conventions and drop the config line.

Powered by blists - more mailing lists