From 5972bd88a5465e13038f41c4e0e1f81a56e4b7fb Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 24 2020 17:54:27 +0000 Subject: [PATCH 1/7] Enable running a command in a specific folder and return the output This way we can use the command's output and start relying on the system's git in more places. Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/lib/repo.py b/pagure/lib/repo.py index 0fe3404..58cdb9f 100644 --- a/pagure/lib/repo.py +++ b/pagure/lib/repo.py @@ -28,10 +28,12 @@ def get_pygit2_version(): return tuple([int(i) for i in pygit2.__version__.split(".")]) -def run_command(command): +def run_command(command, cwd=None): _log.info("Running command: %s", command) try: - out = subprocess.check_output(command, stderr=subprocess.STDOUT) + out = subprocess.check_output( + command, stderr=subprocess.STDOUT, cwd=cwd + ).decode("utf-8") _log.info(" command ran successfully") _log.debug("Output: %s" % out) except subprocess.CalledProcessError as err: @@ -44,6 +46,7 @@ def run_command(command): raise pagure.exceptions.PagureException( "Did not manage to rebase this pull-request" ) + return out class PagureRepo(pygit2.Repository): From 6a1d0029c109180b5e022f93172593af2017468e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 24 2020 17:54:27 +0000 Subject: [PATCH 2/7] Add a method to run git log using the system's git With this method we can run git log against a branch, a ref, a file and we can even specify from which revision/branch we want to run this command. Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/lib/repo.py b/pagure/lib/repo.py index 58cdb9f..4592eb1 100644 --- a/pagure/lib/repo.py +++ b/pagure/lib/repo.py @@ -130,3 +130,29 @@ class PagureRepo(pygit2.Repository): % (pygit2.GIT_MERGE_ANALYSIS_NORMAL) ) raise AssertionError("Unknown merge analysis result") + + @staticmethod + def log(path, log_options=None, target=None, fromref=None): + """ Run git log with the specified options at the specified target. + + This method runs the system's `git log` command since pygit2 doesn't + offer us the possibility to do this via them. + + :kwarg log_options: options to pass to git log + :type log_options: list or None + :kwarg target: the target of the git log command, can be a ref, a + file or nothing + :type path_to: str or None + :kwarg fromref: a reference/commit to use when generating the log + :type path_to: str or None + : + """ + cmd = ["git", "log"] + if log_options: + cmd.extend(log_options) + if fromref: + cmd.append(fromref) + if target: + cmd.extend(["--", target]) + + return run_command(cmd, cwd=path) From 491185472a039b2bbdcdfb1c96a28f5d46d46466 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 24 2020 17:54:27 +0000 Subject: [PATCH 3/7] Add a new endpoint and page to see a file's history Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/templates/file_history.html b/pagure/templates/file_history.html new file mode 100644 index 0000000..fd66f5d --- /dev/null +++ b/pagure/templates/file_history.html @@ -0,0 +1,179 @@ +{% extends "repo_master.html" %} + +{% block title %}Tree - {{ + repo.namespace + '/' if repo.namespace }}{{ repo.name }}{% endblock %} +{% set tag = "home" %} + +{% block header %} + + + +{% endblock %} + +{% block repo %} +
+
+

+ History {{ filename }} +

+
+ +
+
+ {% if branchname %} +
+ +
+ {% for branch in g.branches %} + {% if origin == 'view_tree' %} + + {{ branch }} + + {% elif origin == 'view_file' %} + + {{ branch }} + + {% endif %} + {% endfor %} +
+
+ {% endif %} +
+
+ +
+
+
+
    +
  1. + + +   {{ branchname }} + +
    2. + {% set path = '' %} + {% for file in filename.split('/') %} + {% if loop.first %} + {% set path = file %} + {% else %} + {% set path = path + '/' + file %} + {% endif %} + {% if loop.index != loop.length %}
  2. +   {{ file }} +
    3. + {% elif file %} +
  3. + +   {{ file }} +
    4. + {% endif %} + {% endfor %} +
