[PATCH] kbuild: control extra pacman packages with PACMAN_EXTRAPACKAGES

[PATCH] kbuild: control extra pacman packages with PACMAN_EXTRAPACKAGES

[Jose Fernandez]

Introduce a new variable, PACMAN_EXTRAPACKAGES, in the Makefile.package
to control the creation of additional packages by the pacman-pkg target.

This changes the behavior of the pacman-pkg target to only create the
main kernel package by default. The rest of the packages will be opt-in
going forward.

In a previous patch, there was concern that adding a new debug package
would increase the package time. To address this concern and provide
more flexibility, this change has been added to allow users to decide
which extra packages to include before introducing an optional debug
package [1].

[1] https://lore.kernel.org/lkml/20240801192008.GA3923315@thelio-3990X/T/

Signed-off-by: Jose Fernandez <jose.fernandez@linux.dev>
Reviewed-by: Peter Jung <ptr1337@cachyos.org>
---
 scripts/Makefile.package |  5 +++++
 scripts/package/PKGBUILD | 11 ++++++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/scripts/Makefile.package b/scripts/Makefile.package
index 4a80584ec771..146e828cb4f1 100644
--- a/scripts/Makefile.package
+++ b/scripts/Makefile.package
@@ -144,6 +144,10 @@ snap-pkg:
 # pacman-pkg
 # ---------------------------------------------------------------------------
 
+# Space-separated list of extra packages to build
+# The available extra packages are: headers api-headers
+PACMAN_EXTRAPACKAGES ?=
+
 PHONY += pacman-pkg
 pacman-pkg:
 	@ln -srf $(srctree)/scripts/package/PKGBUILD $(objtree)/PKGBUILD
@@ -152,6 +156,7 @@ pacman-pkg:
 		CARCH="$(UTS_MACHINE)" \
 		KBUILD_MAKEFLAGS="$(MAKEFLAGS)" \
 		KBUILD_REVISION="$(shell $(srctree)/scripts/build-version)" \
+		PACMAN_EXTRAPACKAGES="$(PACMAN_EXTRAPACKAGES)" \
 		makepkg $(MAKEPKGOPTS)
 
 # dir-pkg tar*-pkg - tarball targets
diff --git a/scripts/package/PKGBUILD b/scripts/package/PKGBUILD
index 663ce300dd06..41bd0d387f0a 100644
--- a/scripts/package/PKGBUILD
+++ b/scripts/package/PKGBUILD
@@ -3,10 +3,15 @@
 # Contributor: Jan Alexander Steffens (heftig) <heftig@archlinux.org>
 
 pkgbase=${PACMAN_PKGBASE:-linux-upstream}
-pkgname=("${pkgbase}" "${pkgbase}-api-headers")
-if grep -q CONFIG_MODULES=y include/config/auto.conf; then
-	pkgname+=("${pkgbase}-headers")
+pkgname=("${pkgbase}")
+
+_extrapackages=${PACMAN_EXTRAPACKAGES:-}
+if [ -n "$_extrapackages" ]; then
+	for pkg in $_extrapackages; do
+		pkgname+=("${pkgbase}-$pkg")
+	done
 fi
+
 pkgver="${KERNELRELEASE//-/_}"
 # The PKGBUILD is evaluated multiple times.
 # Running scripts/build-version from here would introduce inconsistencies.
-- 
2.46.0

Re: [PATCH] kbuild: control extra pacman packages with PACMAN_EXTRAPACKAGES

[Thomas Weißschuh]

Hi Jose,

On 2024-08-03 18:01:25+0000, Jose Fernandez wrote:
> Introduce a new variable, PACMAN_EXTRAPACKAGES, in the Makefile.package
> to control the creation of additional packages by the pacman-pkg target.
> 
> This changes the behavior of the pacman-pkg target to only create the
> main kernel package by default. The rest of the packages will be opt-in
> going forward.

I had the impression that by default all extrapackages should be
built. The variable can then be used by expert users where needed.
Other Opinions?

