diff --git a/pagure/lib/git.py b/pagure/lib/git.py index 57dd0df..d4a3ff7 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -1162,9 +1162,11 @@ def merge_pull_request( # Get the fork repopath = pagure.utils.get_remote_repo_path( request.remote_git, request.branch_from) - else: + elif request.project_from: # Get the fork repopath = pagure.utils.get_repo_path(request.project_from) + else: + return fork_obj = PagureRepo(repopath) @@ -1196,19 +1198,11 @@ def merge_pull_request( 'This repo enforces that all commits are ' 'signed off by their author. ') - # Checkout the correct branch - branch_ref = get_branch_ref(new_repo, request.branch) - if not branch_ref: - shutil.rmtree(newpath) - _log.info(' Target branch could not be found') - raise pagure.exceptions.BranchNotFoundException( - 'Branch %s could not be found in the repo %s' % ( - request.branch, request.project.fullname - )) - - new_repo.checkout(branch_ref) - - branch = get_branch_ref(fork_obj, request.branch_from) + # Check/Get the branch from + try: + branch = get_branch_ref(fork_obj, request.branch_from) + except pagure.exceptions.PagureException: + branch = None if not branch: shutil.rmtree(newpath) _log.info(' Branch of origin could not be found') @@ -1218,8 +1212,6 @@ def merge_pull_request( if request.project_from else request.remote_git )) - repo_commit = fork_obj[branch.get_object().hex] - ori_remote = new_repo.remotes[0] # Add the fork as remote repo reponame = '%s_%s' % (request.user.user, request.uid) @@ -1230,6 +1222,71 @@ def merge_pull_request( # Fetch the commits remote.fetch() + # repo_commit = fork_obj[branch.get_object().hex] + repo_commit = new_repo[branch.get_object().hex] + + # Checkout the correct branch + if new_repo.is_empty or new_repo.head_is_unborn: + _log.debug( + ' target repo is empty, so PR can be merged using ' + 'fast-forward, reporting it') + if domerge: + _log.info(' PR merged using fast-forward') + if not request.project.settings.get('always_merge', False): + new_repo.create_branch(request.branch, repo_commit) + commit = repo_commit.oid.hex + else: + tree = new_repo.index.write_tree() + user_obj = pagure.lib.get_user(session, username) + commitname = user_obj.fullname or user_obj.user + author = _make_signature( + commitname, + user_obj.default_email) + commit = new_repo.create_commit( + 'refs/heads/%s' % request.branch, + author, + author, + 'Merge #%s `%s`' % (request.id, request.title), + tree, + [repo_commit.oid.hex]) + + _log.info(' New head: %s', commit) + refname = 'refs/heads/%s:refs/heads/%s' % ( + request.branch, request.branch) + PagureRepo.push(ori_remote, refname) + bare_main_repo.run_hook( + '0' * 40, commit, 'refs/heads/%s' % request.branch, + username) + # Update status + _log.info(' Closing the PR in the DB') + pagure.lib.close_pull_request( + session, request, username, + requestfolder=request_folder, + ) + shutil.rmtree(newpath) + + return 'Changes merged!' + else: + _log.info(' PR merged using fast-forward, reporting it') + request.merge_status = 'FFORWARD' + session.commit() + shutil.rmtree(newpath) + return 'FFORWARD' + + try: + branch_ref = get_branch_ref(new_repo, request.branch) + except pagure.exceptions.PagureException: + branch_ref = None + if not branch_ref: + shutil.rmtree(newpath) + _log.info(' Target branch could not be found') + raise pagure.exceptions.BranchNotFoundException( + 'Branch %s could not be found in the repo %s' % ( + request.branch, request.project.fullname + )) + + new_repo.checkout(branch_ref) + merge = new_repo.merge(repo_commit.oid) _log.debug(' Merge: %s', merge) if merge is None: @@ -1446,6 +1503,7 @@ def get_diff_info(repo_obj, orig_repo, branch_from, branch_to, prid=None): repo_obj = orig_repo if not repo_obj.is_empty and not orig_repo.is_empty: + _log.info('Pulling into an non-empty repo') if branch: orig_commit = orig_repo[branch.get_object().hex] main_walker = orig_repo.walk( @@ -1491,6 +1549,7 @@ def get_diff_info(repo_obj, orig_repo, branch_from, branch_to, prid=None): break diff_commits = diff_commits[:i] + _log.info('Diff commits: %s', diff_commits) if diff_commits: first_commit = repo_obj[diff_commits[-1].oid.hex] if len(first_commit.parents) > 0: @@ -1498,7 +1557,12 @@ def get_diff_info(repo_obj, orig_repo, branch_from, branch_to, prid=None): repo_obj.revparse_single(first_commit.parents[0].oid.hex), repo_obj.revparse_single(diff_commits[0].oid.hex) ) + elif first_commit.oid.hex == diff_commits[0].oid.hex: + _log.info('First commit is also the last commit') + diff = diff_commits[0].tree.diff_to_tree(swap=True) + elif orig_repo.is_empty and not repo_obj.is_empty: + _log.info('Pulling into an empty repo') if 'master' in repo_obj.listall_branches(): repo_commit = repo_obj[repo_obj.head.target] else: @@ -1509,6 +1573,7 @@ def get_diff_info(repo_obj, orig_repo, branch_from, branch_to, prid=None): repo_commit.oid.hex, pygit2.GIT_SORT_TIME): diff_commits.append(commit) + _log.info('Diff commits: %s', diff_commits) diff = repo_commit.tree.diff_to_tree(swap=True) else: raise pagure.exceptions.PagureException( diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index c3e3a14..bbb5301 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -1543,6 +1543,10 @@ def new_remote_request_pull(repo, username=None, namespace=None): flask.flash(str(err), 'error') flask.g.branches = sorted(orig_repo.listall_branches()) + try: + branch_to = orig_repo.head.shorthand + except pygit2.GitError: + branch_to = 'master' return flask.render_template( 'remote_pull_request.html', @@ -1550,7 +1554,7 @@ def new_remote_request_pull(repo, username=None, namespace=None): repo=repo, username=username, form=form, - branch_to=orig_repo.head.shorthand, + branch_to=branch_to, ) diff --git a/tests/test_pagure_flask_ui_remote_pr.py b/tests/test_pagure_flask_ui_remote_pr.py index dc44885..52be4db 100644 --- a/tests/test_pagure_flask_ui_remote_pr.py +++ b/tests/test_pagure_flask_ui_remote_pr.py @@ -278,5 +278,155 @@ class PagureRemotePRtests(tests.Modeltests): self.assertEqual(len(project.requests), 1) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def test_new_remote_pr_empty_target(self): + """ Test creating a new remote PR authenticated against an empty + git repo. """ + + tests.create_projects(self.session) + tests.create_projects_git( + os.path.join(self.path, 'requests'), bare=True) + + # Create empty target git repo + gitrepo = os.path.join(self.path, 'repos', 'test.git') + pygit2.init_repository(gitrepo, bare=True) + + # Create git repo we'll pull from + gitrepo = os.path.join(self.path, 'repos', 'test_origin.git') + repo = pygit2.init_repository(gitrepo) + + # Create a file in that git repo + with open(os.path.join(gitrepo, 'sources'), 'w') as stream: + stream.write('foo\n bar') + repo.index.add('sources') + repo.index.write() + + prev_commit = [] + + # Commits the files added + tree = repo.index.write_tree() + author = _make_signature( + 'Alice Author', 'alice@authors.tld') + committer = _make_signature( + 'Cecil Committer', 'cecil@committers.tld') + repo.create_commit( + 'refs/heads/feature', # the name of the reference to update + author, + committer, + 'Add sources file for testing', + # binary string representing the tree object ID + tree, + # list of binary strings representing parents of the new commit + prev_commit + ) + + # Before + self.session = pagure.lib.create_session(self.dbpath) + project = pagure.lib.get_authorized_project(self.session, 'test') + self.assertEqual(len(project.requests), 0) + + # Try creating a remote PR + user = tests.FakeUser(username='foo') + with tests.user_set(self.app.application, user): + output = self.app.get('/test/diff/remote') + self.assertEqual(output.status_code, 200) + self.assertIn( + '

New remote pull-request

', + output.get_data(as_text=True)) + + csrf_token = self.get_csrf(output=output) + data = { + 'csrf_token': csrf_token, + 'title': 'Remote PR title', + 'branch_from': 'feature', + 'branch_to': 'master', + 'git_repo': gitrepo, + } + output = self.app.post('/test/diff/remote', data=data) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn('

Create pull request

', output_text) + self.assertIn( + '
', output_text) + self.assertNotIn( + '
', output_text) + + # Not saved yet + self.session = pagure.lib.create_session(self.dbpath) + project = pagure.lib.get_authorized_project(self.session, 'test') + self.assertEqual(len(project.requests), 0) + + data = { + 'csrf_token': csrf_token, + 'title': 'Remote PR title', + 'branch_from': 'feature', + 'branch_to': 'master', + 'git_repo': gitrepo, + 'confirm': 1, + } + output = self.app.post( + '/test/diff/remote', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'PR#1: Remote PR title - test\n - Pagure', + output_text) + self.assertIn( + '
', output_text) + self.assertNotIn( + '
', output_text) + + # Show the filename in the diff view + self.assertIn( + '''
+ sources +
+ this is a remote pull-request, so we cannot provide you''', + output_text) + # Show the filename in the Changes summary + self.assertIn( + '' + '\n sources', output_text) + + # Remote PR Created + self.session = pagure.lib.create_session(self.dbpath) + project = pagure.lib.get_authorized_project(self.session, 'test') + self.assertEqual(len(project.requests), 1) + + # Check the merge state of the PR + data = { + 'csrf_token': csrf_token, + 'requestid': project.requests[0].uid, + } + output = self.app.post('/pv/pull-request/merge', data=data) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + data = json.loads(output_text) + self.assertEqual( + data, + { + "code": "FFORWARD", + "message": "The pull-request can be merged and fast-forwarded", + "short_code": "Ok" + } + ) + + user = tests.FakeUser(username='pingou') + with tests.user_set(self.app.application, user): + # Merge the PR + data = { + 'csrf_token': csrf_token, + } + output = self.app.post( + '/test/pull-request/1/merge', data=data, follow_redirects=True) + output_text = output.get_data(as_text=True) + self.assertEqual(output.status_code, 200) + self.assertIn( + 'Overview - test - Pagure', output_text + ) + + if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/tests/test_pagure_lib_git.py b/tests/test_pagure_lib_git.py index 09efc57..073192d 100644 --- a/tests/test_pagure_lib_git.py +++ b/tests/test_pagure_lib_git.py @@ -3072,15 +3072,14 @@ index 0000000..60f7480 self.assertEqual(req.title, 'test PR') # `master` branch not found - self.assertRaises( - pagure.exceptions.PagureException, - pagure.lib.git.merge_pull_request, + msg = pagure.lib.git.merge_pull_request( self.session, request=req, username='pingou', request_folder=os.path.join(self.path, 'requests'), domerge=False ) + self.assertEqual(msg, 'FFORWARD') @patch('pagure.lib.notify.send_email') @patch('pagure.lib.git.update_git')