From 23e9a1bd45b089feb05a7a03ee0d97cd2a6e4e17 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 02 2015 15:12:17 +0000 Subject: [PATCH 1/5] Add a merge_status column to the pull_requests table This is aiming at caching the merge status of the pull-request --- diff --git a/alembic/versions/b5efae6bb23_add_merge_status_to_the_pull_requests_.py b/alembic/versions/b5efae6bb23_add_merge_status_to_the_pull_requests_.py new file mode 100644 index 0000000..aa12d31 --- /dev/null +++ b/alembic/versions/b5efae6bb23_add_merge_status_to_the_pull_requests_.py @@ -0,0 +1,35 @@ +"""Add merge status to the pull_requests table + +Revision ID: b5efae6bb23 +Revises: None +Create Date: 2015-06-02 16:30:06.199128 + +""" + +# revision identifiers, used by Alembic. +revision = 'b5efae6bb23' +down_revision = None + +from alembic import op +from sqlalchemy.dialects.postgresql import ENUM +import sqlalchemy as sa + + +# Sources for the code: https://bitbucket.org/zzzeek/alembic/issue/67 + +def upgrade(): + ''' Add the column merge_status to the table pull_requests. + ''' + enum = ENUM('NO_CHANGE', 'FFORWARD', 'CONFLICTS', 'MERGE', + name='merge_status_enum', create_type=False) + enum.create(op.get_bind(), checkfirst=False) + op.add_column( + 'pull_requests', + sa.Column('merge_status', enum, nullable=True) + ) + +def downgrade(): + ''' Remove the column merge_status from the table pull_requests. + ''' + ENUM(name="merge_status_enum").drop(op.get_bind(), checkfirst=False) + op.drop_column('pull_requests', 'merge_status') diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 44b9fa3..536c698 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -663,6 +663,11 @@ class PullRequest(BASE): sa.ForeignKey('users.id', onupdate='CASCADE'), nullable=True, index=True) + merge_status = sa.Column( + sa.Enum( + 'NO_CHANGE', 'FFORWARD', 'CONFLICTS', 'MERGE', + name='merge_status_enum'), + nullable=True) status = sa.Column(sa.Boolean, nullable=False, default=True) From 7fbb1b60a2f10b8fe326d1aedfa10bd6540aa5f8 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 02 2015 15:15:37 +0000 Subject: [PATCH 2/5] Move all the merge status and text in a main dict and rely on it --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index 6e80331..ba1d64b 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -29,6 +29,25 @@ import pagure.ui.fork from pagure import is_repo_admin, authenticated +MERGE_OPTIONS = { + 'NO_CHANGE': { + 'short_code': 'No changes', + 'message': 'Nothing to change, git is up to date' + }, + 'FFORWARD': { + 'short_code': 'Ok', + 'message': 'The pull-request can be merged and fast-forwarded' + }, + 'CONFLICTS': { + 'short_code': 'Conflicts', + 'message': 'The pull-request cannot be merged due to conflicts' + }, + 'MERGE': { + 'short_code': 'With merge', + 'message': 'The pull-request can be merged with a merge commit' + } +} + # pylint: disable=E1101 @@ -213,21 +232,16 @@ def mergeable_request_pull(): mergecode & pygit2.GIT_MERGE_ANALYSIS_UP_TO_DATE)): shutil.rmtree(newpath) - return flask.jsonify({ - 'code': 'NO_CHANGE', - 'short_code': 'No changes', - 'message': 'Nothing to change, git is up to date'}) + request.merge_status = 'NO_CHANGE' + pagure.SESSION.commit() elif ( (merge is not None and merge.is_fastforward) or (merge is None and mergecode & pygit2.GIT_MERGE_ANALYSIS_FASTFORWARD)): - shutil.rmtree(newpath) - return flask.jsonify({ - 'code': 'FFORWARD', - 'short_code': 'Ok', - 'message': 'The pull-request can be merged and fast-forwarded'}) + request.merge_status = 'FFORWARD' + pagure.SESSION.commit() else: tree = None @@ -235,14 +249,17 @@ def mergeable_request_pull(): tree = new_repo.index.write_tree() except pygit2.GitError: shutil.rmtree(newpath) + request.merge_status = 'CONFLICTS' + pagure.SESSION.commit() return flask.jsonify({ 'code': 'CONFLICTS', - 'short_code': 'Conflicts', - 'message': 'The pull-request cannot be merged due to ' - 'conflicts'}) + 'short_code': MERGE_OPTIONS['CONFLICTS']['short_code'], + 'message': MERGE_OPTIONS['CONFLICTS']['message']}) shutil.rmtree(newpath) - return flask.jsonify({ - 'code': 'MERGE', - 'short_code': 'With merge', - 'message': 'The pull-request can be merged with a merge commit'}) + request.merge_status = 'MERGE' + pagure.SESSION.commit() + return flask.jsonify({ + 'code': request.merge_status, + 'short_code': MERGE_OPTIONS[request.merge_status]['short_code'], + 'message': MERGE_OPTIONS[request.merge_status]['message']}) From 101d1f0ec5c234e7377f6aa656791f9f2a89c73f Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 02 2015 15:15:59 +0000 Subject: [PATCH 3/5] Reset the pull-request's merge_status if things have changed --- diff --git a/pagure/lib/git.py b/pagure/lib/git.py index dde2750..2070491 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -886,6 +886,10 @@ def diff_pull_request( 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) @@ -905,6 +909,10 @@ def diff_pull_request( 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) From 579978a0eebc11da49874aae4550ae118697d409 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 02 2015 15:16:21 +0000 Subject: [PATCH 4/5] If the pull-request already has a merge_status, rely on it --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index ba1d64b..7171c68 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -189,6 +189,12 @@ def mergeable_request_pull(): if not request: flask.abort(404, 'Pull-request not found') + if request.merge_status: + return flask.jsonify({ + 'code': request.merge_status, + 'short_code': MERGE_OPTIONS[request.merge_status]['short_code'], + 'message': MERGE_OPTIONS[request.merge_status]['message']}) + # Get the fork repopath = pagure.get_repo_path(request.project_from) fork_obj = pygit2.Repository(repopath) From 59a959cb73837a0b2b2860320d83a55f6e1a4942 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 02 2015 15:26:34 +0000 Subject: [PATCH 5/5] Add the possibility to by-pass the cache and force the check --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index 7171c68..905e8a7 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -176,6 +176,9 @@ def ticket_add_comment(): def mergeable_request_pull(): """ Returns if the specified pull-request can be merged or not. """ + force = flask.request.form.get('force', False) + if force is not False: + force = True form = pagure.forms.ConfirmationForm() if not form.validate_on_submit(): @@ -189,7 +192,7 @@ def mergeable_request_pull(): if not request: flask.abort(404, 'Pull-request not found') - if request.merge_status: + if request.merge_status and not force: return flask.jsonify({ 'code': request.merge_status, 'short_code': MERGE_OPTIONS[request.merge_status]['short_code'],