> In a previous patch, there was concern that adding a new debug package
> would increase the package time. To address this concern and provide
> more flexibility, this change has been added to allow users to decide
> which extra packages to include before introducing an optional debug
> package [1].

This paragraph seems like it shouldn't be part of the final commit.
If you put it after a line with "---" it will be dropped from the
commit, like so:

---

In a previous patch, ...

> 
> [1] https://lore.kernel.org/lkml/20240801192008.GA3923315@thelio-3990X/T/
> 
> Signed-off-by: Jose Fernandez <jose.fernandez@linux.dev>
> Reviewed-by: Peter Jung <ptr1337@cachyos.org>
> ---
>  scripts/Makefile.package |  5 +++++
>  scripts/package/PKGBUILD | 11 ++++++++---
>  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/Makefile.package b/scripts/Makefile.package
> index 4a80584ec771..146e828cb4f1 100644
> --- a/scripts/Makefile.package
> +++ b/scripts/Makefile.package
> @@ -144,6 +144,10 @@ snap-pkg:
>  # pacman-pkg
>  # ---------------------------------------------------------------------------
>  
> +# Space-separated list of extra packages to build
> +# The available extra packages are: headers api-headers
> +PACMAN_EXTRAPACKAGES ?=

The assignment doesn't do anything.
Do we need the documentation if the default enables all subpackages?

> +
>  PHONY += pacman-pkg
>  pacman-pkg:
>  	@ln -srf $(srctree)/scripts/package/PKGBUILD $(objtree)/PKGBUILD
> @@ -152,6 +156,7 @@ pacman-pkg:
>  		CARCH="$(UTS_MACHINE)" \
>  		KBUILD_MAKEFLAGS="$(MAKEFLAGS)" \
>  		KBUILD_REVISION="$(shell $(srctree)/scripts/build-version)" \
> +		PACMAN_EXTRAPACKAGES="$(PACMAN_EXTRAPACKAGES)" \

This line is superfluous.

>  		makepkg $(MAKEPKGOPTS)
>  
>  # dir-pkg tar*-pkg - tarball targets
> diff --git a/scripts/package/PKGBUILD b/scripts/package/PKGBUILD
> index 663ce300dd06..41bd0d387f0a 100644
> --- a/scripts/package/PKGBUILD
> +++ b/scripts/package/PKGBUILD
> @@ -3,10 +3,15 @@
>  # Contributor: Jan Alexander Steffens (heftig) <heftig@archlinux.org>
>  
>  pkgbase=${PACMAN_PKGBASE:-linux-upstream}
> -pkgname=("${pkgbase}" "${pkgbase}-api-headers")
> -if grep -q CONFIG_MODULES=y include/config/auto.conf; then
> -	pkgname+=("${pkgbase}-headers")
> +pkgname=("${pkgbase}")
> +
> +_extrapackages=${PACMAN_EXTRAPACKAGES:-}
> +if [ -n "$_extrapackages" ]; then

No need for this check. The loop over an empty variable work fine.

> +	for pkg in $_extrapackages; do
> +		pkgname+=("${pkgbase}-$pkg")

Use consistent variable references: "${pkgbase}-${pkg}"

> +	done
>  fi
> +
>  pkgver="${KERNELRELEASE//-/_}"
>  # The PKGBUILD is evaluated multiple times.
>  # Running scripts/build-version from here would introduce inconsistencies.
> -- 
> 2.46.0
> 

Re: [PATCH] kbuild: control extra pacman packages with PACMAN_EXTRAPACKAGES

[Jose Fernandez]

On 24/08/05 04:01PM, Thomas Weißschuh wrote:
> > This changes the behavior of the pacman-pkg target to only create the
> > main kernel package by default. The rest of the packages will be opt-in
> > going forward.
> 
> I had the impression that by default all extrapackages should be
> built. The variable can then be used by expert users where needed.
> Other Opinions?

