From 3716bb68b499a0bbd44486d583060e19ae17d26c Mon Sep 17 00:00:00 2001 From: Oliver Gutierrez Date: Mar 17 2016 12:17:49 +0000 Subject: [PATCH 1/8] Added commit comparison view --- diff --git a/pagure/templates/repo_info.html b/pagure/templates/repo_info.html index 90861e2..9df8570 100644 --- a/pagure/templates/repo_info.html +++ b/pagure/templates/repo_info.html @@ -147,6 +147,68 @@ git push -u origin master {% endif %} {% endif %} + {% if origin == 'compare_commits' %} +

Comparing changes

+
{{ commit1 }} .. {{ commit2 }}
+ + {% if diff %} + {% for patch in diff %} + {% if patch |hasattr('new_id') %} + {% set patch_new_id = patch.new_id %} + {% elif patch |hasattr('delta') %} + {% set patch_new_id = patch.delta.new_file.id %} + {% else %} + {% set patch_new_id = patch.new_oid %} + {% endif %} + + {% if patch | hasattr('new_file_path') %} + {% set patch_new_file_path = patch.new_file_path -%} + {% if patch.new_file_path != patch.old_file_path %} + {% set patch_old_file_path = patch.old_file_path %} + {%- endif -%} + {%- elif patch | hasattr('delta') -%} + {% set patch_new_file_path = patch.delta.new_file.path -%} + {%- if patch.delta.new_file.path != patch.delta.old_file.path -%} + {% set patch_old_file_path = patch.delta.old_file.path %} + {%- endif -%} + {%- endif -%} + +
+
+
+
+ {%- if patch_new_file_path == patch_old_file_path -%} + {{ patch_new_file_path }} (Renamed from "{{ patch_old_file_path }}") + {%- else -%} + {{ patch_new_file_path }} + {%- endif -%} + + View +
+
+ + {% autoescape false %} + {{ patch | patch_to_diff | html_diff | format_loc( + filename=patch_new_file_path, + commit=patch_new_id, + prequest=pull_request, + index=loop.index, + tree_id=diff_commits[0].tree.id)}} + {% endautoescape %} + +
+
+ {% endfor %} + {% endif %} + {% endif %} + {% if origin == 'view_repo' or origin == 'view_repo_branch' %} {% if readme %} diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 149a7d8..e7d8983 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -372,6 +372,73 @@ def view_commits(repo, branchname=None, username=None): ) +@APP.route('//..') +@APP.route('///..') +@APP.route('/fork///..') +@APP.route('/fork////..') +def compare_commits(repo, commit1, commit2, branchname=None, username=None): + """ Compares two commits for specified repo and branch + """ + repo = pagure.lib.get_project(SESSION, repo, user=username) + + if not repo: + flask.abort(404, 'Project not found') + + reponame = pagure.get_repo_path(repo) + + repo_obj = pygit2.Repository(reponame) + + if branchname and branchname not in repo_obj.listall_branches(): + flask.abort(404, 'Branch no found') + + if branchname: + branch = repo_obj.lookup_branch(branchname) + elif not repo_obj.is_empty and not repo_obj.head_is_unborn: + branch = repo_obj.lookup_branch(repo_obj.head.shorthand) + branchname = branch.branch_name + else: + branch = None + + if not repo_obj.is_empty and not repo_obj.head_is_unborn: + head = repo_obj.head.shorthand + else: + head = None + + # Check commit1 and commit2 existence + commit1_obj = repo_obj.get(commit1) + commit2_obj = repo_obj.get(commit2) + if commit1_obj is None: + flask.abort(404, 'First commit does not exist') + if commit2_obj is None: + flask.abort(404, 'Second commit does not exist') + + # TODO: Get commit1 and commit2 diff data + diff_commits = [commit1_obj, commit2_obj] + diff = repo_obj.diff(commit1, commit2) + + # TODO: Pass data to template + + return flask.render_template( + 'repo_info.html', + select='logs', + origin='compare_commits', + repo_obj=repo_obj, + repo=repo, + username=username, + head=head, + commit1=commit1, + commit2=commit2, + commit1_obj=commit1_obj, + commit2_obj=commit2_obj, + diff=diff, + diff_commits=diff_commits, + branches=sorted(repo_obj.listall_branches()), + branchname=branchname, + repo_admin=is_repo_admin(repo), + form=pagure.forms.ConfirmationForm(), + ) + + @APP.route('//blob//f/') @APP.route('/fork///blob//f/') def view_file(repo, identifier, filename, username=None): From 06db6dd087c19b535ff0bcc54adf28978a26fe63 Mon Sep 17 00:00:00 2001 From: Oliver Gutierrez Date: Mar 17 2016 15:55:38 +0000 Subject: [PATCH 2/8] Fixed styling and unused branch statements --- diff --git a/pagure/templates/repo_info.html b/pagure/templates/repo_info.html index 9df8570..635105d 100644 --- a/pagure/templates/repo_info.html +++ b/pagure/templates/repo_info.html @@ -206,9 +206,12 @@ git push -u origin master {% endfor %} + {% else %} +

