diff mbox series

[auh,5/5] Record commit failures as group errors in Updater class so that commit errors are not treated as SUCCESS but as a FAILURE

Message ID 20260104004708.2494403-9-t.f.g.geelen@gmail.com
State New
Headers show
Series [auh,1/5] README: fix version numbering. | expand

Commit Message

Tom Geelen Jan. 4, 2026, 12:47 a.m. UTC
What is happening before: commit_changes() throws, run() catches it and removes the group from succeeded_pkggroups_ctx, but it never sets g['error']. The Statistics.update() logic classifies groups purely by g['error'] is None → “Succeeded”. So you end up with “Succeeded: 1” even though commit failed.

Signed-off-by: Tom Geelen <t.f.g.geelen@gmail.com>
---
 upgrade-helper.py | 2 ++
 1 file changed, 2 insertions(+)

Comments

Alexander Kanavin Jan. 5, 2026, 12:21 p.m. UTC | #1
On Sun, 4 Jan 2026 at 01:49, Tom Geelen <t.f.g.geelen@gmail.com> wrote:

> What is happening before: commit_changes() throws, run() catches it and removes the group from succeeded_pkggroups_ctx, but it never sets g['error']. The Statistics.update() logic classifies groups purely by g['error'] is None → “Succeeded”. So you end up with “Succeeded: 1” even though commit failed.
>
> Signed-off-by: Tom Geelen <t.f.g.geelen@gmail.com>
> ---
>  upgrade-helper.py | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/upgrade-helper.py b/upgrade-helper.py
> index 165fd42..8457c7b 100755
> --- a/upgrade-helper.py
> +++ b/upgrade-helper.py
> @@ -528,6 +528,8 @@ class Updater(object):
>              except Exception as e:
>                  import traceback
>                  E(" Couldn't commit changes to %s:\n%s" % (pkggroup_name, traceback.format_exc()))
> +                # Ensure commit failures are recorded as group errors
> +                g['error'] = e
>                  if g in succeeded_pkggroups_ctx:
>                      succeeded_pkggroups_ctx.remove(g)
>                      failed_pkggroups_ctx.append(g)

The first four commits are fine and I'll apply and push them, but this
one needs a different fix first.

Specifically, commit_changes() runs even when there's nothing to
commit (e.g. devtool upgrade failed and no changes were produced), and
then it fails. In this situation, the actual error is that devtool
failed, not that there was no code to commit, and this fix would
obscure that devtool error.

Looking at commit_changes, it has this bit:

        except Error as e:
            msg = ''

            for line in e.stdout.split("\n"):
                if line.find("nothing to commit") == 0:
                    msg = "Nothing to commit!"
                    I(" %s: %s" % (pns, msg))

            I(" %s: %s" % (pns, e.stdout))
            raise e

So we either need to suppress the 'raise e' when there's nothing to
commit, or run commit_changes() only if there are actual changes ('git
status --porcelain' can help there).

This has been a long-standing issue with autobuilder AUH, it's full of
traceback clutter because of it, e.g.
https://autobuilder.yoctoproject.org/valkyrie/#/builders/38/builds/46/steps/15/logs/stdio
(line 9916 onwards).

Alex
diff mbox series

Patch

diff --git a/upgrade-helper.py b/upgrade-helper.py
index 165fd42..8457c7b 100755
--- a/upgrade-helper.py
+++ b/upgrade-helper.py
@@ -528,6 +528,8 @@  class Updater(object):
             except Exception as e:
                 import traceback
                 E(" Couldn't commit changes to %s:\n%s" % (pkggroup_name, traceback.format_exc()))
+                # Ensure commit failures are recorded as group errors
+                g['error'] = e
                 if g in succeeded_pkggroups_ctx:
                     succeeded_pkggroups_ctx.remove(g)
                     failed_pkggroups_ctx.append(g)