I think switching to defaulting to all packages is a good idea. One concern I 
had was how regular users would discover the customization options. Expert users
will likely look at the Makefile and figure out how to opt out.

I will plan to make this change for v2 unless there are any objections.
 
> > In a previous patch, there was concern that adding a new debug package
> > would increase the package time. To address this concern and provide
> > more flexibility, this change has been added to allow users to decide
> > which extra packages to include before introducing an optional debug
> > package [1].
> 
> This paragraph seems like it shouldn't be part of the final commit.
> If you put it after a line with "---" it will be dropped from the
> commit, like so:
> 
> ---
> 
> In a previous patch, ...

Agreed, I will move this paragraph to below --- for v2.

> 
> > 
> > [1] https://lore.kernel.org/lkml/20240801192008.GA3923315@thelio-3990X/T/
> > 
> > Signed-off-by: Jose Fernandez <jose.fernandez@linux.dev>
> > Reviewed-by: Peter Jung <ptr1337@cachyos.org>
> > ---
> >  scripts/Makefile.package |  5 +++++
> >  scripts/package/PKGBUILD | 11 ++++++++---
> >  2 files changed, 13 insertions(+), 3 deletions(-)
> > 
> > diff --git a/scripts/Makefile.package b/scripts/Makefile.package
> > index 4a80584ec771..146e828cb4f1 100644
> > --- a/scripts/Makefile.package
> > +++ b/scripts/Makefile.package
> > @@ -144,6 +144,10 @@ snap-pkg:
> >  # pacman-pkg
> >  # ---------------------------------------------------------------------------
> >  
> > +# Space-separated list of extra packages to build
> > +# The available extra packages are: headers api-headers
> > +PACMAN_EXTRAPACKAGES ?=
> 
> The assignment doesn't do anything.
> Do we need the documentation if the default enables all subpackages?
> 
> > +
> >  PHONY += pacman-pkg
> >  pacman-pkg:
> >  	@ln -srf $(srctree)/scripts/package/PKGBUILD $(objtree)/PKGBUILD
> > @@ -152,6 +156,7 @@ pacman-pkg:
> >  		CARCH="$(UTS_MACHINE)" \
> >  		KBUILD_MAKEFLAGS="$(MAKEFLAGS)" \
> >  		KBUILD_REVISION="$(shell $(srctree)/scripts/build-version)" \
> > +		PACMAN_EXTRAPACKAGES="$(PACMAN_EXTRAPACKAGES)" \
> 
> This line is superfluous.

Ack.

> >  		makepkg $(MAKEPKGOPTS)
> >  
> >  # dir-pkg tar*-pkg - tarball targets
> > diff --git a/scripts/package/PKGBUILD b/scripts/package/PKGBUILD
> > index 663ce300dd06..41bd0d387f0a 100644
> > --- a/scripts/package/PKGBUILD
> > +++ b/scripts/package/PKGBUILD
> > @@ -3,10 +3,15 @@
> >  # Contributor: Jan Alexander Steffens (heftig) <heftig@archlinux.org>
> >  
> >  pkgbase=${PACMAN_PKGBASE:-linux-upstream}
> > -pkgname=("${pkgbase}" "${pkgbase}-api-headers")
> > -if grep -q CONFIG_MODULES=y include/config/auto.conf; then
> > -	pkgname+=("${pkgbase}-headers")
> > +pkgname=("${pkgbase}")
> > +
> > +_extrapackages=${PACMAN_EXTRAPACKAGES:-}
> > +if [ -n "$_extrapackages" ]; then
> 
> No need for this check. The loop over an empty variable work fine.

Ack. Will update in v2.
 
> > +	for pkg in $_extrapackages; do
> > +		pkgname+=("${pkgbase}-$pkg")
> 
> Use consistent variable references: "${pkgbase}-${pkg}"

Ack. Will update in v2.

