| Message ID | 941a566eb114701685dc44f708f81891b3bd085b.1703042082.git.kevinmbecause@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enable compressed files in EXTRA_FIRMWARE | expand |
Hi Kevin, > Subject: Re: [PATCH 1/2] kbuild: Enable decompression for use by > EXTRA_FIRMWARE The build system can currently only compress files. This > patch adds the functionality to decompress files. Decompression is needed > for building firmware files into the kernel if those files are compressed on > the filesystem. Compressed firmware files are in use by Gentoo, Fedora, > Arch, and others. patch description is squashed into the subject. Did your tooling accidentially remove the empty line between? The patch itself looks good to me. Tested-by: Nicolas Schier <n.schier@avm.de> Kind regards, Nicolas On Wed, Dec 20, 2023 at 05:22:50AM -0500, Kevin Martin wrote: > Signed-off-by: Kevin Martin <kevinmbecause@gmail.com> > --- > scripts/Makefile.lib | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 1a965fe68..d043be3dc 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -523,6 +523,9 @@ quiet_cmd_xzkern_with_size = XZKERN $@ > quiet_cmd_xzmisc = XZMISC $@ > cmd_xzmisc = cat $(real-prereqs) | $(XZ) --check=crc32 --lzma2=dict=1MiB > $@ > > +quiet_cmd_xzdec = XZDEC $@ > + cmd_xzdec = cat $(real-prereqs) | $(XZ) --decompress > $@ > + > # ZSTD > # --------------------------------------------------------------------------- > # Appends the uncompressed size of the data using size_append. The .zst > @@ -548,6 +551,9 @@ quiet_cmd_zstd22 = ZSTD22 $@ > quiet_cmd_zstd22_with_size = ZSTD22 $@ > cmd_zstd22_with_size = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@ > > +quiet_cmd_zstddec = ZSTDDEC $@ > + cmd_zstddec = cat $(real-prereqs) | $(ZSTD) --decompress > $@ > + > # ASM offsets > # --------------------------------------------------------------------------- > > -- > 2.41.0 >
On Wed, Dec 20, 2023 at 7:26 PM Kevin Martin <kevinmbecause@gmail.com> wrote: > > Signed-off-by: Kevin Martin <kevinmbecause@gmail.com> > --- > scripts/Makefile.lib | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 1a965fe68..d043be3dc 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -523,6 +523,9 @@ quiet_cmd_xzkern_with_size = XZKERN $@ > quiet_cmd_xzmisc = XZMISC $@ > cmd_xzmisc = cat $(real-prereqs) | $(XZ) --check=crc32 --lzma2=dict=1MiB > $@ > > +quiet_cmd_xzdec = XZDEC $@ > + cmd_xzdec = cat $(real-prereqs) | $(XZ) --decompress > $@ > + Please do not fork the meaningless 'cat' process. This should be a single process to take just one input file. cmd_xzdec = $(XZ) --decompress --stdout $< > $@ Commit d3dd3b5a29bb9582957451531fed461628dfc834 was a very bad commit. The 'cat' and compression/decompression must be separate rules. We should not repeat the mistake in the past. > # ZSTD > # --------------------------------------------------------------------------- > # Appends the uncompressed size of the data using size_append. The .zst > @@ -548,6 +551,9 @@ quiet_cmd_zstd22 = ZSTD22 $@ > quiet_cmd_zstd22_with_size = ZSTD22 $@ > cmd_zstd22_with_size = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@ > > +quiet_cmd_zstddec = ZSTDDEC $@ > + cmd_zstddec = cat $(real-prereqs) | $(ZSTD) --decompress > $@ > + Same here. Please make this a single process: cmd_zstddec = $(ZSTD) --decompress --force --output=$@ $< One small concern in the future is, if we end up with adding quiet_cmd_bzip2dec, we will run out of the 7-column of the short log. quiet_cmd_bzip2dec = BZIP2DEC$@ We can increase the column size if needed, so I do not think it is a big issue. > # ASM offsets > # --------------------------------------------------------------------------- > > -- > 2.41.0 >
On Wed, 3 Jan 2024, Masahiro Yamada wrote: > On Wed, Dec 20, 2023 at 7:26 PM Kevin Martin <kevinmbecause@gmail.com> wrote: > > > > Signed-off-by: Kevin Martin <kevinmbecause@gmail.com> > > --- > > scripts/Makefile.lib | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > index 1a965fe68..d043be3dc 100644 > > --- a/scripts/Makefile.lib > > +++ b/scripts/Makefile.lib > > @@ -523,6 +523,9 @@ quiet_cmd_xzkern_with_size = XZKERN $@ > > quiet_cmd_xzmisc = XZMISC $@ > > cmd_xzmisc = cat $(real-prereqs) | $(XZ) --check=crc32 --lzma2=dict=1MiB > $@ > > > > +quiet_cmd_xzdec = XZDEC $@ > > + cmd_xzdec = cat $(real-prereqs) | $(XZ) --decompress > $@ > > + > > > > Please do not fork the meaningless 'cat' process. > > This should be a single process to take just one input file. > > cmd_xzdec = $(XZ) --decompress --stdout $< > $@ > > > > > Commit d3dd3b5a29bb9582957451531fed461628dfc834 > was a very bad commit. > > The 'cat' and compression/decompression must be > separate rules. > > We should not repeat the mistake in the past. > Would it be preferable to change all of the compression rules or just the new decompression rules? I could change just the new ones and then begin working on a different patch to clean up the 'cat' processes in the compression rules. > > > > # ZSTD > > # --------------------------------------------------------------------------- > > # Appends the uncompressed size of the data using size_append. The .zst > > @@ -548,6 +551,9 @@ quiet_cmd_zstd22 = ZSTD22 $@ > > quiet_cmd_zstd22_with_size = ZSTD22 $@ > > cmd_zstd22_with_size = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@ > > > > +quiet_cmd_zstddec = ZSTDDEC $@ > > + cmd_zstddec = cat $(real-prereqs) | $(ZSTD) --decompress > $@ > > + > > > Same here. > Please make this a single process: > > cmd_zstddec = $(ZSTD) --decompress --force --output=$@ $< > > > > > > > One small concern in the future is, if we end up with adding > quiet_cmd_bzip2dec, we will run out of the 7-column of the short log. > > quiet_cmd_bzip2dec = BZIP2DEC$@ > > We can increase the column size if needed, so I do not think > it is a big issue. > > > > > > > > > > > > # ASM offsets > > # --------------------------------------------------------------------------- > > > > -- > > 2.41.0 > > > > > -- > Best Regards > Masahiro Yamada >
On Thu, Jan 4, 2024 at 4:04 PM Kevin Martin <kevinmbecause@gmail.com> wrote: > > > On Wed, 3 Jan 2024, Masahiro Yamada wrote: > > > On Wed, Dec 20, 2023 at 7:26 PM Kevin Martin <kevinmbecause@gmail.com> wrote: > > > > > > Signed-off-by: Kevin Martin <kevinmbecause@gmail.com> > > > --- > > > scripts/Makefile.lib | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > > index 1a965fe68..d043be3dc 100644 > > > --- a/scripts/Makefile.lib > > > +++ b/scripts/Makefile.lib > > > @@ -523,6 +523,9 @@ quiet_cmd_xzkern_with_size = XZKERN $@ > > > quiet_cmd_xzmisc = XZMISC $@ > > > cmd_xzmisc = cat $(real-prereqs) | $(XZ) --check=crc32 --lzma2=dict=1MiB > $@ > > > > > > +quiet_cmd_xzdec = XZDEC $@ > > > + cmd_xzdec = cat $(real-prereqs) | $(XZ) --decompress > $@ > > > + > > > > > > > > Please do not fork the meaningless 'cat' process. > > > > This should be a single process to take just one input file. > > > > cmd_xzdec = $(XZ) --decompress --stdout $< > $@ > > > > > > > > > > Commit d3dd3b5a29bb9582957451531fed461628dfc834 > > was a very bad commit. > > > > The 'cat' and compression/decompression must be > > separate rules. > > > > We should not repeat the mistake in the past. > > > > Would it be preferable to change all of the compression rules or just the > new decompression rules? I do not require you to change the existing code. For decompression, it is unlikely that the recipe takes multiple input files. So, 'cat' is unneeded. > I could change just the new ones and then begin working on a different > patch to clean up the 'cat' processes in the compression rules. If you get rid of the 'cat', you need to refactor the user code. arch/x86/boot/compressed/Makefile relies on 'cat' and 'compress', but please double-check no other Makefile uses it. Also, you might need some research about the potential impact onto the reproducible builds. Without 'cat |', the compressed archive might encode the timestamp of the original file. GZIP records the timestamp in the header. > > > > > > > > # ZSTD > > > # --------------------------------------------------------------------------- > > > # Appends the uncompressed size of the data using size_append. The .zst > > > @@ -548,6 +551,9 @@ quiet_cmd_zstd22 = ZSTD22 $@ > > > quiet_cmd_zstd22_with_size = ZSTD22 $@ > > > cmd_zstd22_with_size = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@ > > > > > > +quiet_cmd_zstddec = ZSTDDEC $@ > > > + cmd_zstddec = cat $(real-prereqs) | $(ZSTD) --decompress > $@ > > > + > > > > > > Same here. > > Please make this a single process: > > > > cmd_zstddec = $(ZSTD) --decompress --force --output=$@ $< > > > > > > > > > > > > > > One small concern in the future is, if we end up with adding > > quiet_cmd_bzip2dec, we will run out of the 7-column of the short log. > > > > quiet_cmd_bzip2dec = BZIP2DEC$@ > > > > We can increase the column size if needed, so I do not think > > it is a big issue. > > > > > > > > > > > > > > > > > > > > > > > # ASM offsets > > > # --------------------------------------------------------------------------- > > > > > > -- > > > 2.41.0 > > > > > > > > > -- > > Best Regards > > Masahiro Yamada > >
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 1a965fe68..d043be3dc 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -523,6 +523,9 @@ quiet_cmd_xzkern_with_size = XZKERN $@ quiet_cmd_xzmisc = XZMISC $@ cmd_xzmisc = cat $(real-prereqs) | $(XZ) --check=crc32 --lzma2=dict=1MiB > $@ +quiet_cmd_xzdec = XZDEC $@ + cmd_xzdec = cat $(real-prereqs) | $(XZ) --decompress > $@ + # ZSTD # --------------------------------------------------------------------------- # Appends the uncompressed size of the data using size_append. The .zst @@ -548,6 +551,9 @@ quiet_cmd_zstd22 = ZSTD22 $@ quiet_cmd_zstd22_with_size = ZSTD22 $@ cmd_zstd22_with_size = { cat $(real-prereqs) | $(ZSTD) -22 --ultra; $(size_append); } > $@ +quiet_cmd_zstddec = ZSTDDEC $@ + cmd_zstddec = cat $(real-prereqs) | $(ZSTD) --decompress > $@ + # ASM offsets # ---------------------------------------------------------------------------
Signed-off-by: Kevin Martin <kevinmbecause@gmail.com> --- scripts/Makefile.lib | 6 ++++++ 1 file changed, 6 insertions(+)