From d9d36954a6f73009ffc76c7c0df229beb1200d3c Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 31 2017 10:42:57 +0000 Subject: [PATCH 1/6] Add a diff view for the PR Fixes https://pagure.io/pagure/issue/2723 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index f889a36..a81dcd4 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -266,6 +266,26 @@ def request_pull(repo, requestid, username=None, namespace=None): def request_pull_patch(repo, requestid, username=None, namespace=None): """ Returns the commits from the specified pull-request as patches. """ + return request_pull_to_diff_or_patch( + repo, requestid, username, namespace, diff=False) + + +@APP.route('//pull-request/.diff') +@APP.route('///pull-request/.diff') +@APP.route('/fork///pull-request/.diff') +@APP.route( + '/fork////pull-request/.diff') +def request_pull_diff(repo, requestid, username=None, namespace=None): + """ Returns the commits from the specified pull-request as patches. + """ + return request_pull_to_diff_or_patch( + repo, requestid, username, namespace, diff=True) + + +def request_pull_to_diff_or_patch( + repo, requestid, username=None, namespace=None, diff=False): + """ Returns the commits from the specified pull-request as patches. + """ repo = flask.g.repo if not repo.settings.get('pull_requests', True): @@ -324,7 +344,21 @@ def request_pull_patch(repo, requestid, username=None, namespace=None): 'error') diff_commits.reverse() - patch = pagure.lib.git.commit_to_patch(repo_obj, diff_commits) + if not diff: + patch = pagure.lib.git.commit_to_patch(repo_obj, diff_commits) + else: + if not isinstance(diff_commits, list): + diff_commits = [diff_commits] + + patch = [] + for cnt, commit in enumerate(diff_commits): + if commit.parents: + diff = repo_obj.diff(commit.parents[0], commit) + else: + # First commit in the repo + diff = commit.tree.diff_to_tree(swap=True) + patch.append(diff.patch) + patch = '\n'.join(patch) return flask.Response(patch, content_type="text/plain;charset=UTF-8") diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py index 62d52d5..7ff7845 100644 --- a/tests/test_pagure_flask_ui_fork.py +++ b/tests/test_pagure_flask_ui_fork.py @@ -880,6 +880,61 @@ index 9f44358..2a552bb 100644 self.assertEqual(output.status_code, 404) @patch('pagure.lib.notify.send_email') + def test_request_pull_diff(self, send_email): + """ Test the request_pull_patch endpoint. """ + send_email.return_value = True + + output = self.app.get('/test/pull-request/1.diff') + self.assertEqual(output.status_code, 404) + + tests.create_projects(self.session) + tests.create_projects_git( + os.path.join(self.path, 'requests'), bare=True) + self.set_up_git_repo( + new_project=None, branch_from='feature', mtype='merge') + + output = self.app.get('/test/pull-request/100.diff') + self.assertEqual(output.status_code, 404) + + output = self.app.get('/test/pull-request/1.diff') + self.assertEqual(output.status_code, 200) + + exp = """diff --git a/.gitignore b/.gitignore +new file mode 100644 +index 0000000..e4e5f6c +--- /dev/null ++++ b/.gitignore +@@ -0,0 +1 @@ ++*~ +\ No newline at end of file +diff --git a/sources b/sources +index 9f44358..2a552bb 100644 +--- a/sources ++++ b/sources +@@ -1,2 +1,4 @@ + foo +- bar +\ No newline at end of file ++ bar ++baz ++ boose +\ No newline at end of file +""" + + self.assertEqual(output.data, exp) + + # Project w/o pull-request + repo = pagure.get_authorized_project(self.session, 'test') + settings = repo.settings + settings['pull_requests'] = False + repo.settings = settings + self.session.add(repo) + self.session.commit() + + output = self.app.get('/test/pull-request/1.diff') + self.assertEqual(output.status_code, 404) + + @patch('pagure.lib.notify.send_email') def test_request_pull_patch_close(self, send_email): """ Test the request_pull_patch endpoint with a closed PR. """ send_email.return_value = True From 4f92f04973ddf113abf581138d04a8356b944322 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 31 2017 10:44:26 +0000 Subject: [PATCH 2/6] Optimize generating the patch version of a PR Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/lib/git.py b/pagure/lib/git.py index 40ee099..ad5665e 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -48,7 +48,7 @@ def commit_to_patch(repo_obj, commits): if not isinstance(commits, list): commits = [commits] - patch = "" + patch = [] for cnt, commit in enumerate(commits): if commit.parents: diff = repo_obj.diff(commit.parents[0], commit) @@ -65,7 +65,7 @@ def commit_to_patch(repo_obj, commits): if len(commits) > 1: subject = '[PATCH %s/%s] %s' % (cnt + 1, len(commits), subject) - patch += u"""From {commit} Mon Sep 17 00:00:00 2001 + patch.append(u"""From {commit} Mon Sep 17 00:00:00 2001 From: {author_name} <{author_email}> Date: {date} Subject: {subject} @@ -81,8 +81,8 @@ Subject: {subject} commit.commit_time).strftime('%b %d %Y %H:%M:%S +0000'), subject=subject, msg=message, - patch=diff.patch) - return patch + patch=diff.patch)) + return ''.join(patch) def generate_gitolite_acls(project=None, group=None): From b09b176170fdadca87006e0a63fdf8f16eff5a97 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 31 2017 10:44:42 +0000 Subject: [PATCH 3/6] Fix not showing the Fork and Edit button on binary files Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/templates/file.html b/pagure/templates/file.html index b5541c8..8777091 100644 --- a/pagure/templates/file.html +++ b/pagure/templates/file.html @@ -109,10 +109,18 @@ branchname=branchname, filename=filename | unicode) }}" title="Edit file">Edit {% endif %} - {% if output_type in ('file','markup') and not authenticated - or ( - not repo.is_fork - or (authenticated and repo.user.user != g.fas_user.username)) %} + {% if output_type in ('file','markup') + and ( + not authenticated + or ( + not repo.is_fork + or ( + authenticated + and repo.user.user != g.fas_user.username + ) + ) + ) + %}
' From 4b90cc8737502bdc7bc9a0ef91b791a618848e24 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 31 2017 15:55:55 +0000 Subject: [PATCH 5/6] Consolidate the code to simplify it and add diff view on commits Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/lib/git.py b/pagure/lib/git.py index ad5665e..765307a 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -40,7 +40,7 @@ from pagure.lib import tasks _log = logging.getLogger(__name__) -def commit_to_patch(repo_obj, commits): +def commit_to_patch(repo_obj, commits, diff_view=False): ''' For a given commit (PyGit2 commit object) of a specified git repo, returns a string representation of the changes the commit did in a format that allows it to be used as patch. @@ -55,17 +55,21 @@ def commit_to_patch(repo_obj, commits): else: # First commit in the repo diff = commit.tree.diff_to_tree(swap=True) - - subject = message = '' - if '\n' in commit.message: - subject, message = commit.message.split('\n', 1) + if diff_view: + patch.append(diff.patch) else: - subject = commit.message - if len(commits) > 1: - subject = '[PATCH %s/%s] %s' % (cnt + 1, len(commits), subject) + subject = message = '' + if '\n' in commit.message: + subject, message = commit.message.split('\n', 1) + else: + subject = commit.message + + if len(commits) > 1: + subject = '[PATCH %s/%s] %s' % ( + cnt + 1, len(commits), subject) - patch.append(u"""From {commit} Mon Sep 17 00:00:00 2001 + patch.append(u"""From {commit} Mon Sep 17 00:00:00 2001 From: {author_name} <{author_email}> Date: {date} Subject: {subject} @@ -74,14 +78,15 @@ Subject: {subject} --- {patch} -""".format(commit=commit.oid.hex, - author_name=commit.author.name, - author_email=commit.author.email, - date=datetime.datetime.utcfromtimestamp( - commit.commit_time).strftime('%b %d %Y %H:%M:%S +0000'), - subject=subject, - msg=message, - patch=diff.patch)) +""".format( + commit=commit.oid.hex, + author_name=commit.author.name, + author_email=commit.author.email, + date=datetime.datetime.utcfromtimestamp( + commit.commit_time).strftime('%b %d %Y %H:%M:%S +0000'), + subject=subject, + msg=message, + patch=diff.patch)) return ''.join(patch) diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index a81dcd4..39b292d 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -344,21 +344,8 @@ def request_pull_to_diff_or_patch( 'error') diff_commits.reverse() - if not diff: - patch = pagure.lib.git.commit_to_patch(repo_obj, diff_commits) - else: - if not isinstance(diff_commits, list): - diff_commits = [diff_commits] - - patch = [] - for cnt, commit in enumerate(diff_commits): - if commit.parents: - diff = repo_obj.diff(commit.parents[0], commit) - else: - # First commit in the repo - diff = commit.tree.diff_to_tree(swap=True) - patch.append(diff.patch) - patch = '\n'.join(patch) + patch = pagure.lib.git.commit_to_patch( + repo_obj, diff_commits, diff_view=diff) return flask.Response(patch, content_type="text/plain;charset=UTF-8") diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 5134fa6..ae6ab16 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -789,6 +789,25 @@ def view_commit(repo, commitid, username=None, namespace=None): def view_commit_patch(repo, commitid, username=None, namespace=None): """ Render a commit in a repo as patch """ + return view_commit_patch_or_diff( + repo, commitid, username, namespace, diff=False) + + +@APP.route('//c/.diff') +@APP.route('///c/.diff') +@APP.route('/fork///c/.diff') +@APP.route('/fork////c/.diff') +def view_commit_diff(repo, commitid, username=None, namespace=None): + """ Render a commit in a repo as diff + """ + return view_commit_patch_or_diff( + repo, commitid, username, namespace, diff=True) + + +def view_commit_patch_or_diff( + repo, commitid, username=None, namespace=None, diff=False): + """ Renders a commit either as a patch or as a diff. """ + repo_obj = flask.g.repo_obj try: @@ -799,7 +818,8 @@ def view_commit_patch(repo, commitid, username=None, namespace=None): if commit is None: flask.abort(404, 'Commit not found') - patch = pagure.lib.git.commit_to_patch(repo_obj, commit) + patch = pagure.lib.git.commit_to_patch( + repo_obj, commit, diff_view=diff) return flask.Response(patch, content_type="text/plain;charset=UTF-8") diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index 64a00d4..0d52365 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -2378,7 +2378,6 @@ class PagureFlaskRepotests(tests.Modeltests): '' in output.data) - def test_view_commit_patch(self): """ Test the view_commit_patch endpoint. """ @@ -2507,6 +2506,56 @@ index 0000000..fb7093d +Dev instance: http://209.132.184.222/ (/!\ May change unexpectedly, it's a dev instance ;-)) ''' in output.data) + def test_view_commit_diff(self): + """ Test the view_commit_diff endpoint. """ + + # No project registered in the DB + output = self.app.get('/foo/c/bar.diff') + self.assertEqual(output.status_code, 404) + + tests.create_projects(self.session) + + output = self.app.get('/test/c/bar.diff') + # No git repo associated + self.assertEqual(output.status_code, 404) + + tests.create_projects_git(os.path.join(self.path, 'repos'), bare=True) + + output = self.app.get('/test/c/bar.diff') + self.assertEqual(output.status_code, 404) + + # Add a README to the git repo - First commit + tests.add_readme_git_repo(os.path.join(self.path, 'repos', 'test.git')) + repo = pygit2.Repository(os.path.join(self.path, 'repos', 'test.git')) + commit = repo.revparse_single('HEAD') + + # View first commit + output = self.app.get('/test/c/%s.diff' % commit.oid.hex) + self.assertEqual(output.status_code, 200) + self.assertEqual('''diff --git a/README.rst b/README.rst +new file mode 100644 +index 0000000..fb7093d +--- /dev/null ++++ b/README.rst +@@ -0,0 +1,16 @@ ++Pagure ++====== ++ ++:Author: Pierre-Yves Chibon ++ ++ ++Pagure is a light-weight git-centered forge based on pygit2. ++ ++Currently, Pagure offers a web-interface for git repositories, a ticket ++system and possibilities to create new projects, fork existing ones and ++create/merge pull-requests across or within projects. ++ ++ ++Homepage: https://github.com/pypingou/pagure ++ ++Dev instance: http://209.132.184.222/ (/!\ May change unexpectedly, it's a dev instance ;-)) +''', output.data) + def test_view_tree(self): """ Test the view_tree endpoint. """ output = self.app.get('/foo/tree/') From 7f705397f2add8b9e26c89e8c6d32e13099232a1 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 02 2017 09:00:44 +0000 Subject: [PATCH 6/6] Add missing docblocks where needed, thanks @bowlofeggs! Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/lib/git.py b/pagure/lib/git.py index 765307a..b3ca0db 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -44,6 +44,18 @@ def commit_to_patch(repo_obj, commits, diff_view=False): ''' For a given commit (PyGit2 commit object) of a specified git repo, returns a string representation of the changes the commit did in a format that allows it to be used as patch. + + :arg repo_obj: the `pygit2.Repository` object of the git repo to + retrieve the commits in + :type repo_obj: `pygit2.Repository` + :arg commits: the list of commits to convert to path + :type commits: str or list + :kwarg diff_view: a boolean specifying if what is returned is a git + patch or a git diff + :type diff_view: boolean + :return: the patch or diff representation of the provided commits + :rtype: str + ''' if not isinstance(commits, list): commits = [commits] diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index 39b292d..62a5856 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -285,6 +285,23 @@ def request_pull_diff(repo, requestid, username=None, namespace=None): def request_pull_to_diff_or_patch( repo, requestid, username=None, namespace=None, diff=False): """ Returns the commits from the specified pull-request as patches. + + :arg repo: the `pagure.lib.model.Project` object of the current pagure + project browsed + :type repo: `pagure.lib.model.Project` + :arg requestid: the identifier of the pull-request to convert to patch + or diff + :type requestid: int + :kwarg username: the username of the user who forked then project when + the project viewed is a fork + :type username: str or None + :kwarg namespace: the namespace of the project if it has one + :type namespace: str or None + :kwarg diff: a boolean whether the data returned is a patch or a diff + :type diff: boolean + :return: the patch or diff representation of the specified pull-request + :rtype: str + """ repo = flask.g.repo