diff --git a/fdroidserver/checkupdates.py b/fdroidserver/checkupdates.py index bd5f812e..caf07eda 100644 --- a/fdroidserver/checkupdates.py +++ b/fdroidserver/checkupdates.py @@ -685,7 +685,22 @@ def get_last_build_from_app(app: metadata.App) -> metadata.Build: def push_commits(remote_name='origin', verbose=False): - """Push commits using either appid or 'checkupdates' as branch name.""" + """Make git branch then push commits as merge request. + + 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 + name. This is to support the old way operating, e.g. in batches. + + This uses GitLab "Push Options" to create a merge request. Git + Push Options are config data that can be sent via `git push + --push-option=... origin foo`. + + References + ---------- + * https://docs.gitlab.com/ee/user/project/push_options.html + + """ git_repo = git.Repo.init('.') files = set() upstream_main = 'main' if 'main' in git_repo.remotes.upstream.refs else 'master' @@ -699,43 +714,57 @@ def push_commits(remote_name='origin', verbose=False): m = re.match(r'metadata/([^\s]+)\.yml', files[0]) if m: branch_name = m.group(1) # appid - if len(files) > 0: - if verbose: - from clint.textui import progress + if not files: + return + progress = None + if verbose: + import clint.textui - bar = progress.Bar() + progress_bar = clint.textui.progress.Bar() - class MyProgressPrinter(git.RemoteProgress): - def update(self, op_code, current, maximum=None, message=None): - if isinstance(maximum, float): - bar.show(current, maximum) + class MyProgressPrinter(git.RemoteProgress): + def update(self, op_code, current, maximum=None, message=None): + if isinstance(maximum, float): + progress_bar.show(current, maximum) - progress = MyProgressPrinter() + 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'), + ], + ) + + for pushinfo in pushinfos: + if pushinfo.flags & ( + git.remote.PushInfo.ERROR + | git.remote.PushInfo.REJECTED + | git.remote.PushInfo.REMOTE_FAILURE + | git.remote.PushInfo.REMOTE_REJECTED + ): + # Show potentially useful messages from git remote + if progress: + for line in progress.other_lines: + if line.startswith('remote:'): + logging.debug(line) + raise FDroidException( + f'{remote.url} push failed: {pushinfo.flags} {pushinfo.summary}' + ) else: - progress = None - - 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 - ) - for pushinfo in pushinfos: - if pushinfo.flags & ( - git.remote.PushInfo.ERROR - | git.remote.PushInfo.REJECTED - | git.remote.PushInfo.REMOTE_FAILURE - | git.remote.PushInfo.REMOTE_REJECTED - ): - # Show potentially useful messages from git remote - if progress: - for line in progress.other_lines: - if line.startswith('remote:'): - logging.debug(line) - raise FDroidException( - f'{remote.url} push failed: {pushinfo.flags} {pushinfo.summary}' - ) - else: - logging.debug(remote.url + ': ' + pushinfo.summary) + logging.debug(remote.url + ': ' + pushinfo.summary) def prune_empty_appid_branches(git_repo=None): @@ -792,6 +821,8 @@ def main(): help=_("Only process apps with auto-updates")) parser.add_argument("--commit", action="store_true", default=False, help=_("Commit changes")) + parser.add_argument("--merge-request", action="store_true", default=False, + help=_("Commit changes, push, then make a merge request")) parser.add_argument("--allow-dirty", action="store_true", default=False, help=_("Run on git repo that has uncommitted changes")) metadata.add_metadata_arguments(parser) @@ -806,6 +837,10 @@ def main(): logging.error(_('Build metadata git repo has uncommited changes!')) sys.exit(1) + if options.merge_request and not (options.appid and len(options.appid) == 1): + logging.error(_('--merge-request only runs on a single appid!')) + sys.exit(1) + apps = common.read_app_args(options.appid) processed = [] @@ -821,7 +856,7 @@ def main(): logging.info(msg) try: - checkupdates_app(app, options.auto, options.commit) + checkupdates_app(app, options.auto, options.commit or options.merge_request) processed.append(appid) except Exception as e: msg = _("...checkupdate failed for {appid} : {error}").format(appid=appid, error=e) @@ -830,6 +865,10 @@ def main(): failed[appid] = str(e) exit_code = 1 + if options.appid and options.merge_request: + push_commits(verbose=options.verbose) + prune_empty_appid_branches() + status_update_json(processed, failed) sys.exit(exit_code) diff --git a/tests/checkupdates.TestCase b/tests/checkupdates.TestCase index 90da2666..8c8bb03f 100755 --- a/tests/checkupdates.TestCase +++ b/tests/checkupdates.TestCase @@ -339,9 +339,14 @@ class CheckupdatesTest(unittest.TestCase): git_remote_upstream = os.path.join(testdir, 'git_remote_upstream') upstream_repo = git.Repo.init(git_remote_upstream, bare=True) + with upstream_repo.config_writer() as cw: + cw.set_value('receive', 'advertisePushOptions', True) git_repo.create_remote('upstream', 'file://' + git_remote_upstream) + git_remote_origin = os.path.join(testdir, 'git_remote_origin') origin_repo = git.Repo.init(git_remote_origin, bare=True) + with origin_repo.config_writer() as cw: + cw.set_value('receive', 'advertisePushOptions', True) git_repo.create_remote('origin', 'file://' + git_remote_origin) return git_repo, origin_repo, upstream_repo @@ -425,9 +430,38 @@ class CheckupdatesTest(unittest.TestCase): self.assertNotIn(appid, git_repo.remotes.origin.refs) self.assertNotIn(appid, git_repo.remotes.upstream.refs) - def test_make_merge_request(self): - testdir = self.testdir.name - os.chdir(testdir) + @mock.patch('sys.exit') + @mock.patch('fdroidserver.metadata.read_metadata') + def test_merge_requests_flag(self, read_metadata, sys_exit): + def _sys_exit(return_code=0): + assert return_code != 0 + raise fdroidserver.exception.FDroidException('sys.exit() ran') + + def _read_metadata(a=None, b=None): + raise StopIteration('read_metadata() ran, test is successful') + + appid = 'com.example' + # read_metadata.return_value = dict() # {appid: dict()} + read_metadata.side_effect = _read_metadata + sys_exit.side_effect = _sys_exit + + # set up clean git repo + 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") + + with mock.patch('sys.argv', ['fdroid checkupdates', '--merge-request']): + with self.assertRaises(fdroidserver.exception.FDroidException): + fdroidserver.checkupdates.main() + sys_exit.assert_called() + + sys_exit.reset_mock() + with mock.patch('sys.argv', ['fdroid checkupdates', '--merge-request', appid]): + with self.assertRaises(StopIteration): + fdroidserver.checkupdates.main() + sys_exit.assert_not_called() if __name__ == "__main__":