+
+ +{% if log %} +
+
+
+ + {% for line in log %} + + + + + + {% endfor %} +
{{ line[0] }}
{{ line[-1] }}
+
+
+
+{% else %} +No history found for this file in this repository +{% endif %} +
+ +{% endblock %} + +{% block jscripts %} +{{ super() }} + + + + + + +{% endblock %} diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 0111fe4..cbffdac 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -805,6 +805,50 @@ def view_blame_file(repo, filename, username=None, namespace=None): ) +@UI_NS.route("//history/") +@UI_NS.route("///history/") +@UI_NS.route("/fork///history/") +@UI_NS.route("/fork////history/") +def view_history_file(repo, filename, username=None, namespace=None): + """ Displays the history of a file or a tree for the specified repo. + """ + repo = flask.g.repo + repo_obj = flask.g.repo_obj + + branchname = flask.request.args.get("identifier") + + if repo_obj.is_empty: + flask.abort(404, description="Empty repo cannot have a file") + + try: + log = pagure.lib.repo.PagureRepo.log( + flask.g.reponame, + log_options=["--pretty=oneline", "--abbrev-commit"], + target=filename, + fromref=branchname, + ) + if log.strip(): + log = [l.split(" ", 1) for l in log.strip().split("\n")] + else: + log = [] + except Exception: + log = [] + if not log: + flask.abort(400, description="No history could be found for this file") + + return flask.render_template( + "file_history.html", + select="tree", + repo=repo, + origin="view_file", + username=username, + filename=filename, + branchname=branchname, + output_type="history", + log=log, + ) + + @UI_NS.route("//c//") @UI_NS.route("//c/") @UI_NS.route("///c//") From 16eaabb29d820ebcc3a35a6e070edc5c6b7f1527 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 24 2020 17:54:27 +0000 Subject: [PATCH 4/7] Add a link to the file's history in the file's view page Basically, in the page where you can see the content of a file there is now a button to also see the history of the file, next to the button to see the "blame" view. Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/templates/file.html b/pagure/templates/file.html index 585d0ae..adcfc42 100644 --- a/pagure/templates/file.html +++ b/pagure/templates/file.html @@ -163,6 +163,14 @@ filename=filename) | unicode }}" title="View git blame">Blame History + + Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/templates/blame.html b/pagure/templates/blame.html index 05be5e4..520545f 100644 --- a/pagure/templates/blame.html +++ b/pagure/templates/blame.html @@ -143,6 +143,14 @@ {% endif %} History + + + +""" + +from __future__ import unicode_literals, absolute_import + +import re +import sys +import os +import pygit2 + +sys.path.insert( + 0, os.path.join(os.path.dirname(os.path.abspath(__file__)), "..") +) + +import tests +import pagure.lib.model + + +class PagureFlaskRepoViewHistoryFileSimpletests(tests.Modeltests): + """ Tests for view_history_file endpoint of the flask pagure app """ + + def test_view_history_file_no_project(self): + """ Test the view_history_file endpoint """ + output = self.app.get("/foo/history/sources") + # No project registered in the DB + self.assertEqual(output.status_code, 404) + output_text = output.get_data(as_text=True) + self.assertIn( + "Page not found :'( - Pagure", output_text + ) + self.assertIn("

Page not found (404)

", output_text) + self.assertIn("

Project not found

