diff mbox series

[v6] suggested changes for reproducibility patchset v6

Message ID 20230115215310.732295-1-roberto.foglietta@linuxteam.org
State Superseded, archived
Headers show
Series [v6] suggested changes for reproducibility patchset v6 | expand

Commit Message

roberto.foglietta@linuxteam.org Jan. 15, 2023, 9:53 p.m. UTC 
From: "Roberto A. Foglietta" <roberto.foglietta@gmail.com>

suggested changes for reproducibility patchset

WARNING: eval-image-1.0-r0 do_rootfs_finalize: modified timestamp (1673628837) of 3 files for image reproducibly
         List of files modified could be found here: ./build/tmp/deploy/images/debx86/files.modified_timestamps

v.2: rebased on current ilbers:next

v.3: new script added: wic-extract-rootfs-partition.sh [image.wic]

v.4: example with for epoch generation from git

v.5: reverted the example and rework some few code

v.6: the 1st part of the warning shows up each time the epoch is used
     while the 2nd line appears only when some files has been touched
     This allows the user to know the current situation aboat epoch.

Signed-off-by: Roberto A. Foglietta <roberto.foglietta@gmail.com>
---
 meta-isar/conf/local.conf.sample             |  2 +-
 meta/classes/image-account-extension.bbclass |  6 +--
 meta/classes/image.bbclass                   | 20 ++++----
 meta/classes/initramfs.bbclass               |  4 +-
 wic-extract-rootfs-partition.sh              | 52 ++++++++++++++++++++
 5 files changed, 69 insertions(+), 15 deletions(-)
 create mode 100755 wic-extract-rootfs-partition.sh

Comments

Roberto A. Foglietta Jan. 15, 2023, 10:01 p.m. UTC | #1 
On Sun, 15 Jan 2023 at 22:53, <roberto.foglietta@linuxteam.org> wrote:

>
> v.6: the 1st part of the warning shows up each time the epoch is used
>      while the 2nd line appears only when some files has been touched
>      This allows the user to know the current situation aboat epoch.
>

Please ignore this one, I missed one commit, sorry.
Florian Bezdeka Jan. 15, 2023, 10:31 p.m. UTC | #2 
On Sun, 2023-01-15 at 22:53 +0100, roberto.foglietta@linuxteam.org
wrote:
> From: "Roberto A. Foglietta" <roberto.foglietta@gmail.com>
> 
> suggested changes for reproducibility patchset
> 
> WARNING: eval-image-1.0-r0 do_rootfs_finalize: modified timestamp (1673628837) of 3 files for image reproducibly
>          List of files modified could be found here: ./build/tmp/deploy/images/debx86/files.modified_timestamps
> 

Can't follow. Patches / Commits need proper description (= commit
message). I guess you fixed a warning, but the warning itself (= list
of modified files) was inside the mentioned file, so we have to guess
what changed?

> v.2: rebased on current ilbers:next
> 
> v.3: new script added: wic-extract-rootfs-partition.sh [image.wic]
> 
> v.4: example with for epoch generation from git
> 
> v.5: reverted the example and rework some few code
> 
> v.6: the 1st part of the warning shows up each time the epoch is used
>      while the 2nd line appears only when some files has been touched
>      This allows the user to know the current situation aboat epoch.

Version information does not belong here. See below.

> 
> Signed-off-by: Roberto A. Foglietta <roberto.foglietta@gmail.com>
> ---

Comments like changes between versions of your patches should be
mentioned here. Not inside the commit message.