No changes

{% endif %} {% endif %} + {% if origin == 'view_repo' or origin == 'view_repo_branch' %} {% if readme %} diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index e7d8983..fdc8ab3 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -372,10 +372,10 @@ def view_commits(repo, branchname=None, username=None): ) +@APP.route('//../') @APP.route('//..') -@APP.route('///..') +@APP.route('/fork///../') @APP.route('/fork///..') -@APP.route('/fork////..') def compare_commits(repo, commit1, commit2, branchname=None, username=None): """ Compares two commits for specified repo and branch """ @@ -388,9 +388,6 @@ def compare_commits(repo, commit1, commit2, branchname=None, username=None): repo_obj = pygit2.Repository(reponame) - if branchname and branchname not in repo_obj.listall_branches(): - flask.abort(404, 'Branch no found') - if branchname: branch = repo_obj.lookup_branch(branchname) elif not repo_obj.is_empty and not repo_obj.head_is_unborn: From cfb05874adb0c3c0dfcdb3b3dcb08759679488e0 Mon Sep 17 00:00:00 2001 From: Oliver Gutierrez Date: Mar 17 2016 16:25:42 +0000 Subject: [PATCH 3/8] Added test for commit comparison endpoint --- diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index 39b5140..f9aa1d8 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -816,6 +816,55 @@ class PagureFlaskRepotests(tests.Modeltests): 'test project #3 ', output.data) self.assertIn('Forked from', output.data) + def test_compare_commits(self): + """ Test the compare_commits endpoint. """ + output = self.app.get('/foo/bar') + # No project registered in the DB + self.assertEqual(output.status_code, 404) + + tests.create_projects(self.session) + + output = self.app.get('/test/bar') + # No git repo associated + self.assertEqual(output.status_code, 404) + + tests.create_projects_git(tests.HERE, bare=True) + + output = self.app.get('/test/bar') + self.assertEqual(output.status_code, 404) + + # Add a README to the git repo - First commit + tests.add_readme_git_repo(os.path.join(tests.HERE, 'test.git')) + repo = pygit2.Repository(os.path.join(tests.HERE, 'test.git')) + commit1 = repo.revparse_single('HEAD') + + # Add some content to the git repo + tests.add_content_git_repo(os.path.join(tests.HERE, 'test.git')) + + repo = pygit2.Repository(os.path.join(tests.HERE, 'test.git')) + commit2 = repo.revparse_single('HEAD') + + # View commits comparison + output = self.app.get('/test/%s..%s' % (commit1.oid.hex, commit2.oid.hex)) + self.assertEqual(output.status_code, 200) + self.assertTrue( + '
%s .. %s
' % (commit1.oid.hex, commit2.oid.hex) + in output.data) + self.assertTrue('@@ -0,0 +1,3 @@' in output.data) + self.assertTrue('+ foo' in output.data) + self.assertTrue('+ bar' in output.data) + self.assertTrue('+ baz ' in output.data) + + # View inverse commits comparison + output = self.app.get('/test/%s..%s' % (commit2.oid.hex, commit1.oid.hex)) + self.assertTrue( + '
%s .. %s
' % (commit2.oid.hex, commit1.oid.hex) + in output.data) + self.assertTrue('@@ -1,3 +0,0 @@' in output.data) + self.assertTrue('- foo' in output.data) + self.assertTrue('- bar' in output.data) + self.assertTrue('- baz ' in output.data) + def test_view_file(self): """ Test the view_file endpoint. """ output = self.app.get('/foo/blob/foo/f/sources') From 8e7e66d23ca90f5c8a91ee06add12608f375d4db Mon Sep 17 00:00:00 2001 From: Oliver Gutierrez Date: Mar 17 2016 16:27:19 +0000 Subject: [PATCH 4/8] Fixed python executable path at nosetests script to work with virtual environmentswq --- diff --git a/nosetests b/nosetests index dbf9cf3..2f5271b 100755 --- a/nosetests +++ b/nosetests @@ -1,4 +1,4 @@ -#!/usr/bin/python +#!/usr/bin/env python # EASY-INSTALL-ENTRY-SCRIPT: 'nose==0.10.4','console_scripts','nosetests' __requires__ = ['nose>=0.10.4', 'SQLAlchemy >= 0.7', 'jinja2 >= 2.4'] import sys From 18e619d7f824bd6b30d230d671f2cdee6518e28c Mon Sep 17 00:00:00 2001 From: Oliver Gutierrez Date: Mar 21 2016 10:03:51 +0000 Subject: [PATCH 5/8] Modified compare commit to use pull_request.html template --- diff --git a/pagure/templates/pull_request.html b/pagure/templates/pull_request.html index 38381fa..8dab25c 100644 --- a/pagure/templates/pull_request.html +++ b/pagure/templates/pull_request.html @@ -101,6 +101,11 @@ (tree) + + {% if origin == 'compare_commits' %} +
{{ commit1 }} .. {{ commit2 }}
+ {% endif %} + {% endif %} {% if form and (repo_admin or remote_git) %} diff --git a/pagure/templates/repo_info.html b/pagure/templates/repo_info.html index 635105d..90861e2 100644 --- a/pagure/templates/repo_info.html +++ b/pagure/templates/repo_info.html @@ -147,71 +147,6 @@ git push -u origin master {% endif %} {% endif %} - {% if origin == 'compare_commits' %} -