> > +	done
> >  fi
> > +
> >  pkgver="${KERNELRELEASE//-/_}"
> >  # The PKGBUILD is evaluated multiple times.
> >  # Running scripts/build-version from here would introduce inconsistencies.
> > -- 
> > 2.46.0
> > 

Re: [PATCH] kbuild: control extra pacman packages with PACMAN_EXTRAPACKAGES

[Nathan Chancellor]

On Mon, Aug 05, 2024 at 04:30:15PM -0600, Jose Fernandez wrote:
> On 24/08/05 04:01PM, Thomas Weißschuh wrote:
> > > This changes the behavior of the pacman-pkg target to only create the
> > > main kernel package by default. The rest of the packages will be opt-in
> > > going forward.
> > 
> > I had the impression that by default all extrapackages should be
> > built. The variable can then be used by expert users where needed.
> > Other Opinions?
> 
> I think switching to defaulting to all packages is a good idea. One concern I 
> had was how regular users would discover the customization options. Expert users
> will likely look at the Makefile and figure out how to opt out.

I think that most users will likely want all of the packages built by
default. I think leaving this to be discovered by power users in the
Makefile is reasonable.

> > > In a previous patch, there was concern that adding a new debug package
> > > would increase t.he package time. To address this concern and provide
> > > more flexibility, this change has been added to allow users to decide
> > > which extra packages to include before introducing an optional debug
> > > package [1].
> > 
> > This paragraph seems like it shouldn't be part of the final commit.
> > If you put it after a line with "---" it will be dropped from the
> > commit, like so:
> > 
> > ---
> > 
> > In a previous patch, ...
> 
> Agreed, I will move this paragraph to below --- for v2.
> 
> > 
> > > 
> > > [1] https://lore.kernel.org/lkml/20240801192008.GA3923315@thelio-3990X/T/
> > > 
> > > Signed-off-by: Jose Fernandez <jose.fernandez@linux.dev>
> > > Reviewed-by: Peter Jung <ptr1337@cachyos.org>
> > > ---
> > >  scripts/Makefile.package |  5 +++++
> > >  scripts/package/PKGBUILD | 11 ++++++++---
> > >  2 files changed, 13 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/scripts/Makefile.package b/scripts/Makefile.package
> > > index 4a80584ec771..146e828cb4f1 100644
> > > --- a/scripts/Makefile.package
> > > +++ b/scripts/Makefile.package
> > > @@ -144,6 +144,10 @@ snap-pkg:
> > >  # pacman-pkg
> > >  # ---------------------------------------------------------------------------
> > >  
> > > +# Space-separated list of extra packages to build
> > > +# The available extra packages are: headers api-headers
> > > +PACMAN_EXTRAPACKAGES ?=
> > 
> > The assignment doesn't do anything.
> > Do we need the documentation if the default enables all subpackages?
> > 
> > > +
> > >  PHONY += pacman-pkg
> > >  pacman-pkg:
> > >  	@ln -srf $(srctree)/scripts/package/PKGBUILD $(objtree)/PKGBUILD
> > > @@ -152,6 +156,7 @@ pacman-pkg:
> > >  		CARCH="$(UTS_MACHINE)" \
> > >  		KBUILD_MAKEFLAGS="$(MAKEFLAGS)" \
> > >  		KBUILD_REVISION="$(shell $(srctree)/scripts/build-version)" \
> > > +		PACMAN_EXTRAPACKAGES="$(PACMAN_EXTRAPACKAGES)" \
> > 
> > This line is superfluous.
> 
> Ack.

Is it superfluous if PACMAN_EXTRAPACKAGES is not exported to makepkg? If
I remove this while changing the default of PACMAN_EXTRAPACKAGES in
scripts/Makefile.package, its value is not visible in makepkg, so only
the default package gets built. I think

  export PACMAN_EXTRAPACKAGES

is needed after the '?=' assignment line.

