| Message ID | 20240703114242.1896508-1-ch@denx.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | swupdate.bbclass: make swu_files entries unique | expand |
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
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 >
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
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
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
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
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.
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
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
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
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(-)