[PATCH] swupdate.bbclass: make swu_files entries unique

[PATCH] swupdate.bbclass: make swu_files entries unique

[Claudius Heine]

In case a sw-description contains multiple entries of the same file,
swu_files will contains multiples of the same file and thus later
operations fail.

This change makes the files in the swu_files variable unique, so they
are only operated on once.

Signed-off-by: Claudius Heine <ch@denx.de>
---
 classes/swupdate.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/classes/swupdate.bbclass b/classes/swupdate.bbclass
index e91238d..5e54853 100644
--- a/classes/swupdate.bbclass
+++ b/classes/swupdate.bbclass
@@ -205,7 +205,7 @@ IMAGE_CMD:swu() {
         swu_file_base=$(basename $swu_file)
         # Create symlinks for files used in the update image
         swu_files=$(awk '$1=="filename"{gsub(/[",;]/, "", $3); print $3}' \
-            "${WORKDIR}/$swu_file_base/${SWU_DESCRIPTION_FILE}")
+            "${WORKDIR}/$swu_file_base/${SWU_DESCRIPTION_FILE}" | sort | uniq)
         export swu_files
         for file in $swu_files; do
             if [ -e "${WORKDIR}/$file" ]; then
-- 
2.39.2

Re: [PATCH] swupdate.bbclass: make swu_files entries unique

[MOESSBAUER, Felix]

On Wed, 2024-07-03 at 13:42 +0200, Claudius Heine wrote:
> In case a sw-description contains multiple entries of the same file,
> swu_files will contains multiples of the same file and thus later
> operations fail.

Hi, I'm wondering how that can happen, but OK.

> 
> This change makes the files in the swu_files variable unique, so they
> are only operated on once.
> 
> Signed-off-by: Claudius Heine <ch@denx.de>
> ---
>  classes/swupdate.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/classes/swupdate.bbclass b/classes/swupdate.bbclass
> index e91238d..5e54853 100644
> --- a/classes/swupdate.bbclass
> +++ b/classes/swupdate.bbclass
> @@ -205,7 +205,7 @@ IMAGE_CMD:swu() {
>          swu_file_base=$(basename $swu_file)
>          # Create symlinks for files used in the update image
>          swu_files=$(awk '$1=="filename"{gsub(/[",;]/, "", $3); print
> $3}' \
> -            "${WORKDIR}/$swu_file_base/${SWU_DESCRIPTION_FILE}")
> +            "${WORKDIR}/$swu_file_base/${SWU_DESCRIPTION_FILE}" |
> sort | uniq)

By that, we also change the order of the files in the swu. Are we sure
that there are no unwanted side effects?

Felix

>          export swu_files
>          for file in $swu_files; do
>              if [ -e "${WORKDIR}/$file" ]; then

-- 
Siemens AG, Technology
Linux Expert Center

Re: [PATCH] swupdate.bbclass: make swu_files entries unique

[Claudius Heine]

Hi Felix,

On 2024-07-04 2:15 pm, MOESSBAUER, Felix wrote:
> On Wed, 2024-07-03 at 13:42 +0200, Claudius Heine wrote:
>> In case a sw-description contains multiple entries of the same file,
>> swu_files will contains multiples of the same file and thus later
>> operations fail.
> 
> Hi, I'm wondering how that can happen, but OK.

How it happens that there are multiple mentions of the same file in a 
sw-description, or how it happens that it fails?

Multiple instances of the same file happens when the swupdate software 
collections are used to implement A/B update: 
https://sbabic.github.io/swupdate/sw-description.html#software-collections

There the same image can be flashed to different devices.

The build fails on the `ln -s` just a couple of lines later in that 
file, since `--force` isn't used, the symlink will already exist on the 
second mention of that file.

> 
>>
>> This change makes the files in the swu_files variable unique, so they
>> are only operated on once.
>>
>> Signed-off-by: Claudius Heine <ch@denx.de>
>> ---
>>   classes/swupdate.bbclass | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/classes/swupdate.bbclass b/classes/swupdate.bbclass
>> index e91238d..5e54853 100644
>> --- a/classes/swupdate.bbclass
>> +++ b/classes/swupdate.bbclass
>> @@ -205,7 +205,7 @@ IMAGE_CMD:swu() {
>>           swu_file_base=$(basename $swu_file)
>>           # Create symlinks for files used in the update image
>>           swu_files=$(awk '$1=="filename"{gsub(/[",;]/, "", $3); print
>> $3}' \
>> -            "${WORKDIR}/$swu_file_base/${SWU_DESCRIPTION_FILE}")
>> +            "${WORKDIR}/$swu_file_base/${SWU_DESCRIPTION_FILE}" |
>> sort | uniq)
> 
> By that, we also change the order of the files in the swu. Are we sure
> that there are no unwanted side effects?

