| Message ID | 20260219162117.2565725-1-pkj@axis.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RESEND] sstate.bbclass: Always show a progress bar if an sstate summary is wanted | expand |
On Thu, 2026-02-19 at 17:21 +0100, Peter Kjellerstedt via lists.openembedded.org wrote: > In case sstate_checkhashes() is expected to show an sstate summary, then > always show the process progress bar regardless of how long the task > list is. Without this, the sstate summary could unintentionally > overwrite another active progress bar. > > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com> > --- > meta/classes-global/sstate.bbclass | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass > index 2fd29d7323..6f72490065 100644 > --- a/meta/classes-global/sstate.bbclass > +++ b/meta/classes-global/sstate.bbclass > @@ -1048,7 +1048,7 @@ def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0, summary=True, > > ## thread-safe counter > cnt_tasks_done = itertools.count(start = 1) > - progress = len(tasklist) >= 100 > + progress = summary or len(tasklist) >= 100 > if progress: > msg = "Checking sstate mirror object availability" > bb.event.fire(bb.event.ProcessStarted(msg, len(tasklist)), d) This has sat in the review queue for a while since I haven't found the time to reply. I suspect you're going to disagree with me and I really haven't had the time to have a discussion on it. For reference, whilst the patch was removed from master-next, it is still in the review queue here: https://git.openembedded.org/openembedded-core-contrib/log/?h=master-backlog (and mentioned in the status report about review call changes) Since you're now forcing me to reply about this, I'm confused by what "unintentionally overwrite" means in the commit message since it would seemingly only do that if it shows progress and you're making it always show progress. That would be in theory worse, but consistent. The intent behind the "100" limit was to only show the progress bar if there is a decent chunk of work to be done, rather than flash it onto the screen then off again. I think that reasoning is still valid, showing progress bars for things which happen more quickly than the progress is useful for is a bit pointless. So at the very least we need to be clear what the commit message means but I'm not keen on the removal of the 100 limit. There were other fixes around the progress bar handling and I am also wondering if this issue was still relevant too. Cheers, Richard
> -----Original Message----- > From: Richard Purdie <richard.purdie@linuxfoundation.org> > Sent: den 19 februari 2026 17:48 > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-core@lists.openembedded.org > Subject: Re: [OE-core] [PATCH][RESEND] sstate.bbclass: Always show a progress bar if an sstate summary is wanted > > On Thu, 2026-02-19 at 17:21 +0100, Peter Kjellerstedt via lists.openembedded.org wrote: > > In case sstate_checkhashes() is expected to show an sstate summary, then > > always show the process progress bar regardless of how long the task > > list is. Without this, the sstate summary could unintentionally > > overwrite another active progress bar. > > > > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com> > > --- > > meta/classes-global/sstate.bbclass | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass > > index 2fd29d7323..6f72490065 100644 > > --- a/meta/classes-global/sstate.bbclass > > +++ b/meta/classes-global/sstate.bbclass > > @@ -1048,7 +1048,7 @@ def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0, summary=True, > > > > ## thread-safe counter > > cnt_tasks_done = itertools.count(start = 1) > > - progress = len(tasklist) >= 100 > > + progress = summary or len(tasklist) >= 100 > > if progress: > > msg = "Checking sstate mirror object availability" > > bb.event.fire(bb.event.ProcessStarted(msg, len(tasklist)), d) > > This has sat in the review queue for a while since I haven't found the > time to reply. I suspect you're going to disagree with me and I really > haven't had the time to have a discussion on it. > > For reference, whilst the patch was removed from master-next, it is > still in the review queue here: > > https://git.openembedded.org/openembedded-core-contrib/log/?h=master-backlog > > (and mentioned in the status report about review call changes) > > Since you're now forcing me to reply about this, I'm confused by what > "unintentionally overwrite" means in the commit message since it would > seemingly only do that if it shows progress and you're making it always > show progress. That would be in theory worse, but consistent. > > The intent behind the "100" limit was to only show the progress bar if > there is a decent chunk of work to be done, rather than flash it onto > the screen then off again. I think that reasoning is still valid, > showing progress bars for things which happen more quickly than the > progress is useful for is a bit pointless. Absolutely. The summary argument is only set to True during the initial check of the sstate cache, i.e., when the "Sstate summary: ..." message is printed at the beginning of the build. So my change only affects this case, and does not affect the progress bars that are shown mid-build as a result of using a hash server. > > So at the very least we need to be clear what the commit message means > but I'm not keen on the removal of the 100 limit. > > There were other fixes around the progress bar handling and I am also > wondering if this issue was still relevant too. Yes, it is still relevant. There are actually two progress bars involved in this case, the one for "Initialising tasks" and the one for "Checking sstate mirror object availability". When it works as expected, it will look something like this (I've added the x's and the y's to make the problem more obvious in the next example): Initialising tasks yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy: 100% |###########| Time: 0:00:00 Checking sstate mirror object availability xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: 100% |###| Time: 0:00:00 Sstate summary: Wanted 9 Local 0 Mirrors 0 Missed 9 Current 255 (0% match, 96% complete) However, when there are less than 100 tasks, then it will instead look like this: Sstate summary: Wanted 0 Local 0 Mirrors 0 Missed 0 Current 17 (0% match, 100% complete)yyyy: 99% |######### | ETA: 0:00:00 Initialising tasks yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy: 100% |##########| Time: 0:00:00 I.e., what's happening here is that the "Initialising tasks" progress bar is still active when the "Sstate summary" is output, but since it does not know about that progress bar being active, it overwrites it, resulting in parts of it remaining at the end of the line. What my change does is to effectively make sure that the "Initialising tasks" progress bar is finalized before the sstate summary is shown. This is achieved by forcing the "Checking sstate mirror object availability" progress bar to be shown, and as this progress bar _is_ synchronized with the output of the sstate summary, everything looks as expected. You can see the difference by running these commands without and with my patch: bitbake m4-native -c cleansstate bitbake m4-native One thing I just noticed though is that my patch missed a case. If there actually are no tasks at all, e.g., when everything is in the sstate cache or when running the cleansstate task, then the output will still be wrong. I will send an updated version of my patch to take care of this, and to try and improve the commit message to better explain what is going on. > Cheers, > > Richard //Peter
diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass index 2fd29d7323..6f72490065 100644 --- a/meta/classes-global/sstate.bbclass +++ b/meta/classes-global/sstate.bbclass @@ -1048,7 +1048,7 @@ def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0, summary=True, ## thread-safe counter cnt_tasks_done = itertools.count(start = 1) - progress = len(tasklist) >= 100 + progress = summary or len(tasklist) >= 100 if progress: msg = "Checking sstate mirror object availability" bb.event.fire(bb.event.ProcessStarted(msg, len(tasklist)), d)
In case sstate_checkhashes() is expected to show an sstate summary, then always show the process progress bar regardless of how long the task list is. Without this, the sstate summary could unintentionally overwrite another active progress bar. Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com> --- meta/classes-global/sstate.bbclass | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)