diff --git a/fdroidserver/checkupdates.py b/fdroidserver/checkupdates.py index d812d0b6..bd97135d 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 @@ -41,6 +42,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. @@ -684,17 +689,73 @@ def get_last_build_from_app(app: metadata.App) -> metadata.Build: return metadata.Build() -def get_git_repo_and_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): + """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 = git.Repo.init('.') - main_branch = 'main' - if main_branch not in git_repo.heads: - main_branch = 'master' - return git_repo, main_branch + upstream_main = get_upstream_main_branch(git_repo) + 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}...{start_point}', right_only=True + ): + if commit.committer.email != BOT_EMAIL or commit.author.email != BOT_EMAIL: + return False + else: + start_point = upstream_main + git_repo.git.checkout('-B', appid, start_point) + git_repo.git.rebase(upstream_main, strategy_option='ours', kill_after_timeout=120) + 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 @@ -709,11 +770,10 @@ 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' - 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_main}...HEAD', right_only=True): files.update(commit.stats.files.keys()) files = list(files) @@ -723,6 +783,30 @@ def push_commits(remote_name='origin', branch_name='checkupdates', verbose=False branch_name = m.group(1) # appid 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', + '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 get_changes_versus_ref(git_repo, upstream_main, files[0]): + 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 @@ -736,23 +820,12 @@ 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, force=True, set_upstream=True, progress=progress - ) 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: checkupdates for ' + branch_name, - 'merge_request.description=' - + 'checkupdates-bot run %s' % os.getenv('CI_JOB_URL'), - ], + push_option=push_options, ) for pushinfo in pushinfos: @@ -777,8 +850,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) @@ -861,10 +935,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 8225cdff..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: @@ -453,21 +490,173 @@ 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 ('main', 'master')) - 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 = git.Repo.init('.') + 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 = git.Repo.init('.') + # --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 = git.Repo.init('.') + # --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) + 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( + 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 = git.Repo.init('.') + # --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) + 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), + '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') @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 +688,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)