[PATCH] kheaders: substituting --sort in archive creation

[PATCH] kheaders: substituting --sort in archive creation

[Dmitry Goldin]

From: Dmitry Goldin <dgoldin+lkml@protonmail.ch>

The option --sort=ORDER was only introduced in tar 1.28 (2014), which
is rather new and might not be available in some setups.

This patch tries to replicate the previous behaviour as closely as possible
to fix the kheaders build for older environments. It does not produce identical
archives compared to the previous version due to minor sorting
differences but produces reproducible results itself in my tests.

Signed-off-by: Dmitry Goldin <dgoldin+lkml@protonmail.ch>
---
 kernel/gen_kheaders.sh | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/kernel/gen_kheaders.sh b/kernel/gen_kheaders.sh
index aff79e461fc9..5a0fc0b0403a 100755
--- a/kernel/gen_kheaders.sh
+++ b/kernel/gen_kheaders.sh
@@ -71,10 +71,13 @@ done | cpio --quiet -pd $cpio_dir >/dev/null 2>&1
 find $cpio_dir -type f -print0 |
 	xargs -0 -P8 -n1 perl -pi -e 'BEGIN {undef $/;}; s/\/\*((?!SPDX).)*?\*\///smg;'

-# Create archive and try to normalize metadata for reproducibility
-tar "${KBUILD_BUILD_TIMESTAMP:+--mtime=$KBUILD_BUILD_TIMESTAMP}" \
-    --owner=0 --group=0 --sort=name --numeric-owner \
-    -Jcf $tarfile -C $cpio_dir/ . > /dev/null
+# Create archive and try to normalize metadata for reproducibility.
+# For compatibility with older versions of tar, files are fed to tar
+# pre-sorted, as --sort=name might not be available.
+find $cpio_dir -printf "./%P\n" | LC_ALL=C sort | \
+    tar "${KBUILD_BUILD_TIMESTAMP:+--mtime=$KBUILD_BUILD_TIMESTAMP}" \
+    --owner=0 --group=0 --numeric-owner --no-recursion \
+    -Jcf $tarfile -C $cpio_dir/ -T - > /dev/null

 echo "$src_files_md5" >  kernel/kheaders.md5
 echo "$obj_files_md5" >> kernel/kheaders.md5
--
2.23.0

Re: [PATCH] kheaders: substituting --sort in archive creation

[Dmitry Goldin]

Hi again,

Sorry for the delay, I'm currently traveling and only have access to a
rather weak machine so compiles took ages.

A few remarks regarding this patch;

This version is not fully identical to the previous invocation, as the
sort order differs a little, but I tried to get it as close to the
original as I was able to. Unfortunately no sort flag seems to quite
replicate tars sorting.

Some noteworthy details in this version are `./` in the find format string, LC_ALL
for sort and --no-recursion;

The format string is in place to replicate the exact behaviour of the previous
invocation, which included a leading `./`. I don't mind dropping this,
if anyone feels strongly about it, but initially I set out to reproduce
the result as closely as possible.

LC_ALL=C is required as locale can impact sort order.

--no-recursion is required to prevent duplication in the resulting archive,
because both, folders and files, are already supplied from find and sort.

I checked this part of the script on an old debian lenny (5.0.10) system and it
behaves as expected, except for xz support not being available in
the shipped gnu tar (v1.20).

As far as other testing goes, I have compiled 5.3.5 on NixOS with kheaders as a
module where both runs produced identical results.

Because of the limited computational power available to me right now, I
did not have time yet to try the baked-in version or any additional
compile runs. I would appreciate if reviewers could do a few runs too.

Andreas: Could you give this patch a try and see if this works for you?

--
Thanks and best regards,
    Dmitry

Re: [PATCH] kheaders: substituting --sort in archive creation

[Andreas Schwab]

On Okt 09 2019, Dmitry Goldin <dgoldin@protonmail.ch> wrote:

> Andreas: Could you give this patch a try and see if this works for you?

Thanks, works for me.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] kheaders: substituting --sort in archive creation

[Quentin Perret]

Hi Dmitry,

