From a6eea67e879d826b08425377f16cb5ac0c59c0df Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 05 2015 07:27:09 +0000 Subject: [PATCH 1/7] Checkout the correct branch before trying to merge things --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index 905e8a7..c74d803 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -209,6 +209,22 @@ def mergeable_request_pull(): newpath = tempfile.mkdtemp(prefix='pagure-pr-check') new_repo = pygit2.clone_repository(parentpath, newpath) + # Checkout the correct branch + branchname = request.branch + location = pygit2.GIT_BRANCH_LOCAL + if branchname not in new_repo.listall_branches(): + branchname = 'origin/%s' % request.branch + location = pygit2.GIT_BRANCH_REMOTE + branch_to = new_repo.lookup_branch(branchname, location) + if not branch_to: + shutil.rmtree(newpath) + flask.abort( + 400, + 'Branch %s could not be found in the repo %s' % ( + request.branch, request.project.fullname + )) + new_repo.checkout(branch_to) + branch = fork_obj.lookup_branch(request.branch_from) if not branch: flask.abort( From f7a2ac70670888180757ae85bc6480cf269aadcc Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 05 2015 07:27:09 +0000 Subject: [PATCH 2/7] Remove the temp directory if something goes south --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index c74d803..67909fe 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -227,6 +227,7 @@ def mergeable_request_pull(): branch = fork_obj.lookup_branch(request.branch_from) if not branch: + shutil.rmtree(newpath) flask.abort( 400, 'Branch %s could not be found in the repo %s' % ( From b3be04c3574bfccf0b2cb1ae88d642ad6339e402 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 05 2015 07:27:09 +0000 Subject: [PATCH 3/7] Drop un-used variables --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index 67909fe..6342ad9 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -248,9 +248,6 @@ def mergeable_request_pull(): if merge is None: mergecode = new_repo.merge_analysis(repo_commit.oid)[0] - branch_ref = pagure.lib.git.get_branch_ref(new_repo, request.branch) - - refname = '%s:%s' % (branch_ref.name, branch_ref.name) if ( (merge is not None and merge.is_uptodate) or From 9126bbefa9e92fd36560aede028f3d18a7cd58a6 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 05 2015 07:27:09 +0000 Subject: [PATCH 4/7] Rework the get_branch_ref method to account for remote branches --- diff --git a/pagure/lib/git.py b/pagure/lib/git.py index 3e03d91..e573b01 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -813,17 +813,13 @@ def get_username(abspath): def get_branch_ref(repo, branchname): ''' Return the reference to the specified branch or raises an exception. ''' - branch_ref = None - refs = repo.listall_references() - if branchname in refs: - branch_ref = repo.lookup_reference(branchname).resolve() - elif 'refs/heads/%s' % branchname in refs: - branch_ref = repo.lookup_reference( - 'refs/heads/%s' % branchname).resolve() - elif 'refs/remotes/origin/%s' % branchname in refs: - branch_ref = repo.lookup_reference( - 'refs/remotes/origin/%s' % branchname).resolve() - else: + location = pygit2.GIT_BRANCH_LOCAL + if branchname not in repo.listall_branches(): + branchname = 'origin/%s' % branchname + location = pygit2.GIT_BRANCH_REMOTE + branch_ref = repo.lookup_branch(branchname, location).resolve() + + if not branch_ref: raise pagure.exceptions.PagureException( 'No refs found for %s' % branchname) return branch_ref From 8181ce756434ee8ae7a36954d13b1f9bbabf61c9 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 05 2015 07:27:09 +0000 Subject: [PATCH 5/7] Adjust mergeable_request_pull to use get_branch_ref from the internal library --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index 6342ad9..ded2bae 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -210,12 +210,7 @@ def mergeable_request_pull(): new_repo = pygit2.clone_repository(parentpath, newpath) # Checkout the correct branch - branchname = request.branch - location = pygit2.GIT_BRANCH_LOCAL - if branchname not in new_repo.listall_branches(): - branchname = 'origin/%s' % request.branch - location = pygit2.GIT_BRANCH_REMOTE - branch_to = new_repo.lookup_branch(branchname, location) + branch_ref = pagure.lib.git.get_branch_ref(new_repo, request.branch) if not branch_to: shutil.rmtree(newpath) flask.abort( @@ -225,7 +220,7 @@ def mergeable_request_pull(): )) new_repo.checkout(branch_to) - branch = fork_obj.lookup_branch(request.branch_from) + branch = pagure.lib.git.get_branch_ref(fork_obj, request.branch_from) if not branch: shutil.rmtree(newpath) flask.abort( From bef1de67fe90a569e5ee694860b52672ff82fe05 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 05 2015 07:27:09 +0000 Subject: [PATCH 6/7] Fix the merge_pull_request method - Check and error if the desired branches are not in the repo or the fork - Fix the reference so that we always push to a head - Fix the merge commit to be on the desired branch, not hard-coded to master --- diff --git a/pagure/lib/git.py b/pagure/lib/git.py index e573b01..2aca20f 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -844,8 +844,26 @@ def merge_pull_request(session, repo, request, username, request_folder): session, request, new_repo, fork_obj, requestfolder=request_folder, with_diff=False) - repo_commit = fork_obj[ - fork_obj.lookup_branch(request.branch_from).get_object().hex] + # Checkout the correct branch + branch_ref = get_branch_ref(new_repo, request.branch) + if not branch_ref: + shutil.rmtree(newpath) + raise pagure.exceptions.PagureException( + '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) + if not branch: + shutil.rmtree(newpath) + raise pagure.exceptions.PagureException( + 'Branch %s could not be found in the repo %s' % ( + request.branch_from, request.project_from.fullname + )) + + repo_commit = fork_obj[branch.get_object().hex] ori_remote = new_repo.remotes[0] # Add the fork as remote repo @@ -859,9 +877,7 @@ def merge_pull_request(session, repo, request, username, request_folder): if merge is None: mergecode = new_repo.merge_analysis(repo_commit.oid)[0] - branch_ref = get_branch_ref(new_repo, request.branch) - - refname = '%s:%s' % (branch_ref.name, branch_ref.name) + refname = '%s:refs/heads/%s' % (branch_ref.name, request.branch) if ( (merge is not None and merge.is_uptodate) or @@ -904,7 +920,7 @@ def merge_pull_request(session, repo, request, username, request_folder): head = new_repo.lookup_reference('HEAD').get_object() new_repo.create_commit( - 'refs/heads/master', + 'refs/heads/%s' % request.branch, repo_commit.author, repo_commit.committer, 'Merge #%s `%s`' % (request.id, request.title), From 6b59863487988053664acd5a677602cf5453dd24 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 05 2015 07:27:09 +0000 Subject: [PATCH 7/7] Fix indentation and variable name --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index ded2bae..e5acd6b 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -210,7 +210,7 @@ def mergeable_request_pull(): new_repo = pygit2.clone_repository(parentpath, newpath) # Checkout the correct branch - branch_ref = pagure.lib.git.get_branch_ref(new_repo, request.branch) + branch_to = pagure.lib.git.get_branch_ref(new_repo, request.branch) if not branch_to: shutil.rmtree(newpath) flask.abort(