>  meta-isar/conf/local.conf.sample             |  2 +-
>  meta/classes/image-account-extension.bbclass |  6 +--
>  meta/classes/image.bbclass                   | 20 ++++----
>  meta/classes/initramfs.bbclass               |  4 +-
>  wic-extract-rootfs-partition.sh              | 52 ++++++++++++++++++++
>  5 files changed, 69 insertions(+), 15 deletions(-)
>  create mode 100755 wic-extract-rootfs-partition.sh
> 
> diff --git a/meta-isar/conf/local.conf.sample b/meta-isar/conf/local.conf.sample
> index 6208623e..1d7e178a 100644
> --- a/meta-isar/conf/local.conf.sample
> +++ b/meta-isar/conf/local.conf.sample
> @@ -257,4 +257,4 @@ USER_isar[flags] += "clear-text-password"
>  # Non git repository users can use value from 'stat -c%Y ChangeLog'
>  # To know more details about this variable and how to set the value refer below
>  # https://reproducible-builds.org/docs/source-date-epoch/
> -#SOURCE_DATE_EPOCH =
> +#SOURCE_DATE_EPOCH = ""
> diff --git a/meta/classes/image-account-extension.bbclass b/meta/classes/image-account-extension.bbclass
> index bb173b14..1d49054c 100644
> --- a/meta/classes/image-account-extension.bbclass
> +++ b/meta/classes/image-account-extension.bbclass
> @@ -256,11 +256,11 @@ image_postprocess_accounts() {
>                  # chpasswd adds a random salt when running against a clear-text password.
>                  # For reproducible images, we manually generate the password and use the
>                  # SOURCE_DATE_EPOCH to generate the salt in a deterministic way.
> -                if [ -z "${SOURCE_DATE_EPOCH}"]; then
> +                if [ -z "${SOURCE_DATE_EPOCH}" ]; then
>                      chpasswd_args=""
>                  else
> -                    salt="$(echo "${SOURCE_DATE_EPOCH}" | sha256sum -z | cut -c 1-15)"
> -                    password="$(openssl passwd -6 -salt $salt "$password")"
> +                    salt="$(echo ${SOURCE_DATE_EPOCH} | sha256sum -z | cut -c 1-15)"
> +                    password="$(openssl passwd -6 -salt $salt $password)"
>                  fi
>              fi
>              printf '%s:%s' "$name" "$password" | sudo chroot '${ROOTFSDIR}' \
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index 063b9a3b..bf3dfea8 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -310,8 +310,8 @@ python() {
>  #       invalidate the SSTATE entries for most packages, even if they don't use the
>  #       global SOURCE_DATE_EPOCH variable.
>  rootfs_install_pkgs_install_prepend() {
> -    if [ ! -z "${SOURCE_DATE_EPOCH}" ]; then
> -        export SOURCE_DATE_EPOCH="${SOURCE_DATE_EPOCH}"
> +    if [ -n "${SOURCE_DATE_EPOCH}" ]; then
> +        export SOURCE_DATE_EPOCH
>      fi
>  }
>  
> @@ -443,13 +443,15 @@ EOSUDO
>  
>      # Set same time-stamps to the newly generated file/folders in the
>      # rootfs image for the purpose of reproducible builds.
> -    test ! -z "${SOURCE_DATE_EPOCH}" && \
> -        sudo 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} ';' > ${DEPLOY_DIR_IMAGE}/files.modified_timestamps && \
> -            bbwarn "$(grep ^f ${DEPLOY_DIR_IMAGE}/files.modified_timestamps) \nModified above file timestamps to build image reproducibly"
> -
> +    if [ -n "${SOURCE_DATE_EPOCH}" ]; then
> +        msg=""
> +        fn="${DEPLOY_DIR_IMAGE}/files.modified_timestamps"
> +        if sudo 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} ';' >"$fn"; then
> +            msg="\n        List of files modified could be found here: .${DEPLOY_DIR_IMAGE}/files.modified_timestamps"
> +        fi
> +        bbwarn "Modified timestamp (${SOURCE_DATE_EPOCH}) of $(egrep ^f '$fn' | wc -l) files for image reproducibly.$msg"
> +    fi
>  }
>  addtask rootfs_finalize before do_rootfs after do_rootfs_postprocess
>  
> diff --git a/meta/classes/initramfs.bbclass b/meta/classes/initramfs.bbclass
> index db283347..1b98bc06 100644
> --- a/meta/classes/initramfs.bbclass
> +++ b/meta/classes/initramfs.bbclass
> @@ -33,8 +33,8 @@ do_generate_initramfs() {
>      rootfs_do_qemu
>  
>      # generate reproducible initrd if requested
> -    if [ ! -z "${SOURCE_DATE_EPOCH}" ]; then
> -        export SOURCE_DATE_EPOCH="${SOURCE_DATE_EPOCH}"
> +    if [ -n "${SOURCE_DATE_EPOCH}" ]; then
> +        export SOURCE_DATE_EPOCH
>      fi
>  
>      sudo -E chroot "${INITRAMFS_ROOTFS}" \
> diff --git a/wic-extract-rootfs-partition.sh b/wic-extract-rootfs-partition.sh
> new file mode 100755
> index 00000000..48de0d3a
> --- /dev/null
> +++ b/wic-extract-rootfs-partition.sh
> @@ -0,0 +1,52 @@
> +#!/bin/bash
> +#
> +# Copyright (c) Roberto A. Foglietta, 2023
> +#
> +# Authors:
> +#  Roberto A. Foglietta <roberto.foglietta@gmail.com>
> +#
> +# SPDX-License-Identifier: MIT
> +#
> +#set -ex
> +
> +if [ "$(whoami)" != "root" ]; then
> +    echo
> +    echo "WARNING: this script should run as root, sudo!"
> +    sudo -E $0 "$@"
> +    exit $?
> +fi
> +
> +if [ -e "$1" ]; then
> +    fimg=$(readlink -e $1)
> +fi
> +
> +cd $(dirname $0)
> +
> +if [ ! -n "$1" -a  ! -e "$fimg" ]; then
> +    fimg=$(ls -1 build/tmp/deploy/images/*/*.wic)
> +    n=( $fimg )
> +    if [ ${#n[@]} -gt 1 ]; then
> +        echo
> +        echo "WARNING: more than one image found, choose one:"
> +        echo
> +        echo "$fimg"
> +        echo
> +        exit 1
> +    fi
> +fi
> +
> +if [ ! -e "$fimg" ]; then
> +    echo
> +    echo "ERROR: no any image or block device ${1:+'$1' }found, abort!"
> +    echo
> +    exit 1
> +fi
> +
> +wicf=$fimg
> +losetup -Pf $wicf
> +ldev=$(losetup -j $wicf | cut -d: -f1 | tail -n1)
> +echo loopdev:$ldev
> +dd if=${ldev}p2 bs=1M of=${wicf/.wic/.rootfs} status=progress
> +chown $(id -u).$(id -g) ${wicf/.wic/.rootfs}
> +du -ms ${wicf/.wic/.rootfs}
> +losetup -d $ldev
> -- 
> 2.34.1
>
Roberto A. Foglietta Jan. 15, 2023, 10:46 p.m. UTC | #3 
On Sun, 15 Jan 2023 at 23:32, Florian Bezdeka <florian.bezdeka@siemens.com>
wrote:
>
> On Sun, 2023-01-15 at 22:53 +0100, roberto.foglietta@linuxteam.org
> wrote:
> > From: "Roberto A. Foglietta" <roberto.foglietta@gmail.com>
> >
> > suggested changes for reproducibility patchset
> >
> > WARNING: eval-image-1.0-r0 do_rootfs_finalize: modified timestamp
(1673628837) of 3 files for image reproducibly
> >          List of files modified could be found here:
./build/tmp/deploy/images/debx86/files.modified_timestamps
> >
>
> Can't follow. Patches / Commits need proper description (= commit
> message). I guess you fixed a warning, but the warning itself (= list
> of modified files) was inside the mentioned file, so we have to guess
> what changed?
>

Do not worry, I will do a proper patch when your changes will be included
into ilbers/next - this is just a suggestion for Felix

@Felix
There is no reason to show a warning of long N files but just a summary
with the name of the file to check.
Please forget the v6 because it got out prematurely. I just sent the v7.
Keep in consideration that in my building after the image
finalize, do_install_imager_deps runs and mess-up things.
It is something that I need to investigate.


> >
> > v.6: the 1st part of the warning shows up each time the epoch is used
> >      while the 2nd line appears only when some files has been touched
> >      This allows the user to know the current situation aboat epoch.
>
> Version information does not belong here. See below.
>
> >
> > Signed-off-by: Roberto A. Foglietta <roberto.foglietta@gmail.com>
> > ---
>
> Comments like changes between versions of your patches should be
> mentioned here. Not inside the commit message.
>

Ok, it seems weird to me but it probably is a standard that automatic
software needs. Is that right?
MOESSBAUER, Felix Jan. 16, 2023, 2:55 a.m. UTC | #4 
On Sun, 2023-01-15 at 22:53 +0100, roberto.foglietta@linuxteam.org
wrote:
> From: "Roberto A. Foglietta" <roberto.foglietta@gmail.com>
> 
> suggested changes for reproducibility patchset
> 
> WARNING: eval-image-1.0-r0 do_rootfs_finalize: modified timestamp
> (1673628837) of 3 files for image reproducibly
>          List of files modified could be found here:
> ./build/tmp/deploy/images/debx86/files.modified_timestamps
> 
> v.2: rebased on current ilbers:next
> 
> v.3: new script added: wic-extract-rootfs-partition.sh [image.wic]
> 
> v.4: example with for epoch generation from git
> 
> v.5: reverted the example and rework some few code
> 
> v.6: the 1st part of the warning shows up each time the epoch is used
>      while the 2nd line appears only when some files has been touched
>      This allows the user to know the current situation aboat epoch.

Sorry, but I can't follow either.
Please send the versions as individual patch series, prefixed with
"PATCH v<version>". And please only tackle one issue per patch.

> 
> Signed-off-by: Roberto A. Foglietta <roberto.foglietta@gmail.com>
> ---
>  meta-isar/conf/local.conf.sample             |  2 +-
>  meta/classes/image-account-extension.bbclass |  6 +--
>  meta/classes/image.bbclass                   | 20 ++++----
>  meta/classes/initramfs.bbclass               |  4 +-
>  wic-extract-rootfs-partition.sh              | 52
> ++++++++++++++++++++
>  5 files changed, 69 insertions(+), 15 deletions(-)
>  create mode 100755 wic-extract-rootfs-partition.sh
> 
> diff --git a/meta-isar/conf/local.conf.sample b/meta-
> isar/conf/local.conf.sample
> index 6208623e..1d7e178a 100644
> --- a/meta-isar/conf/local.conf.sample
> +++ b/meta-isar/conf/local.conf.sample
> @@ -257,4 +257,4 @@ USER_isar[flags] += "clear-text-password"
>  # Non git repository users can use value from 'stat -c%Y ChangeLog'
>  # To know more details about this variable and how to set the value
> refer below
>  # https://reproducible-builds.org/docs/source-date-epoch/
> -#SOURCE_DATE_EPOCH =
> +#SOURCE_DATE_EPOCH = ""
> diff --git a/meta/classes/image-account-extension.bbclass
> b/meta/classes/image-account-extension.bbclass
> index bb173b14..1d49054c 100644
> --- a/meta/classes/image-account-extension.bbclass
> +++ b/meta/classes/image-account-extension.bbclass
> @@ -256,11 +256,11 @@ image_postprocess_accounts() {
>                  # chpasswd adds a random salt when running against a
> clear-text password.
>                  # For reproducible images, we manually generate the
> password and use the
>                  # SOURCE_DATE_EPOCH to generate the salt in a
> deterministic way.
> -                if [ -z "${SOURCE_DATE_EPOCH}"]; then
> +                if [ -z "${SOURCE_DATE_EPOCH}" ]; then
>                      chpasswd_args=""
>                  else
> -                    salt="$(echo "${SOURCE_DATE_EPOCH}" | sha256sum
> -z | cut -c 1-15)"
> -                    password="$(openssl passwd -6 -salt $salt
> "$password")"
> +                    salt="$(echo ${SOURCE_DATE_EPOCH} | sha256sum -z
> | cut -c 1-15)"
> +                    password="$(openssl passwd -6 -salt $salt
> $password)"

This "fixup" is simply wrong because the value of the variables are not
escaped correctly anymore. In short: it breaks if salt contains either
reserved characters or spaces. Please run this kind of stuff through
shellcheck before proposing fixes.

Felix

>                  fi
>              fi
>              printf '%s:%s' "$name" "$password" | sudo chroot
> '${ROOTFSDIR}' \
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index 063b9a3b..bf3dfea8 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -310,8 +310,8 @@ python() {
>  #       invalidate the SSTATE entries for most packages, even if
> they don't use the
>  #       global SOURCE_DATE_EPOCH variable.
>  rootfs_install_pkgs_install_prepend() {
> -    if [ ! -z "${SOURCE_DATE_EPOCH}" ]; then
> -        export SOURCE_DATE_EPOCH="${SOURCE_DATE_EPOCH}"
> +    if [ -n "${SOURCE_DATE_EPOCH}" ]; then
> +        export SOURCE_DATE_EPOCH
>      fi
>  }
>  
> @@ -443,13 +443,15 @@ EOSUDO
>  
>      # Set same time-stamps to the newly generated file/folders in
> the
>      # rootfs image for the purpose of reproducible builds.
> -    test ! -z "${SOURCE_DATE_EPOCH}" && \
> -        sudo 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} ';' >
> ${DEPLOY_DIR_IMAGE}/files.modified_timestamps && \
> -            bbwarn "$(grep ^f
> ${DEPLOY_DIR_IMAGE}/files.modified_timestamps) \nModified above file
> timestamps to build image reproducibly"
> -
> +    if [ -n "${SOURCE_DATE_EPOCH}" ]; then
> +        msg=""
> +        fn="${DEPLOY_DIR_IMAGE}/files.modified_timestamps"
> +        if sudo 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} ';' >"$fn"; then
> +            msg="\n        List of files modified could be found
> here: .${DEPLOY_DIR_IMAGE}/files.modified_timestamps"
> +        fi
> +        bbwarn "Modified timestamp (${SOURCE_DATE_EPOCH}) of $(egrep
> ^f '$fn' | wc -l) files for image reproducibly.$msg"
> +    fi
>  }
>  addtask rootfs_finalize before do_rootfs after do_rootfs_postprocess
>  
> diff --git a/meta/classes/initramfs.bbclass
> b/meta/classes/initramfs.bbclass
> index db283347..1b98bc06 100644
> --- a/meta/classes/initramfs.bbclass
> +++ b/meta/classes/initramfs.bbclass
> @@ -33,8 +33,8 @@ do_generate_initramfs() {
>      rootfs_do_qemu
>  
>      # generate reproducible initrd if requested
> -    if [ ! -z "${SOURCE_DATE_EPOCH}" ]; then
> -        export SOURCE_DATE_EPOCH="${SOURCE_DATE_EPOCH}"
> +    if [ -n "${SOURCE_DATE_EPOCH}" ]; then
> +        export SOURCE_DATE_EPOCH
>      fi
>  
>      sudo -E chroot "${INITRAMFS_ROOTFS}" \
> diff --git a/wic-extract-rootfs-partition.sh b/wic-extract-rootfs-
> partition.sh
> new file mode 100755
> index 00000000..48de0d3a
> --- /dev/null
> +++ b/wic-extract-rootfs-partition.sh
> @@ -0,0 +1,52 @@
> +#!/bin/bash
> +#
> +# Copyright (c) Roberto A. Foglietta, 2023
> +#
> +# Authors:
> +#  Roberto A. Foglietta <roberto.foglietta@gmail.com>
> +#
> +# SPDX-License-Identifier: MIT
> +#
> +#set -ex
> +
> +if [ "$(whoami)" != "root" ]; then
> +    echo
> +    echo "WARNING: this script should run as root, sudo!"
> +    sudo -E $0 "$@"
> +    exit $?
> +fi
> +
> +if [ -e "$1" ]; then
> +    fimg=$(readlink -e $1)
> +fi
> +
> +cd $(dirname $0)
> +
> +if [ ! -n "$1" -a  ! -e "$fimg" ]; then
> +    fimg=$(ls -1 build/tmp/deploy/images/*/*.wic)
> +    n=( $fimg )
> +    if [ ${#n[@]} -gt 1 ]; then
> +        echo
> +        echo "WARNING: more than one image found, choose one:"
> +        echo
> +        echo "$fimg"
> +        echo
> +        exit 1
> +    fi
> +fi
> +
> +if [ ! -e "$fimg" ]; then
> +    echo
> +    echo "ERROR: no any image or block device ${1:+'$1' }found,
> abort!"
> +    echo
> +    exit 1
> +fi
> +
> +wicf=$fimg
> +losetup -Pf $wicf
> +ldev=$(losetup -j $wicf | cut -d: -f1 | tail -n1)
> +echo loopdev:$ldev
> +dd if=${ldev}p2 bs=1M of=${wicf/.wic/.rootfs} status=progress
> +chown $(id -u).$(id -g) ${wicf/.wic/.rootfs}
> +du -ms ${wicf/.wic/.rootfs}
> +losetup -d $ldev
> -- 
> 2.34.1
>
Roberto A. Foglietta Jan. 16, 2023, 9:36 a.m. UTC | #5 
On Mon, 16 Jan 2023 at 03:55, Moessbauer, Felix <
felix.moessbauer@siemens.com> wrote:

> On Sun, 2023-01-15 at 22:53 +0100, roberto.foglietta@linuxteam.org
> wrote:
> > From: "Roberto A. Foglietta" <roberto.foglietta@gmail.com>
> >
> > suggested changes for reproducibility patchset
> >
> > WARNING: eval-image-1.0-r0 do_rootfs_finalize: modified timestamp
> > (1673628837) of 3 files for image reproducibly
> >          List of files modified could be found here:
> > ./build/tmp/deploy/images/debx86/files.modified_timestamps
> >
> > v.2: rebased on current ilbers:next
> >
> > v.3: new script added: wic-extract-rootfs-partition.sh [image.wic]
> >
> > v.4: example with for epoch generation from git
> >
> > v.5: reverted the example and rework some few code
> >
> > v.6: the 1st part of the warning shows up each time the epoch is used
> >      while the 2nd line appears only when some files has been touched
> >      This allows the user to know the current situation aboat epoch.
>
> Sorry, but I can't follow either.
>

If 416 files are changed, there is no need to print out a warning of 416
lines but just 2 In case of zero files touched, just one line of warning is
fine.


Please send the versions as individual patch series, prefixed with
> "PATCH v<version>". And please only tackle one issue per patch


Ok. You are right. It is confusing to send suggestions in the form of a
patch.


> +                    password="$(openssl passwd -6 -salt $salt
> > $password)"
>
> This "fixup" is simply wrong because the value of the variables are not
> escaped correctly anymore. In short: it breaks if salt contains either
> reserved characters or spaces.


Correct: thanks.



> Please run this kind of stuff through
> shellcheck before proposing fixes.
>

The suggestion of shellcheck is great, it will be very useful to provide a
code verification in git-functions. However, his line of code of yours did
not even run in a console because it is broken when SOURCE_DATE_EPOCH is
defined - also in dash. In fact, you fixed it in v3. (SMILE)

roberto:~/d$ SOURCE_DATE_EPOCH=42; if [ -z "${SOURCE_DATE_EPOCH}"]; then
echo ciao; fi
bash: [: missing `]'
roberto:~/d$ SOURCE_DATE_EPOCH=""; if [ -z "${SOURCE_DATE_EPOCH}"]; then
echo ciao; fi
ciao

--- a/meta/classes/image-account-extension.bbclass
+++ b/meta/classes/image-account-extension.bbclass
@@ -256,11 +256,11 @@ image_postprocess_accounts() {
                 # chpasswd adds a random salt when running against a
clear-text password.
                 # For reproducible images, we manually generate the
password and use the
                 # SOURCE_DATE_EPOCH to generate the salt in a
deterministic way.
-                if [ -z "${SOURCE_DATE_EPOCH}"]; then
+                if [ -z "${SOURCE_DATE_EPOCH}" ]; then

>
Best regards, R-
Florian Bezdeka Jan. 17, 2023, 11:53 a.m. UTC | #6 
On Sun, 2023-01-15 at 23:46 +0100, Roberto A. Foglietta wrote:
> On Sun, 15 Jan 2023 at 23:32, Florian Bezdeka
> <florian.bezdeka@siemens.com> wrote:
> > 
> > On Sun, 2023-01-15 at 22:53 +0100, roberto.foglietta@linuxteam.org
> > wrote:
> > > From: "Roberto A. Foglietta" <roberto.foglietta@gmail.com>
> > > 
> > > suggested changes for reproducibility patchset
> > > 
> > > WARNING: eval-image-1.0-r0 do_rootfs_finalize: modified timestamp
> > > (1673628837) of 3 files for image reproducibly
> > >          List of files modified could be found here:
> > > ./build/tmp/deploy/images/debx86/files.modified_timestamps
> > > 
> > 
> > Can't follow. Patches / Commits need proper description (= commit
> > message). I guess you fixed a warning, but the warning itself (=
> > list
> > of modified files) was inside the mentioned file, so we have to
> > guess
> > what changed?
> > 
> 
> Do not worry, I will do a proper patch when your changes will be
> included into ilbers/next - this is just a suggestion for Felix
> 
> @Felix 
> There is no reason to show a warning of long N files but just a
> summary with the name of the file to check.
> Please forget the v6 because it got out prematurely. I just sent the
> v7.
> Keep in consideration that in my building after the image
> finalize, do_install_imager_deps runs and mess-up things.
> It is something that I need to investigate.
> 
> 
> > > 
> > > v.6: the 1st part of the warning shows up each time the epoch is
> > > used
> > >      while the 2nd line appears only when some files has been
> > > touched
> > >      This allows the user to know the current situation aboat
> > > epoch.
> > 
> > Version information does not belong here. See below.
> > 
> > > 
> > > Signed-off-by: Roberto A. Foglietta <roberto.foglietta@gmail.com>
> > > ---
> > 
> > Comments like changes between versions of your patches should be
> > mentioned here. Not inside the commit message.
> > 
> 
> Ok, it seems weird to me but it probably is a standard that automatic
> software needs. Is that right?

This is how "git am" works. Everything above the "---" line will go
into the commit message (should be used for story telling), everything
below that line and in front of the first hunk will be thrown away and
can be used for further comments/hints for reviewers/maintainers.
Roberto A. Foglietta Jan. 17, 2023, 1:10 p.m. UTC | #7 
On Tue, 17 Jan 2023 at 12:53, Florian Bezdeka
<florian.bezdeka@siemens.com> wrote:
>
> On Sun, 2023-01-15 at 23:46 +0100, Roberto A. Foglietta wrote:
> > On Sun, 15 Jan 2023 at 23:32, Florian Bezdeka
> > <florian.bezdeka@siemens.com> wrote:

> > > > Signed-off-by: Roberto A. Foglietta <roberto.foglietta@gmail.com>
> > > > ---
> > >
> > > Comments like changes between versions of your patches should be
> > > mentioned here. Not inside the commit message.
> > >
> >
> > Ok, it seems weird to me but it probably is a standard that automatic
> > software needs. Is that right?
>
> This is how "git am" works. Everything above the "---" line will go
> into the commit message (should be used for story telling), everything
> below that line and in front of the first hunk will be thrown away and
> can be used for further comments/hints for reviewers/maintainers.
>

So, considering that I want to maintain the version revision of the
patch along with the commits, it makes perfect sense that I put them
in the description. After all, the description content is arbitrary to
some degrees. However, I can add the revision log also below the
signature in such a way that some other tools that expect to find,
they will find it. Does this sound good to you?

Best regards, R.
Roberto A. Foglietta Jan. 25, 2023, 4:38 p.m. UTC | #8 
On Tue, 17 Jan 2023 at 14:10, Roberto A. Foglietta
<roberto.foglietta@gmail.com> wrote:
>
> On Tue, 17 Jan 2023 at 12:53, Florian Bezdeka
> <florian.bezdeka@siemens.com> wrote:
> >
> > On Sun, 2023-01-15 at 23:46 +0100, Roberto A. Foglietta wrote:
> > > On Sun, 15 Jan 2023 at 23:32, Florian Bezdeka
> > > <florian.bezdeka@siemens.com> wrote:
>
> > > > > Signed-off-by: Roberto A. Foglietta <roberto.foglietta@gmail.com>
> > > > > ---
> > > >
> > > > Comments like changes between versions of your patches should be
> > > > mentioned here. Not inside the commit message.
> > > >
> > >
> > > Ok, it seems weird to me but it probably is a standard that automatic
> > > software needs. Is that right?
> >
> > This is how "git am" works. Everything above the "---" line will go
> > into the commit message (should be used for story telling), everything
> > below that line and in front of the first hunk will be thrown away and
> > can be used for further comments/hints for reviewers/maintainers.
> >
>
> So, considering that I want to maintain the version revision of the
> patch along with the commits, it makes perfect sense that I put them
> in the description. After all, the description content is arbitrary to
> some degrees. However, I can add the revision log also below the
> signature in such a way that some other tools that expect to find,
> they will find it. Does this sound good to you?
>

Put the versioning after the signature, it remains in the comment. Put
the versioning after the --- after the signature is required to edit
manually the patch because git format-patch does not do that. My
versioning is part of the comment to the patch and it is ok to add it
also below --- but I wish to have a way to do that without manually
editing the patch. By the way, if the versioning is not added into the
message commit, there is no way to keep track of the versioning when I
do a commit-patch-commit transfer. So, I am trying to understand how
to keep track of the versioning and populate a field that is lost with
git am patch.

The next patch arriving has been edited manually. I hope it will be
fine under this point of view in the meantime my questions are
pending.

Best regards, R-
diff mbox series

Patch

diff --git a/meta-isar/conf/local.conf.sample b/meta-isar/conf/local.conf.sample
index 6208623e..1d7e178a 100644
--- a/meta-isar/conf/local.conf.sample
+++ b/meta-isar/conf/local.conf.sample
@@ -257,4 +257,4 @@  USER_isar[flags] += "clear-text-password"
 # Non git repository users can use value from 'stat -c%Y ChangeLog'
 # To know more details about this variable and how to set the value refer below
 # https://reproducible-builds.org/docs/source-date-epoch/
-#SOURCE_DATE_EPOCH =
+#SOURCE_DATE_EPOCH = ""
diff --git a/meta/classes/image-account-extension.bbclass b/meta/classes/image-account-extension.bbclass
index bb173b14..1d49054c 100644
--- a/meta/classes/image-account-extension.bbclass
+++ b/meta/classes/image-account-extension.bbclass
@@ -256,11 +256,11 @@  image_postprocess_accounts() {
                 # chpasswd adds a random salt when running against a clear-text password.
                 # For reproducible images, we manually generate the password and use the
                 # SOURCE_DATE_EPOCH to generate the salt in a deterministic way.
-                if [ -z "${SOURCE_DATE_EPOCH}"]; then
+                if [ -z "${SOURCE_DATE_EPOCH}" ]; then
                     chpasswd_args=""
                 else
-                    salt="$(echo "${SOURCE_DATE_EPOCH}" | sha256sum -z | cut -c 1-15)"
-                    password="$(openssl passwd -6 -salt $salt "$password")"
+                    salt="$(echo ${SOURCE_DATE_EPOCH} | sha256sum -z | cut -c 1-15)"
+                    password="$(openssl passwd -6 -salt $salt $password)"
                 fi
             fi
             printf '%s:%s' "$name" "$password" | sudo chroot '${ROOTFSDIR}' \
diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 063b9a3b..bf3dfea8 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -310,8 +310,8 @@  python() {
 #       invalidate the SSTATE entries for most packages, even if they don't use the
 #       global SOURCE_DATE_EPOCH variable.
 rootfs_install_pkgs_install_prepend() {
-    if [ ! -z "${SOURCE_DATE_EPOCH}" ]; then
-        export SOURCE_DATE_EPOCH="${SOURCE_DATE_EPOCH}"
+    if [ -n "${SOURCE_DATE_EPOCH}" ]; then
+        export SOURCE_DATE_EPOCH
     fi
 }
 
@@ -443,13 +443,15 @@  EOSUDO
 
     # Set same time-stamps to the newly generated file/folders in the
     # rootfs image for the purpose of reproducible builds.
-    test ! -z "${SOURCE_DATE_EPOCH}" && \
-        sudo 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} ';' > ${DEPLOY_DIR_IMAGE}/files.modified_timestamps && \
-            bbwarn "$(grep ^f ${DEPLOY_DIR_IMAGE}/files.modified_timestamps) \nModified above file timestamps to build image reproducibly"
-
+    if [ -n "${SOURCE_DATE_EPOCH}" ]; then
+        msg=""
+        fn="${DEPLOY_DIR_IMAGE}/files.modified_timestamps"
+        if sudo 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} ';' >"$fn"; then
+            msg="\n        List of files modified could be found here: .${DEPLOY_DIR_IMAGE}/files.modified_timestamps"
+        fi
+        bbwarn "Modified timestamp (${SOURCE_DATE_EPOCH}) of $(egrep ^f '$fn' | wc -l) files for image reproducibly.$msg"
+    fi
 }
 addtask rootfs_finalize before do_rootfs after do_rootfs_postprocess
 
diff --git a/meta/classes/initramfs.bbclass b/meta/classes/initramfs.bbclass
index db283347..1b98bc06 100644
--- a/meta/classes/initramfs.bbclass
+++ b/meta/classes/initramfs.bbclass
@@ -33,8 +33,8 @@  do_generate_initramfs() {
     rootfs_do_qemu
 
     # generate reproducible initrd if requested
-    if [ ! -z "${SOURCE_DATE_EPOCH}" ]; then
-        export SOURCE_DATE_EPOCH="${SOURCE_DATE_EPOCH}"
+    if [ -n "${SOURCE_DATE_EPOCH}" ]; then
+        export SOURCE_DATE_EPOCH
     fi
 
     sudo -E chroot "${INITRAMFS_ROOTFS}" \
diff --git a/wic-extract-rootfs-partition.sh b/wic-extract-rootfs-partition.sh
new file mode 100755
index 00000000..48de0d3a
--- /dev/null
+++ b/wic-extract-rootfs-partition.sh
@@ -0,0 +1,52 @@ 
+#!/bin/bash
+#
+# Copyright (c) Roberto A. Foglietta, 2023
+#
+# Authors:
+#  Roberto A. Foglietta <roberto.foglietta@gmail.com>
+#
+# SPDX-License-Identifier: MIT
+#
+#set -ex
+
+if [ "$(whoami)" != "root" ]; then
+    echo
+    echo "WARNING: this script should run as root, sudo!"
+    sudo -E $0 "$@"
+    exit $?
+fi
+
+if [ -e "$1" ]; then
+    fimg=$(readlink -e $1)
+fi
+
+cd $(dirname $0)
+
+if [ ! -n "$1" -a  ! -e "$fimg" ]; then
+    fimg=$(ls -1 build/tmp/deploy/images/*/*.wic)
+    n=( $fimg )
+    if [ ${#n[@]} -gt 1 ]; then
+        echo
+        echo "WARNING: more than one image found, choose one:"
+        echo
+        echo "$fimg"
+        echo
+        exit 1
+    fi
+fi
+
+if [ ! -e "$fimg" ]; then
+    echo
+    echo "ERROR: no any image or block device ${1:+'$1' }found, abort!"
+    echo
+    exit 1
+fi
+
+wicf=$fimg
+losetup -Pf $wicf
+ldev=$(losetup -j $wicf | cut -d: -f1 | tail -n1)
+echo loopdev:$ldev
+dd if=${ldev}p2 bs=1M of=${wicf/.wic/.rootfs} status=progress
+chown $(id -u).$(id -g) ${wicf/.wic/.rootfs}
+du -ms ${wicf/.wic/.rootfs}
+losetup -d $ldev