From 97500b8a2c868d6b8e7ca573df9631ecac218f3f Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 24 2017 09:36:25 +0000 Subject: [PATCH 1/3] Optimize the algorithm used to create the diff used to open a new PR Here as well, we used to iterate through all the commits to find which ones are present on the other branch. Now we just iterate through both branch and stop when we encounter the first commit present in both branches. We will need to consolidate our code base a little as this is the first time we are using a similar piece of code, so we definitively have duplication here. --- diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index c1113b8..d22e1a3 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -84,20 +84,31 @@ def _get_pr_info(repo_obj, orig_repo, branch_from, branch_to): 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] - master_commits = [ - commit.oid.hex - for commit in orig_repo.walk( - orig_commit.oid.hex, pygit2.GIT_SORT_TIME) - ] + main_walker = repo_obj.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() - repo_commit = repo_obj[commitid] + while 1: + try: + com = main_walker.next() + main_commits.add(com.hex) + except StopIteration: + pass + try: + branch_commit = branch_walker.next() + except StopIteration: + branch_commit = None - for commit in repo_obj.walk( - repo_commit.oid.hex, pygit2.GIT_SORT_TIME): - if commit.oid.hex in master_commits: + branch_commits.add(branch_commit.oid.hex) + if main_commits.intersection(branch_commits): break - diff_commits.append(commit) + + diff_commits.append(branch_commit) if diff_commits: first_commit = repo_obj[diff_commits[-1].oid.hex] From de491bde10e68d0ac5ee7b989a03266dfb5e21bd Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 24 2017 09:36:25 +0000 Subject: [PATCH 2/3] Fix to the optimizations - Fix the case where both walker reached the end of their loop (so avoid a potential infinite loop) - Check that we retrieved a commit from the branch before adding it to the list of known commits --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index cc5e53e..ed939e6 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -298,14 +298,21 @@ def get_pull_request_ready_branch(): com = main_walker.next() main_commits.add(com.hex) except StopIteration: - pass + com = None try: branch_commit = branch_walker.next() except StopIteration: branch_commit = None - if main_commits.intersection(set( - branch_commits + [branch_commit.hex])): + # We sure never end up here but better safe than sorry + if com is None and branch_commit is None: + break + + if branch_commit: + tmp = set(branch_commits + [branch_commit.hex]) + else: + tmp = set(branch_commits) + if main_commits.intersection(tmp): break branch_commits.append(branch_commit.hex) diff --git a/pagure/lib/git.py b/pagure/lib/git.py index adec451..bcc17cf 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -1379,13 +1379,19 @@ def diff_pull_request( com = main_walker.next() main_commits.add(com.hex) except StopIteration: - pass + com = None + try: branch_commit = branch_walker.next() except StopIteration: branch_commit = None - branch_commits.add(branch_commit.oid.hex) + # 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) if main_commits.intersection(branch_commits): break diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index d22e1a3..1e8d0ce 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -98,13 +98,19 @@ def _get_pr_info(repo_obj, orig_repo, branch_from, branch_to): com = main_walker.next() main_commits.add(com.hex) except StopIteration: - pass + com = None + try: branch_commit = branch_walker.next() except StopIteration: branch_commit = None - branch_commits.add(branch_commit.oid.hex) + # 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) if main_commits.intersection(branch_commits): break From f556c2bda187e0080e7dea28ad425e95257066f0 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 24 2017 09:36:25 +0000 Subject: [PATCH 3/3] Handle the situation where the main branch is ahead of the other one In these case, we do not want to include in our diff the commits that are already in the main branch, so we remove them. (Most often that means we will end up with an empty diff) --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index ed939e6..b07e6c0 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -316,6 +316,14 @@ def get_pull_request_ready_branch(): break branch_commits.append(branch_commit.hex) + # If master is ahead of branch, we need to remove the commits + # that are already in master + branch_commits = [ + com + for com in branch_commits + if com.oid.hex not in main_commits + ] + if branch_commits: branches[branchname] = branch_commits diff --git a/pagure/lib/git.py b/pagure/lib/git.py index bcc17cf..702119a 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -1397,6 +1397,14 @@ def diff_pull_request( diff_commits.append(branch_commit) + # If master is ahead of branch, we need to remove the commits + # that are already in master + diff_commits = [ + com + for com in diff_commits + if com.oid.hex not in main_commits + ] + 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 diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index 1e8d0ce..e8923fc 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -116,6 +116,14 @@ def _get_pr_info(repo_obj, orig_repo, branch_from, branch_to): diff_commits.append(branch_commit) + # If master is ahead of branch, we need to remove the commits + # that are already in master + diff_commits = [ + com + for com in diff_commits + if com.oid.hex not in main_commits + ] + if diff_commits: first_commit = repo_obj[diff_commits[-1].oid.hex] if len(first_commit.parents) > 0: