diff mbox series

[3/3] kbuild: do not create intermediate *.tar for tar packages

Message ID 20230407101629.1298051-3-masahiroy@kernel.org
State New
Headers
Series [1/3] kbuild: merge cmd_archive_linux and cmd_archive_perf |

Commit Message

Masahiro Yamada April 7, 2023, 10:16 a.m. UTC 
  Commit 05e96e96a315 ("kbuild: use git-archive for source package
creation") split the compression as a separate step to factor out
the common build rules.

With the previous commit, we got back to the situation where
compressed source tarballs are created by a single rule.
There is no reason to keep the separate compression rules.

Generate the comressed tar packages directly.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/Makefile.package | 27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)

Comments

Nathan Chancellor April 7, 2023, 6:12 p.m. UTC | #1 
On Fri, Apr 07, 2023 at 07:16:29PM +0900, Masahiro Yamada wrote:
> Commit 05e96e96a315 ("kbuild: use git-archive for source package
> creation") split the compression as a separate step to factor out
> the common build rules.
> 
> With the previous commit, we got back to the situation where
> compressed source tarballs are created by a single rule.
> There is no reason to keep the separate compression rules.
> 
> Generate the comressed tar packages directly.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> 
>  scripts/Makefile.package | 27 +++++++++------------------
>  1 file changed, 9 insertions(+), 18 deletions(-)
> 
> diff --git a/scripts/Makefile.package b/scripts/Makefile.package
> index 7707975f729b..e0e18d7dfbd5 100644
> --- a/scripts/Makefile.package
> +++ b/scripts/Makefile.package
> @@ -2,7 +2,6 @@
>  # Makefile for the different targets used to generate full packages of a kernel
>  
>  include $(srctree)/scripts/Kbuild.include
> -include $(srctree)/scripts/Makefile.lib
>  
>  KERNELPATH := kernel-$(subst -,_,$(KERNELRELEASE))
>  KBUILD_PKG_ROOTCMD ?="fakeroot -u"
> @@ -27,21 +26,6 @@ fi ; \
>  tar -I $(KGZIP) -c $(RCS_TAR_IGNORE) -f $(2).tar.gz \
>  	--transform 's:^:$(2)/:S' $(TAR_CONTENT) $(3)
>  
> -# tarball compression
> -# ---------------------------------------------------------------------------
> -
> -%.tar.gz: %.tar
> -	$(call cmd,gzip)
> -
> -%.tar.bz2: %.tar
> -	$(call cmd,bzip2)
> -
> -%.tar.xz: %.tar
> -	$(call cmd,xzmisc)
> -
> -%.tar.zst: %.tar
> -	$(call cmd,zstd)
> -
>  # Git
>  # ---------------------------------------------------------------------------
>  
> @@ -153,10 +137,17 @@ tar-install: FORCE
>  	$(Q)$(MAKE) -f $(srctree)/Makefile
>  	+$(Q)$(srctree)/scripts/package/buildtar $@
>  
> +compress-tar.gz  = -I "$(KGZIP)"
> +compress-tar.bz2 = -I "$(KBZIP2)"
> +compress-tar.xz  = -I "$(XZ)"
> +compress-tar.zst = -I "$(ZSTD)"
> +
>  quiet_cmd_tar = TAR     $@
> -      cmd_tar = cd $<; tar cf ../$@ --owner=root --group=root --sort=name *
> +      cmd_tar = cd $<; tar cf ../$@ $(compress-tar$(suffix $@)) --owner=root --group=root --sort=name *
> +
> +dir-tarballs := $(addprefix linux-$(KERNELRELEASE)-$(ARCH), .tar .tar.gz .tar.bz2 .tar.xz .tar.zst)
>  
> -linux-$(KERNELRELEASE)-$(ARCH).tar: tar-install
> +$(dir-tarballs): tar-install
>  	$(call cmd,tar)
>  
>  PHONY += dir-pkg
> -- 
> 2.37.2
>
Masahiro Yamada April 11, 2023, 12:33 a.m. UTC | #2 
On Fri, Apr 7, 2023 at 7:16 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Commit 05e96e96a315 ("kbuild: use git-archive for source package
> creation") split the compression as a separate step to factor out
> the common build rules.
>
> With the previous commit, we got back to the situation where
> compressed source tarballs are created by a single rule.
> There is no reason to keep the separate compression rules.
>
> Generate the comressed tar packages directly.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  scripts/Makefile.package | 27 +++++++++------------------
>  1 file changed, 9 insertions(+), 18 deletions(-)
>
> diff --git a/scripts/Makefile.package b/scripts/Makefile.package
> index 7707975f729b..e0e18d7dfbd5 100644
> --- a/scripts/Makefile.package
> +++ b/scripts/Makefile.package
> @@ -2,7 +2,6 @@
>  # Makefile for the different targets used to generate full packages of a kernel
>
>  include $(srctree)/scripts/Kbuild.include
> -include $(srctree)/scripts/Makefile.lib

I noticed a bug.

I will keep this include directive.
Otherwise, perf-tar*-src-pkg targets would be broken.
'cmd_copy' is still used.
Tariq Toukan April 20, 2023, 8:54 a.m. UTC | #3 
On 07/04/2023 21:12, Nathan Chancellor wrote:
> On Fri, Apr 07, 2023 at 07:16:29PM +0900, Masahiro Yamada wrote:
>> Commit 05e96e96a315 ("kbuild: use git-archive for source package
>> creation") split the compression as a separate step to factor out
>> the common build rules.
>>
>> With the previous commit, we got back to the situation where
>> compressed source tarballs are created by a single rule.
>> There is no reason to keep the separate compression rules.
>>
>> Generate the comressed tar packages directly.
>>
>> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> 
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> 

Hi,

We started seeing the failure below in rc7.
We narrowed it down to your patches:

3c65a2704cdd kbuild: do not create intermediate *.tar for tar packages
f8d94++c4e403c kbuild: do not create intermediate *.tar for source tarballs
f6d8283549bc kbuild: merge cmd_archive_linux and cmd_archive_perf
aa7d233f45b4 kbuild: give up untracked files for source package builds

Can you please take a look and advise?

Regards,
Tariq

[root@c-237-113-200-203 linux]# make -j24 rpm-pkg
sh ./scripts/package/mkspec >./kernel.spec
rpmbuild  --target x86_64-linux -bs kernel.spec \
--define='_smp_mflags %{nil}' --define='_sourcedir rpmbuild/SOURCES' 
--define='_srcrpmdir .'
Building target platforms: x86_64-linux
Building for target x86_64-linux
Wrote: ./kernel-6.3.0_rc7+-1.src.rpm
rpmbuild  --target x86_64-linux -rb kernel-6.3.0_rc7+-1.src.rpm \
--define='_smp_mflags %{nil}'
Installing kernel-6.3.0_rc7+-1.src.rpm
Building target platforms: x86_64-linux
Building for target x86_64-linux
Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.yDFEga
+ umask 022
+ cd /root/rpmbuild/BUILD
+ cd /root/rpmbuild/BUILD
+ rm -rf linux
+ /usr/bin/gzip -dc /root/rpmbuild/SOURCES/linux.tar.gz
+ /usr/bin/tar -xof -
+ STATUS=0
+ '[' 0 -ne 0 ']'
+ cd linux
/var/tmp/rpm-tmp.yDFEga: line 37: cd: linux: No such file or directory
error: Bad exit status from /var/tmp/rpm-tmp.yDFEga (%prep)


RPM build errors:
     Bad exit status from /var/tmp/rpm-tmp.yDFEga (%prep)
make[1]: *** [scripts/Makefile.package:69: rpm-pkg] Error 1
make: *** [Makefile:1656: rpm-pkg] Error 2
Nicolas Schier April 20, 2023, 9:06 a.m. UTC | #4 
On Thu, Apr 20, 2023 at 11:54:34AM +0300, Tariq Toukan wrote:
> 
> 
> On 07/04/2023 21:12, Nathan Chancellor wrote:
> > On Fri, Apr 07, 2023 at 07:16:29PM +0900, Masahiro Yamada wrote:
> > > Commit 05e96e96a315 ("kbuild: use git-archive for source package
> > > creation") split the compression as a separate step to factor out
> > > the common build rules.
> > > 
> > > With the previous commit, we got back to the situation where
> > > compressed source tarballs are created by a single rule.
> > > There is no reason to keep the separate compression rules.
> > > 
> > > Generate the comressed tar packages directly.
> > > 
> > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > 
> > Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> > 
> 
> Hi,
> 
> We started seeing the failure below in rc7.
> We narrowed it down to your patches:
> 
> 3c65a2704cdd kbuild: do not create intermediate *.tar for tar packages
> f8d94++c4e403c kbuild: do not create intermediate *.tar for source tarballs
> f6d8283549bc kbuild: merge cmd_archive_linux and cmd_archive_perf
> aa7d233f45b4 kbuild: give up untracked files for source package builds
> 
> Can you please take a look and advise?
> 
> Regards,
> Tariq
> 
> [root@c-237-113-200-203 linux]# make -j24 rpm-pkg
> sh ./scripts/package/mkspec >./kernel.spec
> rpmbuild  --target x86_64-linux -bs kernel.spec \
> --define='_smp_mflags %{nil}' --define='_sourcedir rpmbuild/SOURCES'
> --define='_srcrpmdir .'
> Building target platforms: x86_64-linux
> Building for target x86_64-linux
> Wrote: ./kernel-6.3.0_rc7+-1.src.rpm
> rpmbuild  --target x86_64-linux -rb kernel-6.3.0_rc7+-1.src.rpm \
> --define='_smp_mflags %{nil}'
> Installing kernel-6.3.0_rc7+-1.src.rpm
> Building target platforms: x86_64-linux
> Building for target x86_64-linux
> Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.yDFEga
> + umask 022
> + cd /root/rpmbuild/BUILD
> + cd /root/rpmbuild/BUILD
> + rm -rf linux
> + /usr/bin/gzip -dc /root/rpmbuild/SOURCES/linux.tar.gz
> + /usr/bin/tar -xof -
> + STATUS=0
> + '[' 0 -ne 0 ']'
> + cd linux
> /var/tmp/rpm-tmp.yDFEga: line 37: cd: linux: No such file or directory
> error: Bad exit status from /var/tmp/rpm-tmp.yDFEga (%prep)
> 
> 
> RPM build errors:
>     Bad exit status from /var/tmp/rpm-tmp.yDFEga (%prep)
> make[1]: *** [scripts/Makefile.package:69: rpm-pkg] Error 1
> make: *** [Makefile:1656: rpm-pkg] Error 2

Thanks for the report.  It should/will be fixed with 
https://lore.kernel.org/linux-kbuild/20230419170424.78688-1-masahiroy@kernel.org/

Kind regards,
Nicolas
Leon Romanovsky April 20, 2023, 9:19 a.m. UTC | #5 
On Thu, Apr 20, 2023 at 11:06:06AM +0200, Nicolas Schier wrote:
> On Thu, Apr 20, 2023 at 11:54:34AM +0300, Tariq Toukan wrote:
> > 
> > 
> > On 07/04/2023 21:12, Nathan Chancellor wrote:
> > > On Fri, Apr 07, 2023 at 07:16:29PM +0900, Masahiro Yamada wrote:
> > > > Commit 05e96e96a315 ("kbuild: use git-archive for source package
> > > > creation") split the compression as a separate step to factor out
> > > > the common build rules.
> > > > 
> > > > With the previous commit, we got back to the situation where
> > > > compressed source tarballs are created by a single rule.
> > > > There is no reason to keep the separate compression rules.
> > > > 
> > > > Generate the comressed tar packages directly.
> > > > 
> > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > > 
> > > Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> > > 
> > 
> > Hi,
> > 
> > We started seeing the failure below in rc7.
> > We narrowed it down to your patches:
> > 
> > 3c65a2704cdd kbuild: do not create intermediate *.tar for tar packages
> > f8d94c4e403c kbuild: do not create intermediate *.tar for source tarballs
> > f6d8283549bc kbuild: merge cmd_archive_linux and cmd_archive_perf
> > aa7d233f45b4 kbuild: give up untracked files for source package builds

<...>

> > RPM build errors:
> >     Bad exit status from /var/tmp/rpm-tmp.yDFEga (%prep)
> > make[1]: *** [scripts/Makefile.package:69: rpm-pkg] Error 1
> > make: *** [Makefile:1656: rpm-pkg] Error 2
> 
> Thanks for the report.  It should/will be fixed with 
> https://lore.kernel.org/linux-kbuild/20230419170424.78688-1-masahiroy@kernel.org/

Thanks for the prompt response.

I have a general question, why commits listed by Tariq were not delayed to merge window?

Only one of them has Fixes line, but even that patch doesn't talk about
error, but code refactoring "To simplify the code, ...".

Thanks

> 
> Kind regards,
> Nicolas
Masahiro Yamada April 20, 2023, 12:31 p.m. UTC | #6 
On Thu, Apr 20, 2023 at 6:19 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Thu, Apr 20, 2023 at 11:06:06AM +0200, Nicolas Schier wrote:
> > On Thu, Apr 20, 2023 at 11:54:34AM +0300, Tariq Toukan wrote:
> > >
> > >
> > > On 07/04/2023 21:12, Nathan Chancellor wrote:
> > > > On Fri, Apr 07, 2023 at 07:16:29PM +0900, Masahiro Yamada wrote:
> > > > > Commit 05e96e96a315 ("kbuild: use git-archive for source package
> > > > > creation") split the compression as a separate step to factor out
> > > > > the common build rules.
> > > > >
> > > > > With the previous commit, we got back to the situation where
> > > > > compressed source tarballs are created by a single rule.
> > > > > There is no reason to keep the separate compression rules.
> > > > >
> > > > > Generate the comressed tar packages directly.
> > > > >
> > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > > >
> > > > Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> > > >
> > >
> > > Hi,
> > >
> > > We started seeing the failure below in rc7.
> > > We narrowed it down to your patches:
> > >
> > > 3c65a2704cdd kbuild: do not create intermediate *.tar for tar packages
> > > f8d94c4e403c kbuild: do not create intermediate *.tar for source tarballs
> > > f6d8283549bc kbuild: merge cmd_archive_linux and cmd_archive_perf


Strictly speaking, these are not bugs,
but gave a bad user experience.
(build slowness and extra disk space consumed).


I got a complaint about the intermediate *.tar file.

https://lore.kernel.org/linux-kbuild/20230406152540.8207-1-youling257@gmail.com/



I noticed fixing it was not difficult, so I merged.
(but, apparently I introduced another regression, sorry about that)








> > > aa7d233f45b4 kbuild: give up untracked files for source package builds
>
> <...>
>
> > > RPM build errors:
> > >     Bad exit status from /var/tmp/rpm-tmp.yDFEga (%prep)
> > > make[1]: *** [scripts/Makefile.package:69: rpm-pkg] Error 1
> > > make: *** [Makefile:1656: rpm-pkg] Error 2
> >
> > Thanks for the report.  It should/will be fixed with
> > https://lore.kernel.org/linux-kbuild/20230419170424.78688-1-masahiroy@kernel.org/
>
> Thanks for the prompt response.
>
> I have a general question, why commits listed by Tariq were not delayed to merge window?
>
> Only one of them has Fixes line, but even that patch doesn't talk about
> error, but code refactoring "To simplify the code, ...".
>
> Thanks
>
> >
> > Kind regards,
> > Nicolas
diff mbox series

Patch

diff --git a/scripts/Makefile.package b/scripts/Makefile.package
index 7707975f729b..e0e18d7dfbd5 100644
--- a/scripts/Makefile.package
+++ b/scripts/Makefile.package
@@ -2,7 +2,6 @@ 
 # Makefile for the different targets used to generate full packages of a kernel
 
 include $(srctree)/scripts/Kbuild.include
-include $(srctree)/scripts/Makefile.lib
 
 KERNELPATH := kernel-$(subst -,_,$(KERNELRELEASE))
 KBUILD_PKG_ROOTCMD ?="fakeroot -u"
@@ -27,21 +26,6 @@  fi ; \
 tar -I $(KGZIP) -c $(RCS_TAR_IGNORE) -f $(2).tar.gz \
 	--transform 's:^:$(2)/:S' $(TAR_CONTENT) $(3)
 
-# tarball compression
-# ---------------------------------------------------------------------------
-
-%.tar.gz: %.tar
-	$(call cmd,gzip)
-
-%.tar.bz2: %.tar
-	$(call cmd,bzip2)
-
-%.tar.xz: %.tar
-	$(call cmd,xzmisc)
-
-%.tar.zst: %.tar
-	$(call cmd,zstd)
-
 # Git
 # ---------------------------------------------------------------------------
 
@@ -153,10 +137,17 @@  tar-install: FORCE
 	$(Q)$(MAKE) -f $(srctree)/Makefile
 	+$(Q)$(srctree)/scripts/package/buildtar $@
 
+compress-tar.gz  = -I "$(KGZIP)"
+compress-tar.bz2 = -I "$(KBZIP2)"
+compress-tar.xz  = -I "$(XZ)"
+compress-tar.zst = -I "$(ZSTD)"
+
 quiet_cmd_tar = TAR     $@
-      cmd_tar = cd $<; tar cf ../$@ --owner=root --group=root --sort=name *
+      cmd_tar = cd $<; tar cf ../$@ $(compress-tar$(suffix $@)) --owner=root --group=root --sort=name *
+
+dir-tarballs := $(addprefix linux-$(KERNELRELEASE)-$(ARCH), .tar .tar.gz .tar.bz2 .tar.xz .tar.zst)
 
-linux-$(KERNELRELEASE)-$(ARCH).tar: tar-install
+$(dir-tarballs): tar-install
 	$(call cmd,tar)
 
 PHONY += dir-pkg