", output_text) + + def test_view_history_file_no_git_repo(self): + """ Test the view_history_file endpoint """ + tests.create_projects(self.session) + + output = self.app.get("/test/history/sources") + # No git repo associated + self.assertEqual(output.status_code, 404) + + def test_view_history_file_no_git_content(self): + """ Test the view_history_file endpoint """ + tests.create_projects(self.session) + tests.create_projects_git(os.path.join(self.path, "repos"), bare=True) + + output = self.app.get("/test/history/sources") + # project and associated repo, but no file + self.assertEqual(output.status_code, 404) + output_text = output.get_data(as_text=True) + self.assertIn( + "Page not found :'( - Pagure", output_text + ) + self.assertIn("

Page not found (404)

", output_text) + self.assertIn("

Empty repo cannot have a file

", output_text) + + +class PagureFlaskRepoViewHistoryFiletests(tests.Modeltests): + """ Tests for view_history_file endpoint of the flask pagure app """ + + def setUp(self): + """ Set up the environment, ran before every tests. """ + super(PagureFlaskRepoViewHistoryFiletests, self).setUp() + self.regex = re.compile(r'>(\w+)\n ') + tests.create_projects(self.session) + tests.create_projects_git(os.path.join(self.path, "repos"), bare=True) + + # Add some content to the git repo + tests.add_content_to_git( + os.path.join(self.path, "repos", "test.git"), + message="initial commit", + ) + tests.add_content_to_git( + os.path.join(self.path, "repos", "test.git"), message="foo" + ) + tests.add_content_to_git( + os.path.join(self.path, "repos", "test.git"), + branch="feature", + content="bar", + message="bar", + author=("Aritz Author", "aritz@authors.tld"), + ) + + def test_view_history_file_default_branch_master(self): + """ Test the view_history_file endpoint """ + output = self.app.get("/test/history/sources") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn('', output_text) + self.assertTrue( + '' in output_text + or '' in output_text + ) + self.assertIn( + '', output_text + ) + data = self.regex.findall(output_text) + self.assertEqual(len(data), 2) + + def test_view_history_file_default_branch_non_master(self): + """ Test the view_history_file endpoint """ + repo = pygit2.Repository(os.path.join(self.path, "repos", "test.git")) + reference = repo.lookup_reference("refs/heads/feature").resolve() + repo.set_head(reference.name) + output = self.app.get("/test/history/sources") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn('
foo
', output_text) + self.assertTrue( + '' in output_text + or '' in output_text + ) + self.assertIn( + '', output_text + ) + data = self.regex.findall(output_text) + self.assertEqual(len(data), 3) + + def test_view_history_file_on_commit(self): + """ Test the view_history_file endpoint """ + repo_obj = pygit2.Repository( + os.path.join(self.path, "repos", "test.git") + ) + commit = repo_obj[repo_obj.head.target] + parent = commit.parents[0].oid.hex + + output = self.app.get( + "/test/history/sources?identifier={}".format(parent) + ) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn('
bar
', output_text) + self.assertTrue( + '' in output_text + or '' in output_text + ) + self.assertIn( + '', output_text + ) + data = self.regex.findall(output_text) + self.assertEqual(len(data), 1) + + def test_view_history_file_on_branch(self): + """ Test the view_history_file endpoint """ + output = self.app.get("/test/history/sources?identifier=feature") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn('
initial commit
', output_text) + self.assertTrue( + '' in output_text + or '' in output_text + ) + self.assertIn( + '', output_text + ) + data = self.regex.findall(output_text) + self.assertEqual(len(data), 3) + + def test_view_history_file_on_tag(self): + """ Test the view_history_file endpoint """ + # set a tag on the head's parent commit + repo_obj = pygit2.Repository( + os.path.join(self.path, "repos", "test.git") + ) + commit = repo_obj[repo_obj.head.target] + parent = commit.parents[0].oid.hex + tagger = pygit2.Signature("Alice Doe", "adoe@example.com", 12347, 0) + repo_obj.create_tag( + "v1.0", parent, pygit2.GIT_OBJ_COMMIT, tagger, "Release v1.0" + ) + + output = self.app.get("/test/history/sources?identifier=v1.0") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn('
bar
', output_text) + self.assertTrue( + '' in output_text + or '' in output_text + ) + self.assertIn( + '', output_text + ) + data = self.regex.findall(output_text) + self.assertEqual(len(data), 1) + + def test_view_history_file_binary(self): + """ Test the view_history_file endpoint """ + # Add binary content + tests.add_binary_git_repo( + os.path.join(self.path, "repos", "test.git"), "test.jpg" + ) + output = self.app.get("/test/history/test.jpg") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn("", output_text) + + def test_view_history_file_non_ascii_name(self): + """ Test the view_history_file endpoint """ + tests.add_commit_git_repo( + os.path.join(self.path, "repos", "test.git"), + ncommits=1, + filename="Šource", + ) + output = self.app.get("/test/history/Šource") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertEqual( + output.headers["Content-Type"].lower(), "text/html; charset=utf-8" + ) + self.assertIn("  Šource", output_text) + self.assertIn('
initial commit
Add a fake image file
', output_text) + self.assertTrue( + '' in output_text + or '' in output_text + ) + self.assertIn( + '', + output_text + ) + + def test_view_history_file_fork_of_a_fork(self): + """ Test the view_history_file endpoint """ + item = pagure.lib.model.Project( + user_id=1, # pingou + name="test3", + description="test project #3", + is_fork=True, + parent_id=1, + hook_token="aaabbbppp", + ) + self.session.add(item) + self.session.commit() + + tests.add_content_git_repo( + os.path.join(self.path, "repos", "forks", "pingou", "test3.git") + ) + tests.add_readme_git_repo( + os.path.join(self.path, "repos", "forks", "pingou", "test3.git") + ) + tests.add_commit_git_repo( + os.path.join(self.path, "repos", "forks", "pingou", "test3.git"), + ncommits=10, + ) + tests.add_content_to_git( + os.path.join(self.path, "repos", "forks", "pingou", "test3.git"), + content="✨☃🍰☃✨", + ) + + output = self.app.get("/fork/pingou/test3/history/sources") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn('
Add row 0 to Šource file
', output_text) + self.assertTrue( + '' in output_text + or '' in output_text + ) + self.assertIn( + '', + output_text, + ) + + def test_view_history_file_no_file(self): + """ Test the view_history_file endpoint """ + output = self.app.get("/test/history/foofile") + self.assertEqual(output.status_code, 400) + output_text = output.get_data(as_text=True) + self.assertIn( + 'No history could be found for this file', + output_text + ) + + def test_view_history_file_folder(self): + """ Test the view_history_file endpoint """ + tests.add_commit_git_repo( + os.path.join(self.path, "repos", "test.git/folder1"), + ncommits=1, + filename="sources", + ) + output = self.app.get("/test/history/folder1") + self.assertEqual(output.status_code, 400) + output_text = output.get_data(as_text=True) + self.assertIn( + 'No history could be found for this file', + output_text) + + + def test_view_history_file_unborn_head_no_identifier(self): + repo_obj = pygit2.Repository( + os.path.join(self.path, "repos", "test.git") + ) + repo_obj.set_head("refs/heads/unexistent") + + output = self.app.get("/test/history/sources") + self.assertEqual(output.status_code, 400) + output_text = output.get_data(as_text=True) + self.assertIn( + 'No history could be found for this file', + output_text) From 38cd70c2ba65b8f7088d494f3bd12903241618f9 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 25 2020 08:49:54 +0000 Subject: [PATCH 7/7] Rework the UI for the file history page With this commit the file history page looks a lot closer to the page listing the commits of the project. Adjust the related tests while at it. Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/templates/file_history.html b/pagure/templates/file_history.html index fd66f5d..d2503b0 100644 --- a/pagure/templates/file_history.html +++ b/pagure/templates/file_history.html @@ -1,7 +1,6 @@ {% extends "repo_master.html" %} -{% block title %}Tree - {{ - repo.namespace + '/' if repo.namespace }}{{ repo.name }}{% endblock %} +{% block title %}File history - {{ repo.fullname }}{% endblock %} {% set tag = "home" %} {% block header %} @@ -104,18 +103,56 @@ {% if log %}
-
- 
Add row 2 to sources file
- {% for line in log %} - - - - - - {% endfor %} -
{{ line[0] }}
{{ line[-1] }}
- + +
+ {% for line in log %} + {% set commit = g.repo_obj[line[0]] %} +
+
+
+ + {{ commit.message.split('\n')[0] }} + +
+ {{commit.author|author2user_commits( + link=url_for('ui_ns.view_commits', + repo=repo.name, + branchname=branchname, + username=username, + namespace=repo.namespace, + author=commit.author.email), + cssclass="notblue")|safe}} + • + {{ commit.commit_time|humanize }}   + +
+
+
+
+ + {{ commit.hex|short }} + + +
+
+
+
+ {% endfor %} +
+ {% else %} @@ -127,53 +164,10 @@ No history found for this file in this repository {% block jscripts %} {{ super() }} - - - - {% endblock %} diff --git a/tests/test_pagure_flask_ui_repo_view_history.py b/tests/test_pagure_flask_ui_repo_view_history.py index 109864c..1c53f4c 100644 --- a/tests/test_pagure_flask_ui_repo_view_history.py +++ b/tests/test_pagure_flask_ui_repo_view_history.py @@ -66,7 +66,7 @@ class PagureFlaskRepoViewHistoryFiletests(tests.Modeltests): def setUp(self): """ Set up the environment, ran before every tests. """ super(PagureFlaskRepoViewHistoryFiletests, self).setUp() - self.regex = re.compile(r'>(\w+)\n ') + self.regex = re.compile(r'
', output_text) - self.assertTrue( - '' in output_text - or '' in output_text - ) - self.assertIn( - '
foo
', output_text - ) + self.assertIn("foo", output_text) data = self.regex.findall(output_text) self.assertEqual(len(data), 2) @@ -112,16 +103,7 @@ class PagureFlaskRepoViewHistoryFiletests(tests.Modeltests): output = self.app.get("/test/history/sources") self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) - self.assertIn('', output_text) - self.assertTrue( - '' in output_text - or '' in output_text - ) - self.assertIn( - '', output_text - ) + self.assertIn("bar", output_text) data = self.regex.findall(output_text) self.assertEqual(len(data), 3) @@ -138,16 +120,7 @@ class PagureFlaskRepoViewHistoryFiletests(tests.Modeltests): ) self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) - self.assertIn('
bar
', output_text) - self.assertTrue( - '' in output_text - or '' in output_text - ) - self.assertIn( - '', output_text - ) + self.assertIn("initial commit", output_text) data = self.regex.findall(output_text) self.assertEqual(len(data), 1) @@ -156,16 +129,7 @@ class PagureFlaskRepoViewHistoryFiletests(tests.Modeltests): output = self.app.get("/test/history/sources?identifier=feature") self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) - self.assertIn('
initial commit
', output_text) - self.assertTrue( - '' in output_text - or '' in output_text - ) - self.assertIn( - '', output_text - ) + self.assertIn("bar", output_text) data = self.regex.findall(output_text) self.assertEqual(len(data), 3) @@ -185,16 +149,7 @@ class PagureFlaskRepoViewHistoryFiletests(tests.Modeltests): output = self.app.get("/test/history/sources?identifier=v1.0") self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) - self.assertIn('
bar
', output_text) - self.assertTrue( - '' in output_text - or '' in output_text - ) - self.assertIn( - '', output_text - ) + self.assertIn("initial commit", output_text) data = self.regex.findall(output_text) self.assertEqual(len(data), 1) @@ -207,7 +162,7 @@ class PagureFlaskRepoViewHistoryFiletests(tests.Modeltests): output = self.app.get("/test/history/test.jpg") self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) - self.assertIn("", output_text) + self.assertIn("Add a fake image file", output_text) def test_view_history_file_non_ascii_name(self): """ Test the view_history_file endpoint """ @@ -223,17 +178,7 @@ class PagureFlaskRepoViewHistoryFiletests(tests.Modeltests): output.headers["Content-Type"].lower(), "text/html; charset=utf-8" ) self.assertIn("  Šource", output_text) - self.assertIn('
initial commit
Add a fake image file
', output_text) - self.assertTrue( - '' in output_text - or '' in output_text - ) - self.assertIn( - '', - output_text - ) + self.assertIn("Add row 0 to Šource file", output_text) def test_view_history_file_fork_of_a_fork(self): """ Test the view_history_file endpoint """ @@ -266,16 +211,8 @@ class PagureFlaskRepoViewHistoryFiletests(tests.Modeltests): output = self.app.get("/fork/pingou/test3/history/sources") self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) - self.assertIn('
Add row 0 to Šource file
', output_text) - self.assertTrue( - '' in output_text - or '' in output_text - ) self.assertIn( - '', - output_text, + "Add row 2 to sources file", output_text ) def test_view_history_file_no_file(self): @@ -283,10 +220,7 @@ class PagureFlaskRepoViewHistoryFiletests(tests.Modeltests): output = self.app.get("/test/history/foofile") self.assertEqual(output.status_code, 400) output_text = output.get_data(as_text=True) - self.assertIn( - 'No history could be found for this file', - output_text - ) + self.assertIn("No history could be found for this file", output_text) def test_view_history_file_folder(self): """ Test the view_history_file endpoint """ @@ -298,10 +232,7 @@ class PagureFlaskRepoViewHistoryFiletests(tests.Modeltests): output = self.app.get("/test/history/folder1") self.assertEqual(output.status_code, 400) output_text = output.get_data(as_text=True) - self.assertIn( - 'No history could be found for this file', - output_text) - + self.assertIn("No history could be found for this file", output_text) def test_view_history_file_unborn_head_no_identifier(self): repo_obj = pygit2.Repository( @@ -312,6 +243,4 @@ class PagureFlaskRepoViewHistoryFiletests(tests.Modeltests): output = self.app.get("/test/history/sources") self.assertEqual(output.status_code, 400) output_text = output.get_data(as_text=True) - self.assertIn( - 'No history could be found for this file', - output_text) + self.assertIn("No history could be found for this file", output_text) 
Add row 2 to sources file