> > >  		makepkg $(MAKEPKGOPTS)
> > >  
> > >  # dir-pkg tar*-pkg - tarball targets
> > > diff --git a/scripts/package/PKGBUILD b/scripts/package/PKGBUILD
> > > index 663ce300dd06..41bd0d387f0a 100644
> > > --- a/scripts/package/PKGBUILD
> > > +++ b/scripts/package/PKGBUILD
> > > @@ -3,10 +3,15 @@
> > >  # Contributor: Jan Alexander Steffens (heftig) <heftig@archlinux.org>
> > >  
> > >  pkgbase=${PACMAN_PKGBASE:-linux-upstream}
> > > -pkgname=("${pkgbase}" "${pkgbase}-api-headers")
> > > -if grep -q CONFIG_MODULES=y include/config/auto.conf; then
> > > -	pkgname+=("${pkgbase}-headers")
> > > +pkgname=("${pkgbase}")
> > > +
> > > +_extrapackages=${PACMAN_EXTRAPACKAGES:-}
> > > +if [ -n "$_extrapackages" ]; then
> > 
> > No need for this check. The loop over an empty variable work fine.
> 
> Ack. Will update in v2.
>  
> > > +	for pkg in $_extrapackages; do
> > > +		pkgname+=("${pkgbase}-$pkg")
> > 
> > Use consistent variable references: "${pkgbase}-${pkg}"
> 
> Ack. Will update in v2.
> 
> > > +	done
> > >  fi
> > > +
> > >  pkgver="${KERNELRELEASE//-/_}"
> > >  # The PKGBUILD is evaluated multiple times.
> > >  # Running scripts/build-version from here would introduce inconsistencies.
> > > -- 
> > > 2.46.0
> > > 

Re: [PATCH] kbuild: control extra pacman packages with PACMAN_EXTRAPACKAGES

[Jose Fernandez]

On 24/08/05 07:58PM, Nathan Chancellor wrote:
> On Mon, Aug 05, 2024 at 04:30:15PM -0600, Jose Fernandez wrote:
> > On 24/08/05 04:01PM, Thomas Weißschuh wrote:
> > > > This changes the behavior of the pacman-pkg target to only create the
> > > > main kernel package by default. The rest of the packages will be opt-in
> > > > going forward.
> > > 
> > > I had the impression that by default all extrapackages should be
> > > built. The variable can then be used by expert users where needed.
> > > Other Opinions?
> > 
> > I think switching to defaulting to all packages is a good idea. One concern I 
> > had was how regular users would discover the customization options. Expert users
> > will likely look at the Makefile and figure out how to opt out.
> 
> I think that most users will likely want all of the packages built by
> default. I think leaving this to be discovered by power users in the
> Makefile is reasonable.

Sounds good!

