From patchwork Wed Dec 17 02:28:00 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 954 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=1765938576; bh=GN+h34ksvr9NqVov9Hm8nDhRl1Fmf2CPhpOqRoLL//M=; 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=Vy6ogHLPHnfWW6Zi+Ov5eKuS68e3+4GvfYKwJk3dgDM3SLF/4f8ofyQF840sKq2HT Ilk0XcYw0QIxs8aydhiRcX6Gjo8312QWPUYzgJx4WfAT8r+YfJ7vEO3Ktn9KECnRmH AIDRtzgSnftvwySfgftbADGw5yzqQSrKRvrqs2mrL6/KyiD+G7ieAct1Kxoddmnp+2 h5DCIuy5quO/Xq25hXJRAfh/l3bLYZ7cdx7YKD9e2zD3SR0rAHIbqKIFJy2Kli/K20 3UIgVri7IIIbHDWMpsMAkPYWZzdAp1Pw2XzJq48k949axcGOqg+vyv48lhBwJsapEf 0res8EbYOUR2w== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 7FE4268AEE for ; Tue, 16 Dec 2025 19:29:36 -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 4nYHBa89gPw9 for ; Tue, 16 Dec 2025 19:29:36 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765938576; bh=GN+h34ksvr9NqVov9Hm8nDhRl1Fmf2CPhpOqRoLL//M=; 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=Vy6ogHLPHnfWW6Zi+Ov5eKuS68e3+4GvfYKwJk3dgDM3SLF/4f8ofyQF840sKq2HT Ilk0XcYw0QIxs8aydhiRcX6Gjo8312QWPUYzgJx4WfAT8r+YfJ7vEO3Ktn9KECnRmH AIDRtzgSnftvwySfgftbADGw5yzqQSrKRvrqs2mrL6/KyiD+G7ieAct1Kxoddmnp+2 h5DCIuy5quO/Xq25hXJRAfh/l3bLYZ7cdx7YKD9e2zD3SR0rAHIbqKIFJy2Kli/K20 3UIgVri7IIIbHDWMpsMAkPYWZzdAp1Pw2XzJq48k949axcGOqg+vyv48lhBwJsapEf 0res8EbYOUR2w== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 6ADAE68AFD for ; Tue, 16 Dec 2025 19:29:36 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765938574; bh=m+OY0Kwj1CTcq9DzUWig2WAis3+NakUz6dTIB7g+lT0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=KTZ7KtqZ6I6CWC5lzZMBgkosxQt2aTsX0TS4JPE+8pikL19ZWunKNGoHNmuAoMw41 MDr50aE+uo9brngu4b2VPFs66naT4x8p4OA++T1tH05l6/gtaTGOFJj7xwgXU5gGiZ Zfsp+YzpZyY6ABQVw0VTe0i3jkTNvEQPim4bMOit5J/LLWM5OKkn1XRLuK+RnY9XJs 2LKuw8fphEleMUmieOwI1VKBP0iunTfs5v3ZIriuyD/vtULDvUf4Saa6AtpseNE3VU xLJRs56Rhtvi827BOySYl4bPqWXiMycGmFcNAwhBALEW39bDZEzoHrCwWiJJV9FDBd 8rxu6i90fNsfw== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 1954968BBD; Tue, 16 Dec 2025 19:29:34 -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 0GUir5ivSFhF; Tue, 16 Dec 2025 19:29:34 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765938570; bh=XrlsPmo3AiSbBAiwhc8/2CndGryByURFoQZ4RJPoyaw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=W4U0gGm2cLCZpJ2QRCCiQlfD6lovJo2fqk1qGuv05sI/QDCFb9RIdKgXHdu5glUJ7 dBJZljDTW4D0YjUQlG/ZX3QgArJ6vXQSUK65Td8WOm/KJ3//1eGyYL2pTeJADn5SXG VGDynVwAGxOAe/gINT+ACZUHY8RN+/35O5O22SBch86IMjT82RQ7OshLUjR/p1rlvq o9Yw+MdyibcRdmPg/fWw+rsqV5aLx3KDr91U4PtlmOlcTS/nFc1MS1MRHBFg8zWJqg H6DCr+JNg9uDOS//IkAVtK+Zz7/pqIQbfqCnLKTKm6rwEqMk0dFrN7Hmqjea+6OKzx Rwj0QLdGDCsvg== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id D65C268AEE; Tue, 16 Dec 2025 19:29:29 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Tue, 16 Dec 2025 19:28:00 -0700 Message-ID: <20251217022823.392557-12-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20251217022823.392557-1-sjg@u-boot.org> References: <20251217022823.392557-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: 6LZQKS2SFX3V5FE3OZZFGWMCRF63V23P X-Message-ID-Hash: 6LZQKS2SFX3V5FE3OZZFGWMCRF63V23P 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: Simon Glass , "Claude Opus 4 . 5" X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 11/24] pickman: Fix ancestor check in process_merged_mrs 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 Fix the logic for determining if an MR's last commit is newer than the current database position. The check should verify that current is an ancestor of the MR's commit (meaning the MR's commit is newer), not the other way around. Add a test to cover this. Co-developed-by: Claude Opus 4.5 Signed-off-by: Simon Glass --- tools/pickman/control.py | 16 ++----- tools/pickman/ftest.py | 91 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 12 deletions(-) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 09ffa4a3da3..e6b7539ea34 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -651,20 +651,12 @@ def process_merged_mrs(remote, source, dbs): f"MR !{merge_req.iid}") continue - # Check if this commit is an ancestor of source but not of current - # (meaning it's newer than what we have) + # Check if this commit is newer than current (current is ancestor of it) try: - # Is last_hash reachable from source? - run_git(['merge-base', '--is-ancestor', full_hash, source]) + # 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 # Not reachable, skip - - try: - # Is last_hash already at or before current? - run_git(['merge-base', '--is-ancestor', full_hash, current]) - continue # Already processed - except Exception: # pylint: disable=broad-except - pass # Not an ancestor of current, so it's newer + continue # current is not an ancestor, so last_hash is not newer # Update database short_old = current[:12] diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index 4459f108496..80aa73f8a5b 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -1885,5 +1885,96 @@ class TestDoReviewWithMrs(unittest.TestCase): self.assertIn('Found 1 open pickman MR', stdout.getvalue()) +class TestProcessMergedMrs(unittest.TestCase): + """Tests for process_merged_mrs function.""" + + def setUp(self): + """Set up test fixtures.""" + fd, self.db_path = tempfile.mkstemp(suffix='.db') + os.close(fd) + os.unlink(self.db_path) + database.Database.instances.clear() + + def tearDown(self): + """Clean up test fixtures.""" + if os.path.exists(self.db_path): + os.unlink(self.db_path) + database.Database.instances.clear() + command.TEST_RESULT = None + + def test_process_merged_mrs_updates_newer(self): + """Test that newer commits update the database.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'aaa111aaa111aaa111aaa111aaa111aaa111aaa1') + dbs.commit() + + merged_mrs = [gitlab_api.PickmanMr( + iid=100, + title='[pickman] Test MR', + description='## 2025-01-01: us/next\n\n- bbb222b Subject', + source_branch='cherry-test', + web_url='https://gitlab.com/mr/100', + )] + + def mock_git(args): + if args[0] == 'rev-parse': + return 'bbb222bbb222bbb222bbb222bbb222bbb222bbb2' + if args[0] == 'merge-base': + # current is ancestor of last_hash (newer) + return '' + return '' + + with mock.patch.object(gitlab_api, 'get_merged_pickman_mrs', + return_value=merged_mrs): + with mock.patch.object(control, 'run_git', mock_git): + processed = control.process_merged_mrs('ci', 'us/next', dbs) + + self.assertEqual(processed, 1) + new_commit = dbs.source_get('us/next') + self.assertEqual(new_commit, + 'bbb222bbb222bbb222bbb222bbb222bbb222bbb2') + + dbs.close() + + def test_process_merged_mrs_skips_older(self): + """Test that older commits don't update the database.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'bbb222bbb222bbb222bbb222bbb222bbb222bbb2') + dbs.commit() + + merged_mrs = [gitlab_api.PickmanMr( + iid=100, + title='[pickman] Test MR', + description='## 2025-01-01: us/next\n\n- aaa111a Subject', + source_branch='cherry-test', + web_url='https://gitlab.com/mr/100', + )] + + def mock_git(args): + if args[0] == 'rev-parse': + return 'aaa111aaa111aaa111aaa111aaa111aaa111aaa1' + if args[0] == 'merge-base': + # current is NOT ancestor of last_hash (older) + raise RuntimeError('Not an ancestor') + return '' + + with mock.patch.object(gitlab_api, 'get_merged_pickman_mrs', + return_value=merged_mrs): + with mock.patch.object(control, 'run_git', mock_git): + processed = control.process_merged_mrs('ci', 'us/next', dbs) + + self.assertEqual(processed, 0) + # Should remain unchanged + current = dbs.source_get('us/next') + self.assertEqual(current, + 'bbb222bbb222bbb222bbb222bbb222bbb222bbb2') + + dbs.close() + + if __name__ == '__main__': unittest.main()