From 17c480d299799190a0e9b36b63e6c05085fd9525 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 30 Oct 2024 23:29:53 +0100 Subject: [PATCH 1/8] checkupdates: make iter_commits only include commits in appid branch iter_commits() follows `git rev-list` (which selects different commits than `git diff`). With ... notation, `git rev-list` will return all the commits that are not shared by the two branches. This needs only the commits in the right side of the comparison (like how `git diff` does it). --- fdroidserver/checkupdates.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fdroidserver/checkupdates.py b/fdroidserver/checkupdates.py index d812d0b6..60b696a9 100644 --- a/fdroidserver/checkupdates.py +++ b/fdroidserver/checkupdates.py @@ -712,8 +712,9 @@ def push_commits(remote_name='origin', branch_name='checkupdates', verbose=False git_repo, default = get_git_repo_and_main_branch() files = set() upstream_main = default if default in git_repo.remotes.upstream.refs else 'main' - local_main = default if default in git_repo.refs else 'main' - for commit in git_repo.iter_commits(f'upstream/{upstream_main}...{local_main}'): + for commit in git_repo.iter_commits( + f'upstream/{upstream_main}...HEAD', right_only=True + ): files.update(commit.stats.files.keys()) files = list(files) From 20ff302e89fa8b1ab5d7bf4d93ebf0f131c96e3d Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 30 Oct 2024 23:50:06 +0100 Subject: [PATCH 2/8] checkupdates: remove duplicate push in push_commits() --- fdroidserver/checkupdates.py | 3 --- tests/test_checkupdates.py | 6 +++++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/fdroidserver/checkupdates.py b/fdroidserver/checkupdates.py index 60b696a9..75a59ba1 100644 --- a/fdroidserver/checkupdates.py +++ b/fdroidserver/checkupdates.py @@ -739,9 +739,6 @@ def push_commits(remote_name='origin', branch_name='checkupdates', verbose=False git_repo.create_head(branch_name, force=True) remote = git_repo.remotes[remote_name] - pushinfos = remote.push( - branch_name, force=True, set_upstream=True, progress=progress - ) pushinfos = remote.push( branch_name, progress=progress, diff --git a/tests/test_checkupdates.py b/tests/test_checkupdates.py index 8225cdff..c99d31bf 100755 --- a/tests/test_checkupdates.py +++ b/tests/test_checkupdates.py @@ -464,10 +464,13 @@ class CheckupdatesTest(unittest.TestCase): self.assertTrue(branch in ('main', 'master')) self.assertTrue(branch in repo.heads) + @mock.patch('git.remote.Remote.push') @mock.patch('sys.exit') @mock.patch('fdroidserver.common.read_app_args') @mock.patch('fdroidserver.checkupdates.checkupdates_app') - def test_merge_requests_branch(self, checkupdates_app, read_app_args, sys_exit): + def test_merge_requests_branch( + self, checkupdates_app, read_app_args, sys_exit, push + ): def _sys_exit(return_code=0): self.assertEqual(return_code, 0) @@ -499,5 +502,6 @@ class CheckupdatesTest(unittest.TestCase): self.assertNotIn(appid, git_repo.heads) with mock.patch('sys.argv', ['fdroid checkupdates', '--merge-request', appid]): fdroidserver.checkupdates.main() + push.assert_called_once() sys_exit.assert_called_once() self.assertIn(appid, git_repo.heads) From cd8d4ef88b63f1e3f7225834da58b8b681aac479 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 30 Oct 2024 21:25:10 +0100 Subject: [PATCH 3/8] 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') From fbe9152ee5fbf27350ca6ca0da20420472cf69dd Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 12 Nov 2024 19:12:33 +0200 Subject: [PATCH 4/8] checkupdates: commit summary is merge request title --- fdroidserver/checkupdates.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fdroidserver/checkupdates.py b/fdroidserver/checkupdates.py index 0fafaeb9..0ae3e67f 100644 --- a/fdroidserver/checkupdates.py +++ b/fdroidserver/checkupdates.py @@ -790,9 +790,9 @@ def push_commits(remote_name='origin', branch_name='checkupdates', verbose=False push_option=[ 'merge_request.create', 'merge_request.remove_source_branch', - 'merge_request.title=' + 'bot: checkupdates for ' + branch_name, + 'merge_request.title=bot: ' + git_repo.branches[branch_name].commit.summary, 'merge_request.description=' - + 'checkupdates-bot run %s' % os.getenv('CI_JOB_URL'), + + '~%s checkupdates-bot run %s' % (branch_name, os.getenv('CI_JOB_URL')), ], ) From c97503b5f3312ef435de567570be3599d79ac251 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 18 Nov 2024 12:38:42 +0100 Subject: [PATCH 5/8] checkupdates: get default branch from git config --- fdroidserver/checkupdates.py | 5 ++--- tests/test_checkupdates.py | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/fdroidserver/checkupdates.py b/fdroidserver/checkupdates.py index 0ae3e67f..19a8f853 100644 --- a/fdroidserver/checkupdates.py +++ b/fdroidserver/checkupdates.py @@ -690,9 +690,8 @@ def get_last_build_from_app(app: metadata.App) -> metadata.Build: def get_git_repo_and_main_branch(): git_repo = git.Repo.init('.') - main_branch = 'main' - if main_branch not in git_repo.heads: - main_branch = 'master' + with git_repo.config_reader() as reader: + main_branch = reader.get_value('init', 'defaultBranch') return git_repo, main_branch diff --git a/tests/test_checkupdates.py b/tests/test_checkupdates.py index 87246b06..e9637190 100755 --- a/tests/test_checkupdates.py +++ b/tests/test_checkupdates.py @@ -461,7 +461,6 @@ class CheckupdatesTest(unittest.TestCase): git_repo.index.commit("all files") repo, branch = fdroidserver.checkupdates.get_git_repo_and_main_branch() - self.assertTrue(branch in ('main', 'master')) self.assertTrue(branch in repo.heads) def test_checkout_appid_branch_does_not_exist(self): From fd15ac92765ad98ea80d77e246e02bed38517084 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 18 Nov 2024 13:38:34 +0100 Subject: [PATCH 6/8] checkupdates: mark as Draft when only changing Current Version https://gitlab.com/fdroid/fdroidserver/-/merge_requests/1551#note_2190155816 --- fdroidserver/checkupdates.py | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/fdroidserver/checkupdates.py b/fdroidserver/checkupdates.py index 19a8f853..68bd2cc1 100644 --- a/fdroidserver/checkupdates.py +++ b/fdroidserver/checkupdates.py @@ -766,6 +766,31 @@ def push_commits(remote_name='origin', branch_name='checkupdates', verbose=False branch_name = m.group(1) # appid if not files: return + + git_repo.create_head(branch_name, force=True) + push_options = [ + 'merge_request.create', + 'merge_request.remove_source_branch', + 'merge_request.title=bot: ' + git_repo.branches[branch_name].commit.summary, + 'merge_request.description=' + + '~%s checkupdates-bot run %s' % (branch_name, os.getenv('CI_JOB_URL')), + ] + + # mark as draft if there are only changes to CurrentVersion: + current_version_only = True + for m in re.findall( + r"^[+-].*", + git_repo.git.diff(f"upstream/{upstream_main}...HEAD"), + flags=re.MULTILINE, + ): + if re.match(r"^(\+\+\+|---) ", m): + continue + if not re.match(r"^[-+]CurrentVersion", m): + current_version_only = False + break + if current_version_only: + push_options.append('merge_request.draft') + progress = None if verbose: import clint.textui @@ -779,20 +804,13 @@ def push_commits(remote_name='origin', branch_name='checkupdates', verbose=False progress = MyProgressPrinter() - git_repo.create_head(branch_name, force=True) remote = git_repo.remotes[remote_name] pushinfos = remote.push( branch_name, progress=progress, force=True, set_upstream=True, - push_option=[ - 'merge_request.create', - 'merge_request.remove_source_branch', - 'merge_request.title=bot: ' + git_repo.branches[branch_name].commit.summary, - 'merge_request.description=' - + '~%s checkupdates-bot run %s' % (branch_name, os.getenv('CI_JOB_URL')), - ], + push_option=push_options, ) for pushinfo in pushinfos: From e3f724681a401ff99010c85e42241dc5c8e0717f Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 20 Nov 2024 15:21:36 +0100 Subject: [PATCH 7/8] checkupdates: parse default branch from upstream remote --- fdroidserver/checkupdates.py | 41 +++++++++++++++++------------- tests/test_checkupdates.py | 49 ++++++++++++++++++++++++++++-------- 2 files changed, 63 insertions(+), 27 deletions(-) diff --git a/fdroidserver/checkupdates.py b/fdroidserver/checkupdates.py index 68bd2cc1..d9ad8048 100644 --- a/fdroidserver/checkupdates.py +++ b/fdroidserver/checkupdates.py @@ -18,6 +18,7 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +import configparser import git import os import re @@ -688,11 +689,17 @@ def get_last_build_from_app(app: metadata.App) -> metadata.Build: return metadata.Build() -def get_git_repo_and_main_branch(): - git_repo = git.Repo.init('.') - with git_repo.config_reader() as reader: - main_branch = reader.get_value('init', 'defaultBranch') - return git_repo, main_branch +def get_upstream_main_branch(git_repo): + if len(git_repo.remotes.upstream.refs) == 1: + return git_repo.remotes.upstream.refs[0].name + for name in ('main', 'master'): + if name in git_repo.remotes.upstream.refs: + return f'upstream/{name}' + try: + with git_repo.config_reader() as reader: + return 'upstream/%s' % reader.get_value('init', 'defaultBranch') + except configparser.NoSectionError: + return 'upstream/main' def checkout_appid_branch(appid): @@ -711,7 +718,8 @@ def checkout_appid_branch(appid): """ logging.debug(f'Creating merge request branch for {appid}') - git_repo, main_branch = get_git_repo_and_main_branch() + git_repo = git.Repo.init('.') + upstream_main = get_upstream_main_branch(git_repo) for remote in git_repo.remotes: remote.fetch() try: @@ -721,16 +729,14 @@ def checkout_appid_branch(appid): 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 + f'{upstream_main}...{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}" + start_point = upstream_main git_repo.git.checkout('-B', appid, start_point) - git_repo.git.rebase( - f'upstream/{main_branch}', strategy_option='ours', kill_after_timeout=120 - ) + git_repo.git.rebase(upstream_main, strategy_option='ours', kill_after_timeout=120) return True @@ -751,11 +757,11 @@ def push_commits(remote_name='origin', branch_name='checkupdates', verbose=False * https://docs.gitlab.com/ee/user/project/push_options.html """ - git_repo, default = get_git_repo_and_main_branch() + git_repo = git.Repo.init('.') + upstream_main = get_upstream_main_branch(git_repo) files = set() - upstream_main = default if default in git_repo.remotes.upstream.refs else 'main' for commit in git_repo.iter_commits( - f'upstream/{upstream_main}...HEAD', right_only=True + f'{upstream_main}...HEAD', right_only=True ): files.update(commit.stats.files.keys()) @@ -780,7 +786,7 @@ def push_commits(remote_name='origin', branch_name='checkupdates', verbose=False current_version_only = True for m in re.findall( r"^[+-].*", - git_repo.git.diff(f"upstream/{upstream_main}...HEAD"), + git_repo.git.diff(f"{upstream_main}...HEAD"), flags=re.MULTILINE, ): if re.match(r"^(\+\+\+|---) ", m): @@ -835,8 +841,9 @@ def push_commits(remote_name='origin', branch_name='checkupdates', verbose=False def prune_empty_appid_branches(git_repo=None, main_branch='main'): """Remove empty branches from checkupdates-bot git remote.""" if git_repo is None: - git_repo, main_branch = get_git_repo_and_main_branch() - upstream_main = 'upstream/' + main_branch + git_repo = git.Repo.init('.') + upstream_main = get_upstream_main_branch(git_repo) + main_branch = upstream_main.split('/')[1] remote = git_repo.remotes.origin remote.update(prune=True) diff --git a/tests/test_checkupdates.py b/tests/test_checkupdates.py index e9637190..2e802741 100755 --- a/tests/test_checkupdates.py +++ b/tests/test_checkupdates.py @@ -453,20 +453,47 @@ class CheckupdatesTest(unittest.TestCase): fdroidserver.checkupdates.main() sys_exit.assert_not_called() - def test_get_git_repo_and_main_branch(self): + def test_get_upstream_main_branch(self): os.chdir(self.testdir.name) - git_repo = git.Repo.init() + testvalue = 'foo' + git_repo = git.Repo.init('.', initial_branch=testvalue) + open('foo', 'w').close() git_repo.git.add(all=True) git_repo.index.commit("all files") + git_repo.create_remote('upstream', os.getcwd()).fetch() - repo, branch = fdroidserver.checkupdates.get_git_repo_and_main_branch() - self.assertTrue(branch in repo.heads) + branch = fdroidserver.checkupdates.get_upstream_main_branch(git_repo) + self.assertEqual( + f'upstream/{testvalue}', + branch, + f'The default branch should be called {testvalue}!', + ) + + def test_get_upstream_main_branch_git_config(self): + os.chdir(self.testdir.name) + testvalue = 'foo' + git_repo = git.Repo.init('.', initial_branch=testvalue) + with git_repo.config_writer() as cw: + cw.set_value('init', 'defaultBranch', testvalue) + + open('foo', 'w').close() + git_repo.git.add(all=True) + git_repo.index.commit("all files") + git_repo.git.branch('somethingelse') # make another remote branch + git_repo.create_remote('upstream', os.getcwd()).fetch() + + branch = fdroidserver.checkupdates.get_upstream_main_branch(git_repo) + self.assertEqual( + f'upstream/{testvalue}', + branch, + f'The default branch should be called {testvalue}!', + ) 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() + git_repo = git.Repo.init('.') open('foo', 'w').close() git_repo.git.add(all=True) git_repo.index.commit("all files") @@ -491,7 +518,7 @@ class CheckupdatesTest(unittest.TestCase): 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() + git_repo = git.Repo.init('.') # --merge-request assumes remotes called 'origin' and 'upstream' git_repo.create_remote('upstream', upstream_dir).fetch() @@ -513,7 +540,7 @@ class CheckupdatesTest(unittest.TestCase): 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() + git_repo = git.Repo.init('.') # --merge-request assumes remotes called 'origin' and 'upstream' git_repo.create_remote('upstream', upstream_dir).fetch() @@ -530,7 +557,8 @@ class CheckupdatesTest(unittest.TestCase): # set up starting from remote branch git_repo.remotes.origin.push(appid) - git_repo.git.checkout(main_branch) + upstream_main = fdroidserver.checkupdates.get_upstream_main_branch(git_repo) + git_repo.git.checkout(upstream_main.split('/')[1]) git_repo.delete_head(appid, force=True) self.assertTrue( @@ -552,7 +580,7 @@ class CheckupdatesTest(unittest.TestCase): 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() + git_repo = git.Repo.init('.') # --merge-request assumes remotes called 'origin' and 'upstream' git_repo.create_remote('upstream', upstream_dir).fetch() @@ -577,7 +605,8 @@ class CheckupdatesTest(unittest.TestCase): # set up starting from remote branch git_repo.remotes.origin.push(appid) - git_repo.git.reset(main_branch) + upstream_main = fdroidserver.checkupdates.get_upstream_main_branch(git_repo) + git_repo.git.reset(upstream_main.split('/')[1]) self.assertFalse( fdroidserver.checkupdates.checkout_appid_branch(appid), From 0ec9cd6921b504c48a2a7fd84e4c7c8b2ae7081f Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 19 Nov 2024 09:49:45 +0100 Subject: [PATCH 8/8] checkupdates: only update app branches if metadata file changed --- fdroidserver/checkupdates.py | 33 ++++++++++++++++++++------------ tests/test_checkupdates.py | 37 ++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 12 deletions(-) diff --git a/fdroidserver/checkupdates.py b/fdroidserver/checkupdates.py index d9ad8048..bd97135d 100644 --- a/fdroidserver/checkupdates.py +++ b/fdroidserver/checkupdates.py @@ -740,9 +740,22 @@ def checkout_appid_branch(appid): return True -def push_commits(remote_name='origin', branch_name='checkupdates', verbose=False): +def get_changes_versus_ref(git_repo, ref, f): + changes = [] + for m in re.findall( + r"^[+-].*", git_repo.git.diff(f"{ref}", '--', f), flags=re.MULTILINE + ): + if not re.match(r"^(\+\+\+|---) ", m): + changes.append(m) + return changes + + +def push_commits(branch_name='checkupdates', verbose=False): """Make git branch then push commits as merge request. + The appid is parsed from the actual file that was changed so that + only the right branch is ever updated. + This uses the appid as the standard branch name so that there is only ever one open merge request per-app. If multiple apps are included in the branch, then 'checkupdates' is used as branch @@ -760,9 +773,7 @@ def push_commits(remote_name='origin', branch_name='checkupdates', verbose=False git_repo = git.Repo.init('.') upstream_main = get_upstream_main_branch(git_repo) files = set() - for commit in git_repo.iter_commits( - f'{upstream_main}...HEAD', right_only=True - ): + for commit in git_repo.iter_commits(f'{upstream_main}...HEAD', right_only=True): files.update(commit.stats.files.keys()) files = list(files) @@ -773,6 +784,11 @@ def push_commits(remote_name='origin', branch_name='checkupdates', verbose=False if not files: return + remote = git_repo.remotes.origin + if branch_name in remote.refs: + if not get_changes_versus_ref(git_repo, f'origin/{branch_name}', files[0]): + return + git_repo.create_head(branch_name, force=True) push_options = [ 'merge_request.create', @@ -784,13 +800,7 @@ def push_commits(remote_name='origin', branch_name='checkupdates', verbose=False # mark as draft if there are only changes to CurrentVersion: current_version_only = True - for m in re.findall( - r"^[+-].*", - git_repo.git.diff(f"{upstream_main}...HEAD"), - flags=re.MULTILINE, - ): - if re.match(r"^(\+\+\+|---) ", m): - continue + for m in get_changes_versus_ref(git_repo, upstream_main, files[0]): if not re.match(r"^[-+]CurrentVersion", m): current_version_only = False break @@ -810,7 +820,6 @@ def push_commits(remote_name='origin', branch_name='checkupdates', verbose=False progress = MyProgressPrinter() - remote = git_repo.remotes[remote_name] pushinfos = remote.push( branch_name, progress=progress, diff --git a/tests/test_checkupdates.py b/tests/test_checkupdates.py index 2e802741..87420d2b 100755 --- a/tests/test_checkupdates.py +++ b/tests/test_checkupdates.py @@ -324,6 +324,9 @@ class CheckupdatesTest(unittest.TestCase): for f in (basedir / 'metadata').glob('*.yml'): shutil.copy(f, 'metadata') git_repo = git.Repo.init(testdir) + with git_repo.config_writer() as cw: + cw.set_value('user', 'name', 'Foo Bar') + cw.set_value('user', 'email', 'foo@bar.com') git_repo.git.add(all=True) git_repo.index.commit("all metadata files") @@ -341,6 +344,40 @@ class CheckupdatesTest(unittest.TestCase): return git_repo, origin_repo, upstream_repo + def test_get_changes_versus_ref(self): + def _make_commit_new_app(git_repo, metadata_file): + app = fdroidserver.metadata.App() + fdroidserver.metadata.write_metadata(metadata_file, app) + git_repo.git.add(metadata_file) + git_repo.git.commit(metadata_file, message=f'changed {metadata_file}') + + git_repo, origin_repo, upstream_repo = self._get_test_git_repos() + for remote in git_repo.remotes: + remote.push(git_repo.active_branch) + appid = 'com.testvalue' + metadata_file = f'metadata/{appid}.yml' + + # set up remote branch with change to app + git_repo.git.checkout('-b', appid) + _make_commit_new_app(git_repo, metadata_file) + git_repo.remotes.origin.push(appid) + + # reset local branch and there should be differences + upstream_main = fdroidserver.checkupdates.get_upstream_main_branch(git_repo) + git_repo.git.reset(upstream_main) + self.assertTrue( + fdroidserver.checkupdates.get_changes_versus_ref( + git_repo, f'origin/{appid}', metadata_file + ) + ) + # make new commit that matches the previous, different commit, no diff + _make_commit_new_app(git_repo, metadata_file) + self.assertFalse( + fdroidserver.checkupdates.get_changes_versus_ref( + git_repo, f'origin/{appid}', metadata_file + ) + ) + def test_push_commits(self): git_repo, origin_repo, upstream_repo = self._get_test_git_repos() for remote in git_repo.remotes: