Message ID | 20221027142145.670-1-sam.van.den.berge@gmail.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [meta-swupdate] swupdate-lib.bblcass: replace swupdate_get_size with swupdate_get_decrypted_size | expand |
On Thu, Oct 27, 2022 at 04:21:45PM +0200, sam.van.den.berge@gmail.com wrote: > From: Sam Van Den Berge <sam.van.den.berge@gmail.com> > > Commit c1586c7de422c introduced swupdate_get_size with the intention to > have the unencrypted size of an artifact. However it gets the size from > the artifact in the "s" folder which is the encrypted artifact. > Hence currently swupdate_get_size gives the size of the encrypted > artifact. To avoid any ambiguity I think it's best to rename that > function to swupdate_get_decrypted_size and to get the file from the > DEPLOY_DIR_IMAGE which contains the unencrypted artifact. Then it can be > used without any ambiguity: > > properties: { > decrypted-size = "$swupdate_get_decrypted_size(...)"; > } > > Fixes: c1586c7de422c ("class: add function to get file size") > Signed-off-by: Sam Van Den Berge <sam.van.den.berge@gmail.com> > --- > classes/swupdate-lib.bbclass | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/classes/swupdate-lib.bbclass b/classes/swupdate-lib.bbclass > index 14a2a08..30d2e04 100644 > --- a/classes/swupdate-lib.bbclass > +++ b/classes/swupdate-lib.bbclass > @@ -40,10 +40,11 @@ def swupdate_get_sha256(d, s, filename): > m.update(data) > return m.hexdigest() > > -def swupdate_get_size(d, s, filename): > +def swupdate_get_decrypted_size(d, s, filename): > import os > > - fname = os.path.join(s, filename) > + deploydir = d.getVar('DEPLOY_DIR_IMAGE', True) > + fname = os.path.join(deploydir, filename) > fsize = os.path.getsize(fname) > return str(fsize) > > -- > 2.34.1 > Ping.
Hallo Sam, added Ayoub as author of the original commit: On 27.10.22 16:21, sam.van.den.berge@gmail.com wrote: > From: Sam Van Den Berge <sam.van.den.berge@gmail.com> > > Commit c1586c7de422c introduced swupdate_get_size with the intention to > have the unencrypted size of an artifact. However it gets the size from > the artifact in the "s" folder which is the encrypted artifact. Not sure here. The function simply returns the size of the file passed as parameter. We can write size = "$swupdate_get_size(unencrypted_file)"; or size = "$swupdate_get_size(encrypted_file.enc)"; if you pass the encrypted file, you get the size of the encrypted file. > Hence currently swupdate_get_size gives the size of the encrypted > artifact. To avoid any ambiguity I think it's best to rename that > function to swupdate_get_decrypted_size and to get the file from the > DEPLOY_DIR_IMAGE which contains the unencrypted artifact. This is mostly where I stumbled. The logic is that files are already prepared for any function, and no fix path should be set inside the function itself. > Then it can be > used without any ambiguity: > > properties: { > decrypted-size = "$swupdate_get_decrypted_size(...)"; > } > > Fixes: c1586c7de422c ("class: add function to get file size") > Signed-off-by: Sam Van Den Berge <sam.van.den.berge@gmail.com> > --- > classes/swupdate-lib.bbclass | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/classes/swupdate-lib.bbclass b/classes/swupdate-lib.bbclass > index 14a2a08..30d2e04 100644 > --- a/classes/swupdate-lib.bbclass > +++ b/classes/swupdate-lib.bbclass > @@ -40,10 +40,11 @@ def swupdate_get_sha256(d, s, filename): > m.update(data) > return m.hexdigest() > > -def swupdate_get_size(d, s, filename): > +def swupdate_get_decrypted_size(d, s, filename): > import os > > - fname = os.path.join(s, filename) > + deploydir = d.getVar('DEPLOY_DIR_IMAGE', True) > + fname = os.path.join(deploydir, filename) > fsize = os.path.getsize(fname) > return str(fsize) > Best regards, Stefano Babic
+Ayoub, this time with the right E-Mail address. On 08.11.22 14:41, Stefano Babic wrote: > Hallo Sam, > > added Ayoub as author of the original commit: > > On 27.10.22 16:21, sam.van.den.berge@gmail.com wrote: >> From: Sam Van Den Berge <sam.van.den.berge@gmail.com> >> >> Commit c1586c7de422c introduced swupdate_get_size with the intention to >> have the unencrypted size of an artifact. However it gets the size from >> the artifact in the "s" folder which is the encrypted artifact. > > Not sure here. The function simply returns the size of the file passed > as parameter. We can write > > size = "$swupdate_get_size(unencrypted_file)"; > > or > > size = "$swupdate_get_size(encrypted_file.enc)"; > > if you pass the encrypted file, you get the size of the encrypted file. > >> Hence currently swupdate_get_size gives the size of the encrypted >> artifact. To avoid any ambiguity I think it's best to rename that >> function to swupdate_get_decrypted_size and to get the file from the >> DEPLOY_DIR_IMAGE which contains the unencrypted artifact. > > This is mostly where I stumbled. The logic is that files are already > prepared for any function, and no fix path should be set inside the > function itself. > >> Then it can be >> used without any ambiguity: >> >> properties: { >> decrypted-size = "$swupdate_get_decrypted_size(...)"; >> } >> >> Fixes: c1586c7de422c ("class: add function to get file size") >> Signed-off-by: Sam Van Den Berge <sam.van.den.berge@gmail.com> >> --- >> classes/swupdate-lib.bbclass | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/classes/swupdate-lib.bbclass b/classes/swupdate-lib.bbclass >> index 14a2a08..30d2e04 100644 >> --- a/classes/swupdate-lib.bbclass >> +++ b/classes/swupdate-lib.bbclass >> @@ -40,10 +40,11 @@ def swupdate_get_sha256(d, s, filename): >> m.update(data) >> return m.hexdigest() >> -def swupdate_get_size(d, s, filename): >> +def swupdate_get_decrypted_size(d, s, filename): >> import os >> - fname = os.path.join(s, filename) >> + deploydir = d.getVar('DEPLOY_DIR_IMAGE', True) >> + fname = os.path.join(deploydir, filename) >> fsize = os.path.getsize(fname) >> return str(fsize) > > Best regards, > Stefano Babic >
Hello, Stefano is right you should get the decrypted file size if you want to rename this function. best On Tuesday, November 8, 2022 at 3:58:44 PM UTC+1 Stefano Babic wrote: > +Ayoub, this time with the right E-Mail address. > > On 08.11.22 14:41, Stefano Babic wrote: > > Hallo Sam, > > > > added Ayoub as author of the original commit: > > > > On 27.10.22 16:21, sam.van....@gmail.com wrote: > >> From: Sam Van Den Berge <sam.van....@gmail.com> > >> > >> Commit c1586c7de422c introduced swupdate_get_size with the intention to > >> have the unencrypted size of an artifact. However it gets the size from > >> the artifact in the "s" folder which is the encrypted artifact. > > > > Not sure here. The function simply returns the size of the file passed > > as parameter. We can write > > > > size = "$swupdate_get_size(unencrypted_file)"; > > > > or > > > > size = "$swupdate_get_size(encrypted_file.enc)"; > > > > if you pass the encrypted file, you get the size of the encrypted file. > > > >> Hence currently swupdate_get_size gives the size of the encrypted > >> artifact. To avoid any ambiguity I think it's best to rename that > >> function to swupdate_get_decrypted_size and to get the file from the > >> DEPLOY_DIR_IMAGE which contains the unencrypted artifact. > > > > This is mostly where I stumbled. The logic is that files are already > > prepared for any function, and no fix path should be set inside the > > function itself. > > > >> Then it can be > >> used without any ambiguity: > >> > >> properties: { > >> decrypted-size = "$swupdate_get_decrypted_size(...)"; > >> } > >> > >> Fixes: c1586c7de422c ("class: add function to get file size") > >> Signed-off-by: Sam Van Den Berge <sam.van....@gmail.com> > >> --- > >> classes/swupdate-lib.bbclass | 5 +++-- > >> 1 file changed, 3 insertions(+), 2 deletions(-) > >> > >> diff --git a/classes/swupdate-lib.bbclass b/classes/swupdate-lib.bbclass > >> index 14a2a08..30d2e04 100644 > >> --- a/classes/swupdate-lib.bbclass > >> +++ b/classes/swupdate-lib.bbclass > >> @@ -40,10 +40,11 @@ def swupdate_get_sha256(d, s, filename): > >> m.update(data) > >> return m.hexdigest() > >> -def swupdate_get_size(d, s, filename): > >> +def swupdate_get_decrypted_size(d, s, filename): > >> import os > >> - fname = os.path.join(s, filename) > >> + deploydir = d.getVar('DEPLOY_DIR_IMAGE', True) > >> + fname = os.path.join(deploydir, filename) > >> fsize = os.path.getsize(fname) > >> return str(fsize) > > > > Best regards, > > Stefano Babic > > > > -- > ===================================================================== > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, 82194 Groebenzell, Germany > Phone: +49-8142-66989-53 <+49%208142%206698953> Fax: +49-8142-66989-80 > <+49%208142%206698980> Email: sba...@denx.de > ===================================================================== > >
There may be also other use cases to have 'swupdate_get_size' so I'd propose to have a new function 'swupdate_get_decrypted_size' On Tuesday, November 8, 2022 at 7:09:35 PM UTC+1 ayoub...@googlemail.com wrote: > Hello, > > Stefano is right you should get the decrypted file size if you want to > rename this function. > > best > > On Tuesday, November 8, 2022 at 3:58:44 PM UTC+1 Stefano Babic wrote: > >> +Ayoub, this time with the right E-Mail address. >> >> On 08.11.22 14:41, Stefano Babic wrote: >> > Hallo Sam, >> > >> > added Ayoub as author of the original commit: >> > >> > On 27.10.22 16:21, sam.van....@gmail.com wrote: >> >> From: Sam Van Den Berge <sam.van....@gmail.com> >> >> >> >> Commit c1586c7de422c introduced swupdate_get_size with the intention >> to >> >> have the unencrypted size of an artifact. However it gets the size >> from >> >> the artifact in the "s" folder which is the encrypted artifact. >> > >> > Not sure here. The function simply returns the size of the file passed >> > as parameter. We can write >> > >> > size = "$swupdate_get_size(unencrypted_file)"; >> > >> > or >> > >> > size = "$swupdate_get_size(encrypted_file.enc)"; >> > >> > if you pass the encrypted file, you get the size of the encrypted file. >> > >> >> Hence currently swupdate_get_size gives the size of the encrypted >> >> artifact. To avoid any ambiguity I think it's best to rename that >> >> function to swupdate_get_decrypted_size and to get the file from the >> >> DEPLOY_DIR_IMAGE which contains the unencrypted artifact. >> > >> > This is mostly where I stumbled. The logic is that files are already >> > prepared for any function, and no fix path should be set inside the >> > function itself. >> > >> >> Then it can be >> >> used without any ambiguity: >> >> >> >> properties: { >> >> decrypted-size = "$swupdate_get_decrypted_size(...)"; >> >> } >> >> >> >> Fixes: c1586c7de422c ("class: add function to get file size") >> >> Signed-off-by: Sam Van Den Berge <sam.van....@gmail.com> >> >> --- >> >> classes/swupdate-lib.bbclass | 5 +++-- >> >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/classes/swupdate-lib.bbclass >> b/classes/swupdate-lib.bbclass >> >> index 14a2a08..30d2e04 100644 >> >> --- a/classes/swupdate-lib.bbclass >> >> +++ b/classes/swupdate-lib.bbclass >> >> @@ -40,10 +40,11 @@ def swupdate_get_sha256(d, s, filename): >> >> m.update(data) >> >> return m.hexdigest() >> >> -def swupdate_get_size(d, s, filename): >> >> +def swupdate_get_decrypted_size(d, s, filename): >> >> import os >> >> - fname = os.path.join(s, filename) >> >> + deploydir = d.getVar('DEPLOY_DIR_IMAGE', True) >> >> + fname = os.path.join(deploydir, filename) >> >> fsize = os.path.getsize(fname) >> >> return str(fsize) >> > >> > Best regards, >> > Stefano Babic >> > >> >> -- >> ===================================================================== >> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk >> HRB 165235 Munich, Office: Kirchenstr.5, 82194 Groebenzell, Germany >> Phone: +49-8142-66989-53 <+49%208142%206698953> Fax: +49-8142-66989-80 >> <+49%208142%206698980> Email: sba...@denx.de >> ===================================================================== >> >>
On Tue, Nov 08, 2022 at 03:56:59PM +0100, Stefano Babic wrote: > +Ayoub, this time with the right E-Mail address. > > On 08.11.22 14:41, Stefano Babic wrote: > > Hallo Sam, > > > > added Ayoub as author of the original commit: > > > > On 27.10.22 16:21, sam.van.den.berge@gmail.com wrote: > > > From: Sam Van Den Berge <sam.van.den.berge@gmail.com> > > > > > > Commit c1586c7de422c introduced swupdate_get_size with the intention to > > > have the unencrypted size of an artifact. However it gets the size from > > > the artifact in the "s" folder which is the encrypted artifact. > > > > Not sure here. The function simply returns the size of the file passed > > as parameter. We can write > > > > size = "$swupdate_get_size(unencrypted_file)"; > > > > or > > > > size = "$swupdate_get_size(encrypted_file.enc)"; > > > > if you pass the encrypted file, you get the size of the encrypted file. > > As I understand, if you request to encrypt an image via SWUPDATE_IMAGES_ENCRYPTED, the function add_image_to_swu will encrypt the image and store it in the destination "S". The unencrypted file is not available in the "S" folder so you can't get the "decrypted size" anymore via files in the "S" folder. There is also nowhere a relation anymore to the original source (unencrypted file). I might be missing something but with the current implementation I don't understand how to automatically get the decrypted size of an image. I'm wondering how it could have worked for Ayoub. Unless by coincidence the decrypted and encrypted size were the same. After processing the above feedback I got, I did find one way to get the decrypted size with the current implementation: size = "$swupdate_get_size(@@DEPLOY_DIR_IMAGE@@/image)" In this case, the filename argument in os.path.join(s, filename) in swupdate_get_size will be an absolute path and so os.path.join will return filename which is the absolute path to "image". Then I do seem to get the correct decrypted size. Is this the recommended way of working? > > > Hence currently swupdate_get_size gives the size of the encrypted > > > artifact. To avoid any ambiguity I think it's best to rename that > > > function to swupdate_get_decrypted_size and to get the file from the > > > DEPLOY_DIR_IMAGE which contains the unencrypted artifact. > > > > This is mostly where I stumbled. The logic is that files are already > > prepared for any function, and no fix path should be set inside the > > function itself. > > > > > Then it can be > > > used without any ambiguity: > > > > > > properties: { > > > decrypted-size = "$swupdate_get_decrypted_size(...)"; > > > } > > > > > > Fixes: c1586c7de422c ("class: add function to get file size") > > > Signed-off-by: Sam Van Den Berge <sam.van.den.berge@gmail.com> > > > --- > > > classes/swupdate-lib.bbclass | 5 +++-- > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/classes/swupdate-lib.bbclass b/classes/swupdate-lib.bbclass > > > index 14a2a08..30d2e04 100644 > > > --- a/classes/swupdate-lib.bbclass > > > +++ b/classes/swupdate-lib.bbclass > > > @@ -40,10 +40,11 @@ def swupdate_get_sha256(d, s, filename): > > > m.update(data) > > > return m.hexdigest() > > > -def swupdate_get_size(d, s, filename): > > > +def swupdate_get_decrypted_size(d, s, filename): > > > import os > > > - fname = os.path.join(s, filename) > > > + deploydir = d.getVar('DEPLOY_DIR_IMAGE', True) > > > + fname = os.path.join(deploydir, filename) > > > fsize = os.path.getsize(fname) > > > return str(fsize) > > > > Best regards, > > Stefano Babic > > > > -- > ===================================================================== > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, 82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de > ===================================================================== >
I didn't use it to get the decrypted size ! I was just given as an example, I use it for something different actually. On Thursday, November 10, 2022 at 11:16:11 AM UTC+1 sam.van....@gmail.com wrote: > On Tue, Nov 08, 2022 at 03:56:59PM +0100, Stefano Babic wrote: > > +Ayoub, this time with the right E-Mail address. > > > > On 08.11.22 14:41, Stefano Babic wrote: > > > Hallo Sam, > > > > > > added Ayoub as author of the original commit: > > > > > > On 27.10.22 16:21, sam.van....@gmail.com wrote: > > > > From: Sam Van Den Berge <sam.van....@gmail.com> > > > > > > > > Commit c1586c7de422c introduced swupdate_get_size with the intention > to > > > > have the unencrypted size of an artifact. However it gets the size > from > > > > the artifact in the "s" folder which is the encrypted artifact. > > > > > > Not sure here. The function simply returns the size of the file passed > > > as parameter. We can write > > > > > > size = "$swupdate_get_size(unencrypted_file)"; > > > > > > or > > > > > > size = "$swupdate_get_size(encrypted_file.enc)"; > > > > > > if you pass the encrypted file, you get the size of the encrypted file. > > > > > As I understand, if you request to encrypt an image via > SWUPDATE_IMAGES_ENCRYPTED, the function add_image_to_swu will encrypt the > image > and store it in the destination "S". The unencrypted file is not available > in > the "S" folder so you can't get the "decrypted size" anymore via files in > the > "S" folder. There is also nowhere a relation anymore to the original source > (unencrypted file). > > I might be missing something but with the current implementation I don't > understand how to automatically get the decrypted size of an image. I'm > wondering how it could have worked for Ayoub. Unless by coincidence the > decrypted and encrypted size were the same. > > After processing the above feedback I got, I did find one way to get the > decrypted size with the current implementation: > > size = "$swupdate_get_size(@@DEPLOY_DIR_IMAGE@@/image)" > > In this case, the filename argument in os.path.join(s, filename) in > swupdate_get_size will be an absolute path and so os.path.join will return > filename which is the absolute path to "image". Then I do seem to get the > correct decrypted size. Is this the recommended way of working? > > > > > Hence currently swupdate_get_size gives the size of the encrypted > > > > artifact. To avoid any ambiguity I think it's best to rename that > > > > function to swupdate_get_decrypted_size and to get the file from the > > > > DEPLOY_DIR_IMAGE which contains the unencrypted artifact. > > > > > > This is mostly where I stumbled. The logic is that files are already > > > prepared for any function, and no fix path should be set inside the > > > function itself. > > > > > > > Then it can be > > > > used without any ambiguity: > > > > > > > > properties: { > > > > decrypted-size = "$swupdate_get_decrypted_size(...)"; > > > > } > > > > > > > > Fixes: c1586c7de422c ("class: add function to get file size") > > > > Signed-off-by: Sam Van Den Berge <sam.van....@gmail.com> > > > > --- > > > > classes/swupdate-lib.bbclass | 5 +++-- > > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/classes/swupdate-lib.bbclass > b/classes/swupdate-lib.bbclass > > > > index 14a2a08..30d2e04 100644 > > > > --- a/classes/swupdate-lib.bbclass > > > > +++ b/classes/swupdate-lib.bbclass > > > > @@ -40,10 +40,11 @@ def swupdate_get_sha256(d, s, filename): > > > > m.update(data) > > > > return m.hexdigest() > > > > -def swupdate_get_size(d, s, filename): > > > > +def swupdate_get_decrypted_size(d, s, filename): > > > > import os > > > > - fname = os.path.join(s, filename) > > > > + deploydir = d.getVar('DEPLOY_DIR_IMAGE', True) > > > > + fname = os.path.join(deploydir, filename) > > > > fsize = os.path.getsize(fname) > > > > return str(fsize) > > > > > > Best regards, > > > Stefano Babic > > > > > > > -- > > ===================================================================== > > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > > HRB 165235 Munich, Office: Kirchenstr.5, 82194 Groebenzell, Germany > > Phone: +49-8142-66989-53 <+49%208142%206698953> Fax: +49-8142-66989-80 > <+49%208142%206698980> Email: sba...@denx.de > > ===================================================================== > > >
diff --git a/classes/swupdate-lib.bbclass b/classes/swupdate-lib.bbclass index 14a2a08..30d2e04 100644 --- a/classes/swupdate-lib.bbclass +++ b/classes/swupdate-lib.bbclass @@ -40,10 +40,11 @@ def swupdate_get_sha256(d, s, filename): m.update(data) return m.hexdigest() -def swupdate_get_size(d, s, filename): +def swupdate_get_decrypted_size(d, s, filename): import os - fname = os.path.join(s, filename) + deploydir = d.getVar('DEPLOY_DIR_IMAGE', True) + fname = os.path.join(deploydir, filename) fsize = os.path.getsize(fname) return str(fsize)