[v2] wic: make ext2/3/4 images reproducible

From: Sergei Zhmylev <s.zhmylev@yadro.com>

Ext2/3/4 FS contains not only mtime, but also ctime, atime and crtime.
Currently, all the files are being added into the rootfs image using
mkfs -d functionality which affects all the timestamps excluding mtime.
This patch ensures all the timestamps inside the FS image equal to
the SOURCE_DATE_EPOCH if it is set.

Signed-off-by: Sergei Zhmylev <s.zhmylev@yadro.com>
---
 scripts/lib/wic/partition.py | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

Since this is v2, and v1 was controversial, it is good practice to
document the differences with v1.

Alex

On Mon, 7 Nov 2022 at 14:54, Sergey Zhmylev <s.zhmylev@yadro.com> wrote:
>
> From: Sergei Zhmylev <s.zhmylev@yadro.com>
>
> Ext2/3/4 FS contains not only mtime, but also ctime, atime and crtime.
> Currently, all the files are being added into the rootfs image using
> mkfs -d functionality which affects all the timestamps excluding mtime.
> This patch ensures all the timestamps inside the FS image equal to
> the SOURCE_DATE_EPOCH if it is set.
>
> Signed-off-by: Sergei Zhmylev <s.zhmylev@yadro.com>
> ---
>  scripts/lib/wic/partition.py | 31 ++++++++++++++++++++++++++-----
>  1 file changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
> index bc889bdeb9..140f3c12dc 100644
> --- a/scripts/lib/wic/partition.py
> +++ b/scripts/lib/wic/partition.py
> @@ -294,17 +294,38 @@ class Partition():
>                  f.write("cd etc\n")
>                  f.write("rm fstab\n")
>                  f.write("write %s fstab\n" % (self.updated_fstab_path))
> -                if os.getenv('SOURCE_DATE_EPOCH'):
> -                    fstab_time = int(os.getenv('SOURCE_DATE_EPOCH'))
> -                    for time in ["atime", "mtime", "ctime"]:
> -                        f.write("set_inode_field fstab %s %s\n" % (time, hex(fstab_time)))
> -                        f.write("set_inode_field fstab %s_extra 0\n" % (time))
>              debugfs_cmd = "debugfs -w -f %s %s" % (debugfs_script_path, rootfs)
>              exec_native_cmd(debugfs_cmd, native_sysroot)
>
>          mkfs_cmd = "fsck.%s -pvfD %s" % (self.fstype, rootfs)
>          exec_native_cmd(mkfs_cmd, native_sysroot, pseudo=pseudo)
>
> +        if os.getenv('SOURCE_DATE_EPOCH'):
> +            sde_time = hex(int(os.getenv('SOURCE_DATE_EPOCH')))
> +            debugfs_script_path = os.path.join(cr_workdir, "debugfs_script")
> +            files = []
> +            fields_to_set = ["atime", "ctime", "crtime"]
> +            if 'ROOTFS_RESET_MTIME' in os.environ:
> +                fields_to_set += ["mtime"]
> +            for root, dirs, others in os.walk(rootfs_dir):
> +                base = root.replace(rootfs_dir, "").rstrip(os.sep)
> +                files += [ "/" if base == "" else base ]
> +                files += [ base + "/" + n for n in dirs + others ]
> +            with open(debugfs_script_path, "w") as f:
> +                f.write("set_current_time %s\n" % (sde_time))
> +                f.write("set_inode_field /etc/fstab mtime %s\n" % (sde_time))
> +                f.write("set_inode_field /etc/fstab mtime_extra 0\n")
> +                for file in set(files):
> +                    for time in fields_to_set:
> +                        f.write("set_inode_field \"%s\" %s %s\n" % (file, time, sde_time))
> +                        f.write("set_inode_field \"%s\" %s_extra 0\n" % (file, time))
> +                for time in ["wtime", "mkfs_time", "lastcheck"]:
> +                    f.write("set_super_value %s %s\n" % (time, sde_time))
> +                for time in ["mtime", "first_error_time", "last_error_time"]:
> +                    f.write("set_super_value %s 0\n" % (time))
> +            debugfs_cmd = "debugfs -w -f %s %s" % (debugfs_script_path, rootfs)
> +            exec_native_cmd(debugfs_cmd, native_sysroot)
> +
>          self.check_for_Y2038_problem(rootfs, native_sysroot)
>
>      def prepare_rootfs_btrfs(self, rootfs, cr_workdir, oe_builddir, rootfs_dir,
> --
> 2.37.2
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#172842): https://lists.openembedded.org/g/openembedded-core/message/172842
> Mute This Topic: https://lists.openembedded.org/mt/94866134/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

Difference with v1:
- mtime will be set only in case of user explicitly asked for this
- in all other cases it will be left intact
- mtime for /etc/fstab will be set anytime
- extfs directory hash seed reverted back to original value

Alex, thank you for the comment!
Is it possible to add the note into "git send-email" as it will be
visible in the email but won't go into the commit [message] itself?

On Mon, 2022-11-07 at 16:54 +0300, Sergey Zhmylev wrote:
> From: Sergei Zhmylev <s.zhmylev@yadro.com>
> 
> Ext2/3/4 FS contains not only mtime, but also ctime, atime and crtime.
> Currently, all the files are being added into the rootfs image using
> mkfs -d functionality which affects all the timestamps excluding mtime.
> This patch ensures all the timestamps inside the FS image equal to
> the SOURCE_DATE_EPOCH if it is set.
> 
> Signed-off-by: Sergei Zhmylev <s.zhmylev@yadro.com>
> ---
>  scripts/lib/wic/partition.py | 31 ++++++++++++++++++++++++++-----
>  1 file changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
> index bc889bdeb9..140f3c12dc 100644
> --- a/scripts/lib/wic/partition.py
> +++ b/scripts/lib/wic/partition.py
> @@ -294,17 +294,38 @@ class Partition():
>                  f.write("cd etc\n")
>                  f.write("rm fstab\n")
>                  f.write("write %s fstab\n" % (self.updated_fstab_path))
> -                if os.getenv('SOURCE_DATE_EPOCH'):
> -                    fstab_time = int(os.getenv('SOURCE_DATE_EPOCH'))
> -                    for time in ["atime", "mtime", "ctime"]:
> -                        f.write("set_inode_field fstab %s %s\n" % (time, hex(fstab_time)))
> -                        f.write("set_inode_field fstab %s_extra 0\n" % (time))
>              debugfs_cmd = "debugfs -w -f %s %s" % (debugfs_script_path, rootfs)
>              exec_native_cmd(debugfs_cmd, native_sysroot)
>  
>          mkfs_cmd = "fsck.%s -pvfD %s" % (self.fstype, rootfs)
>          exec_native_cmd(mkfs_cmd, native_sysroot, pseudo=pseudo)
>  
> +        if os.getenv('SOURCE_DATE_EPOCH'):
> +            sde_time = hex(int(os.getenv('SOURCE_DATE_EPOCH')))
> +            debugfs_script_path = os.path.join(cr_workdir, "debugfs_script")
> +            files = []
> +            fields_to_set = ["atime", "ctime", "crtime"]
> +            if 'ROOTFS_RESET_MTIME' in os.environ:
> +                fields_to_set += ["mtime"]

This isn't mentioned in the commit message and isn't documented. I
don't think we should add "magic" variables like this that let us work
around issues.

> +            for root, dirs, others in os.walk(rootfs_dir):
> +                base = root.replace(rootfs_dir, "").rstrip(os.sep)
> +                files += [ "/" if base == "" else base ]
> +                files += [ base + "/" + n for n in dirs + others ]
> +            with open(debugfs_script_path, "w") as f:
> +                f.write("set_current_time %s\n" % (sde_time))
> +                f.write("set_inode_field /etc/fstab mtime %s\n" % (sde_time))
> +                f.write("set_inode_field /etc/fstab mtime_extra 0\n")

What if we didn't modify /etc/fstab? Shouldn't that be left in the code
you remove above instead?

I'd suggest dropping these mtime pieces, then this patch can merge and
we can discuss the other pieces. I do think further work is needed to
handle mtime correctly and not need to hack around the issue.

Cheers,

Richard

On Mon, 7 Nov 2022 at 15:01, Sergey Zhmylev <s.zhmylev@yadro.com> wrote:
> Alex, thank you for the comment!
> Is it possible to add the note into "git send-email" as it will be
> visible in the email but won't go into the commit [message] itself?

I think you can add 'notes' to a commit message after three dashes:

commit title

commit message
---
notes for reviewers

When applying such a patch, git will strip everything that follows ---.

Alex