Hmm... this is a interesting point, it might be the case for streaming 
update files, I will check that.

regards,
Claudius

> 
> Felix
> 
>>           export swu_files
>>           for file in $swu_files; do
>>               if [ -e "${WORKDIR}/$file" ]; then
> 

-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch@denx.de

Re: [PATCH] swupdate.bbclass: make swu_files entries unique

[Jan Kiszka]

On 04.07.24 15:29, Claudius Heine wrote:
> Hi Felix,
> 
> On 2024-07-04 2:15 pm, MOESSBAUER, Felix wrote:
>> On Wed, 2024-07-03 at 13:42 +0200, Claudius Heine wrote:
>>> In case a sw-description contains multiple entries of the same file,
>>> swu_files will contains multiples of the same file and thus later
>>> operations fail.
>>
>> Hi, I'm wondering how that can happen, but OK.
> 
> How it happens that there are multiple mentions of the same file in a
> sw-description, or how it happens that it fails?
> 
> Multiple instances of the same file happens when the swupdate software
> collections are used to implement A/B update:
> https://sbabic.github.io/swupdate/sw-description.html#software-collections
> 
> There the same image can be flashed to different devices.
> 

That's a pattern we can avoid these days by using the roundrobin
handler, see recipes-core/images/swu/sw-description.tmpl. Doesn't make
the collection pattern wrong and this fix unneeded.

Still, you may want to check if there is a real need to have an own
sw-description.tmpl downstream - or if there is anything missing in the
upstream template to make it even more useful.

Jan

-- 
Siemens AG, Technology
Linux Expert Center

Re: [PATCH] swupdate.bbclass: make swu_files entries unique

[Claudius Heine]

Hi Jan,

On 2024-07-04 11:45 pm, Jan Kiszka wrote:
> On 04.07.24 15:29, Claudius Heine wrote:
>> Hi Felix,
>>
>> On 2024-07-04 2:15 pm, MOESSBAUER, Felix wrote:
>>> On Wed, 2024-07-03 at 13:42 +0200, Claudius Heine wrote:
>>>> In case a sw-description contains multiple entries of the same file,
>>>> swu_files will contains multiples of the same file and thus later
>>>> operations fail.
>>>
>>> Hi, I'm wondering how that can happen, but OK.
>>
>> How it happens that there are multiple mentions of the same file in a
>> sw-description, or how it happens that it fails?
>>
>> Multiple instances of the same file happens when the swupdate software
>> collections are used to implement A/B update:
>> https://sbabic.github.io/swupdate/sw-description.html#software-collections
>>
>> There the same image can be flashed to different devices.
>>
> 
> That's a pattern we can avoid these days by using the roundrobin
> handler, see recipes-core/images/swu/sw-description.tmpl. Doesn't make
> the collection pattern wrong and this fix unneeded.

I'm not sure I fully understand your last sentence, but I know about the 
roundrobin handler and used it in other projects before, however AFAIK 
it is something specific to isar-cip-core. I haven't seen this used in 
other projects outside of that.

 From what I experienced, outside of isar-cip-core, the software 
collection approach is much more common. There we have swupdate wrapper 
scripts, that figure out which software collection should be used, and 
call swupdate accordingly.
> 
> Still, you may want to check if there is a real need to have an own
> sw-description.tmpl downstream - or if there is anything missing in the
> upstream template to make it even more useful.

The round-robin handler and the isar-cip-core specific 
sw-description.tmpl might work well enough for projects that start with 
isar-cip-core, but some projects might want to switch from something 
else to isar-cip-core gradually, while preserving update compatibility, 
or have project-specific use-cases and handlers where it doesn't make 
sense to pull in the complexity into isar-cip-core itself.

The platform I am working with, doesn't support EFI, has a two grub 
bootloaders with custom scripts that chainload another, has embedded lua 
scripts in the sw-description, wrapper scripts to implement platform 
specific ways to trigger different parts of the update process 
separately, very specific partition layout, and so on.

