From 5b1bdc2f5a31e8eba054cefbe8fe6d9bf2e7f5ea Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 07 2015 10:55:19 +0000 Subject: [PATCH 1/6] Add a new setting enforcing that all commits in a PR are signed-off by their author --- diff --git a/pagure/lib/model.py b/pagure/lib/model.py index ec24e4c..da2db78 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -320,6 +320,7 @@ class Project(BASE): 'Only_assignee_can_merge_pull-request': False, 'Minimum_score_to_merge_pull-request': -1, 'Web-hooks': None, + 'Enforce_signed-off_commits_in_pull-request': False, } if self._settings: From 6bae37f71e1364131cc35198f1d789906c29c417 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 07 2015 10:55:19 +0000 Subject: [PATCH 2/6] Store the list of commits even if the target repo is empty --- diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index f423a1e..38fe52f 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -671,6 +671,10 @@ def new_request_pull(repo, branch_to, branch_from, username=None): branch = repo_obj.lookup_branch(branch_from) repo_commit = branch.get_object() + for commit in repo_obj.walk( + repo_commit.oid.hex, pygit2.GIT_SORT_TIME): + diff_commits.append(commit) + diff = repo_commit.tree.diff_to_tree(swap=True) else: flask.flash( From d7c61c75d84178c8af56fab3c58f2b2d91931953 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 07 2015 10:55:19 +0000 Subject: [PATCH 3/6] Enforce that all commits are signed-off by the author if the configured If the project is configured to only allow PR with signed-off commits, refuse PR that are not in this situation. --- diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index 38fe52f..5872f16 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -686,6 +686,14 @@ def new_request_pull(repo, branch_to, branch_from, username=None): form = pagure.forms.RequestPullForm() if form.validate_on_submit() and is_repo_admin(repo): try: + if repo.settings.get( + 'Enforce_signed-off_commits_in_pull-request', False): + for commit in diff_commits: + if not 'signed-off-by' in commit.message.lower(): + raise pagure.exceptions.PagureException( + 'This repo enforces that all commits are ' + 'signed off by their author. ') + if orig_commit: orig_commit = orig_commit.oid.hex From e42a1037d462ea83b77b473b2bffd0fa138f01a9 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 07 2015 10:55:19 +0000 Subject: [PATCH 4/6] Also check for the signed-off status before merging a pull-request This is a second check ensuring that, just before merging a pull-request all the commits of the PR are signed-off by their authors. --- diff --git a/pagure/lib/git.py b/pagure/lib/git.py index e55b826..91bd41c 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -844,9 +844,17 @@ def merge_pull_request( new_repo = pygit2.clone_repository(parentpath, newpath) # Update the start and stop commits in the DB, one last time - pagure.lib.git.diff_pull_request( + diff_commits = pagure.lib.git.diff_pull_request( session, request, new_repo, fork_obj, - requestfolder=request_folder, with_diff=False) + requestfolder=request_folder, with_diff=False)[0] + + if request.project.settings.get( + 'Enforce_signed-off_commits_in_pull-request', False): + for commit in diff_commits: + if not 'signed-off-by' in commit.message.lower(): + raise pagure.exceptions.PagureException( + 'This repo enforces that all commits are ' + 'signed off by their author. ') # Checkout the correct branch branch_ref = get_branch_ref(new_repo, request.branch) From 62e43381c401164b233615489e4bf4fa36bfa5b7 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 07 2015 10:55:19 +0000 Subject: [PATCH 5/6] Point to the main project when checking if it can be merged correctly --- diff --git a/pagure/lib/git.py b/pagure/lib/git.py index 91bd41c..2250e91 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -844,8 +844,8 @@ def merge_pull_request( new_repo = pygit2.clone_repository(parentpath, newpath) # Update the start and stop commits in the DB, one last time - diff_commits = pagure.lib.git.diff_pull_request( - session, request, new_repo, fork_obj, + diff_commits = diff_pull_request( + session, request, pygit2.Repository(parentpath), fork_obj, requestfolder=request_folder, with_diff=False)[0] if request.project.settings.get( From 4a383799f57bef75b925ebbfd046b09cf78ed4dd Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 07 2015 10:55:19 +0000 Subject: [PATCH 6/6] Testing against an empty repo means there are no commits in the repo --- diff --git a/tests/test_progit_flask_ui_fork.py b/tests/test_progit_flask_ui_fork.py index 1068b72..920e0b3 100644 --- a/tests/test_progit_flask_ui_fork.py +++ b/tests/test_progit_flask_ui_fork.py @@ -1314,44 +1314,20 @@ index 0000000..2a552bb gitrepo = os.path.join(tests.HERE, 'forks', 'foo', 'test.git') new_repo = pygit2.clone_repository(gitrepo, newpath) - # Edit the sources file again - with open(os.path.join(newpath, 'sources'), 'w') as stream: - stream.write('foo\n bar\nbaz\n boose') - new_repo.index.add('sources') - new_repo.index.write() - - # Commits the files added - tree = new_repo.index.write_tree() - author = pygit2.Signature( - 'Alice Author', 'alice@authors.tld') - committer = pygit2.Signature( - 'Cecil Committer', 'cecil@committers.tld') - new_repo.create_commit( - 'refs/heads/feature', - author, - committer, - 'A commit on branch feature', - tree, - [] - ) - refname = 'refs/heads/feature' - ori_remote = new_repo.remotes[0] - ori_remote.push(refname) - user = tests.FakeUser() user.username = 'foo' with tests.user_set(pagure.APP, user): - output = self.app.get('/fork/foo/test/diff/master..feature') + output = self.app.get( + '/fork/foo/test/diff/master..feature', + follow_redirects=True) self.assertEqual(output.status_code, 200) self.assertIn( - 'Pull request # - test - Pagure', - output.data) - self.assertIn( - '

No commits found

', output.data) + 'Overview - test - Pagure', output.data) self.assertIn( - '', output.data) + '
  • Fork is empty, there are no commits to ' + 'request pulling
    • ', output.data) + output = self.app.get('/test/new_issue') csrf_token = output.data.split( 'name="csrf_token" type="hidden" value="')[1].split('">')[0]