diff mbox series

[meta-swupdate] swupdate classes: fix hash mismatch on master

Message ID 20241201211304.2199299-1-danwalkes@trellis-logic.com
State Changes Requested
Delegated to: Stefano Babic
Headers show
Series [meta-swupdate] swupdate classes: fix hash mismatch on master | expand

Commit Message

Dan Walkes Dec. 1, 2024, 9:13 p.m. UTC
The fix discussed in [1] resolves build issues on styhead but
introduces an issue which causes hash mismatches.

The first build succeeds to generate a working .swu, however
future builds fail with hash mismatch and a message like this:

```
[ERROR] : SWUPDATE failed [0] ERROR cpio_utils.c : __swupdate_copy : 670 : HASH mismatch : 5bb39965bfcedd87fc02bd51fac6636b924aed606d0984dcebae6a3250b01af6 <--> ff70a8dbc50a9ce2bad724554f5f14b8c933ad87fc544d054142755e8ef07f1e
[ERROR] : SWUPDATE failed [0] ERROR archive_handler.c : install_archive_image : 344 : Error copying extracted file
```

This happens because the modifications made to files in the
S directory only happen once.  This means there's a single
conversion of `$swupdate_get_sha256(` to the file sha, and future
builds use the hash previously placed in the file in the S directory
which is no longer correct when the rootfs file hash changes.

It would probably be possible to make a simpler change here which
splits the UNPACKDIR and S and continues to make runtime
edits in S.  However, it seems the intent of S and/or UNPACKDIR
is to contain unmodified source as discussed in [1].

This change instead:
1. Moves the sw-description file to WORKDIR for editing.
2. Continues to use S as source location for sw-description
and other source files which are not edited.
3. Makes all edits in WORKDIR.
4. When preparing the cpio, prepares from WORKDIR instead of
S. using any existing files in WORKDIR if they exist.  If they
do not exist, symlinks the relevant files in S instead.
6. Adds the -L option to cpio to prevent adding symlinks to the
cpio file, and avoid the need to copy from S to WORKDIR when
preparing the cpio.

1: https://groups.google.com/g/swupdate/c/8K-9H7C9o5E/m/a9fqhIOjAAAJ

Signed-off-by: Dan Walkes <danwalkes@trellis-logic.com>
---
 classes-recipe/swupdate-common.bbclass | 37 +++++++++++++++-----------
 classes-recipe/swupdate-image.bbclass  |  2 +-
 2 files changed, 23 insertions(+), 16 deletions(-)

Comments

Dan Walkes Dec. 1, 2024, 9:17 p.m. UTC | #1
Sending this for review and comment.

Khem I think as discussed at 
https://groups.google.com/g/swupdate/c/8K-9H7C9o5E/m/a9fqhIOjAAAJ the 
correct fix here is more complicated.

These changes work for me on 
https://github.com/OE4T/tegra-demo-distro/tree/master/layers/meta-tegrademo/dynamic-layers/meta-swupdate 
but I could use some help testing the encryption/signing setup since I 
don't have a demo setup for this.

Also if you prefer attempting to rollback and split S and UNPACKDIR for a 
simpler version which still modifies S let me know and I can try that 
instead.

On Sunday, December 1, 2024 at 2:13:11 PM UTC-7 Dan Walkes wrote:

