| Message ID | 20260504063614.3831203-5-daniel.turull@ericsson.com |
|---|---|
| State | New |
| Headers | show |
| Series | upgrade_helper: scarthgap compatibility and stable updates | expand |
On Mon, 4 May 2026 at 08:36, <daniel.turull@ericsson.com> wrote: > + prev_runs = sorted(glob.glob(os.path.join(self.uh_dir, "20*"))) > + prev_runs = [r for r in prev_runs if r != self.uh_work_dir] > + if prev_runs: > + prev_dir = prev_runs[-1] This makes me wonder, if AUH was interrupted in its second run, and we need to resume for the second time, then AUH should combine everything from all previous runs into the skip set, not just recipes from the latest one, right? > + if 'workdir' in g: > + if any(g['name'] == s['name'] for s in succeeded_pkggroups_ctx): This is more complex than the condition in the removed block below, why not use that: if g in succeeded_pkggroups_ctx: > + if not os.path.exists(link): > + os.symlink(g['workdir'], link) Why is the existence guard needed? The removed code block doesn't have it. Alex
> -----Original Message----- > From: Alexander Kanavin <alex.kanavin@gmail.com> > Sent: Monday, 4 May 2026 10:12 > To: Daniel Turull <daniel.turull@ericsson.com> > Cc: yocto-patches@lists.yoctoproject.org; paul@pbarker.dev; > ross.burton@arm.com; yoann.congal@smile.fr > Subject: Re: [AUH][PATCH v3 4/4] upgrade-helper.py: Add resume option to > skip already attempted recipes > > On Mon, 4 May 2026 at 08:36, <daniel.turull@ericsson.com> wrote: > > + prev_runs = sorted(glob.glob(os.path.join(self.uh_dir, "20*"))) > > + prev_runs = [r for r in prev_runs if r != self.uh_work_dir] > > + if prev_runs: > > + prev_dir = prev_runs[-1] > > This makes me wonder, if AUH was interrupted in its second run, and we > need to resume for the second time, then AUH should combine everything > from all previous runs into the skip set, not just recipes from the latest one, > right? > Yes, you are right. I'll fix it in a new version. > > + if 'workdir' in g: > > + if any(g['name'] == s['name'] for s in succeeded_pkggroups_ctx): > > This is more complex than the condition in the removed block below, why not > use that: > if g in succeeded_pkggroups_ctx: > I'll add your suggestion. > > + if not os.path.exists(link): > > + os.symlink(g['workdir'], link) > > Why is the existence guard needed? The removed code block doesn't have it. > It's not. It was some transient error that I had but now I cannot reproduce it. Each run creates a fresh timestamped directory so symlinks can't already exist. Removed Thanks Daniel
On Tue, 5 May 2026 at 08:49, Daniel Turull <daniel.turull@ericsson.com> wrote: > > > + if 'workdir' in g: > > > + if any(g['name'] == s['name'] for s in succeeded_pkggroups_ctx): > > > > This is more complex than the condition in the removed block below, why not > > use that: > > if g in succeeded_pkggroups_ctx: > > > I'll add your suggestion. Please be honest. Did AI introduce this inconsistency? Catching and correcting such things is on AI operators, not on reviewers. Alex
Yes, it was introduced by AI and I missed to catch it. I did review it manually but clearly I need to be more focused to catch these types of issues. The last thing I want is to waste maintainers time. You are very important for the project. I apologize for that. Best regards, Daniel > -----Original Message----- > From: Alexander Kanavin <alex.kanavin@gmail.com> > Sent: Tuesday, 5 May 2026 10:06 > To: Daniel Turull <daniel.turull@ericsson.com> > Cc: yocto-patches@lists.yoctoproject.org; paul@pbarker.dev; > ross.burton@arm.com; yoann.congal@smile.fr > Subject: Re: [AUH][PATCH v3 4/4] upgrade-helper.py: Add resume option to > skip already attempted recipes > > On Tue, 5 May 2026 at 08:49, Daniel Turull <daniel.turull@ericsson.com> > wrote: > > > > + if 'workdir' in g: > > > > + if any(g['name'] == s['name'] for s in > succeeded_pkggroups_ctx): > > > > > > This is more complex than the condition in the removed block below, > > > why not use that: > > > if g in succeeded_pkggroups_ctx: > > > > > I'll add your suggestion. > > Please be honest. Did AI introduce this inconsistency? Catching and correcting > such things is on AI operators, not on reviewers. > > Alex
On Tue, 5 May 2026 at 10:21, Daniel Turull <daniel.turull@ericsson.com> wrote: > Yes, it was introduced by AI and I missed to catch it. > > I did review it manually but clearly I need to be more focused to catch these types of issues. What I've seen here, and in other patches is that AI makes code that functions, but has baffling, unexplained logic oddities and maintainability shortcomings that a human wouldn't make. It also doesn't write good commit messages. I don't object to usage of AI per se (that ship has sailed), but you should treat it as a bumbling, confused summer intern that needs to be carefully watched and corrected. Whether you actually end up saving time that way is up to you, just do your utmost to not end up using more of maintainers time. Alex
diff --git a/upgrade-helper.py b/upgrade-helper.py index aecb207..e6ae77f 100755 --- a/upgrade-helper.py +++ b/upgrade-helper.py @@ -42,6 +42,7 @@ import re import signal import sys import configparser +import glob from datetime import datetime from datetime import date import shutil @@ -99,10 +100,12 @@ def parse_cmdline(): parser.add_argument("-t", "--to_version", help="version to upgrade the recipe to") - parser.add_argument("--stable", action="store_true", default=False, help="only upgrade to the next patch version within the stable branch (e.g. 1.2.3 -> 1.2.4)") + parser.add_argument("--resume", action="store_true", default=False, + help="skip recipes already attempted in the last run") + parser.add_argument("-d", "--debug-level", type=int, default=4, choices=range(1, 6), help="set the debug level: CRITICAL=1, ERROR=2, WARNING=3, INFO=4, DEBUG=5") parser.add_argument("-e", "--send-emails", action="store_true", default=False, @@ -493,10 +496,34 @@ class Updater(object): succeeded_pkggroups_ctx = [] failed_pkggroups_ctx = [] attempted_pkggroups = 0 + + # --resume: skip recipes already attempted in the most recent run + resume_skip = set() + if self.args.resume: + prev_runs = sorted(glob.glob(os.path.join(self.uh_dir, "20*"))) + prev_runs = [r for r in prev_runs if r != self.uh_work_dir] + if prev_runs: + prev_dir = prev_runs[-1] + for d in ("succeed", "failed"): + result_dir = os.path.join(prev_dir, d) + if os.path.isdir(result_dir): + resume_skip.update(os.listdir(result_dir)) + if resume_skip: + I(" --resume: skipping %d recipes from %s" % + (len(resume_skip), prev_dir)) + else: + W(" --resume: no previous run found, nothing to skip") for g in pkggroups_ctx: attempted_pkggroups += 1 pkggroup_name = g["name"] + + if g['name'] in resume_skip: + I(" SKIP PACKAGE GROUP %d/%d (already attempted): %s" % + (attempted_pkggroups, total_pkggroups, pkggroup_name)) + continue + I(" ATTEMPT PACKAGE GROUP %d/%d" % (attempted_pkggroups, total_pkggroups)) + try: for pkg_ctx in g['pkgs']: I(" %s: Upgrading to %s" % (pkg_ctx['PN'], pkg_ctx['NPV'])) @@ -541,6 +568,15 @@ class Updater(object): succeeded_pkggroups_ctx.remove(g) failed_pkggroups_ctx.append(g) + # Create symlink immediately so --resume can find it + if 'workdir' in g: + if any(g['name'] == s['name'] for s in succeeded_pkggroups_ctx): + link = os.path.join(self.uh_recipes_succeed_dir, g['name']) + else: + link = os.path.join(self.uh_recipes_failed_dir, g['name']) + if not os.path.exists(link): + os.symlink(g['workdir'], link) + if self.opts['testimage']: ctxs = {} ctxs['succeeded'] = succeeded_pkggroups_ctx @@ -551,15 +587,7 @@ class Updater(object): tim.run() - for g in pkggroups_ctx: - - if g in succeeded_pkggroups_ctx: - os.symlink(g['workdir'], os.path.join( \ - self.uh_recipes_succeed_dir, g['name'])) - else: - os.symlink(g['workdir'], os.path.join( \ - self.uh_recipes_failed_dir, g['name'])) - + for g in succeeded_pkggroups_ctx + failed_pkggroups_ctx: self.statistics.update(g) self.pkg_upgrade_handler(g)