Re: [PATCH] image.bbclass: set file timestamps inside the rootfs and initramfs image

[Henning Schild <henning.schild@siemens.com>]

Am Wed, 9 Nov 2022 09:27:20 +0000
schrieb <Venkata.Pyla@toshiba-tsip.com>:

> Hi Henning Schild,
> 
> Thanks for your review, please find my comments below.
> 
> 
> Thanks,
> Venkata.
> 
> >-----Original Message-----
> >From: Henning Schild <henning.schild@siemens.com>
> >Sent: 07 November 2022 14:24
> >To: pyla venkata(ＴＳＩＰ TMIEC ODG Porting) <Venkata.Pyla@toshiba-  
> >tsip.com>  
> >Cc: isar-users@googlegroups.com; jan.kiszka@siemens.com; hayashi
> >kazuhiro(林和宏 □ＳＷＣ◯ＡＣＴ) <kazuhiro3.hayashi@toshiba.co.jp>;
> >dinesh kumar(ＴＳＩＰ TMIEC ODG Porting)
> ><dinesh.kumar@toshiba-tsip.com> Subject: Re: [PATCH] image.bbclass:
> >set file timestamps inside the rootfs and initramfs image
> >
> >Am Mon,  7 Nov 2022 13:55:03 +0530
> >schrieb venkata.pyla@toshiba-tsip.com:
> >  
> >> From: venkata pyla <venkata.pyla@toshiba-tsip.com>
> >>
> >> As part of reproducible-build work, one of the problem chosen to
> >> solve is the file time stamps inside rootfs and initramfs are not
> >> identical between two builds.
> >>
> >> With the help of reproducible-builds.org and their suggestions, the
> >> above problem can be fixed using 'SOURCE_DATE_EPOCH' variable [2].
> >>
> >> In case of rootfs file time-stamps, set all the files and folders
> >> that are newer than 'SOURCE_DATE_EPOCH' and set it to same.
> >> In case of initramfs, regenerate the initramfs image with
> >> 'SOURCE_DATE_EPOCH' variable set as the mkinitramfs script is
> >> already taken care of creating reproducible initramfs image when
> >> the variable is set in the environment[3].
> >>
> >> The SOURCE_DATE_EPOCH variable should be set to the last
> >> modification of the git repository as explained in the
> >> documentation[2].
> >>
> >> e.g:
> >> SOURCE_DATE_EPOCH=$(git log -1 --pretty=%ct)
> >>
> >> To know more about the reproducible builds and its goals please
> >> refer [1].
> >>
> >> [1] https://reproducible-builds.org/
> >> [2] https://reproducible-builds.org/docs/source-date-epoch/
> >> [3]
> >> https://manpages.debian.org/bullseye/initramfs-tools-core/mkinitramfs.
> >> 8.en.html#ENVIRONMENT
> >>
> >> Signed-off-by: venkata pyla <venkata.pyla@toshiba-tsip.com>
> >> ---
> >>  meta/classes/image.bbclass | 15 +++++++++++++++
> >>  1 file changed, 15 insertions(+)
> >>
> >> diff --git a/meta/classes/image.bbclass
> >> b/meta/classes/image.bbclass index ccff810..c1bb4fd 100644
> >> --- a/meta/classes/image.bbclass
> >> +++ b/meta/classes/image.bbclass
> >> @@ -431,6 +431,21 @@ do_rootfs_finalize() {
> >>              "${ROOTFSDIR}/etc/apt/sources.list.d/bootstrap.list"
> >>
> >>          rm -f "${ROOTFSDIR}/etc/apt/sources-list"
> >> +
> >> +        # Recreate initramfs inorder to set timestamps to
> >> SOURCE_DATE_EPOCH
> >> +        # inorder to make reproducible initramfs
> >> +        test ! -z "${SOURCE_DATE_EPOCH}" && \
> >> +           SOURCE_DATE_EPOCH=${SOURCE_DATE_EPOCH} chroot
> >> "${ROOTFSDIR}" \
> >> +                  update-initramfs -u -v  
> >
> >This should be done where that initial update-initramfs can be found.
> >And not fix things up later. Not every image will have an initrd so
> >that is wrong in multiple ways.  
> 
> Thanks for correcting me, I understood the initrd update should not
> be in the goal of image class, as some of the images may not be
> required initrd as you mentioned. I will find the better place to do
> this and send the another patch.
> 
> >  
> >> +
> >> +	# Set timestamp to files inside the rootfs image inorder
> >> to make
> >> +	# reproducible rootfs
> >> +	test ! -z "${SOURCE_DATE_EPOCH}" && \
> >> +           find ${ROOTFSDIR} -newermt \
> >> +               "$(date -d@${SOURCE_DATE_EPOCH} '+%Y-%m-%d
> >> %H:%M:%S')" \
> >> +               -printf "%y %p\n" \
> >> +               -exec touch '{}' -h -d@${SOURCE_DATE_EPOCH} ';'
> >> +  
> >
> >No! Which files do we care about? My guess would content of packages
> >built with isar. We should export the variable in the dpkg class
> >then the packages will be correct and we do not have to mess around
> >with that rootfs and fix problems that should have never been there.
> > 
> 
> This will only change the files are created during build, and not the
> files come with package.

All files, or very close to all, come with packages. Because isar
builds packages for stuff it does. The wanted timestamp has to be
provided at build time of those packages so that later on one does not
need to run such a find.

You basically first want to make sure the packages (those from
isar-apt) are reproducible and only later look at the whole rootfs. That
whole rootfs will have remaining differences, just from what maintainer
hooks and stuff do.

That find violates the rule that everything should come from a package.
There are some exceptions to that rule in isar, but those are likely
the ones causing repro issues. And anyhow not everyone delivers only
the images, some people also deliver the packages. So those need to be
reproducible as well ... or even first.

> The idea is to set time stamps to the files as same that are modified
> or added during build time (e.g: /etc/*) and they are newer than
> SOURCE_DATE_EPOCH date.
> 
> 
> >
> >I would like to ask for test cases. Ideally first a breaking test
> >and later a commit fixing the issue.  
> 
> I executed this in one of the child project (isar-cip-core) and
> reported the issue here [1], are you expecting to write test cases in
> isar?
> 
> [1] https://gitlab.com/cip-project/cip-core/isar-cip-core/-/issues/31 

The motivation always has to come with the patch. So that manual
diffoscope workflow should be explained in the commit message or
documentation. Ideally you would add a test that actually runs that and
asserts that there are no problems left. Or ignoring the left ones for
starters.

That layer and all other layers will later benefit. But whatever you
try to fix has to be explained and consistent inside isar only. So
issues from some layer clearly do not count.

Say we get the debian package feed to become reproducible, i bet we
could just compare the repo metadata and the package checksums. Not
even dive into the packages and check what might be different.

A good way of explaining a change is also to first write the test and
later the code to fix the problem a test shows. Everybody would
understand easily and if any future change would break it again we
would see before a merge.

regards,
Henning

> >
> >Note that deriving the time from git means one needs git. And when
> >using layers just that one Isar git is clearly not good enough. And
> >we will have to check how that works with sstate. I assume any new
> >commit would rebuild all custom packages. Here i see a conflict
> >between regular dev work and repro pedantics. We might need a way to
> >turn that stuff off.
> >
> >regards,
> >Henning
> >  
> >>  EOSUDO
> >>  }
> >>  addtask rootfs_finalize before do_rootfs after
> >> do_rootfs_postprocess  
>