Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: sitescripts/hg/bin/update_issues.py

Issue 30048555: Issue 7334 - Remove handling of "Fixes XXXX - ..." commit messages (Closed) Base URL: https://hg.adblockplus.org/sitescripts
Patch Set: Created April 18, 2019, 3:48 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: sitescripts/hg/bin/update_issues.py
===================================================================
--- a/sitescripts/hg/bin/update_issues.py
+++ b/sitescripts/hg/bin/update_issues.py
@@ -6,207 +6,96 @@
# published by the Free Software Foundation.
#
# Adblock Plus is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
-# along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.
-
-"""Hooks for integrating Mercurial with Trac.
+# along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.
-Update the issues that are referenced in commit messages when the commits
-are pushed and `master` bookmark is moved. See README.md for more
-information on behavior and configuration.
+"""A changegroup (or pretxnchangegroup) hook for Trac integration.
mathias 2019/05/14 09:32:02 Why do you insist on naming the type(s) of hook?
Vasily Kuznetsov 2019/05/14 10:28:59 This is the intended use and I haven't checked if
+
+Checks commit messages for issue references and posts comments linking to the
+commits into referenced issues.
"""
import collections
-import contextlib
import posixpath
import re
import xmlrpclib
from sitescripts.utils import get_config, get_template
-_IssueRef = collections.namedtuple('IssueRef', 'id commits is_fixed')
-
-ISSUE_NUMBER_REGEX = re.compile(r'\b(issue|fixes)\s+(\d+)\b', re.I)
+ISSUE_NUMBER_REGEX = re.compile(r'\bissue\s+(\d+)\b', re.I)
mathias 2019/05/13 12:23:43 Shouldn't this one include the separating dash? I.
Vasily Kuznetsov 2019/05/13 15:14:15 Yeah I see what you mean, but I'd rather leave it
mathias 2019/05/14 09:32:01 Acknowledged.
NOISSUE_REGEX = re.compile(r'^noissue\b', re.I)
-COMMIT_MESSAGE_REGEX = re.compile(r'\[\S+\ ([^\]]+)\]')
-
-
-@contextlib.contextmanager
-def _trac_proxy(ui, config, action_descr):
- trac_url = config.get('hg', 'trac_xmlrpc_url')
- try:
- yield xmlrpclib.ServerProxy(trac_url)
- except Exception as exc:
- if getattr(exc, 'faultCode', 0) == 404:
- ui.warn('warning: 404 (not found) while {}\n'.format(action_descr))
- else:
- ui.warn('error: {} while {}\n'.format(exc, action_descr))
-
-
-def _update_issue(ui, config, issue_id, changes, comment=''):
- issue_url_template = config.get('hg', 'issue_url_template')
- issue_url = issue_url_template.format(id=issue_id)
-
- updates = []
- if comment:
- for message in COMMIT_MESSAGE_REGEX.findall(comment):
- updates.append(' - referenced a commit: ' + message)
- if 'milestone' in changes:
- updates.append(' - set milestone to ' + changes['milestone'])
- if changes['action'] == 'resolve':
- updates.append(' - closed')
- if not updates:
- return
-
- with _trac_proxy(ui, config, 'updating issue {}'.format(issue_id)) as tp:
- tp.ticket.update(issue_id, comment, changes, True)
- ui.status('updated {}:\n{}\n'.format(issue_url, '\n'.join(updates)))
def _format_description(change):
lines = change.description().splitlines()
message = lines[0].rstrip()
if len(lines) == 1 or lines[1].strip() == '':
return message
return message.rstrip('.') + '...'
-def _post_comments(ui, repo, config, refs):
- repo_name = posixpath.split(repo.url())[1]
+def _generate_comments(repository_name, changes_by_issue):
+ comments = {}
template = get_template('hg/template/issue_commit_comment.tmpl',
autoescape=False)
- for ref in refs:
- comment_text = template.render({
- 'repository_name': repo_name,
- 'changes': ref.commits,
+ for issue_id, changes in changes_by_issue.items():
+ comments[issue_id] = template.render({
+ 'repository_name': repository_name,
+ 'changes': changes,
'format_description': _format_description,
})
- with _trac_proxy(ui, config, 'getting issue {}'.format(ref.id)) as tp:
- attrs = tp.ticket.get(ref.id)[3]
- changes = {'_ts': attrs['_ts'], 'action': 'leave'}
- _update_issue(ui, config, ref.id, changes, comment_text)
+ return comments
-def _compile_module_regexps(ui, config, modules):
- for module, regexp in config.items('hg_module_milestones'):
- try:
- yield module, re.compile('^{}$'.format(regexp), re.I)
- except Exception as exc:
- ui.warn('warning: skipped invalid regexp for module {} in '
- "[hg_module_milestones] config: '{}' ({})\n"
- .format(module, regexp, exc))
-
-
-def _get_module_milestones(ui, config, modules):
- module_regexps = dict(_compile_module_regexps(ui, config, modules))
- modules = module_regexps.keys()
- if not modules:
- return []
-
- milestones_by_module = {}
- with _trac_proxy(ui, config, 'getting milestones') as tp:
- milestone_names = [
- name for name in tp.ticket.milestone.getAll()
- if any(regexp.search(name) for regexp in module_regexps.values())
- ]
- # Using a MultiCall is better because we might have many milestones.
- get_milestones = xmlrpclib.MultiCall(tp)
- for name in milestone_names:
- get_milestones.ticket.milestone.get(name)
- milestones = [ms for ms in get_milestones() if not ms['completed']]
- for module in modules:
- for milestone in milestones:
- if module_regexps[module].search(milestone['name']):
- milestones_by_module[module] = milestone['name']
- break
-
- return milestones_by_module.items()
+def _post_comment(issue_id, comment):
+ issue_id = int(issue_id)
+ url = get_config().get('hg', 'trac_xmlrpc_url')
+ server = xmlrpclib.ServerProxy(url)
+ attributes = server.ticket.get(issue_id)[3]
+ server.ticket.update(
+ issue_id,
+ comment,
+ {
+ '_ts': attributes['_ts'],
+ 'action': 'leave',
+ },
+ True,
+ )
-def _declare_fixed(ui, config, refs):
- updates = []
- # Changes that need milestones added to them, indexed by module.
- need_milestones = collections.defaultdict(list)
-
- for ref in refs:
- with _trac_proxy(ui, config, 'getting issue {}'.format(ref.id)) as tp:
- attrs = tp.ticket.get(ref.id)[3]
- changes = {
- '_ts': attrs['_ts'],
- 'action': 'leave',
- }
- actions = tp.ticket.getActions(ref.id)
- if any(action[0] == 'resolve' for action in actions):
- changes['action'] = 'resolve'
- if not attrs['milestone']:
- need_milestones[attrs['component']].append(changes)
- updates.append((ref.id, changes))
+def hook(ui, repo, node, **kwargs):
+ """Post commit references into Trac issues."""
+ changes_by_issue = collections.defaultdict(list)
- for module, milestone in _get_module_milestones(ui, config,
- need_milestones.keys()):
- for changes in need_milestones[module]:
- changes['milestone'] = milestone
-
- for issue_id, changes in updates:
- _update_issue(ui, config, issue_id, changes)
-
-
-def _collect_references(ui, commits):
- commits_by_issue = collections.defaultdict(list)
- fixed_issues = set()
-
+ first_rev = repo[node].rev()
+ commits = repo[first_rev:]
for commit in commits:
- message = commit.description()
- if ' - ' not in message:
- ui.warn("warning: invalid commit message format: '{}'\n"
- .format(message))
- continue
+ description = commit.description()
+ issue_ids = ISSUE_NUMBER_REGEX.findall(description)
+ if issue_ids:
+ for issue_id in issue_ids:
+ changes_by_issue[issue_id].append(commit)
+ elif not NOISSUE_REGEX.search(description):
+ ui.warn('warning: invalid commit message format in changeset {}\n'
+ .format(commit))
- refs, rest = message.split(' - ', 1)
- issue_refs = ISSUE_NUMBER_REGEX.findall(refs)
- if issue_refs:
- for ref_type, issue_id in issue_refs:
- issue_id = int(issue_id)
- commits_by_issue[issue_id].append(commit)
- if ref_type.lower() == 'fixes':
- fixed_issues.add(issue_id)
- elif not NOISSUE_REGEX.search(refs):
- ui.warn("warning: no issue reference in commit message: '{}'\n"
- .format(message))
-
- for issue_id, commits in sorted(commits_by_issue.items()):
- yield _IssueRef(issue_id, commits, is_fixed=issue_id in fixed_issues)
-
+ repository_name = posixpath.split(repo.url())[1]
+ comments = _generate_comments(repository_name, changes_by_issue)
-def changegroup_hook(ui, repo, node, **kwargs):
- config = get_config()
- first_rev = repo[node].rev()
- pushed_commits = repo[first_rev:]
- refs = _collect_references(ui, pushed_commits)
- _post_comments(ui, repo, config, refs)
-
-
-def pushkey_hook(ui, repo, **kwargs):
- if (kwargs['namespace'] != 'bookmarks' or # Not a bookmark move.
- kwargs['key'] != 'master' or # Not `master` bookmark.
- not kwargs['old']): # The bookmark is just created.
- return
+ issue_url_template = get_config().get('hg', 'issue_url_template')
+ for issue_id, comment in comments.items():
+ issue_url = issue_url_template.format(id=issue_id)
+ try:
+ _post_comment(issue_id, comment)
+ ui.status('updating {}\n'.format(issue_url))
mathias 2019/05/14 09:32:02 This should not be in the `try` block. It should e
Vasily Kuznetsov 2019/05/14 10:28:59 Makes sense. Done.
+ except Exception as exc:
+ ui.warn('warning: failed to update {}\n'.format(issue_url))
+ ui.warn('error message: {}\n'.format(exc))
- config = get_config()
- old_master_rev = repo[kwargs['old']].rev()
- new_master_rev = repo[kwargs['new']].rev()
- added_revs = repo.changelog.findmissingrevs([old_master_rev],
- [new_master_rev])
- added_commits = [repo[rev] for rev in added_revs]
- refs = [ref for ref in _collect_references(ui, added_commits)
- if ref.is_fixed]
- _declare_fixed(ui, config, refs)
-
-
-# Alias for backward compatibility.
-hook = changegroup_hook
+# Alias for smooth migration.
+changegroup_hook = hook
mathias 2019/05/14 09:32:01 This should not be necessary at all: hg@hg-2:
Vasily Kuznetsov 2019/05/14 10:29:00 👍 Done!

Powered by Google App Engine
This is Rietveld