diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index 5420b1d..17ac510 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -23,6 +23,7 @@ from sqlalchemy.exc import SQLAlchemyError PV = flask.Blueprint('internal_ns', __name__, url_prefix='/pv') import pagure +import pagure.exceptions import pagure.forms import pagure.lib import pagure.lib.git @@ -268,62 +269,26 @@ def get_pull_request_ready_branch(): branches = {} if not repo_obj.is_empty and repo_obj.listall_branches() > 1: if not repo_obj.head_is_unborn: - compare_branch = repo_obj.lookup_branch( - repo_obj.head.shorthand) + compare_branch = repo_obj.head.shorthand else: compare_branch = None for branchname in repo_obj.listall_branches(): - branch = repo_obj.lookup_branch(branchname) # Do not compare a branch to itself if compare_branch \ - and compare_branch.branch_name == branch.branch_name: + and compare_branch == branchname: continue - repo_commit = repo_obj[branch.get_object().hex] - - if compare_branch: - main_walker = repo_obj.walk( - compare_branch.get_object().hex, - pygit2.GIT_SORT_TIME) - branch_walker = repo_obj.walk( - repo_commit.oid.hex, - pygit2.GIT_SORT_TIME) - main_commits = set() - branch_commits = list() - while 1: - com = None - if compare_branch: - try: - com = main_walker.next() - main_commits.add(com.oid.hex) - except StopIteration: - com = None - try: - branch_commit = branch_walker.next() - except StopIteration: - branch_commit = None - - # We sure never end up here but better safe than sorry - if com is None and branch_commit is None: - break - - if branch_commit: - branch_commits.append(branch_commit.hex) - if main_commits.intersection(set(branch_commits)): - break - - # If master is ahead of branch, we need to remove the commits - # that are after the first one found in master - i = 0 - for i in range(len(branch_commits)): - if branch_commits[i] in main_commits: - break - branch_commits = branch_commits[:i] + diff_commits = None + try: + _, diff_commits, _ = pagure.lib.git.get_diff_info( + repo_obj, repo_obj, branchname, compare_branch) + except pagure.exceptions.PagureException: + pass - if branch_commits: - branches[branchname] = branch_commits + if diff_commits: + branches[branchname] = [c.oid.hex for c in diff_commits] prs = pagure.lib.search_pull_requests( pagure.SESSION, diff --git a/pagure/lib/git.py b/pagure/lib/git.py index 887672c..020dad3 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -1358,46 +1358,60 @@ def merge_pull_request( return 'Changes merged!' -def diff_pull_request( - session, request, repo_obj, orig_repo, requestfolder, - with_diff=True): - """ Returns the diff and the list of commits between the two git repos - mentionned in the given pull-request. - """ +def get_diff_info(repo_obj, orig_repo, branch_from, branch_to): + ''' Return the info needed to see a diff or make a Pull-Request between + the two specified repo. - commitid = None - diff = None - diff_commits = [] - branch = repo_obj.lookup_branch(request.branch_from) - if branch: - commitid = branch.get_object().hex + :arg repo_obj: The pygit2.Repository object of the first git repo + :arg orig_repo: The pygit2.Repository object of the second git repo + :arg branch_from: the name of the branch having the changes, in the + first git repo + : arg branch_to: the name of the branch in which we want to merge the + changes in the second git repo - if repo_obj.is_empty: - raise pagure.exceptions.PagureException( - 'Fork is empty, there are no commits to create a pull request with' + ''' + frombranch = repo_obj.lookup_branch(branch_from) + if not frombranch and not repo_obj.is_empty: + raise pagure.exceptions.BranchNotFoundException( + 'Branch %s does not exist' % branch_from ) - if not orig_repo.is_empty \ - and request.branch not in orig_repo.listall_branches(): - raise pagure.exceptions.PagureException( - 'The branch into which this pull-request was to be merged: %s ' - 'seems to no longer be present in this repo' % request.branch) + branch = None + if branch_to: + branch = orig_repo.lookup_branch(branch_to) + if not branch and not orig_repo.is_empty: + raise pagure.exceptions.BranchNotFoundException( + 'Branch %s could not be found in the target repo' % branch_to + ) + commitid = None + if frombranch: + commitid = frombranch.get_object().hex + + diff_commits = [] + diff = None + orig_commit = None if not repo_obj.is_empty and not orig_repo.is_empty: + if branch: + orig_commit = orig_repo[branch.get_object().hex] + main_walker = orig_repo.walk( + orig_commit.oid.hex, pygit2.GIT_SORT_TIME) + + repo_commit = repo_obj[commitid] + branch_walker = repo_obj.walk( + repo_commit.oid.hex, pygit2.GIT_SORT_TIME) - main_walker = orig_repo.walk( - orig_repo.lookup_branch(request.branch).get_object().hex, - pygit2.GIT_SORT_TIME) - branch_walker = repo_obj.walk(commitid, pygit2.GIT_SORT_TIME) main_commits = set() branch_commits = set() while 1: - try: - com = main_walker.next() - main_commits.add(com.oid.hex) - except StopIteration: - com = None + com = None + if branch: + try: + com = main_walker.next() + main_commits.add(com.oid.hex) + except StopIteration: + com = None try: branch_commit = branch_walker.next() @@ -1417,82 +1431,111 @@ def diff_pull_request( # If master is ahead of branch, we need to remove the commits # that are after the first one found in master i = 0 - for i in range(len(diff_commits)): - if diff_commits[i].oid.hex in main_commits: - break - diff_commits = diff_commits[:i] + if diff_commits and main_commits: + for i in range(len(diff_commits)): + if diff_commits[i].oid.hex in main_commits: + break + diff_commits = diff_commits[:i] - if request.status and diff_commits: + if diff_commits: first_commit = repo_obj[diff_commits[-1].oid.hex] - # Check if we can still rely on the merge_status - commenttext = None - if request.commit_start != first_commit.oid.hex or\ - request.commit_stop != diff_commits[0].oid.hex: - request.merge_status = None - if request.commit_start: - new_commits_count = 0 - commenttext = "" - for i in diff_commits: - if i.oid.hex == request.commit_stop: - break - new_commits_count = new_commits_count + 1 - commenttext = '%s * %s\n' % ( - commenttext, i.message.strip().split('\n')[0]) - if new_commits_count == 1: - commenttext = "**%d new commit added**\n\n%s" % ( - new_commits_count, commenttext) - else: - commenttext = "**%d new commits added**\n\n%s" % ( - new_commits_count, commenttext) - if request.commit_start and \ - request.commit_start != first_commit.oid.hex: - commenttext = 'rebased' - request.commit_start = first_commit.oid.hex - request.commit_stop = diff_commits[0].oid.hex - session.add(request) - session.commit() - if commenttext: - pagure.lib.add_pull_request_comment( - session, request, - commit=None, tree_id=None, filename=None, row=None, - comment='%s' % commenttext, - user=request.user.username, - requestfolder=requestfolder, - notify=False, notification=True + if len(first_commit.parents) > 0: + diff = repo_obj.diff( + repo_obj.revparse_single(first_commit.parents[0].oid.hex), + repo_obj.revparse_single(diff_commits[0].oid.hex) ) - session.commit() - pagure.lib.git.update_git( - request, repo=request.project, - repofolder=requestfolder) - - if diff_commits and with_diff and len(diff_commits[-1].parents) > 0: - diff = repo_obj.diff( - repo_obj.revparse_single(diff_commits[-1].parents[0].oid.hex), - repo_obj.revparse_single(diff_commits[0].oid.hex) - ) - elif orig_repo.is_empty and not repo_obj.is_empty: - for commit in repo_obj.walk(commitid, pygit2.GIT_SORT_TIME): + if 'master' in repo_obj.listall_branches(): + repo_commit = repo_obj[repo_obj.head.target] + else: + branch = repo_obj.lookup_branch(branch_from) + repo_commit = branch.get_object() + + for commit in repo_obj.walk( + repo_commit.oid.hex, pygit2.GIT_SORT_TIME): diff_commits.append(commit) - if request.status and diff_commits: - first_commit = repo_obj[diff_commits[-1].oid.hex] - # Check if we can still rely on the merge_status - if request.commit_start != first_commit.oid.hex or\ - request.commit_stop != diff_commits[0].oid.hex: - request.merge_status = None - request.commit_start = first_commit.oid.hex - request.commit_stop = diff_commits[0].oid.hex - session.add(request) - session.commit() - pagure.lib.git.update_git( - request, repo=request.project, - repofolder=requestfolder) - repo_commit = repo_obj[request.commit_stop] - if with_diff: - diff = repo_commit.tree.diff_to_tree(swap=True) + diff = repo_commit.tree.diff_to_tree(swap=True) + else: + raise pagure.exceptions.PagureException( + 'Fork is empty, there are no commits to create a pull ' + 'request with' + ) + + return(diff, diff_commits, orig_commit) + + +def diff_pull_request( + session, request, repo_obj, orig_repo, requestfolder, + with_diff=True): + """ Returns the diff and the list of commits between the two git repos + mentionned in the given pull-request. - return (diff_commits, diff) + :arg session: The sqlalchemy session to connect to the database + :arg request: The pagure.lib.model.PullRequest object of the pull-request + to look into + :arg repo_obj: The pygit2.Repository object of the first git repo + :arg orig_repo: The pygit2.Repository object of the second git repo + :arg requestfolder: The folder in which are stored the git repositories + containing the metadata of the different pull-requests + :arg with_diff: A boolean on whether to return the diff with the list + of commits (or just the list of commits) + + """ + + commitid = None + diff = None + diff_commits = [] + diff, diff_commits, _ = get_diff_info( + repo_obj, orig_repo, request.branch_from, request.branch) + + if request.status and diff_commits: + first_commit = repo_obj[diff_commits[-1].oid.hex] + # Check if we can still rely on the merge_status + commenttext = None + if request.commit_start != first_commit.oid.hex or\ + request.commit_stop != diff_commits[0].oid.hex: + request.merge_status = None + if request.commit_start: + new_commits_count = 0 + commenttext = "" + for i in diff_commits: + if i.oid.hex == request.commit_stop: + break + new_commits_count = new_commits_count + 1 + commenttext = '%s * %s\n' % ( + commenttext, i.message.strip().split('\n')[0]) + if new_commits_count == 1: + commenttext = "**%d new commit added**\n\n%s" % ( + new_commits_count, commenttext) + else: + commenttext = "**%d new commits added**\n\n%s" % ( + new_commits_count, commenttext) + if request.commit_start and \ + request.commit_start != first_commit.oid.hex: + commenttext = 'rebased' + request.commit_start = first_commit.oid.hex + request.commit_stop = diff_commits[0].oid.hex + session.add(request) + session.commit() + if commenttext: + pagure.lib.add_pull_request_comment( + session, request, + commit=None, tree_id=None, filename=None, row=None, + comment='%s' % commenttext, + user=request.user.username, + requestfolder=requestfolder, + notify=False, notification=True + ) + session.commit() + pagure.lib.git.update_git( + request, repo=request.project, + repofolder=requestfolder) + + if with_diff: + return (diff_commits, diff) + else: + diff_commits def get_git_tags(project): diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index 75d4818..df2b502 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -58,100 +58,6 @@ def _get_parent_request_repo_path(repo): return parentpath -def _get_pr_info(repo_obj, orig_repo, branch_from, branch_to): - ''' Return the info needed to see a diff or make a Pull-Request between - the two specified repo. - ''' - frombranch = repo_obj.lookup_branch(branch_from) - if not frombranch and not repo_obj.is_empty: - flask.abort( - 400, - 'Branch %s does not exist' % branch_from) - - branch = orig_repo.lookup_branch(branch_to) - if not branch and not orig_repo.is_empty: - flask.abort( - 400, - 'Branch %s could not be found in the target repo' % branch_to) - - branch = repo_obj.lookup_branch(branch_from) - commitid = None - if branch: - commitid = branch.get_object().hex - - diff_commits = [] - diff = None - if not repo_obj.is_empty and not orig_repo.is_empty: - orig_commit = orig_repo[ - orig_repo.lookup_branch(branch_to).get_object().hex] - repo_commit = repo_obj[commitid] - - main_walker = orig_repo.walk( - orig_commit.oid.hex, pygit2.GIT_SORT_TIME) - branch_walker = repo_obj.walk( - repo_commit.oid.hex, pygit2.GIT_SORT_TIME) - main_commits = set() - branch_commits = set() - - while 1: - try: - com = main_walker.next() - main_commits.add(com.oid.hex) - except StopIteration: - com = None - - try: - branch_commit = branch_walker.next() - except StopIteration: - branch_commit = None - - # We sure never end up here but better safe than sorry - if com is None and branch_commit is None: - break - - if branch_commit: - branch_commits.add(branch_commit.oid.hex) - diff_commits.append(branch_commit) - if main_commits.intersection(branch_commits): - break - - # If master is ahead of branch, we need to remove the commits - # that are after the first one found in master - i = 0 - for i in range(len(diff_commits)): - if diff_commits[i].oid.hex in main_commits: - break - diff_commits = diff_commits[:i] - - if diff_commits: - first_commit = repo_obj[diff_commits[-1].oid.hex] - if len(first_commit.parents) > 0: - diff = repo_obj.diff( - repo_obj.revparse_single(first_commit.parents[0].oid.hex), - repo_obj.revparse_single(diff_commits[0].oid.hex) - ) - - elif orig_repo.is_empty and not repo_obj.is_empty: - orig_commit = None - if 'master' in repo_obj.listall_branches(): - repo_commit = repo_obj[repo_obj.head.target] - else: - branch = repo_obj.lookup_branch(branch_from) - repo_commit = branch.get_object() - - for commit in repo_obj.walk( - repo_commit.oid.hex, pygit2.GIT_SORT_TIME): - diff_commits.append(commit) - - diff = repo_commit.tree.diff_to_tree(swap=True) - else: - raise pagure.exceptions.PagureException( - 'Fork is empty, there are no commits to create a pull request with' - ) - - return(diff, diff_commits, orig_commit) - - @APP.route('//pull-requests/') @APP.route('//pull-requests') @APP.route('///pull-requests/') @@ -1059,13 +965,10 @@ def new_request_pull( orig_repo = pygit2.Repository(parentpath) try: - diff, diff_commits, orig_commit = _get_pr_info( + diff, diff_commits, orig_commit = pagure.lib.git.get_diff_info( repo_obj, orig_repo, branch_from, branch_to) except pagure.exceptions.PagureException as err: - flask.flash(err.message, 'error') - return flask.redirect(flask.url_for( - 'view_repo', username=username, repo=repo.name, - namespace=namespace)) + flask.abort(400, str(err)) repo_committer = flask.g.repo_committer @@ -1216,7 +1119,7 @@ def new_remote_request_pull(repo, username=None, namespace=None): repo_obj = pygit2.Repository(repopath) try: - diff, diff_commits, orig_commit = _get_pr_info( + diff, diff_commits, orig_commit = pagure.lib.git.get_diff_info( repo_obj, orig_repo, branch_from, branch_to) except pagure.exceptions.PagureException as err: flask.flash(err.message, 'error') diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py index bab9140..fbe0b33 100644 --- a/tests/test_pagure_flask_ui_fork.py +++ b/tests/test_pagure_flask_ui_fork.py @@ -1536,12 +1536,10 @@ index 0000000..2a552bb output = self.app.get( '/fork/foo/test/diff/master..feature', follow_redirects=True) - self.assertEqual(output.status_code, 200) - self.assertIn( - 'Overview - test - Pagure', output.data) + self.assertEqual(output.status_code, 400) self.assertIn( - '\n Fork is empty, there are ' - 'no commits to create a pull request with', output.data) + '

