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-