From cd8d4ef88b63f1e3f7225834da58b8b681aac479 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 30 Oct 2024 21:25:10 +0100 Subject: [PATCH] checkupdates: reuse per-app branches when making merge requests https://gitlab.com/fdroid/fdroidserver/-/merge_requests/1551#note_2206085258 --- fdroidserver/checkupdates.py | 55 ++++++++++++++-- tests/test_checkupdates.py | 121 +++++++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 4 deletions(-) diff --git a/fdroidserver/checkupdates.py b/fdroidserver/checkupdates.py index 75a59ba1..0fafaeb9 100644 --- a/fdroidserver/checkupdates.py +++ b/fdroidserver/checkupdates.py @@ -41,6 +41,10 @@ from . import net from .exception import VCSException, NoSubmodulesException, FDroidException, MetaDataException +# https://gitlab.com/fdroid/checkupdates-runner/-/blob/1861899262a62a4ed08fa24e5449c0368dfb7617/.gitlab-ci.yml#L36 +BOT_EMAIL = 'fdroidci@bubu1.eu' + + def check_http(app: metadata.App) -> tuple[Optional[str], Optional[int]]: """Check for a new version by looking at a document retrieved via HTTP. @@ -692,6 +696,45 @@ def get_git_repo_and_main_branch(): return git_repo, main_branch +def checkout_appid_branch(appid): + """Prepare the working branch named after the appid. + + This sets up everything for checkupdates_app() to run and add + commits. If there is an existing branch named after the appid, + and it has commits from users other than the checkupdates-bot, + then this will return False. Otherwise, it returns True. + + The checkupdates-runner must set the committer email address in + the git config. Then any commit with a committer or author that + does not match that will be considered to have human edits. That + email address is currently set in: + https://gitlab.com/fdroid/checkupdates-runner/-/blob/1861899262a62a4ed08fa24e5449c0368dfb7617/.gitlab-ci.yml#L36 + + """ + logging.debug(f'Creating merge request branch for {appid}') + git_repo, main_branch = get_git_repo_and_main_branch() + for remote in git_repo.remotes: + remote.fetch() + try: + git_repo.remotes.origin.fetch(f'{appid}:refs/remotes/origin/{appid}') + except Exception as e: + logging.warning('"%s" branch not found on origin remote:\n\t%s', appid, e) + if appid in git_repo.remotes.origin.refs: + start_point = f"origin/{appid}" + for commit in git_repo.iter_commits( + f'upstream/{main_branch}...{start_point}', right_only=True + ): + if commit.committer.email != BOT_EMAIL or commit.author.email != BOT_EMAIL: + return False + else: + start_point = f"upstream/{main_branch}" + git_repo.git.checkout('-B', appid, start_point) + git_repo.git.rebase( + f'upstream/{main_branch}', strategy_option='ours', kill_after_timeout=120 + ) + return True + + def push_commits(remote_name='origin', branch_name='checkupdates', verbose=False): """Make git branch then push commits as merge request. @@ -859,10 +902,14 @@ def main(): try: if options.merge_request: - logging.info(f'Creating merge request branch for {appid}') - git_repo, main_branch = get_git_repo_and_main_branch() - git_repo.create_head(appid, f"upstream/{main_branch}", force=True) - git_repo.git.checkout(appid) + if not checkout_appid_branch(appid): + msg = _("...checkupdate failed for {appid} : {error}").format( + appid=appid, + error='Open merge request with human edits, skipped.', + ) + logging.warning(msg) + failed[appid] = msg + continue checkupdates_app(app, options.auto, options.commit or options.merge_request) processed.append(appid) diff --git a/tests/test_checkupdates.py b/tests/test_checkupdates.py index c99d31bf..87246b06 100755 --- a/tests/test_checkupdates.py +++ b/tests/test_checkupdates.py @@ -464,6 +464,127 @@ class CheckupdatesTest(unittest.TestCase): self.assertTrue(branch in ('main', 'master')) self.assertTrue(branch in repo.heads) + def test_checkout_appid_branch_does_not_exist(self): + appid = 'com.example' + os.chdir(self.testdir.name) + git_repo, main_branch = fdroidserver.checkupdates.get_git_repo_and_main_branch() + open('foo', 'w').close() + git_repo.git.add(all=True) + git_repo.index.commit("all files") + # --merge-request assumes remotes called 'origin' and 'upstream' + git_repo.create_remote('origin', os.getcwd()).fetch() + git_repo.create_remote('upstream', os.getcwd()).fetch() + self.assertNotIn(appid, git_repo.heads) + fdroidserver.checkupdates.checkout_appid_branch(appid) + self.assertIn(appid, git_repo.heads) + + def test_checkout_appid_branch_exists(self): + appid = 'com.example' + + upstream_dir = os.path.join(self.testdir.name, 'upstream_git') + os.mkdir(upstream_dir) + upstream_repo = git.Repo.init(upstream_dir) + (Path(upstream_dir) / 'README').write_text('README') + upstream_repo.git.add(all=True) + upstream_repo.index.commit("README") + upstream_repo.create_head(appid) + + local_dir = os.path.join(self.testdir.name, 'local_git') + git.Repo.clone_from(upstream_dir, local_dir) + os.chdir(local_dir) + git_repo, main_branch = fdroidserver.checkupdates.get_git_repo_and_main_branch() + # --merge-request assumes remotes called 'origin' and 'upstream' + git_repo.create_remote('upstream', upstream_dir).fetch() + + self.assertNotIn(appid, git_repo.heads) + fdroidserver.checkupdates.checkout_appid_branch(appid) + self.assertIn(appid, git_repo.heads) + + def test_checkout_appid_branch_skip_bot_commit(self): + appid = 'com.example' + + upstream_dir = os.path.join(self.testdir.name, 'upstream_git') + os.mkdir(upstream_dir) + upstream_repo = git.Repo.init(upstream_dir) + (Path(upstream_dir) / 'README').write_text('README') + upstream_repo.git.add(all=True) + upstream_repo.index.commit("README") + upstream_repo.create_head(appid) + + local_dir = os.path.join(self.testdir.name, 'local_git') + git.Repo.clone_from(upstream_dir, local_dir) + os.chdir(local_dir) + git_repo, main_branch = fdroidserver.checkupdates.get_git_repo_and_main_branch() + # --merge-request assumes remotes called 'origin' and 'upstream' + git_repo.create_remote('upstream', upstream_dir).fetch() + + os.mkdir('metadata') + git_repo.create_head(appid, f'origin/{appid}', force=True) + git_repo.git.checkout(appid) + + # fake checkupdates-bot commit + Path(f'metadata/{appid}.yml').write_text('AutoName: Example\n') + with git_repo.config_writer() as cw: + cw.set_value('user', 'email', fdroidserver.checkupdates.BOT_EMAIL) + git_repo.git.add(all=True) + git_repo.index.commit("Example") + + # set up starting from remote branch + git_repo.remotes.origin.push(appid) + git_repo.git.checkout(main_branch) + git_repo.delete_head(appid, force=True) + + self.assertTrue( + fdroidserver.checkupdates.checkout_appid_branch(appid), + 'This should have been true since there are only bot commits.', + ) + + def test_checkout_appid_branch_skip_human_edits(self): + appid = 'com.example' + + upstream_dir = os.path.join(self.testdir.name, 'upstream_git') + os.mkdir(upstream_dir) + upstream_repo = git.Repo.init(upstream_dir) + (Path(upstream_dir) / 'README').write_text('README') + upstream_repo.git.add(all=True) + upstream_repo.index.commit("README") + upstream_repo.create_head(appid) + + local_dir = os.path.join(self.testdir.name, 'local_git') + git.Repo.clone_from(upstream_dir, local_dir) + os.chdir(local_dir) + git_repo, main_branch = fdroidserver.checkupdates.get_git_repo_and_main_branch() + # --merge-request assumes remotes called 'origin' and 'upstream' + git_repo.create_remote('upstream', upstream_dir).fetch() + + os.mkdir('metadata') + git_repo.create_head(appid, f'origin/{appid}', force=True) + git_repo.git.checkout(appid) + + with git_repo.config_writer() as cw: + cw.set_value('user', 'email', fdroidserver.checkupdates.BOT_EMAIL) + + # fake checkupdates-bot commit + Path(f'metadata/{appid}.yml').write_text('AutoName: Example\n') + git_repo.git.add(all=True) + git_repo.index.commit("Example") + + # fake commit added on top by a human + Path(f'metadata/{appid}.yml').write_text('AutoName: Example\nName: Foo\n') + with git_repo.config_writer() as cw: + cw.set_value('user', 'email', 'human@bar.com') + git_repo.git.add(all=True) + git_repo.index.commit("Example") + + # set up starting from remote branch + git_repo.remotes.origin.push(appid) + git_repo.git.reset(main_branch) + + self.assertFalse( + fdroidserver.checkupdates.checkout_appid_branch(appid), + 'This should have been false since there are human edits.', + ) + @mock.patch('git.remote.Remote.push') @mock.patch('sys.exit') @mock.patch('fdroidserver.common.read_app_args')