Preventing alternative ways to implement the product-layer from a 
base-layer makes it difficult to adopt it.

IMO having a default isar-cip-core way, is great, but it should be 
possible to go a custom route as well, and to deactivate or change 
whatever is necessary. IMO, this is generally how bitbake meta-layers 
should work. Otherwise people will either require to carry downstream 
patches to layers, which should only be done in very specific 
circumstances, or have to copy & modify classes and files into their 
layers, which will also make development and maintenance more difficult.

regards,
Claudius

-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch@denx.de

Re: [PATCH] swupdate.bbclass: make swu_files entries unique

[Claudius Heine]

Hi Felix,

On 2024-07-04 2:15 pm, MOESSBAUER, Felix wrote:
>> diff --git a/classes/swupdate.bbclass b/classes/swupdate.bbclass
>> index e91238d..5e54853 100644
>> --- a/classes/swupdate.bbclass
>> +++ b/classes/swupdate.bbclass
>> @@ -205,7 +205,7 @@ IMAGE_CMD:swu() {
>>           swu_file_base=$(basename $swu_file)
>>           # Create symlinks for files used in the update image
>>           swu_files=$(awk '$1=="filename"{gsub(/[",;]/, "", $3); print
>> $3}' \
>> -            "${WORKDIR}/$swu_file_base/${SWU_DESCRIPTION_FILE}")
>> +            "${WORKDIR}/$swu_file_base/${SWU_DESCRIPTION_FILE}" |
>> sort | uniq)
> By that, we also change the order of the files in the swu. Are we sure
> that there are no unwanted side effects?

So when doing streaming update, the order in which the files are inside 
the CPIO archive is the order in which the files are written. So 
changing of these files will effect that. However this order shouldn't 
really matter when updating.

When doing non-streaming update, the artifacts are applied in the order 
they are mentioned in the sw-description, so the order in which they are 
inside the cpio archive should not matter.

regards,
Claudius

-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch@denx.de

Re: [PATCH] swupdate.bbclass: make swu_files entries unique

[Jan Kiszka]

On 05.07.24 09:55, Claudius Heine wrote:
> Hi Jan,
> 
> On 2024-07-04 11:45 pm, Jan Kiszka wrote:
>> On 04.07.24 15:29, Claudius Heine wrote:
>>> Hi Felix,
>>>
>>> On 2024-07-04 2:15 pm, MOESSBAUER, Felix wrote:
>>>> On Wed, 2024-07-03 at 13:42 +0200, Claudius Heine wrote:
>>>>> In case a sw-description contains multiple entries of the same file,
>>>>> swu_files will contains multiples of the same file and thus later
>>>>> operations fail.
>>>>
>>>> Hi, I'm wondering how that can happen, but OK.
>>>
>>> How it happens that there are multiple mentions of the same file in a
>>> sw-description, or how it happens that it fails?
>>>
>>> Multiple instances of the same file happens when the swupdate software
>>> collections are used to implement A/B update:
>>> https://sbabic.github.io/swupdate/sw-description.html#software-collections
>>>
>>> There the same image can be flashed to different devices.
>>>
>>
>> That's a pattern we can avoid these days by using the roundrobin
>> handler, see recipes-core/images/swu/sw-description.tmpl. Doesn't make
>> the collection pattern wrong and this fix unneeded.
> 
> I'm not sure I fully understand your last sentence, but I know about the
> roundrobin handler and used it in other projects before, however AFAIK
> it is something specific to isar-cip-core. I haven't seen this used in
> other projects outside of that.

I mean I'm still happy to take your fix.

> 
> From what I experienced, outside of isar-cip-core, the software
> collection approach is much more common. There we have swupdate wrapper
> scripts, that figure out which software collection should be used, and
> call swupdate accordingly.

Right, and that additional, possibly ugly wrapping is what the pattern
in isar-cip-core avoids: You just call "swupdate -i my.swu" (or
equivalent triggers), and everything works as it should.

Christian, seems there is a gap in awareness for this option outside of
the isar-cip-core universe.