Comparing changes

-
{{ commit1 }} .. {{ commit2 }}
- - {% if diff %} - {% for patch in diff %} - {% if patch |hasattr('new_id') %} - {% set patch_new_id = patch.new_id %} - {% elif patch |hasattr('delta') %} - {% set patch_new_id = patch.delta.new_file.id %} - {% else %} - {% set patch_new_id = patch.new_oid %} - {% endif %} - - {% if patch | hasattr('new_file_path') %} - {% set patch_new_file_path = patch.new_file_path -%} - {% if patch.new_file_path != patch.old_file_path %} - {% set patch_old_file_path = patch.old_file_path %} - {%- endif -%} - {%- elif patch | hasattr('delta') -%} - {% set patch_new_file_path = patch.delta.new_file.path -%} - {%- if patch.delta.new_file.path != patch.delta.old_file.path -%} - {% set patch_old_file_path = patch.delta.old_file.path %} - {%- endif -%} - {%- endif -%} - -
-
-
-
- {%- if patch_new_file_path == patch_old_file_path -%} - {{ patch_new_file_path }} (Renamed from "{{ patch_old_file_path }}") - {%- else -%} - {{ patch_new_file_path }} - {%- endif -%} - - View -
-
- - {% autoescape false %} - {{ patch | patch_to_diff | html_diff | format_loc( - filename=patch_new_file_path, - commit=patch_new_id, - prequest=pull_request, - index=loop.index, - tree_id=diff_commits[0].tree.id)}} - {% endautoescape %} - -
-
- {% endfor %} - {% else %} -

No changes

