fix notes appearing in releases older than they should
Use topological sort instead of date sort so that release notes appear in the release where they are merged, rather than the one closest to the same commit time, which might be much older. In order to support listing tags on versions of git with different output formats, we also limit the tags we care about to things that look like version numbers (some sequence of digits, dots, and maybe "a" or "b" for alpha & beta). Change-Id: I95278eab25f331262a4c9eeac2260ca7290b5421 Closes-Bug: #1522153
This commit is contained in:
Doug Hellmann
parent
ddec6c5bc7
commit
4b45743e49
|
|
@ -0,0 +1,6 @@
|
|||
---
|
||||
fixes:
|
||||
- Fixes `bug 1522153
|
||||
<https://bugs.launchpad.net/reno/+bug/1522153>`__ so that notes
|
||||
added in commits that are merged after tags are associated with
|
||||
the correct version.
|
||||
119
reno/scanner.py
119
reno/scanner.py
|
|
@ -79,7 +79,70 @@ def _get_unique_id(filename):
|
|||
return uniqueid
|
||||
|
||||
|
||||
# TODO(dhellmann): Add branch arg?
|
||||
# The git log output from _get_tags_on_branch() looks like this sample
|
||||
# from the openstack/nova repository for git 1.9.1:
|
||||
#
|
||||
# git log --simplify-by-decoration --pretty="%d"
|
||||
# (HEAD, origin/master, origin/HEAD, gerrit/master, master)
|
||||
# (apu/master)
|
||||
# (tag: 13.0.0.0b1)
|
||||
# (tag: 12.0.0.0rc3, tag: 12.0.0)
|
||||
#
|
||||
# And this for git 1.7.1 (RHEL):
|
||||
#
|
||||
# $ git log --simplify-by-decoration --pretty="%d"
|
||||
# (HEAD, origin/master, origin/HEAD, master)
|
||||
# (tag: 13.0.0.0b1)
|
||||
# (tag: 12.0.0.0rc3, tag: 12.0.0)
|
||||
# (tag: 12.0.0.0rc2)
|
||||
# (tag: 2015.1.0rc3, tag: 2015.1.0)
|
||||
# ...
|
||||
# (tag: folsom-2)
|
||||
# (tag: folsom-1)
|
||||
# (essex-1)
|
||||
# (diablo-2)
|
||||
# (diablo-1)
|
||||
# (2011.2)
|
||||
#
|
||||
# The difference in the tags with "tag:" and without appears to be
|
||||
# caused by some being annotated and others not.
|
||||
#
|
||||
# So we might have multiple tags on a given commit, and we might have
|
||||
# lines that have no tags or are completely blank, and we might have
|
||||
# "tag:" or not. This pattern is used to find the tag entries on each
|
||||
# line, ignoring tags that don't look like version numbers.
|
||||
TAG_RE = re.compile('(?:[(]|tag: )([\d.ab]+)[,)]')
|
||||
|
||||
|
||||
def _get_version_tags_on_branch(reporoot, branch):
|
||||
"""Return tags from the branch, in date order.
|
||||
|
||||
Need to get the list of tags in the right order, because the topo
|
||||
search breaks the date ordering. Use git to ask for the tags in
|
||||
order, rather than trying to sort them, because many repositories
|
||||
have "non-standard" tags or have renumbered projects (from
|
||||
date-based to SemVer), for which sorting would require complex
|
||||
logic.
|
||||
|
||||
"""
|
||||
tags = []
|
||||
tag_cmd = [
|
||||
'git', 'log',
|
||||
'--simplify-by-decoration',
|
||||
'--pretty="%d"',
|
||||
]
|
||||
if branch:
|
||||
tag_cmd.append(branch)
|
||||
LOG.debug('running %s' % ' '.join(tag_cmd))
|
||||
tag_results = utils.check_output(tag_cmd, cwd=reporoot)
|
||||
LOG.debug(tag_results)
|
||||
for line in tag_results.splitlines():
|
||||
LOG.debug('line %r' % line)
|
||||
for match in TAG_RE.findall(line):
|
||||
tags.append(match)
|
||||
return tags
|
||||
|
||||
|
||||
def get_notes_by_version(reporoot, notesdir, branch=None):
|
||||
"""Return an OrderedDict mapping versions to lists of notes files.
|
||||
|
||||
|
|
@ -91,13 +154,26 @@ def get_notes_by_version(reporoot, notesdir, branch=None):
|
|||
|
||||
LOG.debug('scanning %s/%s (branch=%s)' % (reporoot, notesdir, branch))
|
||||
|
||||
# Determine all of the tags known on the branch, in their date
|
||||
# order. We scan the commit history in topological order to ensure
|
||||
# we have the commits in the right version, so we might encounter
|
||||
# the tags in a different order during that phase.
|
||||
versions_by_date = _get_version_tags_on_branch(reporoot, branch)
|
||||
LOG.debug('versions by date %r' % (versions_by_date,))
|
||||
versions = []
|
||||
earliest_seen = collections.OrderedDict()
|
||||
|
||||
# Determine the current version, which might be an unreleased or dev
|
||||
# version.
|
||||
# Determine the current version, which might be an unreleased or
|
||||
# dev version if there are unreleased commits at the head of the
|
||||
# branch in question. Since the version may not already be known,
|
||||
# make sure it is in the list of versions by date. And since it is
|
||||
# the most recent version, go ahead and insert it at the front of
|
||||
# the list.
|
||||
current_version = _get_current_version(reporoot, branch)
|
||||
LOG.debug('current repository version: %s' % current_version)
|
||||
if current_version not in versions_by_date:
|
||||
LOG.debug('adding %s to versions by date' % current_version)
|
||||
versions_by_date.insert(0, current_version)
|
||||
|
||||
# Remember the most current filename for each id, to allow for
|
||||
# renames.
|
||||
|
|
@ -105,7 +181,12 @@ def get_notes_by_version(reporoot, notesdir, branch=None):
|
|||
|
||||
# FIXME(dhellmann): This might need to be more line-oriented for
|
||||
# longer histories.
|
||||
log_cmd = ['git', 'log', '--pretty=%x00%H %d', '--name-only']
|
||||
log_cmd = [
|
||||
'git', 'log',
|
||||
'--topo-order', # force traversal order rather than date order
|
||||
'--pretty=%x00%H %d', # output contents in parsable format
|
||||
'--name-only' # only include the names of the files in the patch
|
||||
]
|
||||
if branch is not None:
|
||||
log_cmd.append(branch)
|
||||
LOG.debug('running %s' % ' '.join(log_cmd))
|
||||
|
|
@ -188,20 +269,26 @@ def get_notes_by_version(reporoot, notesdir, branch=None):
|
|||
except KeyError:
|
||||
# Unable to find the file again, skip it to avoid breaking
|
||||
# the build.
|
||||
print(
|
||||
'[reno] unable to find file associated '
|
||||
'with unique id %r, skipping' %
|
||||
uniqueid,
|
||||
file=sys.stderr,
|
||||
)
|
||||
msg = ('[reno] unable to find file associated '
|
||||
'with unique id %r, skipping') % uniqueid
|
||||
LOG.debug(msg)
|
||||
print(msg, file=sys.stderr)
|
||||
|
||||
# Only return the parts of files_and_tags that actually have
|
||||
# filenames associated with the versions.
|
||||
trimmed = {
|
||||
k: list(reversed(v))
|
||||
for (k, v)
|
||||
in files_and_tags.items()
|
||||
if v
|
||||
}
|
||||
trimmed = collections.OrderedDict()
|
||||
for ov in versions_by_date:
|
||||
if not files_and_tags.get(ov):
|
||||
continue
|
||||
# Sort the notes associated with the version so they are in a
|
||||
# deterministic order, to avoid having the same data result in
|
||||
# different output depending on random factors. Earlier
|
||||
# versions of the scanner assumed the notes were recorded in
|
||||
# chronological order based on the commit date, but with the
|
||||
# change to use topological sorting that is no longer
|
||||
# necessarily true. We want the notes to always show up in the
|
||||
# same order, but it doesn't really matter what order that is,
|
||||
# so just sort based on the unique id.
|
||||
trimmed[ov] = sorted(files_and_tags[ov])
|
||||
|
||||
return trimmed
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ import logging
|
|||
import os.path
|
||||
import re
|
||||
import subprocess
|
||||
import textwrap
|
||||
|
||||
from reno import create
|
||||
from reno import scanner
|
||||
|
|
@ -24,6 +25,7 @@ from reno.tests import base
|
|||
from reno import utils
|
||||
|
||||
import fixtures
|
||||
import mock
|
||||
from testtools.content import text_content
|
||||
|
||||
|
||||
|
|
@ -458,6 +460,145 @@ class BasicTest(Base):
|
|||
)
|
||||
|
||||
|
||||
class MergeCommitTest(Base):
|
||||
|
||||
def test_1(self):
|
||||
# Create changes on master and in the branch
|
||||
# in order so the history is "normal"
|
||||
n1 = self._add_notes_file()
|
||||
self._run_git('tag', '-s', '-m', 'first tag', '1.0.0')
|
||||
self._run_git('checkout', '-b', 'test_merge_commit')
|
||||
n2 = self._add_notes_file()
|
||||
self._run_git('checkout', 'master')
|
||||
self._add_other_file('ignore-1.txt')
|
||||
self._run_git('merge', '--no-ff', 'test_merge_commit')
|
||||
self._add_other_file('ignore-2.txt')
|
||||
self._run_git('tag', '-s', '-m', 'second tag', '2.0.0')
|
||||
raw_results = scanner.get_notes_by_version(
|
||||
self.reporoot,
|
||||
'releasenotes/notes',
|
||||
)
|
||||
results = {
|
||||
k: [f for (f, n) in v]
|
||||
for (k, v) in raw_results.items()
|
||||
}
|
||||
self.assertEqual(
|
||||
{'1.0.0': [n1],
|
||||
'2.0.0': [n2]},
|
||||
results,
|
||||
)
|
||||
self.assertEqual(
|
||||
['2.0.0', '1.0.0'],
|
||||
list(raw_results.keys()),
|
||||
)
|
||||
|
||||
def test_2(self):
|
||||
# Create changes on the branch before the tag into which it is
|
||||
# actually merged.
|
||||
self._add_other_file('ignore-0.txt')
|
||||
self._run_git('checkout', '-b', 'test_merge_commit')
|
||||
n1 = self._add_notes_file()
|
||||
self._run_git('checkout', 'master')
|
||||
n2 = self._add_notes_file()
|
||||
self._run_git('tag', '-s', '-m', 'first tag', '1.0.0')
|
||||
self._add_other_file('ignore-1.txt')
|
||||
self._run_git('merge', '--no-ff', 'test_merge_commit')
|
||||
self._add_other_file('ignore-2.txt')
|
||||
self._run_git('tag', '-s', '-m', 'second tag', '2.0.0')
|
||||
raw_results = scanner.get_notes_by_version(
|
||||
self.reporoot,
|
||||
'releasenotes/notes',
|
||||
)
|
||||
results = {
|
||||
k: [f for (f, n) in v]
|
||||
for (k, v) in raw_results.items()
|
||||
}
|
||||
self.assertEqual(
|
||||
{'1.0.0': [n2],
|
||||
'2.0.0': [n1]},
|
||||
results,
|
||||
)
|
||||
self.assertEqual(
|
||||
['2.0.0', '1.0.0'],
|
||||
list(raw_results.keys()),
|
||||
)
|
||||
|
||||
def test_3(self):
|
||||
# Create changes on the branch before the tag into which it is
|
||||
# actually merged, with another tag in between the time of the
|
||||
# commit and the time of the merge. This should reflect the
|
||||
# order of events described in bug #1522153.
|
||||
self._add_other_file('ignore-0.txt')
|
||||
self._run_git('checkout', '-b', 'test_merge_commit')
|
||||
n1 = self._add_notes_file()
|
||||
self._run_git('checkout', 'master')
|
||||
n2 = self._add_notes_file()
|
||||
self._run_git('tag', '-s', '-m', 'first tag', '1.0.0')
|
||||
self._add_other_file('ignore-1.txt')
|
||||
self._run_git('tag', '-s', '-m', 'second tag', '1.1.0')
|
||||
self._run_git('merge', '--no-ff', 'test_merge_commit')
|
||||
self._add_other_file('ignore-2.txt')
|
||||
self._run_git('tag', '-s', '-m', 'third tag', '2.0.0')
|
||||
self._add_other_file('ignore-3.txt')
|
||||
raw_results = scanner.get_notes_by_version(
|
||||
self.reporoot,
|
||||
'releasenotes/notes',
|
||||
)
|
||||
results = {
|
||||
k: [f for (f, n) in v]
|
||||
for (k, v) in raw_results.items()
|
||||
}
|
||||
# Since the 1.1.0 tag has no notes files, it does not appear
|
||||
# in the output. It's only there to trigger the bug as it was
|
||||
# originally reported.
|
||||
self.assertEqual(
|
||||
{'1.0.0': [n2],
|
||||
'2.0.0': [n1]},
|
||||
results,
|
||||
)
|
||||
self.assertEqual(
|
||||
['2.0.0', '1.0.0'],
|
||||
list(raw_results.keys()),
|
||||
)
|
||||
|
||||
def test_4(self):
|
||||
# Create changes on the branch before the tag into which it is
|
||||
# actually merged, with another tag in between the time of the
|
||||
# commit and the time of the merge. This should reflect the
|
||||
# order of events described in bug #1522153.
|
||||
self._add_other_file('ignore-0.txt')
|
||||
self._run_git('checkout', '-b', 'test_merge_commit')
|
||||
n1 = self._add_notes_file()
|
||||
self._run_git('checkout', 'master')
|
||||
n2 = self._add_notes_file()
|
||||
self._run_git('tag', '-s', '-m', 'first tag', '1.0.0')
|
||||
self._add_other_file('ignore-1.txt')
|
||||
n3 = self._add_notes_file()
|
||||
self._run_git('tag', '-s', '-m', 'second tag', '1.1.0')
|
||||
self._run_git('merge', '--no-ff', 'test_merge_commit')
|
||||
self._add_other_file('ignore-2.txt')
|
||||
self._run_git('tag', '-s', '-m', 'third tag', '2.0.0')
|
||||
self._add_other_file('ignore-3.txt')
|
||||
raw_results = scanner.get_notes_by_version(
|
||||
self.reporoot,
|
||||
'releasenotes/notes',
|
||||
)
|
||||
results = {
|
||||
k: [f for (f, n) in v]
|
||||
for (k, v) in raw_results.items()
|
||||
}
|
||||
self.assertEqual(
|
||||
{'1.0.0': [n2],
|
||||
'1.1.0': [n3],
|
||||
'2.0.0': [n1]},
|
||||
results,
|
||||
)
|
||||
self.assertEqual(
|
||||
['2.0.0', '1.1.0', '1.0.0'],
|
||||
list(raw_results.keys()),
|
||||
)
|
||||
|
||||
|
||||
class UniqueIdTest(Base):
|
||||
|
||||
def test_legacy(self):
|
||||
|
|
@ -533,3 +674,165 @@ class BranchTest(Base):
|
|||
},
|
||||
results,
|
||||
)
|
||||
|
||||
|
||||
class GetTagsParseTest(base.TestCase):
|
||||
|
||||
EXPECTED = [
|
||||
'2.0.0',
|
||||
'1.8.1',
|
||||
'1.8.0',
|
||||
'1.7.1',
|
||||
'1.7.0',
|
||||
'1.6.0',
|
||||
'1.5.0',
|
||||
'1.4.0',
|
||||
'1.3.0',
|
||||
'1.2.0',
|
||||
'1.1.0',
|
||||
'1.0.0',
|
||||
'0.11.2',
|
||||
'0.11.1',
|
||||
'0.11.0',
|
||||
'0.10.1',
|
||||
'0.10.0',
|
||||
'0.9.0',
|
||||
'0.8.0',
|
||||
'0.7.1',
|
||||
'0.7.0',
|
||||
'0.6.0',
|
||||
'0.5.1',
|
||||
'0.5.0',
|
||||
'0.4.2',
|
||||
'0.4.1',
|
||||
'0.4.0',
|
||||
'0.3.2',
|
||||
'0.3.1',
|
||||
'0.3.0',
|
||||
'0.2.5',
|
||||
'0.2.4',
|
||||
'0.2.3',
|
||||
'0.2.2',
|
||||
'0.2.1',
|
||||
'0.2.0',
|
||||
'0.1.3',
|
||||
'0.1.2',
|
||||
'0.1.1',
|
||||
'0.1.0',
|
||||
]
|
||||
|
||||
def test_keystoneclient_ubuntu_1_9_1(self):
|
||||
# git 1.9.1 as it produces output on ubuntu for python-keystoneclient
|
||||
# git log --simplify-by-decoration --pretty="%d"
|
||||
tag_list_output = textwrap.dedent("""
|
||||
(HEAD, origin/master, origin/HEAD, gerrit/master, master)
|
||||
(apu/master)
|
||||
(tag: 2.0.0)
|
||||
(tag: 1.8.1)
|
||||
(tag: 1.8.0)
|
||||
(tag: 1.7.1)
|
||||
(tag: 1.7.0)
|
||||
(tag: 1.6.0)
|
||||
(tag: 1.5.0)
|
||||
(tag: 1.4.0)
|
||||
(uncap-requirements)
|
||||
(tag: 1.3.0)
|
||||
(tag: 1.2.0)
|
||||
(tag: 1.1.0)
|
||||
(tag: 1.0.0)
|
||||
(tag: 0.11.2)
|
||||
(tag: 0.11.1)
|
||||
(tag: 0.11.0)
|
||||
(tag: 0.10.1)
|
||||
(tag: 0.10.0)
|
||||
(tag: 0.9.0)
|
||||
(tag: 0.8.0)
|
||||
(tag: 0.7.1)
|
||||
(tag: 0.7.0)
|
||||
(tag: 0.6.0)
|
||||
(tag: 0.5.1)
|
||||
(tag: 0.5.0)
|
||||
(tag: 0.4.2)
|
||||
(tag: 0.4.1)
|
||||
(tag: 0.4.0)
|
||||
(tag: 0.3.2)
|
||||
(tag: 0.3.1)
|
||||
(tag: 0.3.0)
|
||||
(tag: 0.2.5)
|
||||
(tag: 0.2.4)
|
||||
(tag: 0.2.3)
|
||||
(tag: 0.2.2)
|
||||
(tag: 0.2.1)
|
||||
(tag: 0.2.0)
|
||||
|
||||
(origin/feature/keystone-v3, gerrit/feature/keystone-v3)
|
||||
(tag: 0.1.3)
|
||||
(tag: 0.1.2)
|
||||
(tag: 0.1.1)
|
||||
(tag: 0.1.0)
|
||||
(tag: folsom-1)
|
||||
(tag: essex-rc1)
|
||||
(tag: essex-4)
|
||||
(tag: essex-3)
|
||||
""")
|
||||
with mock.patch('reno.utils.check_output') as co:
|
||||
co.return_value = tag_list_output
|
||||
actual = scanner._get_version_tags_on_branch('reporoot',
|
||||
branch=None)
|
||||
self.assertEqual(self.EXPECTED, actual)
|
||||
|
||||
def test_keystoneclient_rhel_1_7_1(self):
|
||||
# git 1.7.1 as it produces output on RHEL 6 for python-keystoneclient
|
||||
# git log --simplify-by-decoration --pretty="%d"
|
||||
tag_list_output = textwrap.dedent("""
|
||||
(HEAD, origin/master, origin/HEAD, master)
|
||||
(tag: 2.0.0)
|
||||
(tag: 1.8.1)
|
||||
(tag: 1.8.0)
|
||||
(tag: 1.7.1)
|
||||
(tag: 1.7.0)
|
||||
(tag: 1.6.0)
|
||||
(tag: 1.5.0)
|
||||
(tag: 1.4.0)
|
||||
(tag: 1.3.0)
|
||||
(tag: 1.2.0)
|
||||
(tag: 1.1.0)
|
||||
(tag: 1.0.0)
|
||||
(tag: 0.11.2)
|
||||
(tag: 0.11.1)
|
||||
(tag: 0.11.0)
|
||||
(tag: 0.10.1)
|
||||
(tag: 0.10.0)
|
||||
(tag: 0.9.0)
|
||||
(tag: 0.8.0)
|
||||
(tag: 0.7.1)
|
||||
(tag: 0.7.0)
|
||||
(tag: 0.6.0)
|
||||
(tag: 0.5.1)
|
||||
(tag: 0.5.0)
|
||||
(tag: 0.4.2)
|
||||
(tag: 0.4.1)
|
||||
(tag: 0.4.0)
|
||||
(tag: 0.3.2)
|
||||
(tag: 0.3.1)
|
||||
(tag: 0.3.0)
|
||||
(tag: 0.2.5)
|
||||
(tag: 0.2.4)
|
||||
(tag: 0.2.3)
|
||||
(tag: 0.2.2)
|
||||
(tag: 0.2.1)
|
||||
(tag: 0.2.0)
|
||||
(tag: 0.1.3)
|
||||
(0.1.2)
|
||||
(tag: 0.1.1)
|
||||
(0.1.0)
|
||||
(tag: folsom-1)
|
||||
(tag: essex-rc1)
|
||||
(essex-4)
|
||||
(essex-3)
|
||||
""")
|
||||
with mock.patch('reno.utils.check_output') as co:
|
||||
co.return_value = tag_list_output
|
||||
actual = scanner._get_version_tags_on_branch('reporoot',
|
||||
branch=None)
|
||||
self.assertEqual(self.EXPECTED, actual)
|
||||
|
|
|
|||
|
|
@ -4,6 +4,8 @@
|
|||
|
||||
hacking<0.11,>=0.10.0
|
||||
|
||||
mock>=1.2
|
||||
|
||||
coverage>=3.6
|
||||
discover
|
||||
python-subunit>=0.0.18
|
||||
|
|
|
|||
Loading…
Reference in New Issue