| Message ID | 20240804000130.841636-1-jose.fernandez@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | kbuild: control extra pacman packages with PACMAN_EXTRAPACKAGES | expand |
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 >
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 > >
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 > > >
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 > > > >
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.