- {% endif %} - {% endif %} - - {% if origin == 'view_repo' or origin == 'view_repo_branch' %} {% if readme %} diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index fdc8ab3..236f37c 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -376,8 +376,8 @@ def view_commits(repo, branchname=None, username=None): @APP.route('//..') @APP.route('/fork///../') @APP.route('/fork///..') -def compare_commits(repo, commit1, commit2, branchname=None, username=None): - """ Compares two commits for specified repo and branch +def compare_commits(repo, commit1, commit2, username=None): + """ Compares two commits for specified repo """ repo = pagure.lib.get_project(SESSION, repo, user=username) @@ -388,14 +388,6 @@ def compare_commits(repo, commit1, commit2, branchname=None, username=None): repo_obj = pygit2.Repository(reponame) - if branchname: - branch = repo_obj.lookup_branch(branchname) - elif not repo_obj.is_empty and not repo_obj.head_is_unborn: - branch = repo_obj.lookup_branch(repo_obj.head.shorthand) - branchname = branch.branch_name - else: - branch = None - if not repo_obj.is_empty and not repo_obj.head_is_unborn: head = repo_obj.head.shorthand else: @@ -409,14 +401,12 @@ def compare_commits(repo, commit1, commit2, branchname=None, username=None): if commit2_obj is None: flask.abort(404, 'Second commit does not exist') - # TODO: Get commit1 and commit2 diff data + # Get commit1 and commit2 diff data diff_commits = [commit1_obj, commit2_obj] diff = repo_obj.diff(commit1, commit2) - # TODO: Pass data to template - return flask.render_template( - 'repo_info.html', + 'pull_request.html', select='logs', origin='compare_commits', repo_obj=repo_obj, @@ -430,7 +420,7 @@ def compare_commits(repo, commit1, commit2, branchname=None, username=None): diff=diff, diff_commits=diff_commits, branches=sorted(repo_obj.listall_branches()), - branchname=branchname, + # branchname=branchname, repo_admin=is_repo_admin(repo), form=pagure.forms.ConfirmationForm(), ) diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index f9aa1d8..d75988e 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -847,23 +847,19 @@ class PagureFlaskRepotests(tests.Modeltests): # View commits comparison output = self.app.get('/test/%s..%s' % (commit1.oid.hex, commit2.oid.hex)) self.assertEqual(output.status_code, 200) - self.assertTrue( - '
%s .. %s
' % (commit1.oid.hex, commit2.oid.hex) - in output.data) - self.assertTrue('@@ -0,0 +1,3 @@' in output.data) - self.assertTrue('+ foo' in output.data) - self.assertTrue('+ bar' in output.data) - self.assertTrue('+ baz ' in output.data) + self.assertIn('
%s .. %s
' % (commit1.oid.hex, commit2.oid.hex), output.data) + self.assertIn('@@ -0,0 +1,3 @@', output.data) + self.assertIn('+ foo', output.data) + self.assertIn('+ bar', output.data) + self.assertIn('+ baz ', output.data) # View inverse commits comparison output = self.app.get('/test/%s..%s' % (commit2.oid.hex, commit1.oid.hex)) - self.assertTrue( - '
%s .. %s
' % (commit2.oid.hex, commit1.oid.hex) - in output.data) - self.assertTrue('@@ -1,3 +0,0 @@' in output.data) - self.assertTrue('- foo' in output.data) - self.assertTrue('- bar' in output.data) - self.assertTrue('- baz ' in output.data) + self.assertIn('
%s .. %s
' % (commit2.oid.hex, commit1.oid.hex), output.data) + self.assertIn('@@ -1,3 +0,0 @@', output.data) + self.assertIn('- foo', output.data) + self.assertIn('- bar', output.data) + self.assertIn('- baz ', output.data) def test_view_file(self): """ Test the view_file endpoint. """ From 048bf153a0419eba6000b0626ba75239ceb76e57 Mon Sep 17 00:00:00 2001 From: Oliver Gutierrez Date: Mar 28 2016 07:35:48 +0000 Subject: [PATCH 6/8] Code styling fixes for PEP8 compliance --- diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index d75988e..03eef4a 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -836,27 +836,36 @@ class PagureFlaskRepotests(tests.Modeltests): # Add a README to the git repo - First commit tests.add_readme_git_repo(os.path.join(tests.HERE, 'test.git')) repo = pygit2.Repository(os.path.join(tests.HERE, 'test.git')) - commit1 = repo.revparse_single('HEAD') + c1 = repo.revparse_single('HEAD') # Add some content to the git repo tests.add_content_git_repo(os.path.join(tests.HERE, 'test.git')) repo = pygit2.Repository(os.path.join(tests.HERE, 'test.git')) - commit2 = repo.revparse_single('HEAD') + c2 = repo.revparse_single('HEAD') # View commits comparison - output = self.app.get('/test/%s..%s' % (commit1.oid.hex, commit2.oid.hex)) + output = self.app.get('/test/%s..%s' % (c1.oid.hex, c2.oid.hex)) self.assertEqual(output.status_code, 200) - self.assertIn('
%s .. %s
' % (commit1.oid.hex, commit2.oid.hex), output.data) - self.assertIn('@@ -0,0 +1,3 @@', output.data) + self.assertIn( + '
%s .. %s
' % (c1.oid.hex, c2.oid.hex), + output.data) + self.assertIn( + '@@ -0,0 +1,3 @@', + output.data) self.assertIn('+ foo', output.data) self.assertIn('+ bar', output.data) self.assertIn('+ baz ', output.data) # View inverse commits comparison - output = self.app.get('/test/%s..%s' % (commit2.oid.hex, commit1.oid.hex)) - self.assertIn('
%s .. %s
' % (commit2.oid.hex, commit1.oid.hex), output.data) - self.assertIn('@@ -1,3 +0,0 @@', output.data) + output = self.app.get( + '/test/%s..%s' % (c2.oid.hex, c1.oid.hex)) + self.assertIn( + '
%s .. %s
' % (c2.oid.hex, c1.oid.hex), + output.data) + self.assertIn( + '@@ -1,3 +0,0 @@', + output.data) self.assertIn('- foo', output.data) self.assertIn('- bar', output.data) self.assertIn('- baz ', output.data) From f6beda24caeb32fc0ba86c4c3330c78df52a7bbb Mon Sep 17 00:00:00 2001 From: Oliver Gutierrez Date: Apr 05 2016 12:14:39 +0000 Subject: [PATCH 7/8] Changed routes to new schema --- diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 236f37c..161ea43 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -372,10 +372,10 @@ def view_commits(repo, branchname=None, username=None): ) -@APP.route('//../') -@APP.route('//..') -@APP.route('/fork///../') -@APP.route('/fork///..') +@APP.route('//c/../') +@APP.route('//c/..') +@APP.route('/fork///c/../') +@APP.route('/fork///c/..') def compare_commits(repo, commit1, commit2, username=None): """ Compares two commits for specified repo """ diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index 03eef4a..e23cd4a 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -845,7 +845,7 @@ class PagureFlaskRepotests(tests.Modeltests): c2 = repo.revparse_single('HEAD') # View commits comparison - output = self.app.get('/test/%s..%s' % (c1.oid.hex, c2.oid.hex)) + output = self.app.get('/test/c/%s..%s' % (c1.oid.hex, c2.oid.hex)) self.assertEqual(output.status_code, 200) self.assertIn( '
%s .. %s
' % (c1.oid.hex, c2.oid.hex), @@ -859,7 +859,7 @@ class PagureFlaskRepotests(tests.Modeltests): # View inverse commits comparison output = self.app.get( - '/test/%s..%s' % (c2.oid.hex, c1.oid.hex)) + '/test/c/%s..%s' % (c2.oid.hex, c1.oid.hex)) self.assertIn( '
%s .. %s
' % (c2.oid.hex, c1.oid.hex), output.data) From c279504abaeb77fed08a5043ffa0cf9c80bbd2c8 Mon Sep 17 00:00:00 2001 From: Oliver Gutierrez Date: Apr 05 2016 12:57:29 +0000 Subject: [PATCH 8/8] Removed commit parameter enforcement to 40 chars string --- diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 161ea43..9df5002 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -372,10 +372,10 @@ def view_commits(repo, branchname=None, username=None): ) -@APP.route('//c/../') -@APP.route('//c/..') -@APP.route('/fork///c/../') -@APP.route('/fork///c/..') +@APP.route('//c/../') +@APP.route('//c/..') +@APP.route('/fork///c/../') +@APP.route('/fork///c/..') def compare_commits(repo, commit1, commit2, username=None): """ Compares two commits for specified repo """