> The fix discussed in [1] resolves build issues on styhead but
> introduces an issue which causes hash mismatches.
>
> The first build succeeds to generate a working .swu, however
> future builds fail with hash mismatch and a message like this:
>
> ```
> [ERROR] : SWUPDATE failed [0] ERROR cpio_utils.c : __swupdate_copy : 670 : 
> HASH mismatch : 
> 5bb39965bfcedd87fc02bd51fac6636b924aed606d0984dcebae6a3250b01af6 <--> 
> ff70a8dbc50a9ce2bad724554f5f14b8c933ad87fc544d054142755e8ef07f1e
> [ERROR] : SWUPDATE failed [0] ERROR archive_handler.c : 
> install_archive_image : 344 : Error copying extracted file
> ```
>
> This happens because the modifications made to files in the
> S directory only happen once. This means there's a single
> conversion of `$swupdate_get_sha256(` to the file sha, and future
> builds use the hash previously placed in the file in the S directory
> which is no longer correct when the rootfs file hash changes.
>
> It would probably be possible to make a simpler change here which
> splits the UNPACKDIR and S and continues to make runtime
> edits in S. However, it seems the intent of S and/or UNPACKDIR
> is to contain unmodified source as discussed in [1].
>
> This change instead:
> 1. Moves the sw-description file to WORKDIR for editing.
> 2. Continues to use S as source location for sw-description
> and other source files which are not edited.
> 3. Makes all edits in WORKDIR.
> 4. When preparing the cpio, prepares from WORKDIR instead of
> S. using any existing files in WORKDIR if they exist. If they
> do not exist, symlinks the relevant files in S instead.
> 6. Adds the -L option to cpio to prevent adding symlinks to the
> cpio file, and avoid the need to copy from S to WORKDIR when
> preparing the cpio.
>
> 1: https://groups.google.com/g/swupdate/c/8K-9H7C9o5E/m/a9fqhIOjAAAJ
>
> Signed-off-by: Dan Walkes <danw...@trellis-logic.com>
> ---
> classes-recipe/swupdate-common.bbclass | 37 +++++++++++++++-----------
> classes-recipe/swupdate-image.bbclass | 2 +-
> 2 files changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/classes-recipe/swupdate-common.bbclass 
> b/classes-recipe/swupdate-common.bbclass
> index 7b49561..085a7b5 100644
> --- a/classes-recipe/swupdate-common.bbclass
> +++ b/classes-recipe/swupdate-common.bbclass
> @@ -66,22 +66,22 @@ def swupdate_getdepends(d):
>
> return depstr
>
> -def swupdate_write_sha256(s):
> +def swupdate_write_sha256(workdir):
> import re
> write_lines = []
> - with open(os.path.join(s, "sw-description"), 'r') as f:
> + with open(os.path.join(workdir, "sw-description"), 'r') as f:
> for line in f:
> shastr = r"sha256.+=.+@(.+\")"
> m = 
> re.match(r"^(?P<before_placeholder>.+)(sha256|version).+[=:].*(?P<quote>[\'\"])@(?P<filename>.*)(?P=quote)", 
> line)
> if m:
> filename = m.group('filename')
> bb.warn("Syntax for sha256 changed, please use $swupdate_get_sha256(%s)" % 
> filename)
> - hash = swupdate_get_sha256(None, s, filename)
> + hash = swupdate_get_sha256(None, workdir, filename)
> write_lines.append(line.replace("@%s" % (filename), hash))
> else:
> write_lines.append(line)
>
> - with open(os.path.join(s, "sw-description"), 'w+') as f:
> + with open(os.path.join(workdir, "sw-description"), 'w+') as f:
> for line in write_lines:
> f.write(line)
>
> @@ -97,7 +97,7 @@ def swupdate_exec_functions(d, s, write_lines):
> write_lines[index] = line
>
>
> -def swupdate_expand_bitbake_variables(d, s):
> +def swupdate_expand_bitbake_variables(d, s, workdir):
> write_lines = []
>
> with open(os.path.join(s, "sw-description"), 'r') as f:
> @@ -131,7 +131,7 @@ def swupdate_expand_bitbake_variables(d, s):
>
> swupdate_exec_functions(d, s, write_lines)
>
> - with open(os.path.join(s, "sw-description"), 'w+') as f:
> + with open(os.path.join(workdir, "sw-description"), 'w+') as f:
> for line in write_lines:
> f.write(line)
>
> @@ -173,17 +173,18 @@ def prepare_sw_description(d):
> import subprocess
>
> s = d.getVar('S')
> - swupdate_expand_bitbake_variables(d, s)
> + workdir = d.getVar('WORKDIR')
> + swupdate_expand_bitbake_variables(d, s, workdir)
>
> - swupdate_write_sha256(s)
> + swupdate_write_sha256(workdir)
>
> encrypt = d.getVar('SWUPDATE_ENCRYPT_SWDESC')
> if encrypt:
> bb.note("Encryption of sw-description")
> - shutil.copyfile(os.path.join(s, 'sw-description'), os.path.join(s, 
> 'sw-description.plain'))
> + shutil.copyfile(os.path.join(workdir, 'sw-description'), 
> os.path.join(workdir, 'sw-description.plain'))
> key,iv = swupdate_extract_keys(d.getVar('SWUPDATE_AES_FILE'))
> - iv = swupdate_get_IV(d, s, 'sw-description')
> - swupdate_encrypt_file(os.path.join(s, 'sw-description.plain'), 
> os.path.join(s, 'sw-description'), key, iv)
> + iv = swupdate_get_IV(d, workdir, 'sw-description')
> + swupdate_encrypt_file(os.path.join(workdir, 'sw-description.plain'), 
> os.path.join(workdir, 'sw-description'), key, iv)
>
> signing = d.getVar('SWUPDATE_SIGNING')
> if signing == "1":
> @@ -191,8 +192,8 @@ def prepare_sw_description(d):
> signing = "RSA"
> if signing:
>
> - sw_desc_sig = os.path.join(s, 'sw-description.sig')
> - sw_desc = os.path.join(s, 'sw-description.plain' if encrypt else 
> 'sw-description')
> + sw_desc_sig = os.path.join(workdir, 'sw-description.sig')
> + sw_desc = os.path.join(workdir, 'sw-description.plain' if encrypt else 
> 'sw-description')
>
> if signing == "CUSTOM":
> signcmd = []
> @@ -233,6 +234,7 @@ def swupdate_add_src_uri(d, list_for_cpio):
> import shutil
>
> s = d.getVar('S')
> + workdir = d.getVar('WORKDIR')
> exclude = (d.getVar("SWUPDATE_SRC_URI_EXCLUDE") or "").split()
>
> fetch = bb.fetch2.Fetch([], d)
> @@ -310,9 +312,14 @@ def swupdate_add_artifacts(d, list_for_cpio):
>
> def swupdate_create_cpio(d, swudeploydir, list_for_cpio):
> s = d.getVar('S')
> - os.chdir(s)
> + workdir = d.getVar('WORKDIR')
> + os.chdir(workdir)
> + for file in list_for_cpio:
> + if not os.path.exists(file):
> + os.symlink(os.path.join(s, file), file)
> +
> updateimage = d.getVar('IMAGE_NAME') + '.swu'
> - line = 'for i in ' + ' '.join(list_for_cpio) + '; do echo $i;done | cpio 
> -ov -H crc --reproducible > ' + os.path.join(swudeploydir, updateimage)
> + line = 'for i in ' + ' '.join(list_for_cpio) + '; do echo $i;done | cpio 
> -ov -H crc --reproducible -L > ' + os.path.join(swudeploydir, updateimage)
> os.system(line)
> os.chdir(swudeploydir)
> updateimage_link = d.getVar('IMAGE_LINK_NAME')
> diff --git a/classes-recipe/swupdate-image.bbclass 
> b/classes-recipe/swupdate-image.bbclass
> index 1cd3eeb..bf8c5cc 100644
> --- a/classes-recipe/swupdate-image.bbclass
> +++ b/classes-recipe/swupdate-image.bbclass
> @@ -40,7 +40,7 @@ python do_swupdate_copy_swdescription() {
>
> import shutil
>
> - workdir = d.getVar('S')
> + workdir = d.getVar('WORKDIR')
> filespath = d.getVar('FILESPATH')
> sw_desc_path = bb.utils.which(filespath, "sw-description")
> shutil.copyfile(sw_desc_path, os.path.join(workdir, "sw-description"))
> -- 
> 2.34.1
>
>
Dan Walkes July 16, 2025, 9:14 p.m. UTC | #2
I've verified this is still an issue with latest master, and have updated 
the patch 
at https://github.com/Trellis-Logic/meta-swupdate/tree/fix-hash-issues and 
https://github.com/Trellis-Logic/meta-swupdate/commit/f829424cd38a1f549de050bffcc26434799e28ff 
with more details about how to reproduce and about the underlying issue 
causing it and also to rebase on the latest master.

The biggest new realization is that this only happens when `rm_work` is not 
enabled.  If you use `rm_work` this effectively is a workaround for this 
issue, at least as far as I've tested.  This likely explains why I appear 
to be the only one seeing this, as the underlying demo distro 
at https://github.com/OE4T/tegra-demo-distro does not use `rm_work` and 
this is how I've been testing it.

I think the patch I have is possibly not the simplest way to solve the 
problem and I'm willing to implement in a different way if anyone has 
suggestions.

If everyone else is content to just use rm_work as a workaround I'll just 
keep this in my local branch too, and perhaps I can change the defaults on 
tegra-demo-distro to use rm_work so we won't hit this either using upstream 
master.

If you'd like me to resend the current patch rebased on the latest master 
to the mailing list I'll do that too.  The only change is the description, 
however.

Dan
On Sunday, December 1, 2024 at 2:17:08 PM UTC-7 Dan Walkes wrote:

> Sending this for review and comment.
>
> Khem I think as discussed at 
> https://groups.google.com/g/swupdate/c/8K-9H7C9o5E/m/a9fqhIOjAAAJ the 
> correct fix here is more complicated.
>
> These changes work for me on 
> https://github.com/OE4T/tegra-demo-distro/tree/master/layers/meta-tegrademo/dynamic-layers/meta-swupdate 
> but I could use some help testing the encryption/signing setup since I 
> don't have a demo setup for this.
>
> Also if you prefer attempting to rollback and split S and UNPACKDIR for a 
> simpler version which still modifies S let me know and I can try that 
> instead.
>
> On Sunday, December 1, 2024 at 2:13:11 PM UTC-7 Dan Walkes wrote:
>
>> The fix discussed in [1] resolves build issues on styhead but 
>> introduces an issue which causes hash mismatches. 
>>
>> The first build succeeds to generate a working .swu, however 
>> future builds fail with hash mismatch and a message like this: 
>>
>> ``` 
>> [ERROR] : SWUPDATE failed [0] ERROR cpio_utils.c : __swupdate_copy : 670 
>> : HASH mismatch : 
>> 5bb39965bfcedd87fc02bd51fac6636b924aed606d0984dcebae6a3250b01af6 <--> 
>> ff70a8dbc50a9ce2bad724554f5f14b8c933ad87fc544d054142755e8ef07f1e 
>> [ERROR] : SWUPDATE failed [0] ERROR archive_handler.c : 
>> install_archive_image : 344 : Error copying extracted file 
>> ``` 
>>
>> This happens because the modifications made to files in the 
>> S directory only happen once. This means there's a single 
>> conversion of `$swupdate_get_sha256(` to the file sha, and future 
>> builds use the hash previously placed in the file in the S directory 
>> which is no longer correct when the rootfs file hash changes. 
>>
>> It would probably be possible to make a simpler change here which 
>> splits the UNPACKDIR and S and continues to make runtime 
>> edits in S. However, it seems the intent of S and/or UNPACKDIR 
>> is to contain unmodified source as discussed in [1]. 
>>
>> This change instead: 
>> 1. Moves the sw-description file to WORKDIR for editing. 
>> 2. Continues to use S as source location for sw-description 
>> and other source files which are not edited. 
>> 3. Makes all edits in WORKDIR. 
>> 4. When preparing the cpio, prepares from WORKDIR instead of 
>> S. using any existing files in WORKDIR if they exist. If they 
>> do not exist, symlinks the relevant files in S instead. 
>> 6. Adds the -L option to cpio to prevent adding symlinks to the 
>> cpio file, and avoid the need to copy from S to WORKDIR when 
>> preparing the cpio. 
>>
>> 1: https://groups.google.com/g/swupdate/c/8K-9H7C9o5E/m/a9fqhIOjAAAJ 
>>
>> Signed-off-by: Dan Walkes <danw...@trellis-logic.com> 
>> --- 
>> classes-recipe/swupdate-common.bbclass | 37 +++++++++++++++----------- 
>> classes-recipe/swupdate-image.bbclass | 2 +- 
>> 2 files changed, 23 insertions(+), 16 deletions(-) 
>>
>> diff --git a/classes-recipe/swupdate-common.bbclass 
>> b/classes-recipe/swupdate-common.bbclass 
>> index 7b49561..085a7b5 100644 
>> --- a/classes-recipe/swupdate-common.bbclass 
>> +++ b/classes-recipe/swupdate-common.bbclass 
>> @@ -66,22 +66,22 @@ def swupdate_getdepends(d): 
>>
>> return depstr 
>>
>> -def swupdate_write_sha256(s): 
>> +def swupdate_write_sha256(workdir): 
>> import re 
>> write_lines = [] 
>> - with open(os.path.join(s, "sw-description"), 'r') as f: 
>> + with open(os.path.join(workdir, "sw-description"), 'r') as f: 
>> for line in f: 
>> shastr = r"sha256.+=.+@(.+\")" 
>> m = 
>> re.match(r"^(?P<before_placeholder>.+)(sha256|version).+[=:].*(?P<quote>[\'\"])@(?P<filename>.*)(?P=quote)", 
>> line) 
>> if m: 
>> filename = m.group('filename') 
>> bb.warn("Syntax for sha256 changed, please use $swupdate_get_sha256(%s)" 
>> % filename) 
>> - hash = swupdate_get_sha256(None, s, filename) 
>> + hash = swupdate_get_sha256(None, workdir, filename) 
>> write_lines.append(line.replace("@%s" % (filename), hash)) 
>> else: 
>> write_lines.append(line) 
>>
>> - with open(os.path.join(s, "sw-description"), 'w+') as f: 
>> + with open(os.path.join(workdir, "sw-description"), 'w+') as f: 
>> for line in write_lines: 
>> f.write(line) 
>>
>> @@ -97,7 +97,7 @@ def swupdate_exec_functions(d, s, write_lines): 
>> write_lines[index] = line 
>>
>>
>> -def swupdate_expand_bitbake_variables(d, s): 
>> +def swupdate_expand_bitbake_variables(d, s, workdir): 
>> write_lines = [] 
>>
>> with open(os.path.join(s, "sw-description"), 'r') as f: 
>> @@ -131,7 +131,7 @@ def swupdate_expand_bitbake_variables(d, s): 
>>
>> swupdate_exec_functions(d, s, write_lines) 
>>
>> - with open(os.path.join(s, "sw-description"), 'w+') as f: 
>> + with open(os.path.join(workdir, "sw-description"), 'w+') as f: 
>> for line in write_lines: 
>> f.write(line) 
>>
>> @@ -173,17 +173,18 @@ def prepare_sw_description(d): 
>> import subprocess 
>>
>> s = d.getVar('S') 
>> - swupdate_expand_bitbake_variables(d, s) 
>> + workdir = d.getVar('WORKDIR') 
>> + swupdate_expand_bitbake_variables(d, s, workdir) 
>>
>> - swupdate_write_sha256(s) 
>> + swupdate_write_sha256(workdir) 
>>
>> encrypt = d.getVar('SWUPDATE_ENCRYPT_SWDESC') 
>> if encrypt: 
>> bb.note("Encryption of sw-description") 
>> - shutil.copyfile(os.path.join(s, 'sw-description'), os.path.join(s, 
>> 'sw-description.plain')) 
>> + shutil.copyfile(os.path.join(workdir, 'sw-description'), 
>> os.path.join(workdir, 'sw-description.plain')) 
>> key,iv = swupdate_extract_keys(d.getVar('SWUPDATE_AES_FILE')) 
>> - iv = swupdate_get_IV(d, s, 'sw-description') 
>> - swupdate_encrypt_file(os.path.join(s, 'sw-description.plain'), 
>> os.path.join(s, 'sw-description'), key, iv) 
>> + iv = swupdate_get_IV(d, workdir, 'sw-description') 
>> + swupdate_encrypt_file(os.path.join(workdir, 'sw-description.plain'), 
>> os.path.join(workdir, 'sw-description'), key, iv) 
>>
>> signing = d.getVar('SWUPDATE_SIGNING') 
>> if signing == "1": 
>> @@ -191,8 +192,8 @@ def prepare_sw_description(d): 
>> signing = "RSA" 
>> if signing: 
>>
>> - sw_desc_sig = os.path.join(s, 'sw-description.sig') 
>> - sw_desc = os.path.join(s, 'sw-description.plain' if encrypt else 
>> 'sw-description') 
>> + sw_desc_sig = os.path.join(workdir, 'sw-description.sig') 
>> + sw_desc = os.path.join(workdir, 'sw-description.plain' if encrypt else 
>> 'sw-description') 
>>
>> if signing == "CUSTOM": 
>> signcmd = [] 
>> @@ -233,6 +234,7 @@ def swupdate_add_src_uri(d, list_for_cpio): 
>> import shutil 
>>
>> s = d.getVar('S') 
>> + workdir = d.getVar('WORKDIR') 
>> exclude = (d.getVar("SWUPDATE_SRC_URI_EXCLUDE") or "").split() 
>>
>> fetch = bb.fetch2.Fetch([], d) 
>> @@ -310,9 +312,14 @@ def swupdate_add_artifacts(d, list_for_cpio): 
>>
>> def swupdate_create_cpio(d, swudeploydir, list_for_cpio): 
>> s = d.getVar('S') 
>> - os.chdir(s) 
>> + workdir = d.getVar('WORKDIR') 
>> + os.chdir(workdir) 
>> + for file in list_for_cpio: 
>> + if not os.path.exists(file): 
>> + os.symlink(os.path.join(s, file), file) 
>> + 
>> updateimage = d.getVar('IMAGE_NAME') + '.swu' 
>> - line = 'for i in ' + ' '.join(list_for_cpio) + '; do echo $i;done | 
>> cpio -ov -H crc --reproducible > ' + os.path.join(swudeploydir, 
>> updateimage) 
>> + line = 'for i in ' + ' '.join(list_for_cpio) + '; do echo $i;done | 
>> cpio -ov -H crc --reproducible -L > ' + os.path.join(swudeploydir, 
>> updateimage) 
>> os.system(line) 
>> os.chdir(swudeploydir) 
>> updateimage_link = d.getVar('IMAGE_LINK_NAME') 
>> diff --git a/classes-recipe/swupdate-image.bbclass 
>> b/classes-recipe/swupdate-image.bbclass 
>> index 1cd3eeb..bf8c5cc 100644 
>> --- a/classes-recipe/swupdate-image.bbclass 
>> +++ b/classes-recipe/swupdate-image.bbclass 
>> @@ -40,7 +40,7 @@ python do_swupdate_copy_swdescription() { 
>>
>> import shutil 
>>
>> - workdir = d.getVar('S') 
>> + workdir = d.getVar('WORKDIR') 
>> filespath = d.getVar('FILESPATH') 
>> sw_desc_path = bb.utils.which(filespath, "sw-description") 
>> shutil.copyfile(sw_desc_path, os.path.join(workdir, "sw-description")) 
>> -- 
>> 2.34.1 
>>
>>
Stefano Babic July 17, 2025, 3:14 p.m. UTC | #3
Hi Dan,

On 7/16/25 23:14, Dan Walkes wrote:
> I've verified this is still an issue with latest master, and have 
> updated the patch at https://github.com/Trellis-Logic/meta-swupdate/ 
> tree/fix-hash-issues and https://github.com/Trellis-Logic/meta-swupdate/ 
> commit/f829424cd38a1f549de050bffcc26434799e28ff with more details about 
> how to reproduce and about the underlying issue causing it and also to 
> rebase on the latest master.

Does it mean you have a new patch to be posted here for review or is the 
posted patch enough ?

I will first ask something - the issue you report seems due to some 
missing dependencies, and the task do_swupdate does not run again if you 
change rootfs. I am quite missing the role of rm_work, because it 
shouldn't play a role if dependency is correct.

You are mentioning that changes in rootfs does not reflect into the SWU. 
Is just sw-description affected in your test ?

Surely, sw-description must be regenerated if one of the artifacts changed.

> 
> The biggest new realization is that this only happens when `rm_work` is 
> not enabled. 

Well, I have not understood why this makes a difference. Can you explain 
this ?

> If you use `rm_work` this effectively is a workaround for 
> this issue, at least as far as I've tested.  This likely explains why I 
> appear to be the only one seeing this, as the underlying demo distro 
> at https://github.com/OE4T/tegra-demo-distro does not use `rm_work` and 
> this is how I've been testing it.
 > > I think the patch I have is possibly not the simplest way to solve the
> problem and I'm willing to implement in a different way if anyone has 
> suggestions.
> 
> If everyone else is content to just use rm_work as a workaround I'll 
> just keep this in my local branch too, and perhaps I can change the 
> defaults on tegra-demo-distro to use rm_work so we won't hit this either 
> using upstream master.
> 
> If you'd like me to resend the current patch rebased on the latest 
> master to the mailing list I'll do that too.  The only change is the 
> description, however.

Ok, the just wait. I review the original patch, and you can resend then 
after comments.

Best regards,
Stefano Babic

> 
> Dan
> On Sunday, December 1, 2024 at 2:17:08 PM UTC-7 Dan Walkes wrote:
> 
>     Sending this for review and comment.
> 
>     Khem I think as discussed at https://groups.google.com/g/swupdate/
>     c/8K-9H7C9o5E/m/a9fqhIOjAAAJ <https://groups.google.com/g/swupdate/
>     c/8K-9H7C9o5E/m/a9fqhIOjAAAJ> the correct fix here is more complicated.
> 
>     These changes work for me on https://github.com/OE4T/tegra-demo-
>     distro/tree/master/layers/meta-tegrademo/dynamic-layers/meta-
>     swupdate <https://github.com/OE4T/tegra-demo-distro/tree/master/
>     layers/meta-tegrademo/dynamic-layers/meta-swupdate> but I could use
>     some help testing the encryption/signing setup since I don't have a
>     demo setup for this.
> 
>     Also if you prefer attempting to rollback and split S and UNPACKDIR
>     for a simpler version which still modifies S let me know and I can
>     try that instead.
> 
>     On Sunday, December 1, 2024 at 2:13:11 PM UTC-7 Dan Walkes wrote:
> 
>         The fix discussed in [1] resolves build issues on styhead but
>         introduces an issue which causes hash mismatches.
> 
>         The first build succeeds to generate a working .swu, however
>         future builds fail with hash mismatch and a message like this:
> 
>         ```
>         [ERROR] : SWUPDATE failed [0] ERROR cpio_utils.c :
>         __swupdate_copy : 670 : HASH mismatch :
>         5bb39965bfcedd87fc02bd51fac6636b924aed606d0984dcebae6a3250b01af6
>         <-->
>         ff70a8dbc50a9ce2bad724554f5f14b8c933ad87fc544d054142755e8ef07f1e
>         [ERROR] : SWUPDATE failed [0] ERROR archive_handler.c :
>         install_archive_image : 344 : Error copying extracted file
>         ```
> 
>         This happens because the modifications made to files in the
>         S directory only happen once. This means there's a single
>         conversion of `$swupdate_get_sha256(` to the file sha, and future
>         builds use the hash previously placed in the file in the S
>         directory
>         which is no longer correct when the rootfs file hash changes.
> 
>         It would probably be possible to make a simpler change here which
>         splits the UNPACKDIR and S and continues to make runtime
>         edits in S. However, it seems the intent of S and/or UNPACKDIR
>         is to contain unmodified source as discussed in [1].
> 
>         This change instead:
>         1. Moves the sw-description file to WORKDIR for editing.
>         2. Continues to use S as source location for sw-description
>         and other source files which are not edited.
>         3. Makes all edits in WORKDIR.
>         4. When preparing the cpio, prepares from WORKDIR instead of
>         S. using any existing files in WORKDIR if they exist. If they
>         do not exist, symlinks the relevant files in S instead.
>         6. Adds the -L option to cpio to prevent adding symlinks to the
>         cpio file, and avoid the need to copy from S to WORKDIR when
>         preparing the cpio.
> 
>         1: https://groups.google.com/g/swupdate/c/8K-9H7C9o5E/m/
>         a9fqhIOjAAAJ <https://groups.google.com/g/swupdate/
>         c/8K-9H7C9o5E/m/a9fqhIOjAAAJ>
> 
>         Signed-off-by: Dan Walkes <danw...@trellis-logic.com>
>         ---
>         classes-recipe/swupdate-common.bbclass | 37 ++++++++++++++
>         +-----------
>         classes-recipe/swupdate-image.bbclass | 2 +-
>         2 files changed, 23 insertions(+), 16 deletions(-)
> 
>         diff --git a/classes-recipe/swupdate-common.bbclass b/classes-
>         recipe/swupdate-common.bbclass
>         index 7b49561..085a7b5 100644
>         --- a/classes-recipe/swupdate-common.bbclass
>         +++ b/classes-recipe/swupdate-common.bbclass
>         @@ -66,22 +66,22 @@ def swupdate_getdepends(d):
> 
>         return depstr
> 
>         -def swupdate_write_sha256(s):
>         +def swupdate_write_sha256(workdir):
>         import re
>         write_lines = []
>         - with open(os.path.join(s, "sw-description"), 'r') as f:
>         + with open(os.path.join(workdir, "sw-description"), 'r') as f:
>         for line in f:
>         shastr = r"sha256.+=.+@(.+\")"
>         m = re.match(r"^(?P<before_placeholder>.+)(sha256|version).
>         +[=:].*(?P<quote>[\'\"])@(?P<filename>.*)(?P=quote)", line)
>         if m:
>         filename = m.group('filename')
>         bb.warn("Syntax for sha256 changed, please use
>         $swupdate_get_sha256(%s)" % filename)
>         - hash = swupdate_get_sha256(None, s, filename)
>         + hash = swupdate_get_sha256(None, workdir, filename)
>         write_lines.append(line.replace("@%s" % (filename), hash))
>         else:
>         write_lines.append(line)
> 
>         - with open(os.path.join(s, "sw-description"), 'w+') as f:
>         + with open(os.path.join(workdir, "sw-description"), 'w+') as f:
>         for line in write_lines:
>         f.write(line)
> 
>         @@ -97,7 +97,7 @@ def swupdate_exec_functions(d, s, write_lines):
>         write_lines[index] = line
> 
> 
>         -def swupdate_expand_bitbake_variables(d, s):
>         +def swupdate_expand_bitbake_variables(d, s, workdir):
>         write_lines = []
> 
>         with open(os.path.join(s, "sw-description"), 'r') as f:
>         @@ -131,7 +131,7 @@ def swupdate_expand_bitbake_variables(d, s):
> 
>         swupdate_exec_functions(d, s, write_lines)
> 
>         - with open(os.path.join(s, "sw-description"), 'w+') as f:
>         + with open(os.path.join(workdir, "sw-description"), 'w+') as f:
>         for line in write_lines:
>         f.write(line)
> 
>         @@ -173,17 +173,18 @@ def prepare_sw_description(d):
>         import subprocess
> 
>         s = d.getVar('S')
>         - swupdate_expand_bitbake_variables(d, s)
>         + workdir = d.getVar('WORKDIR')
>         + swupdate_expand_bitbake_variables(d, s, workdir)
> 
>         - swupdate_write_sha256(s)
>         + swupdate_write_sha256(workdir)
> 
>         encrypt = d.getVar('SWUPDATE_ENCRYPT_SWDESC')
>         if encrypt:
>         bb.note("Encryption of sw-description")
>         - shutil.copyfile(os.path.join(s, 'sw-description'),
>         os.path.join(s, 'sw-description.plain'))
>         + shutil.copyfile(os.path.join(workdir, 'sw-description'),
>         os.path.join(workdir, 'sw-description.plain'))
>         key,iv = swupdate_extract_keys(d.getVar('SWUPDATE_AES_FILE'))
>         - iv = swupdate_get_IV(d, s, 'sw-description')
>         - swupdate_encrypt_file(os.path.join(s, 'sw-description.plain'),
>         os.path.join(s, 'sw-description'), key, iv)
>         + iv = swupdate_get_IV(d, workdir, 'sw-description')
>         + swupdate_encrypt_file(os.path.join(workdir, 'sw-
>         description.plain'), os.path.join(workdir, 'sw-description'),
>         key, iv)
> 
>         signing = d.getVar('SWUPDATE_SIGNING')
>         if signing == "1":
>         @@ -191,8 +192,8 @@ def prepare_sw_description(d):
>         signing = "RSA"
>         if signing:
> 
>         - sw_desc_sig = os.path.join(s, 'sw-description.sig')
>         - sw_desc = os.path.join(s, 'sw-description.plain' if encrypt
>         else 'sw-description')
>         + sw_desc_sig = os.path.join(workdir, 'sw-description.sig')
>         + sw_desc = os.path.join(workdir, 'sw-description.plain' if
>         encrypt else 'sw-description')
> 
>         if signing == "CUSTOM":
>         signcmd = []
>         @@ -233,6 +234,7 @@ def swupdate_add_src_uri(d, list_for_cpio):
>         import shutil
> 
>         s = d.getVar('S')
>         + workdir = d.getVar('WORKDIR')
>         exclude = (d.getVar("SWUPDATE_SRC_URI_EXCLUDE") or "").split()
> 
>         fetch = bb.fetch2.Fetch([], d)
>         @@ -310,9 +312,14 @@ def swupdate_add_artifacts(d, list_for_cpio):
> 
>         def swupdate_create_cpio(d, swudeploydir, list_for_cpio):
>         s = d.getVar('S')
>         - os.chdir(s)
>         + workdir = d.getVar('WORKDIR')
>         + os.chdir(workdir)
>         + for file in list_for_cpio:
>         + if not os.path.exists(file):
>         + os.symlink(os.path.join(s, file), file)
>         +
>         updateimage = d.getVar('IMAGE_NAME') + '.swu'
>         - line = 'for i in ' + ' '.join(list_for_cpio) + '; do echo
>         $i;done | cpio -ov -H crc --reproducible > ' +
>         os.path.join(swudeploydir, updateimage)
>         + line = 'for i in ' + ' '.join(list_for_cpio) + '; do echo
>         $i;done | cpio -ov -H crc --reproducible -L > ' +
>         os.path.join(swudeploydir, updateimage)
>         os.system(line)
>         os.chdir(swudeploydir)
>         updateimage_link = d.getVar('IMAGE_LINK_NAME')
>         diff --git a/classes-recipe/swupdate-image.bbclass b/classes-
>         recipe/swupdate-image.bbclass
>         index 1cd3eeb..bf8c5cc 100644
>         --- a/classes-recipe/swupdate-image.bbclass
>         +++ b/classes-recipe/swupdate-image.bbclass
>         @@ -40,7 +40,7 @@ python do_swupdate_copy_swdescription() {
> 
>         import shutil
> 
>         - workdir = d.getVar('S')
>         + workdir = d.getVar('WORKDIR')
>         filespath = d.getVar('FILESPATH')
>         sw_desc_path = bb.utils.which(filespath, "sw-description")
>         shutil.copyfile(sw_desc_path, os.path.join(workdir, "sw-
>         description"))
>         -- 
>         2.34.1
> 
> -- 
> You received this message because you are subscribed to the Google 
> Groups "swupdate" group.
> To unsubscribe from this group and stop receiving emails from it, send 
> an email to swupdate+unsubscribe@googlegroups.com 
> <mailto:swupdate+unsubscribe@googlegroups.com>.
> To view this discussion visit https://groups.google.com/d/msgid/ 
> swupdate/1cf15b67-2723-4c0f-8e66-8d2d32af1ec9n%40googlegroups.com 
> <https://groups.google.com/d/msgid/ 
> swupdate/1cf15b67-2723-4c0f-8e66-8d2d32af1ec9n%40googlegroups.com? 
> utm_medium=email&utm_source=footer>.
Stefano Babic July 17, 2025, 3:30 p.m. UTC | #4
Hi Dan,

On 12/1/24 22:13, Dan Walkes wrote:
> The fix discussed in [1] resolves build issues on styhead but
> introduces an issue which causes hash mismatches.
> 
> The first build succeeds to generate a working .swu, however
> future builds fail with hash mismatch and a message like this:
> 
> ```
> [ERROR] : SWUPDATE failed [0] ERROR cpio_utils.c : __swupdate_copy : 670 : HASH mismatch : 5bb39965bfcedd87fc02bd51fac6636b924aed606d0984dcebae6a3250b01af6 <--> ff70a8dbc50a9ce2bad724554f5f14b8c933ad87fc544d054142755e8ef07f1e
> [ERROR] : SWUPDATE failed [0] ERROR archive_handler.c : install_archive_image : 344 : Error copying extracted file
> ```
> 
> This happens because the modifications made to files in the
> S directory only happen once.  This means there's a single
> conversion of `$swupdate_get_sha256(` to the file sha, and future
> builds use the hash previously placed in the file in the S directory
> which is no longer correct when the rootfs file hash changes.
> 
> It would probably be possible to make a simpler change here which
> splits the UNPACKDIR and S and continues to make runtime
> edits in S.  However, it seems the intent of S and/or UNPACKDIR
> is to contain unmodified source as discussed in [1].

In principle it is ok to move from $S to $WORKDIR - until scarthgap, $S 
was $WORKDIR.

swupdate_add_artifacts() already copies files from DEPLOY_DIR (listed in 
SWUPDATE_IMAGES) into a work directory (it was $S). Is it not enough by 
copying them into $WORKDIR then without links ?

> 
> This change instead:
> 1. Moves the sw-description file to WORKDIR for editing.
> 2. Continues to use S as source location for sw-description
> and other source files which are not edited.
> 3. Makes all edits in WORKDIR.
> 4. When preparing the cpio, prepares from WORKDIR instead of
> S. using any existing files in WORKDIR if they exist.  If they
> do not exist, symlinks the relevant files in S instead.
> 6. Adds the -L option to cpio to prevent adding symlinks to the
> cpio file, and avoid the need to copy from S to WORKDIR when
> preparing the cpio.
> 
> 1: https://groups.google.com/g/swupdate/c/8K-9H7C9o5E/m/a9fqhIOjAAAJ
> 
> Signed-off-by: Dan Walkes <danwalkes@trellis-logic.com>
> ---
>   classes-recipe/swupdate-common.bbclass | 37 +++++++++++++++-----------
>   classes-recipe/swupdate-image.bbclass  |  2 +-
>   2 files changed, 23 insertions(+), 16 deletions(-)
> 
> diff --git a/classes-recipe/swupdate-common.bbclass b/classes-recipe/swupdate-common.bbclass
> index 7b49561..085a7b5 100644
> --- a/classes-recipe/swupdate-common.bbclass
> +++ b/classes-recipe/swupdate-common.bbclass
> @@ -66,22 +66,22 @@ def swupdate_getdepends(d):
>   
>       return depstr
>   
> -def swupdate_write_sha256(s):
> +def swupdate_write_sha256(workdir):

"s" here is just the parameter of the function. What it counts, it is 
how the function is called. Do we really need to change it ?

>       import re
>       write_lines = []
> -    with open(os.path.join(s, "sw-description"), 'r') as f:
> +    with open(os.path.join(workdir, "sw-description"), 'r') as f:
>          for line in f:
>             shastr = r"sha256.+=.+@(.+\")"
>             m = re.match(r"^(?P<before_placeholder>.+)(sha256|version).+[=:].*(?P<quote>[\'\"])@(?P<filename>.*)(?P=quote)", line)
>             if m:
>                 filename = m.group('filename')
>                 bb.warn("Syntax for sha256 changed, please use $swupdate_get_sha256(%s)" % filename)
> -              hash = swupdate_get_sha256(None, s, filename)
> +              hash = swupdate_get_sha256(None, workdir, filename)
>                 write_lines.append(line.replace("@%s" % (filename), hash))
>             else:
>                 write_lines.append(line)
>   
> -    with open(os.path.join(s, "sw-description"), 'w+') as f:
> +    with open(os.path.join(workdir, "sw-description"), 'w+') as f:
>           for line in write_lines:
>               f.write(line)
>   
> @@ -97,7 +97,7 @@ def swupdate_exec_functions(d, s, write_lines):
>               write_lines[index] = line
>   
>   
> -def swupdate_expand_bitbake_variables(d, s):
> +def swupdate_expand_bitbake_variables(d, s, workdir):
>       write_lines = []
>   
>       with open(os.path.join(s, "sw-description"), 'r') as f:
> @@ -131,7 +131,7 @@ def swupdate_expand_bitbake_variables(d, s):
>   
>       swupdate_exec_functions(d, s, write_lines)
>   
> -    with open(os.path.join(s, "sw-description"), 'w+') as f:
> +    with open(os.path.join(workdir, "sw-description"), 'w+') as f:
>           for line in write_lines:
>               f.write(line)

ok - so we have original sw-description in $S, generated sw-description 
in $WORKDIR

>   
> @@ -173,17 +173,18 @@ def prepare_sw_description(d):
>       import subprocess
>   
>       s = d.getVar('S')
> -    swupdate_expand_bitbake_variables(d, s)
> +    workdir = d.getVar('WORKDIR')
> +    swupdate_expand_bitbake_variables(d, s, workdir)
>   
> -    swupdate_write_sha256(s)
> +    swupdate_write_sha256(workdir)
>   
>       encrypt = d.getVar('SWUPDATE_ENCRYPT_SWDESC')
>       if encrypt:
>           bb.note("Encryption of sw-description")
> -        shutil.copyfile(os.path.join(s, 'sw-description'), os.path.join(s, 'sw-description.plain'))
> +        shutil.copyfile(os.path.join(workdir, 'sw-description'), os.path.join(workdir, 'sw-description.plain'))
>           key,iv = swupdate_extract_keys(d.getVar('SWUPDATE_AES_FILE'))
> -        iv = swupdate_get_IV(d, s, 'sw-description')
> -        swupdate_encrypt_file(os.path.join(s, 'sw-description.plain'), os.path.join(s, 'sw-description'), key, iv)
> +        iv = swupdate_get_IV(d, workdir, 'sw-description')
> +        swupdate_encrypt_file(os.path.join(workdir, 'sw-description.plain'), os.path.join(workdir, 'sw-description'), key, iv)
>   
>       signing = d.getVar('SWUPDATE_SIGNING')
>       if signing == "1":
> @@ -191,8 +192,8 @@ def prepare_sw_description(d):
>           signing = "RSA"
>       if signing:
>   
> -        sw_desc_sig = os.path.join(s, 'sw-description.sig')
> -        sw_desc =  os.path.join(s, 'sw-description.plain' if encrypt else 'sw-description')
> +        sw_desc_sig = os.path.join(workdir, 'sw-description.sig')
> +        sw_desc =  os.path.join(workdir, 'sw-description.plain' if encrypt else 'sw-description')
>   
>           if signing == "CUSTOM":
>               signcmd = []
> @@ -233,6 +234,7 @@ def swupdate_add_src_uri(d, list_for_cpio):
>       import shutil
>   
>       s = d.getVar('S')
> +    workdir = d.getVar('WORKDIR')
>       exclude = (d.getVar("SWUPDATE_SRC_URI_EXCLUDE") or "").split()
>   
>       fetch = bb.fetch2.Fetch([], d)
> @@ -310,9 +312,14 @@ def swupdate_add_artifacts(d, list_for_cpio):
>   
>   def swupdate_create_cpio(d, swudeploydir, list_for_cpio):
>       s = d.getVar('S')
> -    os.chdir(s)
> +    workdir = d.getVar('WORKDIR')
> +    os.chdir(workdir)
> +    for file in list_for_cpio:
> +        if not os.path.exists(file):
> +            os.symlink(os.path.join(s, file), file)

So we are talking here about files in SRC_URI, right ? Images listed are 
also searched in $DEPLOY_DIR_IMAGE. They are worked in 
swupdate_add_src_uri(), and they are copied into WORKDIR by changing 
"dst" in that function.

> +
>       updateimage = d.getVar('IMAGE_NAME') + '.swu'
> -    line = 'for i in ' + ' '.join(list_for_cpio) + '; do echo $i;done | cpio -ov -H crc --reproducible > ' + os.path.join(swudeploydir, updateimage)
> +    line = 'for i in ' + ' '.join(list_for_cpio) + '; do echo $i;done | cpio -ov -H crc --reproducible -L > ' + os.path.join(swudeploydir, updateimage)
>       os.system(line)
>       os.chdir(swudeploydir)
>       updateimage_link = d.getVar('IMAGE_LINK_NAME')
> diff --git a/classes-recipe/swupdate-image.bbclass b/classes-recipe/swupdate-image.bbclass
> index 1cd3eeb..bf8c5cc 100644
> --- a/classes-recipe/swupdate-image.bbclass
> +++ b/classes-recipe/swupdate-image.bbclass
> @@ -40,7 +40,7 @@ python do_swupdate_copy_swdescription() {
>   
>       import shutil
>   
> -    workdir = d.getVar('S')
> +    workdir = d.getVar('WORKDIR')
>       filespath = d.getVar('FILESPATH')
>       sw_desc_path = bb.utils.which(filespath, "sw-description")
>       shutil.copyfile(sw_desc_path, os.path.join(workdir, "sw-description"))

Best regards,
Stefano
Dan Walkes July 17, 2025, 4:21 p.m. UTC | #5
Hi Stefano,
Thanks for the response.

On Thu, Jul 17, 2025 at 9:14 AM Stefano Babic
<stefano.babic@swupdate.org> wrote:
>
> Hi Dan,
>
> On 7/16/25 23:14, Dan Walkes wrote:
> > I've verified this is still an issue with latest master, and have
> > updated the patch at https://github.com/Trellis-Logic/meta-swupdate/
> > tree/fix-hash-issues and https://github.com/Trellis-Logic/meta-swupdate/
> > commit/f829424cd38a1f549de050bffcc26434799e28ff with more details about
> > how to reproduce and about the underlying issue causing it and also to
> > rebase on the latest master.
>
> Does it mean you have a new patch to be posted here for review or is the
> posted patch enough ?

The posted patch is enough, just rebased on the latest master.  I just
added more detail to the description.

>
> I will first ask something - the issue you report seems due to some
> missing dependencies, and the task do_swupdate does not run again if you
> change rootfs. I am quite missing the role of rm_work, because it
> shouldn't play a role if dependency is correct.

The task do_swupdate does run, and the sw-description file is rebuilt
(timestamp changes).  It's just rebuilt from the copy used in the
previous build, which has overwritten the `$swupdate_get_sha256(` with
the actual file sha.  Therefore the logic at
https://github.com/sbabic/meta-swupdate/blob/cbd0ed50a61dd7cbad09740d5d0edd295e56f16a/classes-recipe/swupdate-common.bbclass#L72-L74
does not recognize the sha field and does not overwrite the sha from
previous build with the new sha.

If rm_work is used, then the sw-description file is removed from the S
directory after the build.  The next build generates the
sw-description from the copy provided in the SRC_URI which includes
the `$swupdate_get_sha256(` placeholders, and everything works.

>
> You are mentioning that changes in rootfs does not reflect into the SWU.
> Is just sw-description affected in your test ?

It's just the sha field in the sw-description I've noticed this
problem with, but I think any variable expansion could have the same
issue, if the variable changes without re-initializing the
sw-description file from the SRC_URI and instead using a copy where
variable expansion has already been performed as a part of

>
> Surely, sw-description must be regenerated if one of the artifacts changed.

It's definitely regenerated in that the timestamp changes and the
logic in swupdate_expand_bitbake_variables and swupdate_write_sha256
is run.  However both functions run on a sw-description where variable
placeholders have already been replaced in a previous run.  The reason
is that the sw-description is not replaced in the S directory with the
copy in SRC_URI unless rm_work is used, at least in my
testing/configuration.

Maybe one other way to solve this would be for prepare_sw_description
to overwrite the sw-description in S with the copy from SRC_URI.  It
looks like there's some logic today which specifically avoids this for
sw-description at
https://github.com/sbabic/meta-swupdate/blob/cbd0ed50a61dd7cbad09740d5d0edd295e56f16a/classes-recipe/swupdate-common.bbclass#L256
which went in at
https://github.com/sbabic/meta-swupdate/commit/cfb7ca5b3dc37f2889205bd79757f0c0b4410ef4
however.

>
> >
> > The biggest new realization is that this only happens when `rm_work` is
> > not enabled.
>
> Well, I have not understood why this makes a difference. Can you explain
> this ?

I think I have above but let me know if this is unclear.  Also if
there's a public example you'd like me to demonstrate this on, let me
know.  I think I could try this with raspberry pi on
https://github.com/sbabic/meta-swupdate-boards.

I expect you'll be able to reproduce this in any build setup based on
master if you:

1. Turn off rm_work on your builds, or for at least your swu image recipe.
2. Build the swu target at least once.
3. Make some modification to the rootfs.
4. Build the swu target again.
5. Run the swupdate on target.

At step 4 if you look at the $S directory in the build/tmp/work subdir
you'll see the timestamp of the sw-description file is updated but the
hash has not changed and does not match the hash of the updated rootfs
file, but instead contains the has for the rootfs built in step 2.

>
> > If you use `rm_work` this effectively is a workaround for
> > this issue, at least as far as I've tested.  This likely explains why I
> > appear to be the only one seeing this, as the underlying demo distro
> > at https://github.com/OE4T/tegra-demo-distro does not use `rm_work` and
> > this is how I've been testing it.
>  > > I think the patch I have is possibly not the simplest way to solve the
> > problem and I'm willing to implement in a different way if anyone has
> > suggestions.
> >
> > If everyone else is content to just use rm_work as a workaround I'll
> > just keep this in my local branch too, and perhaps I can change the
> > defaults on tegra-demo-distro to use rm_work so we won't hit this either
> > using upstream master.
> >
> > If you'd like me to resend the current patch rebased on the latest
> > master to the mailing list I'll do that too.  The only change is the
> > description, however.
>
> Ok, the just wait. I review the original patch, and you can resend then
> after comments.
>

OK I'll respond to the review now, thank you.

> Best regards,
> Stefano Babic
>
> >
> > Dan
> > On Sunday, December 1, 2024 at 2:17:08 PM UTC-7 Dan Walkes wrote:
> >
> >     Sending this for review and comment.
> >
> >     Khem I think as discussed at https://groups.google.com/g/swupdate/
> >     c/8K-9H7C9o5E/m/a9fqhIOjAAAJ <https://groups.google.com/g/swupdate/
> >     c/8K-9H7C9o5E/m/a9fqhIOjAAAJ> the correct fix here is more complicated.
> >
> >     These changes work for me on https://github.com/OE4T/tegra-demo-
> >     distro/tree/master/layers/meta-tegrademo/dynamic-layers/meta-
> >     swupdate <https://github.com/OE4T/tegra-demo-distro/tree/master/
> >     layers/meta-tegrademo/dynamic-layers/meta-swupdate> but I could use
> >     some help testing the encryption/signing setup since I don't have a
> >     demo setup for this.
> >
> >     Also if you prefer attempting to rollback and split S and UNPACKDIR
> >     for a simpler version which still modifies S let me know and I can
> >     try that instead.
> >
> >     On Sunday, December 1, 2024 at 2:13:11 PM UTC-7 Dan Walkes wrote:
> >
> >         The fix discussed in [1] resolves build issues on styhead but
> >         introduces an issue which causes hash mismatches.
> >
> >         The first build succeeds to generate a working .swu, however
> >         future builds fail with hash mismatch and a message like this:
> >
> >         ```
> >         [ERROR] : SWUPDATE failed [0] ERROR cpio_utils.c :
> >         __swupdate_copy : 670 : HASH mismatch :
> >         5bb39965bfcedd87fc02bd51fac6636b924aed606d0984dcebae6a3250b01af6
> >         <-->
> >         ff70a8dbc50a9ce2bad724554f5f14b8c933ad87fc544d054142755e8ef07f1e
> >         [ERROR] : SWUPDATE failed [0] ERROR archive_handler.c :
> >         install_archive_image : 344 : Error copying extracted file
> >         ```
> >
> >         This happens because the modifications made to files in the
> >         S directory only happen once. This means there's a single
> >         conversion of `$swupdate_get_sha256(` to the file sha, and future
> >         builds use the hash previously placed in the file in the S
> >         directory
> >         which is no longer correct when the rootfs file hash changes.
> >
> >         It would probably be possible to make a simpler change here which
> >         splits the UNPACKDIR and S and continues to make runtime
> >         edits in S. However, it seems the intent of S and/or UNPACKDIR
> >         is to contain unmodified source as discussed in [1].
> >
> >         This change instead:
> >         1. Moves the sw-description file to WORKDIR for editing.
> >         2. Continues to use S as source location for sw-description
> >         and other source files which are not edited.
> >         3. Makes all edits in WORKDIR.
> >         4. When preparing the cpio, prepares from WORKDIR instead of
> >         S. using any existing files in WORKDIR if they exist. If they
> >         do not exist, symlinks the relevant files in S instead.
> >         6. Adds the -L option to cpio to prevent adding symlinks to the
> >         cpio file, and avoid the need to copy from S to WORKDIR when
> >         preparing the cpio.
> >
> >         1: https://groups.google.com/g/swupdate/c/8K-9H7C9o5E/m/
> >         a9fqhIOjAAAJ <https://groups.google.com/g/swupdate/
> >         c/8K-9H7C9o5E/m/a9fqhIOjAAAJ>
> >
> >         Signed-off-by: Dan Walkes <danw...@trellis-logic.com>
> >         ---
> >         classes-recipe/swupdate-common.bbclass | 37 ++++++++++++++
> >         +-----------
> >         classes-recipe/swupdate-image.bbclass | 2 +-
> >         2 files changed, 23 insertions(+), 16 deletions(-)
> >
> >         diff --git a/classes-recipe/swupdate-common.bbclass b/classes-
> >         recipe/swupdate-common.bbclass
> >         index 7b49561..085a7b5 100644
> >         --- a/classes-recipe/swupdate-common.bbclass
> >         +++ b/classes-recipe/swupdate-common.bbclass
> >         @@ -66,22 +66,22 @@ def swupdate_getdepends(d):
> >
> >         return depstr
> >
> >         -def swupdate_write_sha256(s):
> >         +def swupdate_write_sha256(workdir):
> >         import re
> >         write_lines = []
> >         - with open(os.path.join(s, "sw-description"), 'r') as f:
> >         + with open(os.path.join(workdir, "sw-description"), 'r') as f:
> >         for line in f:
> >         shastr = r"sha256.+=.+@(.+\")"
> >         m = re.match(r"^(?P<before_placeholder>.+)(sha256|version).
> >         +[=:].*(?P<quote>[\'\"])@(?P<filename>.*)(?P=quote)", line)
> >         if m:
> >         filename = m.group('filename')
> >         bb.warn("Syntax for sha256 changed, please use
> >         $swupdate_get_sha256(%s)" % filename)
> >         - hash = swupdate_get_sha256(None, s, filename)
> >         + hash = swupdate_get_sha256(None, workdir, filename)
> >         write_lines.append(line.replace("@%s" % (filename), hash))
> >         else:
> >         write_lines.append(line)
> >
> >         - with open(os.path.join(s, "sw-description"), 'w+') as f:
> >         + with open(os.path.join(workdir, "sw-description"), 'w+') as f:
> >         for line in write_lines:
> >         f.write(line)
> >
> >         @@ -97,7 +97,7 @@ def swupdate_exec_functions(d, s, write_lines):
> >         write_lines[index] = line
> >
> >
> >         -def swupdate_expand_bitbake_variables(d, s):
> >         +def swupdate_expand_bitbake_variables(d, s, workdir):
> >         write_lines = []
> >
> >         with open(os.path.join(s, "sw-description"), 'r') as f:
> >         @@ -131,7 +131,7 @@ def swupdate_expand_bitbake_variables(d, s):
> >
> >         swupdate_exec_functions(d, s, write_lines)
> >
> >         - with open(os.path.join(s, "sw-description"), 'w+') as f:
> >         + with open(os.path.join(workdir, "sw-description"), 'w+') as f:
> >         for line in write_lines:
> >         f.write(line)
> >
> >         @@ -173,17 +173,18 @@ def prepare_sw_description(d):
> >         import subprocess
> >
> >         s = d.getVar('S')
> >         - swupdate_expand_bitbake_variables(d, s)
> >         + workdir = d.getVar('WORKDIR')
> >         + swupdate_expand_bitbake_variables(d, s, workdir)
> >
> >         - swupdate_write_sha256(s)
> >         + swupdate_write_sha256(workdir)
> >
> >         encrypt = d.getVar('SWUPDATE_ENCRYPT_SWDESC')
> >         if encrypt:
> >         bb.note("Encryption of sw-description")
> >         - shutil.copyfile(os.path.join(s, 'sw-description'),
> >         os.path.join(s, 'sw-description.plain'))
> >         + shutil.copyfile(os.path.join(workdir, 'sw-description'),
> >         os.path.join(workdir, 'sw-description.plain'))
> >         key,iv = swupdate_extract_keys(d.getVar('SWUPDATE_AES_FILE'))
> >         - iv = swupdate_get_IV(d, s, 'sw-description')
> >         - swupdate_encrypt_file(os.path.join(s, 'sw-description.plain'),
> >         os.path.join(s, 'sw-description'), key, iv)
> >         + iv = swupdate_get_IV(d, workdir, 'sw-description')
> >         + swupdate_encrypt_file(os.path.join(workdir, 'sw-
> >         description.plain'), os.path.join(workdir, 'sw-description'),
> >         key, iv)
> >
> >         signing = d.getVar('SWUPDATE_SIGNING')
> >         if signing == "1":
> >         @@ -191,8 +192,8 @@ def prepare_sw_description(d):
> >         signing = "RSA"
> >         if signing:
> >
> >         - sw_desc_sig = os.path.join(s, 'sw-description.sig')
> >         - sw_desc = os.path.join(s, 'sw-description.plain' if encrypt
> >         else 'sw-description')
> >         + sw_desc_sig = os.path.join(workdir, 'sw-description.sig')
> >         + sw_desc = os.path.join(workdir, 'sw-description.plain' if
> >         encrypt else 'sw-description')
> >
> >         if signing == "CUSTOM":
> >         signcmd = []
> >         @@ -233,6 +234,7 @@ def swupdate_add_src_uri(d, list_for_cpio):
> >         import shutil
> >
> >         s = d.getVar('S')
> >         + workdir = d.getVar('WORKDIR')
> >         exclude = (d.getVar("SWUPDATE_SRC_URI_EXCLUDE") or "").split()
> >
> >         fetch = bb.fetch2.Fetch([], d)
> >         @@ -310,9 +312,14 @@ def swupdate_add_artifacts(d, list_for_cpio):
> >
> >         def swupdate_create_cpio(d, swudeploydir, list_for_cpio):
> >         s = d.getVar('S')
> >         - os.chdir(s)
> >         + workdir = d.getVar('WORKDIR')
> >         + os.chdir(workdir)
> >         + for file in list_for_cpio:
> >         + if not os.path.exists(file):
> >         + os.symlink(os.path.join(s, file), file)
> >         +
> >         updateimage = d.getVar('IMAGE_NAME') + '.swu'
> >         - line = 'for i in ' + ' '.join(list_for_cpio) + '; do echo
> >         $i;done | cpio -ov -H crc --reproducible > ' +
> >         os.path.join(swudeploydir, updateimage)
> >         + line = 'for i in ' + ' '.join(list_for_cpio) + '; do echo
> >         $i;done | cpio -ov -H crc --reproducible -L > ' +
> >         os.path.join(swudeploydir, updateimage)
> >         os.system(line)
> >         os.chdir(swudeploydir)
> >         updateimage_link = d.getVar('IMAGE_LINK_NAME')
> >         diff --git a/classes-recipe/swupdate-image.bbclass b/classes-
> >         recipe/swupdate-image.bbclass
> >         index 1cd3eeb..bf8c5cc 100644
> >         --- a/classes-recipe/swupdate-image.bbclass
> >         +++ b/classes-recipe/swupdate-image.bbclass
> >         @@ -40,7 +40,7 @@ python do_swupdate_copy_swdescription() {
> >
> >         import shutil
> >
> >         - workdir = d.getVar('S')
> >         + workdir = d.getVar('WORKDIR')
> >         filespath = d.getVar('FILESPATH')
> >         sw_desc_path = bb.utils.which(filespath, "sw-description")
> >         shutil.copyfile(sw_desc_path, os.path.join(workdir, "sw-
> >         description"))
> >         --
> >         2.34.1
> >
> > --
> > You received this message because you are subscribed to the Google
> > Groups "swupdate" group.
> > To unsubscribe from this group and stop receiving emails from it, send
> > an email to swupdate+unsubscribe@googlegroups.com
> > <mailto:swupdate+unsubscribe@googlegroups.com>.
> > To view this discussion visit https://groups.google.com/d/msgid/
> > swupdate/1cf15b67-2723-4c0f-8e66-8d2d32af1ec9n%40googlegroups.com
> > <https://groups.google.com/d/msgid/
> > swupdate/1cf15b67-2723-4c0f-8e66-8d2d32af1ec9n%40googlegroups.com?
> > utm_medium=email&utm_source=footer>.
>
Dan Walkes July 17, 2025, 4:34 p.m. UTC | #6
Hi Stefano,

Thanks for the review.

On Thu, Jul 17, 2025 at 9:31 AM Stefano Babic
<stefano.babic@swupdate.org> wrote:
>
> Hi Dan,
>
> On 12/1/24 22:13, Dan Walkes wrote:
> > The fix discussed in [1] resolves build issues on styhead but
> > introduces an issue which causes hash mismatches.
> >
> > The first build succeeds to generate a working .swu, however
> > future builds fail with hash mismatch and a message like this:
> >
> > ```
> > [ERROR] : SWUPDATE failed [0] ERROR cpio_utils.c : __swupdate_copy : 670 : HASH mismatch : 5bb39965bfcedd87fc02bd51fac6636b924aed606d0984dcebae6a3250b01af6 <--> ff70a8dbc50a9ce2bad724554f5f14b8c933ad87fc544d054142755e8ef07f1e
> > [ERROR] : SWUPDATE failed [0] ERROR archive_handler.c : install_archive_image : 344 : Error copying extracted file
> > ```
> >
> > This happens because the modifications made to files in the
> > S directory only happen once.  This means there's a single
> > conversion of `$swupdate_get_sha256(` to the file sha, and future
> > builds use the hash previously placed in the file in the S directory
> > which is no longer correct when the rootfs file hash changes.
> >
> > It would probably be possible to make a simpler change here which
> > splits the UNPACKDIR and S and continues to make runtime
> > edits in S.  However, it seems the intent of S and/or UNPACKDIR
> > is to contain unmodified source as discussed in [1].
>
> In principle it is ok to move from $S to $WORKDIR - until scarthgap, $S
> was $WORKDIR.
>
> swupdate_add_artifacts() already copies files from DEPLOY_DIR (listed in
> SWUPDATE_IMAGES) into a work directory (it was $S). Is it not enough by
> copying them into $WORKDIR then without links ?

I hadn't considered that approach but that might be easier, and I
think it will work.  We shouldn't need symlinks in WORKDIR if we have
the content there, and we don't need content in both S and WORKDIR.  I
think we'd need a similar change in swupdate_add_src_uri to copy to
WORKDIR or we'd need symlinks for those files as well.

>
> >
> > This change instead:
> > 1. Moves the sw-description file to WORKDIR for editing.
> > 2. Continues to use S as source location for sw-description
> > and other source files which are not edited.
> > 3. Makes all edits in WORKDIR.
> > 4. When preparing the cpio, prepares from WORKDIR instead of
> > S. using any existing files in WORKDIR if they exist.  If they
> > do not exist, symlinks the relevant files in S instead.
> > 6. Adds the -L option to cpio to prevent adding symlinks to the
> > cpio file, and avoid the need to copy from S to WORKDIR when
> > preparing the cpio.
> >
> > 1: https://groups.google.com/g/swupdate/c/8K-9H7C9o5E/m/a9fqhIOjAAAJ
> >
> > Signed-off-by: Dan Walkes <danwalkes@trellis-logic.com>
> > ---
> >   classes-recipe/swupdate-common.bbclass | 37 +++++++++++++++-----------
> >   classes-recipe/swupdate-image.bbclass  |  2 +-
> >   2 files changed, 23 insertions(+), 16 deletions(-)
> >
> > diff --git a/classes-recipe/swupdate-common.bbclass b/classes-recipe/swupdate-common.bbclass
> > index 7b49561..085a7b5 100644
> > --- a/classes-recipe/swupdate-common.bbclass
> > +++ b/classes-recipe/swupdate-common.bbclass
> > @@ -66,22 +66,22 @@ def swupdate_getdepends(d):
> >
> >       return depstr
> >
> > -def swupdate_write_sha256(s):
> > +def swupdate_write_sha256(workdir):
>
> "s" here is just the parameter of the function. What it counts, it is
> how the function is called. Do we really need to change it ?

The important thing is that we only write the sw-description file in
WORKDIR and not in UNPACKDIR/S, so we can keep a version of
sw-description without modification in UNPACKDIR/S.  This is
accomplished with the current code by writing to WORKDIR but reading
from S in swupdate_expand_bitbake_variables.

>
> >       import re
> >       write_lines = []
> > -    with open(os.path.join(s, "sw-description"), 'r') as f:
> > +    with open(os.path.join(workdir, "sw-description"), 'r') as f:
> >          for line in f:
> >             shastr = r"sha256.+=.+@(.+\")"
> >             m = re.match(r"^(?P<before_placeholder>.+)(sha256|version).+[=:].*(?P<quote>[\'\"])@(?P<filename>.*)(?P=quote)", line)
> >             if m:
> >                 filename = m.group('filename')
> >                 bb.warn("Syntax for sha256 changed, please use $swupdate_get_sha256(%s)" % filename)
> > -              hash = swupdate_get_sha256(None, s, filename)
> > +              hash = swupdate_get_sha256(None, workdir, filename)
> >                 write_lines.append(line.replace("@%s" % (filename), hash))
> >             else:
> >                 write_lines.append(line)
> >
> > -    with open(os.path.join(s, "sw-description"), 'w+') as f:
> > +    with open(os.path.join(workdir, "sw-description"), 'w+') as f:
> >           for line in write_lines:
> >               f.write(line)
> >
> > @@ -97,7 +97,7 @@ def swupdate_exec_functions(d, s, write_lines):
> >               write_lines[index] = line
> >
> >
> > -def swupdate_expand_bitbake_variables(d, s):
> > +def swupdate_expand_bitbake_variables(d, s, workdir):
> >       write_lines = []
> >
> >       with open(os.path.join(s, "sw-description"), 'r') as f:
> > @@ -131,7 +131,7 @@ def swupdate_expand_bitbake_variables(d, s):
> >
> >       swupdate_exec_functions(d, s, write_lines)
> >
> > -    with open(os.path.join(s, "sw-description"), 'w+') as f:
> > +    with open(os.path.join(workdir, "sw-description"), 'w+') as f:
> >           for line in write_lines:
> >               f.write(line)
>
> ok - so we have original sw-description in $S, generated sw-description
> in $WORKDIR

Yes.

>
> >
> > @@ -173,17 +173,18 @@ def prepare_sw_description(d):
> >       import subprocess
> >
> >       s = d.getVar('S')
> > -    swupdate_expand_bitbake_variables(d, s)
> > +    workdir = d.getVar('WORKDIR')
> > +    swupdate_expand_bitbake_variables(d, s, workdir)
> >
> > -    swupdate_write_sha256(s)
> > +    swupdate_write_sha256(workdir)
> >
> >       encrypt = d.getVar('SWUPDATE_ENCRYPT_SWDESC')
> >       if encrypt:
> >           bb.note("Encryption of sw-description")
> > -        shutil.copyfile(os.path.join(s, 'sw-description'), os.path.join(s, 'sw-description.plain'))
> > +        shutil.copyfile(os.path.join(workdir, 'sw-description'), os.path.join(workdir, 'sw-description.plain'))
> >           key,iv = swupdate_extract_keys(d.getVar('SWUPDATE_AES_FILE'))
> > -        iv = swupdate_get_IV(d, s, 'sw-description')
> > -        swupdate_encrypt_file(os.path.join(s, 'sw-description.plain'), os.path.join(s, 'sw-description'), key, iv)
> > +        iv = swupdate_get_IV(d, workdir, 'sw-description')
> > +        swupdate_encrypt_file(os.path.join(workdir, 'sw-description.plain'), os.path.join(workdir, 'sw-description'), key, iv)
> >
> >       signing = d.getVar('SWUPDATE_SIGNING')
> >       if signing == "1":
> > @@ -191,8 +192,8 @@ def prepare_sw_description(d):
> >           signing = "RSA"
> >       if signing:
> >
> > -        sw_desc_sig = os.path.join(s, 'sw-description.sig')
> > -        sw_desc =  os.path.join(s, 'sw-description.plain' if encrypt else 'sw-description')
> > +        sw_desc_sig = os.path.join(workdir, 'sw-description.sig')
> > +        sw_desc =  os.path.join(workdir, 'sw-description.plain' if encrypt else 'sw-description')
> >
> >           if signing == "CUSTOM":
> >               signcmd = []
> > @@ -233,6 +234,7 @@ def swupdate_add_src_uri(d, list_for_cpio):
> >       import shutil
> >
> >       s = d.getVar('S')
> > +    workdir = d.getVar('WORKDIR')
> >       exclude = (d.getVar("SWUPDATE_SRC_URI_EXCLUDE") or "").split()
> >
> >       fetch = bb.fetch2.Fetch([], d)
> > @@ -310,9 +312,14 @@ def swupdate_add_artifacts(d, list_for_cpio):
> >
> >   def swupdate_create_cpio(d, swudeploydir, list_for_cpio):
> >       s = d.getVar('S')
> > -    os.chdir(s)
> > +    workdir = d.getVar('WORKDIR')
> > +    os.chdir(workdir)
> > +    for file in list_for_cpio:
> > +        if not os.path.exists(file):
> > +            os.symlink(os.path.join(s, file), file)
>
> So we are talking here about files in SRC_URI, right ? Images listed are
> also searched in $DEPLOY_DIR_IMAGE. They are worked in
> swupdate_add_src_uri(), and they are copied into WORKDIR by changing
> "dst" in that function.
>

Symlinks will exist for all content including swupdate_add_artifacts
and swupdate_add_src_uri with this patch, since both still only add to
S as implemented. I could experiment with doing away with the symlinks
by copying content from these functions into WORKDIR instead if you
prefer.

> > +
> >       updateimage = d.getVar('IMAGE_NAME') + '.swu'
> > -    line = 'for i in ' + ' '.join(list_for_cpio) + '; do echo $i;done | cpio -ov -H crc --reproducible > ' + os.path.join(swudeploydir, updateimage)
> > +    line = 'for i in ' + ' '.join(list_for_cpio) + '; do echo $i;done | cpio -ov -H crc --reproducible -L > ' + os.path.join(swudeploydir, updateimage)
> >       os.system(line)
> >       os.chdir(swudeploydir)
> >       updateimage_link = d.getVar('IMAGE_LINK_NAME')
> > diff --git a/classes-recipe/swupdate-image.bbclass b/classes-recipe/swupdate-image.bbclass
> > index 1cd3eeb..bf8c5cc 100644
> > --- a/classes-recipe/swupdate-image.bbclass
> > +++ b/classes-recipe/swupdate-image.bbclass
> > @@ -40,7 +40,7 @@ python do_swupdate_copy_swdescription() {
> >
> >       import shutil
> >
> > -    workdir = d.getVar('S')
> > +    workdir = d.getVar('WORKDIR')
> >       filespath = d.getVar('FILESPATH')
> >       sw_desc_path = bb.utils.which(filespath, "sw-description")
> >       shutil.copyfile(sw_desc_path, os.path.join(workdir, "sw-description"))
>
> Best regards,
> Stefano
>
Thanks
Dan
Stefano Babic July 17, 2025, 8:11 p.m. UTC | #7
Hi Dan,

On 17.07.25 18:34, Dan Walkes wrote:
> Hi Stefano,
> 
> Thanks for the review.
> 
> On Thu, Jul 17, 2025 at 9:31 AM Stefano Babic
> <stefano.babic@swupdate.org> wrote:
>>
>> Hi Dan,
>>
>> On 12/1/24 22:13, Dan Walkes wrote:
>>> The fix discussed in [1] resolves build issues on styhead but
>>> introduces an issue which causes hash mismatches.
>>>
>>> The first build succeeds to generate a working .swu, however
>>> future builds fail with hash mismatch and a message like this:
>>>
>>> ```
>>> [ERROR] : SWUPDATE failed [0] ERROR cpio_utils.c : __swupdate_copy : 670 : HASH mismatch : 5bb39965bfcedd87fc02bd51fac6636b924aed606d0984dcebae6a3250b01af6 <--> ff70a8dbc50a9ce2bad724554f5f14b8c933ad87fc544d054142755e8ef07f1e
>>> [ERROR] : SWUPDATE failed [0] ERROR archive_handler.c : install_archive_image : 344 : Error copying extracted file
>>> ```
>>>
>>> This happens because the modifications made to files in the
>>> S directory only happen once.  This means there's a single
>>> conversion of `$swupdate_get_sha256(` to the file sha, and future
>>> builds use the hash previously placed in the file in the S directory
>>> which is no longer correct when the rootfs file hash changes.
>>>
>>> It would probably be possible to make a simpler change here which
>>> splits the UNPACKDIR and S and continues to make runtime
>>> edits in S.  However, it seems the intent of S and/or UNPACKDIR
>>> is to contain unmodified source as discussed in [1].
>>
>> In principle it is ok to move from $S to $WORKDIR - until scarthgap, $S
>> was $WORKDIR.
>>
>> swupdate_add_artifacts() already copies files from DEPLOY_DIR (listed in
>> SWUPDATE_IMAGES) into a work directory (it was $S). Is it not enough by
>> copying them into $WORKDIR then without links ?
> 
> I hadn't considered that approach but that might be easier,

I think so.

> and I
> think it will work.  We shouldn't need symlinks in WORKDIR if we have
> the content there, and we don't need content in both S and WORKDIR.  I
> think we'd need a similar change in swupdate_add_src_uri to copy to
> WORKDIR or we'd need symlinks for those files as well.
> 
>>
>>>
>>> This change instead:
>>> 1. Moves the sw-description file to WORKDIR for editing.
>>> 2. Continues to use S as source location for sw-description
>>> and other source files which are not edited.
>>> 3. Makes all edits in WORKDIR.
>>> 4. When preparing the cpio, prepares from WORKDIR instead of
>>> S. using any existing files in WORKDIR if they exist.  If they
>>> do not exist, symlinks the relevant files in S instead.
>>> 6. Adds the -L option to cpio to prevent adding symlinks to the
>>> cpio file, and avoid the need to copy from S to WORKDIR when
>>> preparing the cpio.
>>>
>>> 1: https://groups.google.com/g/swupdate/c/8K-9H7C9o5E/m/a9fqhIOjAAAJ
>>>
>>> Signed-off-by: Dan Walkes <danwalkes@trellis-logic.com>
>>> ---
>>>    classes-recipe/swupdate-common.bbclass | 37 +++++++++++++++-----------
>>>    classes-recipe/swupdate-image.bbclass  |  2 +-
>>>    2 files changed, 23 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/classes-recipe/swupdate-common.bbclass b/classes-recipe/swupdate-common.bbclass
>>> index 7b49561..085a7b5 100644
>>> --- a/classes-recipe/swupdate-common.bbclass
>>> +++ b/classes-recipe/swupdate-common.bbclass
>>> @@ -66,22 +66,22 @@ def swupdate_getdepends(d):
>>>
>>>        return depstr
>>>
>>> -def swupdate_write_sha256(s):
>>> +def swupdate_write_sha256(workdir):
>>
>> "s" here is just the parameter of the function. What it counts, it is
>> how the function is called. Do we really need to change it ?
> 
> The important thing is that we only write the sw-description file in
> WORKDIR and not in UNPACKDIR/S,

Right, full agree.

> so we can keep a version of
> sw-description without modification in UNPACKDIR/S.  This is
> accomplished with the current code by writing to WORKDIR but reading
> from S in swupdate_expand_bitbake_variables.

Right

> 
>>
>>>        import re
>>>        write_lines = []
>>> -    with open(os.path.join(s, "sw-description"), 'r') as f:
>>> +    with open(os.path.join(workdir, "sw-description"), 'r') as f:
>>>           for line in f:
>>>              shastr = r"sha256.+=.+@(.+\")"
>>>              m = re.match(r"^(?P<before_placeholder>.+)(sha256|version).+[=:].*(?P<quote>[\'\"])@(?P<filename>.*)(?P=quote)", line)
>>>              if m:
>>>                  filename = m.group('filename')
>>>                  bb.warn("Syntax for sha256 changed, please use $swupdate_get_sha256(%s)" % filename)
>>> -              hash = swupdate_get_sha256(None, s, filename)
>>> +              hash = swupdate_get_sha256(None, workdir, filename)
>>>                  write_lines.append(line.replace("@%s" % (filename), hash))
>>>              else:
>>>                  write_lines.append(line)
>>>
>>> -    with open(os.path.join(s, "sw-description"), 'w+') as f:
>>> +    with open(os.path.join(workdir, "sw-description"), 'w+') as f:
>>>            for line in write_lines:
>>>                f.write(line)
>>>
>>> @@ -97,7 +97,7 @@ def swupdate_exec_functions(d, s, write_lines):
>>>                write_lines[index] = line
>>>
>>>
>>> -def swupdate_expand_bitbake_variables(d, s):
>>> +def swupdate_expand_bitbake_variables(d, s, workdir):
>>>        write_lines = []
>>>
>>>        with open(os.path.join(s, "sw-description"), 'r') as f:
>>> @@ -131,7 +131,7 @@ def swupdate_expand_bitbake_variables(d, s):
>>>
>>>        swupdate_exec_functions(d, s, write_lines)
>>>
>>> -    with open(os.path.join(s, "sw-description"), 'w+') as f:
>>> +    with open(os.path.join(workdir, "sw-description"), 'w+') as f:
>>>            for line in write_lines:
>>>                f.write(line)
>>
>> ok - so we have original sw-description in $S, generated sw-description
>> in $WORKDIR
> 
> Yes.
> 
>>
>>>
>>> @@ -173,17 +173,18 @@ def prepare_sw_description(d):
>>>        import subprocess
>>>
>>>        s = d.getVar('S')
>>> -    swupdate_expand_bitbake_variables(d, s)
>>> +    workdir = d.getVar('WORKDIR')
>>> +    swupdate_expand_bitbake_variables(d, s, workdir)
>>>
>>> -    swupdate_write_sha256(s)
>>> +    swupdate_write_sha256(workdir)
>>>
>>>        encrypt = d.getVar('SWUPDATE_ENCRYPT_SWDESC')
>>>        if encrypt:
>>>            bb.note("Encryption of sw-description")
>>> -        shutil.copyfile(os.path.join(s, 'sw-description'), os.path.join(s, 'sw-description.plain'))
>>> +        shutil.copyfile(os.path.join(workdir, 'sw-description'), os.path.join(workdir, 'sw-description.plain'))
>>>            key,iv = swupdate_extract_keys(d.getVar('SWUPDATE_AES_FILE'))
>>> -        iv = swupdate_get_IV(d, s, 'sw-description')
>>> -        swupdate_encrypt_file(os.path.join(s, 'sw-description.plain'), os.path.join(s, 'sw-description'), key, iv)
>>> +        iv = swupdate_get_IV(d, workdir, 'sw-description')
>>> +        swupdate_encrypt_file(os.path.join(workdir, 'sw-description.plain'), os.path.join(workdir, 'sw-description'), key, iv)
>>>
>>>        signing = d.getVar('SWUPDATE_SIGNING')
>>>        if signing == "1":
>>> @@ -191,8 +192,8 @@ def prepare_sw_description(d):
>>>            signing = "RSA"
>>>        if signing:
>>>
>>> -        sw_desc_sig = os.path.join(s, 'sw-description.sig')
>>> -        sw_desc =  os.path.join(s, 'sw-description.plain' if encrypt else 'sw-description')
>>> +        sw_desc_sig = os.path.join(workdir, 'sw-description.sig')
>>> +        sw_desc =  os.path.join(workdir, 'sw-description.plain' if encrypt else 'sw-description')
>>>
>>>            if signing == "CUSTOM":
>>>                signcmd = []
>>> @@ -233,6 +234,7 @@ def swupdate_add_src_uri(d, list_for_cpio):
>>>        import shutil
>>>
>>>        s = d.getVar('S')
>>> +    workdir = d.getVar('WORKDIR')
>>>        exclude = (d.getVar("SWUPDATE_SRC_URI_EXCLUDE") or "").split()
>>>
>>>        fetch = bb.fetch2.Fetch([], d)
>>> @@ -310,9 +312,14 @@ def swupdate_add_artifacts(d, list_for_cpio):
>>>
>>>    def swupdate_create_cpio(d, swudeploydir, list_for_cpio):
>>>        s = d.getVar('S')
>>> -    os.chdir(s)
>>> +    workdir = d.getVar('WORKDIR')
>>> +    os.chdir(workdir)
>>> +    for file in list_for_cpio:
>>> +        if not os.path.exists(file):
>>> +            os.symlink(os.path.join(s, file), file)
>>
>> So we are talking here about files in SRC_URI, right ? Images listed are
>> also searched in $DEPLOY_DIR_IMAGE. They are worked in
>> swupdate_add_src_uri(), and they are copied into WORKDIR by changing
>> "dst" in that function.
>>
> 
> Symlinks will exist for all content including swupdate_add_artifacts
> and swupdate_add_src_uri with this patch, since both still only add to
> S as implemented. I could experiment with doing away with the symlinks
> by copying content from these functions into WORKDIR instead if you
> prefer.

Yes, please do it.

> 
>>> +
>>>        updateimage = d.getVar('IMAGE_NAME') + '.swu'
>>> -    line = 'for i in ' + ' '.join(list_for_cpio) + '; do echo $i;done | cpio -ov -H crc --reproducible > ' + os.path.join(swudeploydir, updateimage)
>>> +    line = 'for i in ' + ' '.join(list_for_cpio) + '; do echo $i;done | cpio -ov -H crc --reproducible -L > ' + os.path.join(swudeploydir, updateimage)
>>>        os.system(line)
>>>        os.chdir(swudeploydir)
>>>        updateimage_link = d.getVar('IMAGE_LINK_NAME')
>>> diff --git a/classes-recipe/swupdate-image.bbclass b/classes-recipe/swupdate-image.bbclass
>>> index 1cd3eeb..bf8c5cc 100644
>>> --- a/classes-recipe/swupdate-image.bbclass
>>> +++ b/classes-recipe/swupdate-image.bbclass
>>> @@ -40,7 +40,7 @@ python do_swupdate_copy_swdescription() {
>>>
>>>        import shutil
>>>
>>> -    workdir = d.getVar('S')
>>> +    workdir = d.getVar('WORKDIR')
>>>        filespath = d.getVar('FILESPATH')
>>>        sw_desc_path = bb.utils.which(filespath, "sw-description")
>>>        shutil.copyfile(sw_desc_path, os.path.join(workdir, "sw-description"))
>>


Best regards,
Stefano
diff mbox series

Patch

diff --git a/classes-recipe/swupdate-common.bbclass b/classes-recipe/swupdate-common.bbclass
index 7b49561..085a7b5 100644
--- a/classes-recipe/swupdate-common.bbclass
+++ b/classes-recipe/swupdate-common.bbclass
@@ -66,22 +66,22 @@  def swupdate_getdepends(d):
 
     return depstr
 
-def swupdate_write_sha256(s):
+def swupdate_write_sha256(workdir):
     import re
     write_lines = []
-    with open(os.path.join(s, "sw-description"), 'r') as f:
+    with open(os.path.join(workdir, "sw-description"), 'r') as f:
        for line in f:
           shastr = r"sha256.+=.+@(.+\")"
           m = re.match(r"^(?P<before_placeholder>.+)(sha256|version).+[=:].*(?P<quote>[\'\"])@(?P<filename>.*)(?P=quote)", line)
           if m:
               filename = m.group('filename')
               bb.warn("Syntax for sha256 changed, please use $swupdate_get_sha256(%s)" % filename)
-              hash = swupdate_get_sha256(None, s, filename)
+              hash = swupdate_get_sha256(None, workdir, filename)
               write_lines.append(line.replace("@%s" % (filename), hash))
           else:
               write_lines.append(line)
 
-    with open(os.path.join(s, "sw-description"), 'w+') as f:
+    with open(os.path.join(workdir, "sw-description"), 'w+') as f:
         for line in write_lines:
             f.write(line)
 
@@ -97,7 +97,7 @@  def swupdate_exec_functions(d, s, write_lines):
             write_lines[index] = line
 
 
-def swupdate_expand_bitbake_variables(d, s):
+def swupdate_expand_bitbake_variables(d, s, workdir):
     write_lines = []
 
     with open(os.path.join(s, "sw-description"), 'r') as f:
@@ -131,7 +131,7 @@  def swupdate_expand_bitbake_variables(d, s):
 
     swupdate_exec_functions(d, s, write_lines)
 
-    with open(os.path.join(s, "sw-description"), 'w+') as f:
+    with open(os.path.join(workdir, "sw-description"), 'w+') as f:
         for line in write_lines:
             f.write(line)
 
@@ -173,17 +173,18 @@  def prepare_sw_description(d):
     import subprocess
 
     s = d.getVar('S')
-    swupdate_expand_bitbake_variables(d, s)
+    workdir = d.getVar('WORKDIR')
+    swupdate_expand_bitbake_variables(d, s, workdir)
 
-    swupdate_write_sha256(s)
+    swupdate_write_sha256(workdir)
 
     encrypt = d.getVar('SWUPDATE_ENCRYPT_SWDESC')
     if encrypt:
         bb.note("Encryption of sw-description")
-        shutil.copyfile(os.path.join(s, 'sw-description'), os.path.join(s, 'sw-description.plain'))
+        shutil.copyfile(os.path.join(workdir, 'sw-description'), os.path.join(workdir, 'sw-description.plain'))
         key,iv = swupdate_extract_keys(d.getVar('SWUPDATE_AES_FILE'))
-        iv = swupdate_get_IV(d, s, 'sw-description')
-        swupdate_encrypt_file(os.path.join(s, 'sw-description.plain'), os.path.join(s, 'sw-description'), key, iv)
+        iv = swupdate_get_IV(d, workdir, 'sw-description')
+        swupdate_encrypt_file(os.path.join(workdir, 'sw-description.plain'), os.path.join(workdir, 'sw-description'), key, iv)
 
     signing = d.getVar('SWUPDATE_SIGNING')
     if signing == "1":
@@ -191,8 +192,8 @@  def prepare_sw_description(d):
         signing = "RSA"
     if signing:
 
-        sw_desc_sig = os.path.join(s, 'sw-description.sig')
-        sw_desc =  os.path.join(s, 'sw-description.plain' if encrypt else 'sw-description')
+        sw_desc_sig = os.path.join(workdir, 'sw-description.sig')
+        sw_desc =  os.path.join(workdir, 'sw-description.plain' if encrypt else 'sw-description')
 
         if signing == "CUSTOM":
             signcmd = []
@@ -233,6 +234,7 @@  def swupdate_add_src_uri(d, list_for_cpio):
     import shutil
 
     s = d.getVar('S')
+    workdir = d.getVar('WORKDIR')
     exclude = (d.getVar("SWUPDATE_SRC_URI_EXCLUDE") or "").split()
 
     fetch = bb.fetch2.Fetch([], d)
@@ -310,9 +312,14 @@  def swupdate_add_artifacts(d, list_for_cpio):
 
 def swupdate_create_cpio(d, swudeploydir, list_for_cpio):
     s = d.getVar('S')
-    os.chdir(s)
+    workdir = d.getVar('WORKDIR')
+    os.chdir(workdir)
+    for file in list_for_cpio:
+        if not os.path.exists(file):
+            os.symlink(os.path.join(s, file), file)
+
     updateimage = d.getVar('IMAGE_NAME') + '.swu'
-    line = 'for i in ' + ' '.join(list_for_cpio) + '; do echo $i;done | cpio -ov -H crc --reproducible > ' + os.path.join(swudeploydir, updateimage)
+    line = 'for i in ' + ' '.join(list_for_cpio) + '; do echo $i;done | cpio -ov -H crc --reproducible -L > ' + os.path.join(swudeploydir, updateimage)
     os.system(line)
     os.chdir(swudeploydir)
     updateimage_link = d.getVar('IMAGE_LINK_NAME')
diff --git a/classes-recipe/swupdate-image.bbclass b/classes-recipe/swupdate-image.bbclass
index 1cd3eeb..bf8c5cc 100644
--- a/classes-recipe/swupdate-image.bbclass
+++ b/classes-recipe/swupdate-image.bbclass
@@ -40,7 +40,7 @@  python do_swupdate_copy_swdescription() {
 
     import shutil
 
-    workdir = d.getVar('S')
+    workdir = d.getVar('WORKDIR')
     filespath = d.getVar('FILESPATH')
     sw_desc_path = bb.utils.which(filespath, "sw-description")
     shutil.copyfile(sw_desc_path, os.path.join(workdir, "sw-description"))