On Wednesday 09 Oct 2019 at 13:42:14 (+0000), Dmitry Goldin wrote:
> From: Dmitry Goldin <dgoldin+lkml@protonmail.ch>
> 
> The option --sort=ORDER was only introduced in tar 1.28 (2014), which
> is rather new and might not be available in some setups.
> 
> This patch tries to replicate the previous behaviour as closely as possible
> to fix the kheaders build for older environments. It does not produce identical
> archives compared to the previous version due to minor sorting
> differences but produces reproducible results itself in my tests.
> 
> Signed-off-by: Dmitry Goldin <dgoldin+lkml@protonmail.ch>
> ---
>  kernel/gen_kheaders.sh | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/gen_kheaders.sh b/kernel/gen_kheaders.sh
> index aff79e461fc9..5a0fc0b0403a 100755
> --- a/kernel/gen_kheaders.sh
> +++ b/kernel/gen_kheaders.sh
> @@ -71,10 +71,13 @@ done | cpio --quiet -pd $cpio_dir >/dev/null 2>&1
>  find $cpio_dir -type f -print0 |
>  	xargs -0 -P8 -n1 perl -pi -e 'BEGIN {undef $/;}; s/\/\*((?!SPDX).)*?\*\///smg;'
> 
> -# Create archive and try to normalize metadata for reproducibility
> -tar "${KBUILD_BUILD_TIMESTAMP:+--mtime=$KBUILD_BUILD_TIMESTAMP}" \
> -    --owner=0 --group=0 --sort=name --numeric-owner \
> -    -Jcf $tarfile -C $cpio_dir/ . > /dev/null
> +# Create archive and try to normalize metadata for reproducibility.
> +# For compatibility with older versions of tar, files are fed to tar
> +# pre-sorted, as --sort=name might not be available.
> +find $cpio_dir -printf "./%P\n" | LC_ALL=C sort | \
> +    tar "${KBUILD_BUILD_TIMESTAMP:+--mtime=$KBUILD_BUILD_TIMESTAMP}" \
> +    --owner=0 --group=0 --numeric-owner --no-recursion \
> +    -Jcf $tarfile -C $cpio_dir/ -T - > /dev/null
> 
>  echo "$src_files_md5" >  kernel/kheaders.md5
>  echo "$obj_files_md5" >> kernel/kheaders.md5
> --
> 2.23.0

FWIW:

  Tested-by: Quentin Perret <qperret@google.com>

It turns out this issue broke something in our CI, could this patch be
queued as a -rc4 fix ?

Thanks,
Quentin

Re: [PATCH] kheaders: substituting --sort in archive creation

[Masahiro Yamada]

On Mon, Oct 14, 2019 at 10:40 PM Quentin Perret <qperret@google.com> wrote:
>
> Hi Dmitry,
>
> On Wednesday 09 Oct 2019 at 13:42:14 (+0000), Dmitry Goldin wrote:
> > From: Dmitry Goldin <dgoldin+lkml@protonmail.ch>
> >
> > The option --sort=ORDER was only introduced in tar 1.28 (2014), which
> > is rather new and might not be available in some setups.
> >
> > This patch tries to replicate the previous behaviour as closely as possible
> > to fix the kheaders build for older environments. It does not produce identical
> > archives compared to the previous version due to minor sorting
> > differences but produces reproducible results itself in my tests.
> >
> > Signed-off-by: Dmitry Goldin <dgoldin+lkml@protonmail.ch>
> > ---
> >  kernel/gen_kheaders.sh | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/kernel/gen_kheaders.sh b/kernel/gen_kheaders.sh
> > index aff79e461fc9..5a0fc0b0403a 100755
> > --- a/kernel/gen_kheaders.sh
> > +++ b/kernel/gen_kheaders.sh
> > @@ -71,10 +71,13 @@ done | cpio --quiet -pd $cpio_dir >/dev/null 2>&1
> >  find $cpio_dir -type f -print0 |
> >       xargs -0 -P8 -n1 perl -pi -e 'BEGIN {undef $/;}; s/\/\*((?!SPDX).)*?\*\///smg;'
> >
> > -# Create archive and try to normalize metadata for reproducibility
> > -tar "${KBUILD_BUILD_TIMESTAMP:+--mtime=$KBUILD_BUILD_TIMESTAMP}" \
> > -    --owner=0 --group=0 --sort=name --numeric-owner \
> > -    -Jcf $tarfile -C $cpio_dir/ . > /dev/null
> > +# Create archive and try to normalize metadata for reproducibility.
> > +# For compatibility with older versions of tar, files are fed to tar
> > +# pre-sorted, as --sort=name might not be available.
> > +find $cpio_dir -printf "./%P\n" | LC_ALL=C sort | \
> > +    tar "${KBUILD_BUILD_TIMESTAMP:+--mtime=$KBUILD_BUILD_TIMESTAMP}" \
> > +    --owner=0 --group=0 --numeric-owner --no-recursion \
> > +    -Jcf $tarfile -C $cpio_dir/ -T - > /dev/null
> >
> >  echo "$src_files_md5" >  kernel/kheaders.md5
> >  echo "$obj_files_md5" >> kernel/kheaders.md5
> > --
> > 2.23.0
>
> FWIW:
>
>   Tested-by: Quentin Perret <qperret@google.com>
>
> It turns out this issue broke something in our CI, could this patch be
> queued as a -rc4 fix ?


Applied to linux-kbuild/fixes. Thanks.

I will send a pull request for -rc4.



-- 
Best Regards
Masahiro Yamada