> > > > In a previous patch, there was concern that adding a new debug package
> > > > would increase t.he package time. To address this concern and provide
> > > > more flexibility, this change has been added to allow users to decide
> > > > which extra packages to include before introducing an optional debug
> > > > package [1].
> > > 
> > > This paragraph seems like it shouldn't be part of the final commit.
> > > If you put it after a line with "---" it will be dropped from the
> > > commit, like so:
> > > 
> > > ---
> > > 
> > > In a previous patch, ...
> > 
> > Agreed, I will move this paragraph to below --- for v2.
> > 
> > > 
> > > > 
> > > > [1] https://lore.kernel.org/lkml/20240801192008.GA3923315@thelio-3990X/T/
> > > > 
> > > > Signed-off-by: Jose Fernandez <jose.fernandez@linux.dev>
> > > > Reviewed-by: Peter Jung <ptr1337@cachyos.org>
> > > > ---
> > > >  scripts/Makefile.package |  5 +++++
> > > >  scripts/package/PKGBUILD | 11 ++++++++---
> > > >  2 files changed, 13 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/scripts/Makefile.package b/scripts/Makefile.package
> > > > index 4a80584ec771..146e828cb4f1 100644
> > > > --- a/scripts/Makefile.package
> > > > +++ b/scripts/Makefile.package
> > > > @@ -144,6 +144,10 @@ snap-pkg:
> > > >  # pacman-pkg
> > > >  # ---------------------------------------------------------------------------
> > > >  
> > > > +# Space-separated list of extra packages to build
> > > > +# The available extra packages are: headers api-headers
> > > > +PACMAN_EXTRAPACKAGES ?=
> > > 
> > > The assignment doesn't do anything.
> > > Do we need the documentation if the default enables all subpackages?
> > > 
> > > > +
> > > >  PHONY += pacman-pkg
> > > >  pacman-pkg:
> > > >  	@ln -srf $(srctree)/scripts/package/PKGBUILD $(objtree)/PKGBUILD
> > > > @@ -152,6 +156,7 @@ pacman-pkg:
> > > >  		CARCH="$(UTS_MACHINE)" \
> > > >  		KBUILD_MAKEFLAGS="$(MAKEFLAGS)" \
> > > >  		KBUILD_REVISION="$(shell $(srctree)/scripts/build-version)" \
> > > > +		PACMAN_EXTRAPACKAGES="$(PACMAN_EXTRAPACKAGES)" \
> > > 
> > > This line is superfluous.
> > 
> > Ack.
> 
> Is it superfluous if PACMAN_EXTRAPACKAGES is not exported to makepkg? If
> I remove this while changing the default of PACMAN_EXTRAPACKAGES in
> scripts/Makefile.package, its value is not visible in makepkg, so only
> the default package gets built. I think
> 
>   export PACMAN_EXTRAPACKAGES
> 
> is needed after the '?=' assignment line.

Nathan, I removed the line and it appears to work as expected.

If I set the default packages to:

PACMAN_EXTRAPACKAGES ?= headers api-headers

Then any of these commands builds only the main kernel package:

make pacman-pkg PACMAN_EXTRAPACKAGES=
make pacman-pkg PACMAN_EXTRAPACKAGES=""

This command builds the main package + headers package:

make pacman-pkg PACMAN_EXTRAPACKAGES="headers"

I'm not quite sure how PACMAN_EXTRAPACKAGES makes it to makepkg without that
line. But it appears like it does.

> 
> > > >  		makepkg $(MAKEPKGOPTS)
> > > >  
> > > >  # dir-pkg tar*-pkg - tarball targets
> > > > diff --git a/scripts/package/PKGBUILD b/scripts/package/PKGBUILD
> > > > index 663ce300dd06..41bd0d387f0a 100644
> > > > --- a/scripts/package/PKGBUILD
> > > > +++ b/scripts/package/PKGBUILD
> > > > @@ -3,10 +3,15 @@
> > > >  # Contributor: Jan Alexander Steffens (heftig) <heftig@archlinux.org>
> > > >  
> > > >  pkgbase=${PACMAN_PKGBASE:-linux-upstream}
> > > > -pkgname=("${pkgbase}" "${pkgbase}-api-headers")
> > > > -if grep -q CONFIG_MODULES=y include/config/auto.conf; then
> > > > -	pkgname+=("${pkgbase}-headers")
> > > > +pkgname=("${pkgbase}")
> > > > +
> > > > +_extrapackages=${PACMAN_EXTRAPACKAGES:-}
> > > > +if [ -n "$_extrapackages" ]; then
> > > 
> > > No need for this check. The loop over an empty variable work fine.
> > 
> > Ack. Will update in v2.
> >  
> > > > +	for pkg in $_extrapackages; do
> > > > +		pkgname+=("${pkgbase}-$pkg")
> > > 
> > > Use consistent variable references: "${pkgbase}-${pkg}"
> > 
> > Ack. Will update in v2.
> > 
> > > > +	done
> > > >  fi
> > > > +
> > > >  pkgver="${KERNELRELEASE//-/_}"
> > > >  # The PKGBUILD is evaluated multiple times.
> > > >  # Running scripts/build-version from here would introduce inconsistencies.
> > > > -- 
> > > > 2.46.0
> > > >