From d80b6cb34633a9eab3e7054c0600c161052519de Mon Sep 17 00:00:00 2001 From: Benjamin Pflanz Date: Tue, 16 Jun 2015 20:19:50 -0400 Subject: [PATCH] Don't parse git log header for topic When looking for candidate topic in bug or blueprint ID, get the git log without header lines, e.g., commit hash, author, committer, dates, so that accidental matches such as commit hash (which could look like a bug ID) or user name (which could look like anything) don't accidentally get taken as the topic. Change-Id: Ie45ed7509e7f873e566f078b55cadd547da704dd Closes-Bug: #2000296 --- git_review/cmd.py | 9 +++- git_review/tests/test_git_review.py | 68 ++++++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 3 deletions(-) diff --git a/git_review/cmd.py b/git_review/cmd.py index cac99a5..5b49044 100755 --- a/git_review/cmd.py +++ b/git_review/cmd.py @@ -900,7 +900,14 @@ def get_topic(target_branch): "for the topic of the change submitted", "/".join(branch_parts[2:])) - log_output = run_command("git log HEAD^1..HEAD") + preferred_log_format = "%B" + log_output = run_command("git log --pretty='" + preferred_log_format + + "' HEAD^1..HEAD") + if log_output == preferred_log_format: + # The %B format specifier is supported starting at Git v1.7.2. If it's + # not supported, we'll just get back '%B', so we try something else. + # The downside of %s is that it removes newlines in the subject. + log_output = run_command("git log --pretty='%s%n%b' HEAD^1..HEAD") bug_re = r'''(?x) # verbose regexp \b([Bb]ug|[Ll][Pp]) # bug or lp [ \t\f\v]* # don't want to match newline diff --git a/git_review/tests/test_git_review.py b/git_review/tests/test_git_review.py index 1519c83..ffaa34a 100644 --- a/git_review/tests/test_git_review.py +++ b/git_review/tests/test_git_review.py @@ -330,7 +330,7 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase): def test_bug_topic_newline(self): self._run_git_review('-s') - self._simple_change('a change', 'new change not for bug\n123') + self._simple_change('a change', 'new change not for bug\n\n123') self._assert_branch_would_be('master') def test_bp_topic(self): @@ -340,9 +340,73 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase): def test_bp_topic_newline(self): self._run_git_review('-s') - self._simple_change('a change', 'new change not for bluepring\nasdf') + self._simple_change('a change', 'new change not for blueprint\n\nasdf') self._assert_branch_would_be('master') + def test_author_name_topic_bp(self): + old_author = None + if 'GIT_AUTHOR_NAME' in os.environ: + old_author = os.environ['GIT_AUTHOR_NAME'] + try: + os.environ['GIT_AUTHOR_NAME'] = 'BPNAME' + self._run_git_review('-s') + self._simple_change('a change', + 'new change 1 with name but no topic') + self._assert_branch_would_be('master') + finally: + if old_author: + os.environ['GIT_AUTHOR_NAME'] = old_author + else: + del os.environ['GIT_AUTHOR_NAME'] + + def test_author_email_topic_bp(self): + old_author = None + if 'GIT_AUTHOR_EMAIL' in os.environ: + old_author = os.environ['GIT_AUTHOR_EMAIL'] + try: + os.environ['GIT_AUTHOR_EMAIL'] = 'bpemail@example.com' + self._run_git_review('-s') + self._simple_change('a change', + 'new change 1 with email but no topic') + self._assert_branch_would_be('master') + finally: + if old_author: + os.environ['GIT_AUTHOR_EMAIL'] = old_author + else: + del os.environ['GIT_AUTHOR_EMAIL'] + + def test_author_name_topic_bug(self): + old_author = None + if 'GIT_AUTHOR_NAME' in os.environ: + old_author = os.environ['GIT_AUTHOR_NAME'] + try: + os.environ['GIT_AUTHOR_NAME'] = 'Bug: #1234' + self._run_git_review('-s') + self._simple_change('a change', + 'new change 2 with name but no topic') + self._assert_branch_would_be('master') + finally: + if old_author: + os.environ['GIT_AUTHOR_NAME'] = old_author + else: + del os.environ['GIT_AUTHOR_NAME'] + + def test_author_email_topic_bug(self): + old_author = None + if 'GIT_AUTHOR_EMAIL' in os.environ: + old_author = os.environ['GIT_AUTHOR_EMAIL'] + try: + os.environ['GIT_AUTHOR_EMAIL'] = 'bug5678@example.com' + self._run_git_review('-s') + self._simple_change('a change', + 'new change 2 with email but no topic') + self._assert_branch_would_be('master') + finally: + if old_author: + os.environ['GIT_AUTHOR_EMAIL'] = old_author + else: + del os.environ['GIT_AUTHOR_EMAIL'] + def test_git_review_T(self): self._run_git_review('-s') self._simple_change('test file modified', 'commit message for bug 456')