From patchwork Wed Dec 17 02:26:04 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 942 Return-Path: X-Original-To: u-boot-concept@u-boot.org Delivered-To: u-boot-concept@u-boot.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765938465; bh=1d+NzfGwqQOJSsXVDwk4coOnc0P8QsY4xZ+u6A1tUFA=; h=From:To:Date:In-Reply-To:References:CC:Subject:List-Id: List-Archive:List-Help:List-Owner:List-Post:List-Subscribe: List-Unsubscribe:From; b=j7aUJzGMMqDhaHrDIG5cCdYnutDRvAwL2tQUg+jZD8FnU3eMy89NA3CThpErV+t6j N3cAOj68YLygqF6Fg5Jjm391RotuPjyn5aTor0ck/VphscK4Db7hhxJqq2INp8g57K 5cI0TkaznrRVfjqw0p8mo/769WZIVYfBpzOdJiVczo25ZGUIm0o8vAI6O/ufAnQWYz Uf84xhcSvbHChh7hLHuIodgkgaG06G5qPtCFVlROv+3GelECCY6aVjAM7u1K0P78nT G4GqUnLP1SVvnCcLgsSX8nCckYZu+D0dMOOKoX93l3l7E5WEsG1XHK6Odc7oybLOfQ CtP/8sHYthKLQ== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id F083B68BB6 for ; Tue, 16 Dec 2025 19:27:45 -0700 (MST) X-Virus-Scanned: Debian amavis at Received: from mail.u-boot.org ([127.0.0.1]) by localhost (mail.u-boot.org [127.0.0.1]) (amavis, port 10024) with ESMTP id y2d9oA13mnD3 for ; Tue, 16 Dec 2025 19:27:45 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765938465; bh=1d+NzfGwqQOJSsXVDwk4coOnc0P8QsY4xZ+u6A1tUFA=; h=From:To:Date:In-Reply-To:References:CC:Subject:List-Id: List-Archive:List-Help:List-Owner:List-Post:List-Subscribe: List-Unsubscribe:From; b=j7aUJzGMMqDhaHrDIG5cCdYnutDRvAwL2tQUg+jZD8FnU3eMy89NA3CThpErV+t6j N3cAOj68YLygqF6Fg5Jjm391RotuPjyn5aTor0ck/VphscK4Db7hhxJqq2INp8g57K 5cI0TkaznrRVfjqw0p8mo/769WZIVYfBpzOdJiVczo25ZGUIm0o8vAI6O/ufAnQWYz Uf84xhcSvbHChh7hLHuIodgkgaG06G5qPtCFVlROv+3GelECCY6aVjAM7u1K0P78nT G4GqUnLP1SVvnCcLgsSX8nCckYZu+D0dMOOKoX93l3l7E5WEsG1XHK6Odc7oybLOfQ CtP/8sHYthKLQ== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id D3ADC68AFF for ; Tue, 16 Dec 2025 19:27:45 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765938464; bh=eDF7sI/s9exrpXehPGMeKoU+aQOFRTyFi7LBv1FUY2U=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=QAWi7O8y28lnZcpwEbToLrFkEVzXOmIZhS5OBj9BcaalT1p84jAzqB0nun/iquGbx DnpgFetpF0pE9dKOlauKxYq5Sps5ig7chbkPTZgji4EGDdRM9A51EF2nEH7tCPXxNW tTkidkSnm0tAnZLqFauZOndpYxfNe/VZhYRYaYYyPMGNzin+4QkE23WhItDfMVlUrH T4qMS5aDaST+RHS/Tf+JqCzaZj9Rv0ZxQzoLEYshnVuPuFQbSqqvb2qxLJlHSXd63l 4wjbnJTngdNDtGT8XR15I0Nlv3cmwgdVnyAwdExo/KyP0zSsWkC13+Jet2CejXdf/d tEpbbmiujxLYg== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 93C4A68BBD; Tue, 16 Dec 2025 19:27:44 -0700 (MST) X-Virus-Scanned: Debian amavis at Received: from mail.u-boot.org ([127.0.0.1]) by localhost (mail.u-boot.org [127.0.0.1]) (amavis, port 10026) with ESMTP id XuZoGf4PVM2H; Tue, 16 Dec 2025 19:27:44 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765938459; bh=8ZgFg7hHAVMg9OT4GTQxwJeELG/3GF6rTPSnExdfYZ0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=qGklYBLrfXuEAOoGXJFkIaWaGHd6x8D7yTqwtC65182kI8z+uAflGYp7Nx1xkXhSN cbwGF8YI+qDlvEiBYPDVk2uYLEfr3UKDLG/M2ArUmG7fpWz8BUzlihuVajeyWSSLSw T/CNvzVTGcNVTEn/HodIDFFiqrpD2TkexMUgu6Idifa0whMFfcWBjykmeUCTZYu8xf 09xo02XgmDdFtZi8k69dtCXoNkxVRbHorL44QxzaTLZ7SykE8P3BfBjiG7t/IFT17Q tMMwm2RLYgGeaz1OZaOH1Sx2aV5iz0BJ4m4qcrURP5SEpDwTVvlM6sLgc16ezXMiQV qdE24sACdjySA== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id EDDAB6884F; Tue, 16 Dec 2025 19:27:38 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Tue, 16 Dec 2025 19:26:04 -0700 Message-ID: <20251217022611.389379-17-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20251217022611.389379-1-sjg@u-boot.org> References: <20251217022611.389379-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: VBZM2SYIR3WVBO42NK5JEJP3MIYH3EX3 X-Message-ID-Hash: VBZM2SYIR3WVBO42NK5JEJP3MIYH3EX3 X-MailFrom: sjg@u-boot.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Heinrich Schuchardt , Simon Glass , "Claude Opus 4 . 5" X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 16/17] pickman: Update database when MRs are merged List-Id: Discussion and patches related to U-Boot Concept Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: From: Simon Glass Add automatic database updates when pickman MRs are merged. The step command now: 1. Checks for merged pickman MRs via GitLab API 2. Parses the MR description to find the source branch and last commit 3. Updates the database's last_commit for the source branch 4. Then proceeds to check for open MRs and create new ones as before This removes the need to manually run commit-source after each MR is merged, enabling fully automated cherry-pick workflows with poll. Add get_merged_pickman_mrs() and refactor get_open_pickman_mrs() to use a common get_pickman_mrs() function with a state parameter. Co-developed-by: Claude Opus 4.5 --- tools/pickman/README.rst | 13 +++-- tools/pickman/control.py | 94 ++++++++++++++++++++++++++++++++++++- tools/pickman/ftest.py | 74 ++++++++++++++++++++++++++--- tools/pickman/gitlab_api.py | 38 +++++++++++++-- 4 files changed, 203 insertions(+), 16 deletions(-) diff --git a/tools/pickman/README.rst b/tools/pickman/README.rst index 7a33378a32b..07b9408afa0 100644 --- a/tools/pickman/README.rst +++ b/tools/pickman/README.rst @@ -117,9 +117,16 @@ To automatically create an MR if none is pending:: ./tools/pickman/pickman step us/next -This checks for open pickman MRs (those with ``[pickman]`` in the title) and if -none exist, runs ``apply`` with ``--push`` to create a new one. This is useful -for automated workflows where only one MR should be active at a time. +This command performs the following: + +1. Checks for merged pickman MRs and updates the database with the last + cherry-picked commit from each merged MR +2. Checks for open pickman MRs (those with ``[pickman]`` in the title) +3. If no open MRs exist, runs ``apply`` with ``--push`` to create a new one + +This is useful for automated workflows where only one MR should be active at a +time. The automatic database update on merge means you don't need to manually +run ``commit-source`` after each MR is merged. Options for the step command: diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 6efb20df1d5..aaed06c1e35 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -513,11 +513,95 @@ def do_review(args, dbs): # pylint: disable=unused-argument return 0 +def parse_mr_description(description): + """Parse a pickman MR description to extract source and last commit + + Args: + description (str): MR description text + + Returns: + tuple: (source_branch, last_commit_hash) or (None, None) if not parseable + """ + import re + + # Extract source branch from "## date: source_branch" line + source_match = re.search(r'^## [^:]+: (.+)$', description, re.MULTILINE) + if not source_match: + return None, None + source = source_match.group(1) + + # Extract commits from "- hash subject" lines + commit_matches = re.findall(r'^- ([a-f0-9]+) ', description, re.MULTILINE) + if not commit_matches: + return None, None + + # Last commit is the last one in the list + last_hash = commit_matches[-1] + + return source, last_hash + + +def process_merged_mrs(remote, source, dbs): + """Check for merged MRs and update the database + + Args: + remote (str): Remote name + source (str): Source branch name to filter by + dbs (Database): Database instance + + Returns: + int: Number of MRs processed, or -1 on error + """ + merged_mrs = gitlab_api.get_merged_pickman_mrs(remote) + if merged_mrs is None: + return -1 + + processed = 0 + for merge_req in merged_mrs: + mr_source, last_hash = parse_mr_description(merge_req['description']) + + # Only process MRs for the requested source branch + if mr_source != source: + continue + + # Check if this MR's last commit is newer than current database + current = dbs.source_get(source) + if not current: + continue + + # Resolve the short hash to full hash + try: + full_hash = run_git(['rev-parse', last_hash]) + except Exception: # pylint: disable=broad-except + tout.warning(f"Could not resolve commit '{last_hash}' from " + f"MR !{merge_req['iid']}") + continue + + # Check if this commit is newer than current (current is ancestor of it) + try: + # Is current an ancestor of last_hash? (meaning last_hash is newer) + run_git(['merge-base', '--is-ancestor', current, full_hash]) + except Exception: # pylint: disable=broad-except + continue # current is not an ancestor, so last_hash is not newer + + # Update database + short_old = current[:12] + short_new = full_hash[:12] + tout.info(f"MR !{merge_req['iid']} merged, updating '{source}': " + f'{short_old} -> {short_new}') + dbs.source_set(source, full_hash) + dbs.commit() + processed += 1 + + return processed + + def do_step(args, dbs): """Create an MR if none is pending - Checks for open pickman MRs and if none exist, runs apply with push - to create a new one. + Checks for merged pickman MRs and updates the database, then checks for + open pickman MRs and if none exist, runs apply with push to create a new + one. Args: args (Namespace): Parsed arguments with 'source', 'remote', 'target' @@ -527,6 +611,12 @@ def do_step(args, dbs): int: 0 on success, 1 on failure """ remote = args.remote + source = args.source + + # First check for merged MRs and update database + processed = process_merged_mrs(remote, source, dbs) + if processed < 0: + return 1 # Check for open pickman MRs mrs = gitlab_api.get_open_pickman_mrs(remote) diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index 252fb79d2be..e5ed187bdf6 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -1126,6 +1126,54 @@ class TestParseStep(unittest.TestCase): self.assertEqual(args.target, 'main') +class TestParseMrDescription(unittest.TestCase): + """Tests for parse_mr_description function.""" + + def test_parse_mr_description(self): + """Test parsing a valid MR description.""" + description = """## 2025-01-15: us/next + +Branch: cherry-abc123 + +Commits: +- abc123a First commit +- def456b Second commit +- caf789c Third commit + +### Conversation log +Some log text""" + source, last_hash = control.parse_mr_description(description) + self.assertEqual(source, 'us/next') + self.assertEqual(last_hash, 'caf789c') + + def test_parse_mr_description_single_commit(self): + """Test parsing MR description with single commit.""" + description = """## 2025-01-15: feature/branch + +Branch: cherry-xyz + +Commits: +- abc1234 Only commit""" + source, last_hash = control.parse_mr_description(description) + self.assertEqual(source, 'feature/branch') + self.assertEqual(last_hash, 'abc1234') + + def test_parse_mr_description_invalid(self): + """Test parsing invalid MR description.""" + source, last_hash = control.parse_mr_description('invalid description') + self.assertIsNone(source) + self.assertIsNone(last_hash) + + def test_parse_mr_description_no_commits(self): + """Test parsing MR description without commits.""" + description = """## 2025-01-15: us/next + +Branch: cherry-abc""" + source, last_hash = control.parse_mr_description(description) + self.assertIsNone(source) + self.assertIsNone(last_hash) + + class TestStep(unittest.TestCase): """Tests for step command.""" @@ -1136,17 +1184,19 @@ class TestStep(unittest.TestCase): 'title': '[pickman] Test MR', 'web_url': 'https://gitlab.com/mr/123', } - with mock.patch.object(gitlab_api, 'get_open_pickman_mrs', - return_value=[mock_mr]): - args = argparse.Namespace(cmd='step', source='us/next', - remote='ci', target='master') - ret = control.do_step(args, None) + with mock.patch.object(gitlab_api, 'get_merged_pickman_mrs', + return_value=[]): + with mock.patch.object(gitlab_api, 'get_open_pickman_mrs', + return_value=[mock_mr]): + args = argparse.Namespace(cmd='step', source='us/next', + remote='ci', target='master') + ret = control.do_step(args, None) self.assertEqual(ret, 0) def test_step_gitlab_error(self): """Test step when GitLab API returns error.""" - with mock.patch.object(gitlab_api, 'get_open_pickman_mrs', + with mock.patch.object(gitlab_api, 'get_merged_pickman_mrs', return_value=None): args = argparse.Namespace(cmd='step', source='us/next', remote='ci', target='master') @@ -1154,6 +1204,18 @@ class TestStep(unittest.TestCase): self.assertEqual(ret, 1) + def test_step_open_mrs_error(self): + """Test step when get_open_pickman_mrs returns error.""" + with mock.patch.object(gitlab_api, 'get_merged_pickman_mrs', + return_value=[]): + with mock.patch.object(gitlab_api, 'get_open_pickman_mrs', + return_value=None): + args = argparse.Namespace(cmd='step', source='us/next', + remote='ci', target='master') + ret = control.do_step(args, None) + + self.assertEqual(ret, 1) + class TestParseReview(unittest.TestCase): """Tests for review command argument parsing.""" diff --git a/tools/pickman/gitlab_api.py b/tools/pickman/gitlab_api.py index ca239c41271..6720e0a1526 100644 --- a/tools/pickman/gitlab_api.py +++ b/tools/pickman/gitlab_api.py @@ -152,15 +152,16 @@ def create_mr(host, proj_path, source, target, title, desc=''): return None -def get_open_pickman_mrs(remote): - """Get open merge requests created by pickman +def get_pickman_mrs(remote, state='opened'): + """Get merge requests created by pickman Args: remote (str): Remote name + state (str): MR state ('opened', 'merged', 'closed', 'all') Returns: - list: List of dicts with 'iid', 'title', 'web_url', 'source_branch' keys, - or None on failure + list: List of dicts with 'iid', 'title', 'web_url', 'source_branch', + 'description' keys, or None on failure """ if not check_available(): return None @@ -181,7 +182,7 @@ def get_open_pickman_mrs(remote): glab = gitlab.Gitlab(f'https://{host}', private_token=token) project = glab.projects.get(proj_path) - mrs = project.mergerequests.list(state='opened', get_all=True) + mrs = project.mergerequests.list(state=state, get_all=True) pickman_mrs = [] for merge_req in mrs: if '[pickman]' in merge_req.title: @@ -190,6 +191,7 @@ def get_open_pickman_mrs(remote): 'title': merge_req.title, 'web_url': merge_req.web_url, 'source_branch': merge_req.source_branch, + 'description': merge_req.description or '', }) return pickman_mrs except gitlab.exceptions.GitlabError as exc: @@ -197,6 +199,32 @@ def get_open_pickman_mrs(remote): return None +def get_open_pickman_mrs(remote): + """Get open merge requests created by pickman + + Args: + remote (str): Remote name + + Returns: + list: List of dicts with 'iid', 'title', 'web_url', 'source_branch' keys, + or None on failure + """ + return get_pickman_mrs(remote, state='opened') + + +def get_merged_pickman_mrs(remote): + """Get merged merge requests created by pickman + + Args: + remote (str): Remote name + + Returns: + list: List of dicts with 'iid', 'title', 'web_url', 'source_branch', + 'description' keys, or None on failure + """ + return get_pickman_mrs(remote, state='merged') + + def get_mr_comments(remote, mr_iid): """Get human comments on a merge request (excluding bot/system notes)