Re: [PATCH v6] suggested changes for reproducibility patchset v6

["Roberto A. Foglietta" <roberto.foglietta@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2801 bytes --]

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-

[-- Attachment #2: Type: text/html, Size: 4361 bytes --]