Fork is empty, there are no commits to create a ' + 'pull request with

', output.data) output = self.app.get('/test/new_issue') csrf_token = output.data.split( @@ -1554,12 +1552,10 @@ index 0000000..2a552bb output = self.app.post( '/test/diff/master..feature', data=data, follow_redirects=True) - self.assertEqual(output.status_code, 200) - self.assertIn( - 'Overview - test - Pagure', output.data) + self.assertEqual(output.status_code, 400) self.assertIn( - '\n Fork is empty, there are ' - 'no commits to create a pull request with', output.data) + '

Fork is empty, there are no commits to create a ' + 'pull request with

', output.data) shutil.rmtree(newpath) @@ -1591,11 +1587,10 @@ index 0000000..2a552bb with tests.user_set(pagure.APP, user): output = self.app.get( '/fork/foo/test/diff/master..master', follow_redirects=True) + self.assertEqual(output.status_code, 400) self.assertIn( - 'Overview - test - Pagure', output.data) - self.assertIn( - '\n Fork is empty, there are ' - 'no commits to create a pull request with', output.data) + '

Fork is empty, there are no commits to create a ' + 'pull request with

', output.data) shutil.rmtree(newpath) diff --git a/tests/test_pagure_flask_ui_fork_pr.py b/tests/test_pagure_flask_ui_fork_pr.py index cc30493..d02ea1e 100644 --- a/tests/test_pagure_flask_ui_fork_pr.py +++ b/tests/test_pagure_flask_ui_fork_pr.py @@ -160,6 +160,8 @@ class PagureFlaskForkPrtests(tests.Modeltests): refname = 'refs/heads/feature_foo:refs/heads/feature_foo' PagureRepo.push(ori_remote, refname) + shutil.rmtree(newpath) + def test_get_pr_info(self): """ Test pagure.ui.fork._get_pr_info """ @@ -167,7 +169,7 @@ class PagureFlaskForkPrtests(tests.Modeltests): gitrepo2 = os.path.join( self.path, 'repos', 'forks', 'pingou', 'test.git') - diff, diff_commits, orig_commit = pagure.ui.fork._get_pr_info( + diff, diff_commits, orig_commit = pagure.lib.git.get_diff_info( repo_obj=PagureRepo(gitrepo2), orig_repo=PagureRepo(gitrepo), branch_from='feature_foo',