| Message ID | 20250827064940.16074-1-r.gruenert@pironex.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v4] scripts/runqemu: raise an error when bitbake was not found | expand |
Thanks, I think this is good now. Alex On Wed, 27 Aug 2025 at 08:50, Richard Grünert <r.gruenert@pironex.com> wrote: > > Running 'scrupts/runqemu' without bitbake in PATH causes the > following error: > > ``` > Traceback (most recent call last): > File "/home/rg/temp_stuff/oe_2/./scripts/runqemu", line 1807, in main > config.check_args() > ~~~~~~~~~~~~~~~~~^^ > File "/home/rg/temp_stuff/oe_2/./scripts/runqemu", line 624, in check_args > s = re.search('^DEPLOY_DIR_IMAGE="(.*)"', self.bitbake_e, re.M) > File "/usr/lib/python3.13/re/__init__.py", line 177, in search > return _compile(pattern, flags).search(string) > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^ > TypeError: expected string or bytes-like object, got 'NoneType' > ``` > > This patch adds a more helpful error message to inform the user that > bitbake was not found, e.g. because oe-init-build-env was not sourced. > > This is an example of the new error message after the patch: > > ``` > runqemu - ERROR - In order for this script to dynamically infer paths > kernels or filesystem images, you either need bitbake in your PATH > or to source oe-init-build-env before running this script. > > Dynamic path inference can be avoided by passing a *.qemuboot.conf to > runqemu, i.e. `runqemu /path/to/my-image-name.qemuboot.conf` > > Bitbake is needed to run 'bitbake -e', but it is not found in PATH. Please source the bitbake build environment. > ``` > > CC: Richard Purdie <richard.purdie@linuxfoundation.org> > CC: Alexander Kanavin <alex.kanavin@gmail.com> > > Signed-off-by: Richard Grünert <r.gruenert@pironex.com> > --- > changes in v4: > Changed commit message and error message string according > to suggestions by Alexander Kanavin. > > changes in v3: > Messed up the CC tag :) > > changes in v2: > Changed the position according to suggestion by Alexander Kanavin. > I also changed the error to OEPathError as this seems to be made for > this exact situation. > --- > scripts/runqemu | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/scripts/runqemu b/scripts/runqemu > index c24528eac8..69872f6de8 100755 > --- a/scripts/runqemu > +++ b/scripts/runqemu > @@ -1713,9 +1713,6 @@ to your build configuration. > self.cleaned = True > > def run_bitbake_env(self, mach=None, target=''): > - bitbake = shutil.which('bitbake') > - if not bitbake: > - return > > if not mach: > mach = self.get('MACHINE') > @@ -1732,6 +1729,10 @@ to your build configuration. > else: > cmd = 'bitbake -e %s %s' % (multiconfig, target) > > + bitbake = shutil.which('bitbake') > + if not bitbake: > + raise OEPathError("Bitbake is needed to run '%s', but it is not found in PATH. Please source the bitbake build environment." % cmd.strip()) > + > logger.info('Running %s...' % cmd) > try: > return subprocess.check_output(cmd, shell=True).decode('utf-8')
On Wed, 2025-08-27 at 08:49 +0200, Richard Grünert wrote: > Running 'scrupts/runqemu' without bitbake in PATH causes the > following error: > > ``` > Traceback (most recent call last): > File "/home/rg/temp_stuff/oe_2/./scripts/runqemu", line 1807, in main > config.check_args() > ~~~~~~~~~~~~~~~~~^^ > File "/home/rg/temp_stuff/oe_2/./scripts/runqemu", line 624, in check_args > s = re.search('^DEPLOY_DIR_IMAGE="(.*)"', self.bitbake_e, re.M) > File "/usr/lib/python3.13/re/__init__.py", line 177, in search > return _compile(pattern, flags).search(string) > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^ > TypeError: expected string or bytes-like object, got 'NoneType' > ``` > > This patch adds a more helpful error message to inform the user that > bitbake was not found, e.g. because oe-init-build-env was not sourced. > > This is an example of the new error message after the patch: > > ``` > runqemu - ERROR - In order for this script to dynamically infer paths > kernels or filesystem images, you either need bitbake in your PATH > or to source oe-init-build-env before running this script. > > Dynamic path inference can be avoided by passing a *.qemuboot.conf to > runqemu, i.e. `runqemu /path/to/my-image-name.qemuboot.conf` > > Bitbake is needed to run 'bitbake -e', but it is not found in PATH. Please source the bitbake build environment. > ``` > > CC: Richard Purdie <richard.purdie@linuxfoundation.org> > CC: Alexander Kanavin <alex.kanavin@gmail.com> > > Signed-off-by: Richard Grünert <r.gruenert@pironex.com> > --- > changes in v4: > Changed commit message and error message string according > to suggestions by Alexander Kanavin. > > changes in v3: > Messed up the CC tag :) > > changes in v2: > Changed the position according to suggestion by Alexander Kanavin. > I also changed the error to OEPathError as this seems to be made for > this exact situation. > --- > scripts/runqemu | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/scripts/runqemu b/scripts/runqemu > index c24528eac8..69872f6de8 100755 > --- a/scripts/runqemu > +++ b/scripts/runqemu > @@ -1713,9 +1713,6 @@ to your build configuration. > self.cleaned = True > > def run_bitbake_env(self, mach=None, target=''): > - bitbake = shutil.which('bitbake') > - if not bitbake: > - return > > if not mach: > mach = self.get('MACHINE') > @@ -1732,6 +1729,10 @@ to your build configuration. > else: > cmd = 'bitbake -e %s %s' % (multiconfig, target) > > + bitbake = shutil.which('bitbake') > + if not bitbake: > + raise OEPathError("Bitbake is needed to run '%s', but it is not found in PATH. Please source the bitbake build environment." % cmd.strip()) > + > logger.info('Running %s...' % cmd) > try: > return subprocess.check_output(cmd, shell=True).decode('utf-8') Sorry this has sat in review for a while, I needed to check into something as I had a feeling there was once a codepath this would break. In runqemu there are these comments: # if we're running under testimage, or similarly as a child # of an existing bitbake invocation, we can't invoke bitbake # to validate the MACHINE setting and must assume it's correct... and more specifically this code: if not self.bitbake_e: self.load_bitbake_env() if self.bitbake_e: [...] else: # when we're invoked from a running bitbake instance we won't # be able to call `bitbake -e`, then try: # - get OE_TMPDIR from environment and guess paths based on it # - get OECORE_NATIVE_SYSROOT from environment (for sdk) tmpdir = self.get('OE_TMPDIR') i.e. there is a case where we do run the bitbake -e command, bitbake isn't found and we don't expect the issue to be fatal. OE_TMPDIR is only set by our internal qemu runners: meta/lib/oeqa/utils/qemurunner.py: env["OE_TMPDIR"] = self.tmpdir meta/lib/oeqa/utils/qemutinyrunner.py: os.environ["OE_TMPDIR"] = self.tmpdir i.e. in our automated testing, but that is a key workflow we need to support. What is interesting is that your patches do pass testing. It is therefore possible we've changed things and this codepath isn't actually needed any more. That may be due to the qemuboot config files, which we probably didn't have when OE_TMPDIR was added. Either way, we probably do need to try and clean up this codepath before this change can merge. Cheers, Richard
On Thu, 11 Sept 2025 at 15:31, Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > What is interesting is that your patches do pass testing. It is > therefore possible we've changed things and this codepath isn't > actually needed any more. That may be due to the qemuboot config files, > which we probably didn't have when OE_TMPDIR was added. > > Either way, we probably do need to try and clean up this codepath > before this change can merge. There's another spot at the end of the environment getter function where a failure to obtain the environment results in the getter returning nothing: try: return subprocess.check_output(cmd, shell=True).decode('utf-8') except subprocess.CalledProcessError as err: logger.warning("Couldn't run '%s' to gather environment information, giving up with 'bitbake -e':\n%s" % (cmd, err.output.decode('utf-8'))) return '' If this code path has been executed this *might* explain why testing had passed. Anyway, I think this is all horrible code, and the environment getter should fail if it can't get the environment, for any reason. Then it's on the caller to either not try to get the environment in the first place (by checking for presence of OE_TMPDIR first), or handle the fail. I apologize that a simple usability tweak has turned into a total rabbit hole. So my suggestion is: 1. Keep the current patch. 2. Add a patch that removes the above try... except block so that the environment getter function does not return nothing in any circumstance, and always fails if it can't get the environment. We take that on the autobuilder and see what fails and when, or if nothing does. Then we can formulate what can be removed/reworked around the OE_TMPDIR getting and setting. Alex
On Thu, 11 Sept 2025 at 17:55, Alexander Kanavin via lists.openembedded.org <alex.kanavin=gmail.com@lists.openembedded.org> wrote: > > On Thu, 11 Sept 2025 at 15:31, Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: > > > What is interesting is that your patches do pass testing. It is > > therefore possible we've changed things and this codepath isn't > > actually needed any more. That may be due to the qemuboot config files, > > which we probably didn't have when OE_TMPDIR was added. > > > > Either way, we probably do need to try and clean up this codepath > > before this change can merge. > > There's another spot at the end of the environment getter function > where a failure to obtain the environment results in the getter > returning nothing: > > try: > return subprocess.check_output(cmd, shell=True).decode('utf-8') > except subprocess.CalledProcessError as err: > logger.warning("Couldn't run '%s' to gather > environment information, giving up with 'bitbake -e':\n%s" % (cmd, > err.output.decode('utf-8'))) > return '' > > If this code path has been executed this *might* explain why testing had passed. > > Anyway, I think this is all horrible code, and the environment getter > should fail if it can't get the environment, for any reason. Then it's > on the caller to either not try to get the environment in the first > place (by checking for presence of OE_TMPDIR first), or handle the > fail. > > I apologize that a simple usability tweak has turned into a total rabbit hole. > > So my suggestion is: > 1. Keep the current patch. > 2. Add a patch that removes the above try... except block so that the > environment getter function does not return nothing in any > circumstance, and always fails if it can't get the environment. > > We take that on the autobuilder and see what fails and when, or if > nothing does. Then we can formulate what can be removed/reworked > around the OE_TMPDIR getting and setting. I added the patch in point 2, and ran a-full on the AB. The outcome is that nothing failed: https://autobuilder.yoctoproject.org/valkyrie/#/builders/29/builds/2401 (the three oe-selftest fails are due to an unrelated issue in bitbake-selftest for which a fix has been sent) This means we can indeed remove the code paths that set and get of OE_TMPDIR, as they are never used, and then change the environment getter function in runqemu to always either return the environment or fail with an exception. Richard G., are you still able to work on this subject? Cheers, Alex
Hi Alex,
I want to make sure I'm understanding you correctly, so
here are some questions regarding the changes:
1.
When removing the try/except-block, should we just return the error and
take the python-exception or re-raise the error?, i.e. like this:
*
[...]
*
logger.info('Running %s...' % cmd)
*
return subprocess.check_output(cmd, shell=True).decode('utf-8')
*
2.
or this:
try:
*
return subprocess.check_output(cmd, shell=True).decode('utf-8')
*
except:
*
raise QuemuError("abcdefg")
*
1.
Then, basically, all this can simply be removed:
# if we're running under testimage, or similarly as a child
# of an existing bitbake invocation, we can't invoke bitbake
# to validate the MACHINE setting and must assume it's correct...
# FIXME: testimage.bbclass exports these two variables into env,
# are there other scenarios in which we need to support being
# invoked by bitbake?
deploy = self.get('DEPLOY_DIR_IMAGE')
image_link_name = self.get('IMAGE_LINK_NAME')
bbchild = deploy and self.get('OE_TMPDIR')
if bbchild:
self.set_machine_deploy_dir(arg, deploy)
return
as well as (validate_paths()):
...
*
if self.bitbake_e:
*
native_vars = ...
*
...
*
else:
*
tmpdir = self.get('OE_TMPDIR')
*
...
*
1.
so the latter becomes this:
*
if not havenative:
*
if not self.bitbake_e:
*
self.load_bitbake_env()
*
native_vars = ..
*
...
*
2.
I suppose these changes should be in a patch separate from the initial patch
(scripts/runqemu: raise an error when bitbake was not found)?
Best Regards
Richard G.
________________________________
Von: Alexander Kanavin <alex.kanavin@gmail.com>
Gesendet: Donnerstag, 18. September 2025 19:11
An: alex.kanavin@gmail.com <alex.kanavin@gmail.com>
Cc: Richard Grünert | pironex technology GmbH <r.gruenert@pironex.com>; openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org>; Richard Purdie <richard.purdie@linuxfoundation.org>
Betreff: Re: [OE-core] [PATCH v4] scripts/runqemu: raise an error when bitbake was not found
>>>> Achtung: Diese E-Mail stammt von einem externen Absender (außerhalb von pironex.com)<<<<
On Thu, 11 Sept 2025 at 17:55, Alexander Kanavin via
lists.openembedded.org <alex.kanavin=gmail.com@lists.openembedded.org>
wrote:
>
> On Thu, 11 Sept 2025 at 15:31, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
>
> > What is interesting is that your patches do pass testing. It is
> > therefore possible we've changed things and this codepath isn't
> > actually needed any more. That may be due to the qemuboot config files,
> > which we probably didn't have when OE_TMPDIR was added.
> >
> > Either way, we probably do need to try and clean up this codepath
> > before this change can merge.
>
> There's another spot at the end of the environment getter function
> where a failure to obtain the environment results in the getter
> returning nothing:
>
> try:
> return subprocess.check_output(cmd, shell=True).decode('utf-8')
> except subprocess.CalledProcessError as err:
> logger.warning("Couldn't run '%s' to gather
> environment information, giving up with 'bitbake -e':\n%s" % (cmd,
> err.output.decode('utf-8')))
> return ''
>
> If this code path has been executed this *might* explain why testing had passed.
>
> Anyway, I think this is all horrible code, and the environment getter
> should fail if it can't get the environment, for any reason. Then it's
> on the caller to either not try to get the environment in the first
> place (by checking for presence of OE_TMPDIR first), or handle the
> fail.
>
> I apologize that a simple usability tweak has turned into a total rabbit hole.
>
> So my suggestion is:
> 1. Keep the current patch.
> 2. Add a patch that removes the above try... except block so that the
> environment getter function does not return nothing in any
> circumstance, and always fails if it can't get the environment.
>
> We take that on the autobuilder and see what fails and when, or if
> nothing does. Then we can formulate what can be removed/reworked
> around the OE_TMPDIR getting and setting.
I added the patch in point 2, and ran a-full on the AB. The outcome is
that nothing failed:
https://autobuilder.yoctoproject.org/valkyrie/#/builders/29/builds/2401
(the three oe-selftest fails are due to an unrelated issue in
bitbake-selftest for which a fix has been sent)
This means we can indeed remove the code paths that set and get of
OE_TMPDIR, as they are never used, and then change the environment
getter function in runqemu to always either return the environment or
fail with an exception.
Richard G., are you still able to work on this subject?
Cheers,
Alex
On Fri, 19 Sept 2025 at 09:27, Richard Grünert | pironex technology GmbH <r.gruenert@pironex.com> wrote: > I want to make sure I'm understanding you correctly, so > here are some questions regarding the changes: > > When removing the try/except-block, should we just return the error and > take the python-exception or re-raise the error?, i.e. like this: > > [...] > > logger.info('Running %s...' % cmd) > > return subprocess.check_output(cmd, shell=True).decode('utf-8') This is correct. Any errors will be coming from this call directly. I've done this in the additional patch to test usage of OE_TMPDIR (if there was any), so you can take that and extend the commit message with additional explanation: https://git.yoctoproject.org/poky-contrib/commit/?h=akanavin/package-version-updates&id=fa2fb452940e9876ff7266a5678c37db559a5d56 > Then, basically, all this can simply be removed: > > # if we're running under testimage, or similarly as a child > # of an existing bitbake invocation, we can't invoke bitbake > # to validate the MACHINE setting and must assume it's correct... > # FIXME: testimage.bbclass exports these two variables into env, > # are there other scenarios in which we need to support being > # invoked by bitbake? > deploy = self.get('DEPLOY_DIR_IMAGE') > image_link_name = self.get('IMAGE_LINK_NAME') > bbchild = deploy and self.get('OE_TMPDIR') > if bbchild: > self.set_machine_deploy_dir(arg, deploy) > return This code path is not subject to empty bitbake_e, so it might still be executed and things might break if it's taken out. I would keep it in. > as well as (validate_paths()): > > > ... > > if self.bitbake_e: > > native_vars = ... > > ... > > else: > > tmpdir = self.get('OE_TMPDIR') > > ... > > > so the latter becomes this: > > if not havenative: > > if not self.bitbake_e: > > self.load_bitbake_env() > > native_vars = .. > > ... This has been confirmed to never be run, so it can be taken out, yes. > I suppose these changes should be in a patch separate from the initial patch > (scripts/runqemu: raise an error when bitbake was not found)? Yes please, each code change should be in a separate patch, so three patches altogether. Alex
diff --git a/scripts/runqemu b/scripts/runqemu index c24528eac8..69872f6de8 100755 --- a/scripts/runqemu +++ b/scripts/runqemu @@ -1713,9 +1713,6 @@ to your build configuration. self.cleaned = True def run_bitbake_env(self, mach=None, target=''): - bitbake = shutil.which('bitbake') - if not bitbake: - return if not mach: mach = self.get('MACHINE') @@ -1732,6 +1729,10 @@ to your build configuration. else: cmd = 'bitbake -e %s %s' % (multiconfig, target) + bitbake = shutil.which('bitbake') + if not bitbake: + raise OEPathError("Bitbake is needed to run '%s', but it is not found in PATH. Please source the bitbake build environment." % cmd.strip()) + logger.info('Running %s...' % cmd) try: return subprocess.check_output(cmd, shell=True).decode('utf-8')
Running 'scrupts/runqemu' without bitbake in PATH causes the following error: ``` Traceback (most recent call last): File "/home/rg/temp_stuff/oe_2/./scripts/runqemu", line 1807, in main config.check_args() ~~~~~~~~~~~~~~~~~^^ File "/home/rg/temp_stuff/oe_2/./scripts/runqemu", line 624, in check_args s = re.search('^DEPLOY_DIR_IMAGE="(.*)"', self.bitbake_e, re.M) File "/usr/lib/python3.13/re/__init__.py", line 177, in search return _compile(pattern, flags).search(string) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^ TypeError: expected string or bytes-like object, got 'NoneType' ``` This patch adds a more helpful error message to inform the user that bitbake was not found, e.g. because oe-init-build-env was not sourced. This is an example of the new error message after the patch: ``` runqemu - ERROR - In order for this script to dynamically infer paths kernels or filesystem images, you either need bitbake in your PATH or to source oe-init-build-env before running this script. Dynamic path inference can be avoided by passing a *.qemuboot.conf to runqemu, i.e. `runqemu /path/to/my-image-name.qemuboot.conf` Bitbake is needed to run 'bitbake -e', but it is not found in PATH. Please source the bitbake build environment. ``` CC: Richard Purdie <richard.purdie@linuxfoundation.org> CC: Alexander Kanavin <alex.kanavin@gmail.com> Signed-off-by: Richard Grünert <r.gruenert@pironex.com> --- changes in v4: Changed commit message and error message string according to suggestions by Alexander Kanavin. changes in v3: Messed up the CC tag :) changes in v2: Changed the position according to suggestion by Alexander Kanavin. I also changed the error to OEPathError as this seems to be made for this exact situation. --- scripts/runqemu | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)