>>
>> Still, you may want to check if there is a real need to have an own
>> sw-description.tmpl downstream - or if there is anything missing in the
>> upstream template to make it even more useful.
> 
> The round-robin handler and the isar-cip-core specific
> sw-description.tmpl might work well enough for projects that start with
> isar-cip-core, but some projects might want to switch from something
> else to isar-cip-core gradually, while preserving update compatibility,
> or have project-specific use-cases and handlers where it doesn't make
> sense to pull in the complexity into isar-cip-core itself.
> 
> The platform I am working with, doesn't support EFI, has a two grub
> bootloaders with custom scripts that chainload another, has embedded lua
> scripts in the sw-description, wrapper scripts to implement platform
> specific ways to trigger different parts of the update process
> separately, very specific partition layout, and so on.

OK, that special one. Well, if you have to maintain special
swu-descriptions for other reasons, then it is hard to reuse things from
here, true.

> 
> Preventing alternative ways to implement the product-layer from a
> base-layer makes it difficult to adopt it.
> 
> IMO having a default isar-cip-core way, is great, but it should be
> possible to go a custom route as well, and to deactivate or change
> whatever is necessary. IMO, this is generally how bitbake meta-layers
> should work. Otherwise people will either require to carry downstream
> patches to layers, which should only be done in very specific
> circumstances, or have to copy & modify classes and files into their
> layers, which will also make development and maintenance more difficult.

Again, I'm fine with allowing software collections as well.

Jan

-- 
Siemens AG, Technology
Linux Expert Center

Re: [PATCH] swupdate.bbclass: make swu_files entries unique

[Claudius Heine]

On 2024-07-05 10:08 am, Jan Kiszka wrote:
>>> That's a pattern we can avoid these days by using the roundrobin
>>> handler, see recipes-core/images/swu/sw-description.tmpl. Doesn't make
>>> the collection pattern wrong and this fix unneeded.
>> I'm not sure I fully understand your last sentence, but I know about the
>> roundrobin handler and used it in other projects before, however AFAIK
>> it is something specific to isar-cip-core. I haven't seen this used in
>> other projects outside of that.
> I mean I'm still happy to take your fix.

Got it now. Somehow my reading comprehension has forsaken me for that bit.

-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch@denx.de

Re: [PATCH] swupdate.bbclass: make swu_files entries unique

[Storm, Christian]

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


From what I experienced, outside of isar-cip-core, the software
collection approach is much more common. There we have swupdate wrapper
scripts, that figure out which software collection should be used, and
call swupdate accordingly.

Right, and that additional, possibly ugly wrapping is what the pattern
in isar-cip-core avoids: You just call "swupdate -i my.swu" (or
equivalent triggers), and everything works as it should.

Christian, seems there is a gap in awareness for this option outside of
the isar-cip-core universe.

Hm, it's well known to the readers of the SWUpdate mailing list and has been announced officially (https://swupdate.org/2021/06/14) but yes, we may market that more prominently.
Nowadays, having all the infrastructure in SWUpdate itself, we may even pursue upstreaming that to SWUpdate instead of maintaining it ourselves... On the other hand, we may as well advertise it in CIP more prominently?

Christian

--
Dr. Christian Storm
Siemens AG, Technology, T CED OES-DE
Friedrich-Ludwig-Bauer-Str. 3, 85748 Garching, Germany


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

Re: [PATCH] swupdate.bbclass: make swu_files entries unique

[Jan Kiszka]

On 03.07.24 13:42, Claudius Heine wrote:
> In case a sw-description contains multiple entries of the same file,
> swu_files will contains multiples of the same file and thus later
> operations fail.
> 
> This change makes the files in the swu_files variable unique, so they
> are only operated on once.
> 
> Signed-off-by: Claudius Heine <ch@denx.de>
> ---
>  classes/swupdate.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/classes/swupdate.bbclass b/classes/swupdate.bbclass
> index e91238d..5e54853 100644
> --- a/classes/swupdate.bbclass
> +++ b/classes/swupdate.bbclass
> @@ -205,7 +205,7 @@ IMAGE_CMD:swu() {
>          swu_file_base=$(basename $swu_file)
>          # Create symlinks for files used in the update image
>          swu_files=$(awk '$1=="filename"{gsub(/[",;]/, "", $3); print $3}' \
> -            "${WORKDIR}/$swu_file_base/${SWU_DESCRIPTION_FILE}")
> +            "${WORKDIR}/$swu_file_base/${SWU_DESCRIPTION_FILE}" | sort | uniq)
>          export swu_files
>          for file in $swu_files; do
>              if [ -e "${WORKDIR}/$file" ]; then

Thanks, applied.

Jan

-- 
Siemens AG, Technology
Linux Expert Center