From dade63be547111d267254dace6c962343bb566df Mon Sep 17 00:00:00 2001 From: Slavek Kabrda Date: Feb 28 2019 12:39:16 +0000 Subject: Fix displaying diffs that contain symlinks. Fixes #4006 --- diff --git a/pagure/lib/git.py b/pagure/lib/git.py index fd4881a..98592e7 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -2664,20 +2664,22 @@ def get_stats_patch(patch): output["old_id"] = str(patch.old_id) elif hasattr(patch, "delta"): # Newer pygit2 - if patch.delta.new_file.mode == 0 and patch.delta.old_file.mode in [ - 33188, - 33261, - ]: + # we recognize non-executable file, executable file and symlink + expected_modes = [33188, 33261, 40960] + if ( + patch.delta.new_file.mode == 0 + and patch.delta.old_file.mode in expected_modes + ): status = "D" elif ( - patch.delta.new_file.mode in [33188, 33261] + patch.delta.new_file.mode in expected_modes and patch.delta.old_file.mode == 0 ): status = "A" - elif patch.delta.new_file.mode in [ - 33188, - 33261, - ] and patch.delta.old_file.mode in [33188, 33261]: + elif ( + patch.delta.new_file.mode in expected_modes + and patch.delta.old_file.mode in expected_modes + ): status = "M" if patch.delta.new_file.path != patch.delta.old_file.path: status = "R" diff --git a/pagure/templates/commit.html b/pagure/templates/commit.html index 53c0e60..2207b71 100644 --- a/pagure/templates/commit.html +++ b/pagure/templates/commit.html @@ -16,6 +16,8 @@ {% endblock %} {% block repo %} {% set splitted_message = commit.message.split('\n') %} +{# we recognize non-executable file, executable file and symlink #} +{% set expected_modes = [33188, 33261, 40960] %}
{% block overviewtabs %}{{ super() }}{% endblock %} @@ -189,13 +191,13 @@ {%- elif patch | hasattr('delta') -%} {%- if patch.delta.new_file.path == patch.delta.old_file.path -%} {%- if patch.delta.new_file.mode == 0 - and patch.delta.old_file.mode in [33188, 33261] -%} + and patch.delta.old_file.mode in expected_modes -%} {{ changesdeletedfile(patch.delta.new_file.path, linesadded, linesremoved, loop.index) }} - {%- elif patch.delta.new_file.mode in [33188, 33261] + {%- elif patch.delta.new_file.mode in expected_modes and patch.delta.old_file.mode == 0 -%} {{ changesaddedfile( patch.delta.new_file.path, linesadded, linesremoved, loop.index) }} - {%- elif patch.delta.new_file.mode in [33188, 33261] - and patch.delta.old_file.mode in [33188, 33261] -%} + {%- elif patch.delta.new_file.mode in expected_modes + and patch.delta.old_file.mode in expected_modes -%} {{ changeschangedfile(patch.delta.new_file.path, linesadded, linesremoved, loop.index) }} {%-endif-%} {%- else -%} @@ -397,7 +399,7 @@ {%- elif patch | hasattr('delta') -%} {%- if patch.delta.new_file.path == patch.delta.old_file.path -%} {%- if patch.delta.new_file.mode == 0 - and patch.delta.old_file.mode in [33188, 33261] -%} + and patch.delta.old_file.mode in expected_modes -%} {% set patchtype = "removed"%}
{{ viewfilelink(patch.delta.new_file.path) }} @@ -408,7 +410,7 @@ {{ viewfilelinkbutton(patch.delta.new_file.path, disabled=True) }} {{ diffcollapsebtn() }}
- {%-elif patch.delta.new_file.mode in [33188, 33261] + {%-elif patch.delta.new_file.mode in expected_modes and patch.delta.old_file.mode == 0 -%} {% set patchtype = "added"%}
@@ -422,8 +424,8 @@ {{ diffcollapsebtn() }} {% endif %}
- {%-elif patch.delta.new_file.mode in [33188, 33261] - and patch.delta.old_file.mode in [33188, 33261] -%} + {%-elif patch.delta.new_file.mode in expected_modes + and patch.delta.old_file.mode in expected_modes -%} {% set patchtype = "changed"%}
{{ viewfilelink(patch.delta.new_file.path) }} diff --git a/tests/__init__.py b/tests/__init__.py index 446b047..ecdf3c8 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -878,15 +878,21 @@ that should never get displayed on the website if there is a README.rst in the r def add_commit_git_repo(folder, ncommits=10, filename='sources', - branch='master'): + branch='master', symlink_to=None): """ Create some more commits for the specified git repo. """ repo, newfolder, branch_ref_obj = _clone_and_top_commits( folder, branch, branch_ref=True) for index in range(ncommits): # Create a file in that git repo - with open(os.path.join(newfolder, filename), 'a') as stream: - stream.write('Row %s\n' % index) + if symlink_to: + os.symlink( + symlink_to, + os.path.join(newfolder, filename), + ) + else: + with open(os.path.join(newfolder, filename), 'a') as stream: + stream.write('Row %s\n' % index) repo.index.add(filename) repo.index.write() diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index be77354..e0e8905 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -2163,6 +2163,20 @@ class PagureFlaskRepotests(tests.Modeltests): '
\n' ' file removed\n', output_text) + def compare_with_symlink(c3, c4): + # View comparison of commits with symlink + # we only test that the patch itself renders correctly, + # the rest of the logic is already tested in the other functions + output = self.app.get('/test/c/%s..%s' % (c3.oid.hex, c4.oid.hex)) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + print(output_text) + self.assertIn( + 'Diff from %s to %s - test\n - Pagure' + % (c3.oid.hex, c4.oid.hex), + output_text) + self.assertIn('
+ Source
', output_text) + output = self.app.get('/foo/bar') # No project registered in the DB self.assertEqual(output.status_code, 404) @@ -2196,14 +2210,22 @@ class PagureFlaskRepotests(tests.Modeltests): ncommits=1, filename='Šource') c3 = repo.revparse_single('HEAD') + tests.add_commit_git_repo( + os.path.join(self.path, 'repos', 'test.git'), + ncommits=1, filename='Source-sl', symlink_to='Source' + ) + c4 = repo.revparse_single('HEAD') + compare_first_two(c1, c2) compare_all(c1, c3) + compare_with_symlink(c3, c4) user = tests.FakeUser() # Set user logged in with tests.user_set(self.app.application, user): compare_first_two(c1, c2) compare_all(c1, c3) + compare_with_symlink(c3, c4) def test_view_file(self): """ Test the view_file endpoint. """ @@ -2780,6 +2802,12 @@ class PagureFlaskRepotests(tests.Modeltests): repo = pygit2.Repository(os.path.join(self.path, 'repos', 'test.git')) commit = repo.revparse_single('HEAD') + # Add a symlink to test that it displays correctly + tests.add_commit_git_repo( + os.path.join(self.path, 'repos', 'test.git'), + ncommits=1, filename='sources-sl', symlink_to='sources' + ) + commit_sl = repo.revparse_single('HEAD') # View another commit output = self.app.get('/test/c/%s' % commit.oid.hex) @@ -2791,6 +2819,16 @@ class PagureFlaskRepotests(tests.Modeltests): self.assertIn('Authored by Alice Author', output_text) self.assertIn('Committed by Cecil Committer', output_text) + # Make sure that diff containing symlink displays the header correctly + output = self.app.get('/test/c/%s' % commit_sl.oid.hex) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + # check the link to the file + self.assertIn('>sources-sl', output_text) + # check that the header contains "file added" and a +1 for one added line + self.assertIn('>file added', output_text) + self.assertIn('>+1', output_text) + #View the commit when branch name is provided output = self.app.get('/test/c/%s?branch=master' % commit.oid.hex) self.assertEqual(output.status_code, 200)