* [PATCH] classes: decode output data to text @ 2022-12-22 10:38 Pawel Zalewski 2023-01-06 12:06 ` [OE-core] " Richard Purdie 2023-01-16 11:01 ` Richard Purdie 0 siblings, 2 replies; 9+ messages in thread From: Pawel Zalewski @ 2022-12-22 10:38 UTC (permalink / raw) To: openembedded-core; +Cc: Pawel Zalewski The default return value from subprocess.check_output is an encoded byte. The applied fix will decode the value to a string. Signed-off-by: Pawel Zalewski <pzalewski@thegoodpenguin.co.uk> --- meta/classes/fs-uuid.bbclass | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meta/classes/fs-uuid.bbclass b/meta/classes/fs-uuid.bbclass index 9b53dfba7a..731ea575bd 100644 --- a/meta/classes/fs-uuid.bbclass +++ b/meta/classes/fs-uuid.bbclass @@ -4,7 +4,7 @@ def get_rootfs_uuid(d): import subprocess rootfs = d.getVar('ROOTFS') - output = subprocess.check_output(['tune2fs', '-l', rootfs]) + output = subprocess.check_output(['tune2fs', '-l', rootfs], text=True) for line in output.split('\n'): if line.startswith('Filesystem UUID:'): uuid = line.split()[-1] -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [OE-core] [PATCH] classes: decode output data to text 2022-12-22 10:38 [PATCH] classes: decode output data to text Pawel Zalewski @ 2023-01-06 12:06 ` Richard Purdie 2023-01-06 12:40 ` Pawel Zalewski 2023-01-16 11:01 ` Richard Purdie 1 sibling, 1 reply; 9+ messages in thread From: Richard Purdie @ 2023-01-06 12:06 UTC (permalink / raw) To: Pawel Zalewski, openembedded-core On Thu, 2022-12-22 at 10:38 +0000, Pawel Zalewski wrote: > The default return value from subprocess.check_output is an encoded byte. > The applied fix will decode the value to a string. > > Signed-off-by: Pawel Zalewski <pzalewski@thegoodpenguin.co.uk> > --- > meta/classes/fs-uuid.bbclass | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/meta/classes/fs-uuid.bbclass b/meta/classes/fs-uuid.bbclass > index 9b53dfba7a..731ea575bd 100644 > --- a/meta/classes/fs-uuid.bbclass > +++ b/meta/classes/fs-uuid.bbclass > @@ -4,7 +4,7 @@ > def get_rootfs_uuid(d): > import subprocess > rootfs = d.getVar('ROOTFS') > - output = subprocess.check_output(['tune2fs', '-l', rootfs]) > + output = subprocess.check_output(['tune2fs', '-l', rootfs], text=True) > for line in output.split('\n'): > if line.startswith('Filesystem UUID:'): > uuid = line.split()[-1] That looks reasonable, I just wonder how this has worked until now? Why aren't we seeing errors due to this? Does it mean we don't have some test coverage? or was there silent breakage of some kind this fixes? Cheers, Richard ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [OE-core] [PATCH] classes: decode output data to text 2023-01-06 12:06 ` [OE-core] " Richard Purdie @ 2023-01-06 12:40 ` Pawel Zalewski 2023-01-06 12:43 ` Alexander Kanavin 0 siblings, 1 reply; 9+ messages in thread From: Pawel Zalewski @ 2023-01-06 12:40 UTC (permalink / raw) To: Richard Purdie; +Cc: openembedded-core It is not actually being used by default in Yocto ? This was found in kirkstone. But regardless, it is wrong and will drop an error. Kind regards, Pawel On Fri, 6 Jan 2023 at 12:06, Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > > On Thu, 2022-12-22 at 10:38 +0000, Pawel Zalewski wrote: > > The default return value from subprocess.check_output is an encoded byte. > > The applied fix will decode the value to a string. > > > > Signed-off-by: Pawel Zalewski <pzalewski@thegoodpenguin.co.uk> > > --- > > meta/classes/fs-uuid.bbclass | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/meta/classes/fs-uuid.bbclass b/meta/classes/fs-uuid.bbclass > > index 9b53dfba7a..731ea575bd 100644 > > --- a/meta/classes/fs-uuid.bbclass > > +++ b/meta/classes/fs-uuid.bbclass > > @@ -4,7 +4,7 @@ > > def get_rootfs_uuid(d): > > import subprocess > > rootfs = d.getVar('ROOTFS') > > - output = subprocess.check_output(['tune2fs', '-l', rootfs]) > > + output = subprocess.check_output(['tune2fs', '-l', rootfs], text=True) > > for line in output.split('\n'): > > if line.startswith('Filesystem UUID:'): > > uuid = line.split()[-1] > > > That looks reasonable, I just wonder how this has worked until now? Why > aren't we seeing errors due to this? > > Does it mean we don't have some test coverage? or was there silent > breakage of some kind this fixes? > > Cheers, > > Richard ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [OE-core] [PATCH] classes: decode output data to text 2023-01-06 12:40 ` Pawel Zalewski @ 2023-01-06 12:43 ` Alexander Kanavin 2023-01-06 13:10 ` Pawel Zalewski 0 siblings, 1 reply; 9+ messages in thread From: Alexander Kanavin @ 2023-01-06 12:43 UTC (permalink / raw) To: Pawel Zalewski; +Cc: Richard Purdie, openembedded-core [-- Attachment #1: Type: text/plain, Size: 2121 bytes --] Can you please describe steps to reproduce? Alex On Fri 6. Jan 2023 at 13.40, Pawel Zalewski <pzalewski@thegoodpenguin.co.uk> wrote: > It is not actually being used by default in Yocto ? > This was found in kirkstone. > But regardless, it is wrong and will drop an error. > > Kind regards, > Pawel > > On Fri, 6 Jan 2023 at 12:06, Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: > > > > On Thu, 2022-12-22 at 10:38 +0000, Pawel Zalewski wrote: > > > The default return value from subprocess.check_output is an encoded > byte. > > > The applied fix will decode the value to a string. > > > > > > Signed-off-by: Pawel Zalewski <pzalewski@thegoodpenguin.co.uk> > > > --- > > > meta/classes/fs-uuid.bbclass | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/meta/classes/fs-uuid.bbclass > b/meta/classes/fs-uuid.bbclass > > > index 9b53dfba7a..731ea575bd 100644 > > > --- a/meta/classes/fs-uuid.bbclass > > > +++ b/meta/classes/fs-uuid.bbclass > > > @@ -4,7 +4,7 @@ > > > def get_rootfs_uuid(d): > > > import subprocess > > > rootfs = d.getVar('ROOTFS') > > > - output = subprocess.check_output(['tune2fs', '-l', rootfs]) > > > + output = subprocess.check_output(['tune2fs', '-l', rootfs], > text=True) > > > for line in output.split('\n'): > > > if line.startswith('Filesystem UUID:'): > > > uuid = line.split()[-1] > > > > > > That looks reasonable, I just wonder how this has worked until now? Why > > aren't we seeing errors due to this? > > > > Does it mean we don't have some test coverage? or was there silent > > breakage of some kind this fixes? > > > > Cheers, > > > > Richard > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#175583): > https://lists.openembedded.org/g/openembedded-core/message/175583 > Mute This Topic: https://lists.openembedded.org/mt/95823853/1686489 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [ > alex.kanavin@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- > > [-- Attachment #2: Type: text/html, Size: 3528 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [OE-core] [PATCH] classes: decode output data to text 2023-01-06 12:43 ` Alexander Kanavin @ 2023-01-06 13:10 ` Pawel Zalewski 2023-01-06 13:32 ` Alexander Kanavin 0 siblings, 1 reply; 9+ messages in thread From: Pawel Zalewski @ 2023-01-06 13:10 UTC (permalink / raw) To: Alexander Kanavin; +Cc: Richard Purdie, openembedded-core 1. inherit fs-uuid in some-recipe 2. set ROOTFS to some-existing-rootfs 3. make call to get_rootfs_uuid somewhere in that recipe ie. "${@get_rootfs_uuid(d)}" 3. bitbake some-recipe Pawel On Fri, 6 Jan 2023 at 12:43, Alexander Kanavin <alex.kanavin@gmail.com> wrote: > > Can you please describe steps to reproduce? > > Alex > > On Fri 6. Jan 2023 at 13.40, Pawel Zalewski <pzalewski@thegoodpenguin.co.uk> wrote: >> >> It is not actually being used by default in Yocto ? >> This was found in kirkstone. >> But regardless, it is wrong and will drop an error. >> >> Kind regards, >> Pawel >> >> On Fri, 6 Jan 2023 at 12:06, Richard Purdie >> <richard.purdie@linuxfoundation.org> wrote: >> > >> > On Thu, 2022-12-22 at 10:38 +0000, Pawel Zalewski wrote: >> > > The default return value from subprocess.check_output is an encoded byte. >> > > The applied fix will decode the value to a string. >> > > >> > > Signed-off-by: Pawel Zalewski <pzalewski@thegoodpenguin.co.uk> >> > > --- >> > > meta/classes/fs-uuid.bbclass | 2 +- >> > > 1 file changed, 1 insertion(+), 1 deletion(-) >> > > >> > > diff --git a/meta/classes/fs-uuid.bbclass b/meta/classes/fs-uuid.bbclass >> > > index 9b53dfba7a..731ea575bd 100644 >> > > --- a/meta/classes/fs-uuid.bbclass >> > > +++ b/meta/classes/fs-uuid.bbclass >> > > @@ -4,7 +4,7 @@ >> > > def get_rootfs_uuid(d): >> > > import subprocess >> > > rootfs = d.getVar('ROOTFS') >> > > - output = subprocess.check_output(['tune2fs', '-l', rootfs]) >> > > + output = subprocess.check_output(['tune2fs', '-l', rootfs], text=True) >> > > for line in output.split('\n'): >> > > if line.startswith('Filesystem UUID:'): >> > > uuid = line.split()[-1] >> > >> > >> > That looks reasonable, I just wonder how this has worked until now? Why >> > aren't we seeing errors due to this? >> > >> > Does it mean we don't have some test coverage? or was there silent >> > breakage of some kind this fixes? >> > >> > Cheers, >> > >> > Richard >> >> -=-=-=-=-=-=-=-=-=-=-=- >> Links: You receive all messages sent to this group. >> View/Reply Online (#175583): https://lists.openembedded.org/g/openembedded-core/message/175583 >> Mute This Topic: https://lists.openembedded.org/mt/95823853/1686489 >> Group Owner: openembedded-core+owner@lists.openembedded.org >> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com] >> -=-=-=-=-=-=-=-=-=-=-=- >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [OE-core] [PATCH] classes: decode output data to text 2023-01-06 13:10 ` Pawel Zalewski @ 2023-01-06 13:32 ` Alexander Kanavin 2023-01-16 10:58 ` Richard Purdie 0 siblings, 1 reply; 9+ messages in thread From: Alexander Kanavin @ 2023-01-06 13:32 UTC (permalink / raw) To: Pawel Zalewski; +Cc: Richard Purdie, openembedded-core Now I see what's going on. The fs-uuid class contains two functions: get_rootfs_uuid, and replace_rootfs_uuid. The former is not used anywhere, but the latter is. Can you introduce get_rootfs_uuid() into something in oe-core, perhaps it could enhance existing code somewhere (maybe complement replace_rootfs_uuid usage?), or a test could be made better? Otherwise, it's still not tested, and will regress without notice. Alex On Fri, 6 Jan 2023 at 14:11, Pawel Zalewski <pzalewski@thegoodpenguin.co.uk> wrote: > > 1. inherit fs-uuid in some-recipe > 2. set ROOTFS to some-existing-rootfs > 3. make call to get_rootfs_uuid somewhere in that recipe ie. > "${@get_rootfs_uuid(d)}" > 3. bitbake some-recipe > > Pawel > > On Fri, 6 Jan 2023 at 12:43, Alexander Kanavin <alex.kanavin@gmail.com> wrote: > > > > Can you please describe steps to reproduce? > > > > Alex > > > > On Fri 6. Jan 2023 at 13.40, Pawel Zalewski <pzalewski@thegoodpenguin.co.uk> wrote: > >> > >> It is not actually being used by default in Yocto ? > >> This was found in kirkstone. > >> But regardless, it is wrong and will drop an error. > >> > >> Kind regards, > >> Pawel > >> > >> On Fri, 6 Jan 2023 at 12:06, Richard Purdie > >> <richard.purdie@linuxfoundation.org> wrote: > >> > > >> > On Thu, 2022-12-22 at 10:38 +0000, Pawel Zalewski wrote: > >> > > The default return value from subprocess.check_output is an encoded byte. > >> > > The applied fix will decode the value to a string. > >> > > > >> > > Signed-off-by: Pawel Zalewski <pzalewski@thegoodpenguin.co.uk> > >> > > --- > >> > > meta/classes/fs-uuid.bbclass | 2 +- > >> > > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > > >> > > diff --git a/meta/classes/fs-uuid.bbclass b/meta/classes/fs-uuid.bbclass > >> > > index 9b53dfba7a..731ea575bd 100644 > >> > > --- a/meta/classes/fs-uuid.bbclass > >> > > +++ b/meta/classes/fs-uuid.bbclass > >> > > @@ -4,7 +4,7 @@ > >> > > def get_rootfs_uuid(d): > >> > > import subprocess > >> > > rootfs = d.getVar('ROOTFS') > >> > > - output = subprocess.check_output(['tune2fs', '-l', rootfs]) > >> > > + output = subprocess.check_output(['tune2fs', '-l', rootfs], text=True) > >> > > for line in output.split('\n'): > >> > > if line.startswith('Filesystem UUID:'): > >> > > uuid = line.split()[-1] > >> > > >> > > >> > That looks reasonable, I just wonder how this has worked until now? Why > >> > aren't we seeing errors due to this? > >> > > >> > Does it mean we don't have some test coverage? or was there silent > >> > breakage of some kind this fixes? > >> > > >> > Cheers, > >> > > >> > Richard > >> > >> -=-=-=-=-=-=-=-=-=-=-=- > >> Links: You receive all messages sent to this group. > >> View/Reply Online (#175583): https://lists.openembedded.org/g/openembedded-core/message/175583 > >> Mute This Topic: https://lists.openembedded.org/mt/95823853/1686489 > >> Group Owner: openembedded-core+owner@lists.openembedded.org > >> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com] > >> -=-=-=-=-=-=-=-=-=-=-=- > >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [OE-core] [PATCH] classes: decode output data to text 2023-01-06 13:32 ` Alexander Kanavin @ 2023-01-16 10:58 ` Richard Purdie 0 siblings, 0 replies; 9+ messages in thread From: Richard Purdie @ 2023-01-16 10:58 UTC (permalink / raw) To: Alexander Kanavin, Pawel Zalewski; +Cc: openembedded-core On Fri, 2023-01-06 at 14:32 +0100, Alexander Kanavin wrote: > Now I see what's going on. The fs-uuid class contains two functions: > get_rootfs_uuid, and replace_rootfs_uuid. The former is not used > anywhere, but the latter is. Can you introduce get_rootfs_uuid() into > something in oe-core, perhaps it could enhance existing code somewhere > (maybe complement replace_rootfs_uuid usage?), or a test could be made > better? > > Otherwise, it's still not tested, and will regress without notice. I still didn't quite understand this explanation since replace_rootfs_uuid() calls get_rootfs_uuid(). The issue is that UUID_PLACEHOLDER isn't used anywhere in core: UUID_PLACEHOLDER = '<<uuid-of-rootfs>>' which means that get_rootfs_uuid() is never called. There are some hints about how this should be used here: https://git.yoctoproject.org/poky/commit/meta/?id=6d7bcd4df5722f3ccebbbf73361838cb0a5dc0ee The fix is therefore probably ok but a test of UUID_PLACEHOLDER is missing. Looking at the class code, these two functions should be moved to lib/oe/ as well, we don't need a bbclass for this. Cheers, Richard ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [OE-core] [PATCH] classes: decode output data to text 2022-12-22 10:38 [PATCH] classes: decode output data to text Pawel Zalewski 2023-01-06 12:06 ` [OE-core] " Richard Purdie @ 2023-01-16 11:01 ` Richard Purdie 2023-01-16 11:14 ` Pawel Zalewski 1 sibling, 1 reply; 9+ messages in thread From: Richard Purdie @ 2023-01-16 11:01 UTC (permalink / raw) To: Pawel Zalewski, openembedded-core On Thu, 2022-12-22 at 10:38 +0000, Pawel Zalewski wrote: > The default return value from subprocess.check_output is an encoded byte. > The applied fix will decode the value to a string. > > Signed-off-by: Pawel Zalewski <pzalewski@thegoodpenguin.co.uk> > --- > meta/classes/fs-uuid.bbclass | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/meta/classes/fs-uuid.bbclass b/meta/classes/fs-uuid.bbclass > index 9b53dfba7a..731ea575bd 100644 > --- a/meta/classes/fs-uuid.bbclass > +++ b/meta/classes/fs-uuid.bbclass > @@ -4,7 +4,7 @@ > def get_rootfs_uuid(d): > import subprocess > rootfs = d.getVar('ROOTFS') > - output = subprocess.check_output(['tune2fs', '-l', rootfs]) > + output = subprocess.check_output(['tune2fs', '-l', rootfs], text=True) > for line in output.split('\n'): > if line.startswith('Filesystem UUID:'): > uuid = line.split()[-1] You mentioned running into this on kirkstone. One problem is that text=True is python 3.7 syntax so whilst this is fine for master, kirkstone supports older versions of python. Cheers, Richard ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [OE-core] [PATCH] classes: decode output data to text 2023-01-16 11:01 ` Richard Purdie @ 2023-01-16 11:14 ` Pawel Zalewski 0 siblings, 0 replies; 9+ messages in thread From: Pawel Zalewski @ 2023-01-16 11:14 UTC (permalink / raw) To: Richard Purdie; +Cc: openembedded-core [-- Attachment #1: Type: text/plain, Size: 1408 bytes --] That is a good point, looking at the codebase I think that the go-to for dealing with it is using the call to ".decode('utf8')". Thanks, Pawel On Mon, 16 Jan 2023 at 11:01, Richard Purdie < richard.purdie@linuxfoundation.org> wrote: > On Thu, 2022-12-22 at 10:38 +0000, Pawel Zalewski wrote: > > The default return value from subprocess.check_output is an encoded byte. > > The applied fix will decode the value to a string. > > > > Signed-off-by: Pawel Zalewski <pzalewski@thegoodpenguin.co.uk> > > --- > > meta/classes/fs-uuid.bbclass | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/meta/classes/fs-uuid.bbclass b/meta/classes/fs-uuid.bbclass > > index 9b53dfba7a..731ea575bd 100644 > > --- a/meta/classes/fs-uuid.bbclass > > +++ b/meta/classes/fs-uuid.bbclass > > @@ -4,7 +4,7 @@ > > def get_rootfs_uuid(d): > > import subprocess > > rootfs = d.getVar('ROOTFS') > > - output = subprocess.check_output(['tune2fs', '-l', rootfs]) > > + output = subprocess.check_output(['tune2fs', '-l', rootfs], > text=True) > > for line in output.split('\n'): > > if line.startswith('Filesystem UUID:'): > > uuid = line.split()[-1] > > You mentioned running into this on kirkstone. One problem is that > text=True is python 3.7 syntax so whilst this is fine for master, > kirkstone supports older versions of python. > > Cheers, > > Richard > [-- Attachment #2: Type: text/html, Size: 2085 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-01-16 11:14 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-12-22 10:38 [PATCH] classes: decode output data to text Pawel Zalewski 2023-01-06 12:06 ` [OE-core] " Richard Purdie 2023-01-06 12:40 ` Pawel Zalewski 2023-01-06 12:43 ` Alexander Kanavin 2023-01-06 13:10 ` Pawel Zalewski 2023-01-06 13:32 ` Alexander Kanavin 2023-01-16 10:58 ` Richard Purdie 2023-01-16 11:01 ` Richard Purdie 2023-01-16 11:14 ` Pawel Zalewski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.