From 1db83bde73b94d9aa11cd6265f4c69a5af233b87 Mon Sep 17 00:00:00 2001 From: Doug Hellmann Date: Wed, 1 Feb 2017 13:10:30 -0500 Subject: [PATCH] fix logic for deciding when to stop scanning a branch When scanning a given branch, we don't want to stop at the base of the current branch, we want to stop at the base of the *previous* branch on master. That pulls in the full history of the current branch from where the older branch diverged. Given this history: (master) | 1.0.0 | \ (stable/a) | \ | 1.0.1 2.0.0 | 2.0.1 | \ (stable/b) | \ | 2.0.2 2.1.0 The notes for stable/b branch should include versions 2.0.2, 2.0.1, and 2.0.0 (which is on master) but not 1.0.0 which is the previous release. Change-Id: If1feddadc1a8e24b163667cd84f5b9e098951c69 Signed-off-by: Doug Hellmann --- ...ption-branch-name-re-8ecfe93195b8824e.yaml | 15 ++ reno/config.py | 5 + reno/scanner.py | 77 ++++++++- reno/tests/test_scanner.py | 163 +++++++++++++----- 4 files changed, 205 insertions(+), 55 deletions(-) create mode 100644 releasenotes/notes/config-option-branch-name-re-8ecfe93195b8824e.yaml diff --git a/releasenotes/notes/config-option-branch-name-re-8ecfe93195b8824e.yaml b/releasenotes/notes/config-option-branch-name-re-8ecfe93195b8824e.yaml new file mode 100644 index 0000000..c3150ca --- /dev/null +++ b/releasenotes/notes/config-option-branch-name-re-8ecfe93195b8824e.yaml @@ -0,0 +1,15 @@ +--- +features: + - | + Add a configuration option ``branch_name_re`` to hold a regular expression + for choosing "interesting" branches when trying to automatically detect + how far back the scanner should look. The default is 'stable/.+', which + works for the OpenStack practice of creating branches named after the + stable series of releases. +fixes: + - | + Fixes the logic for determining how far back in history to look when + scanning a given branch. Reno now looks for the base of the "previous" + branch, as determined by looking at branches matching ``branch_name_re`` + in lexical order. This may not work if branches are created using + version numbers as their names. diff --git a/reno/config.py b/reno/config.py index 95563f1..47d44b1 100644 --- a/reno/config.py +++ b/reno/config.py @@ -135,6 +135,11 @@ class Config(object): 'pre_release_tag_re': ''' (?P\.\d+(?:[ab]|rc)+\d*)$ ''', + + # The pattern for names for branches that are relevant when + # scanning history to determine where to stop, to find the + # "base" of a branch. Other branches are ignored. + 'branch_name_re': 'stable/.+', } @classmethod diff --git a/reno/scanner.py b/reno/scanner.py index 7b12571..25ffa98 100644 --- a/reno/scanner.py +++ b/reno/scanner.py @@ -470,6 +470,10 @@ class Scanner(object): self.conf.pre_release_tag_re, flags=re.VERBOSE | re.UNICODE, ) + self.branch_name_re = re.compile( + self.conf.branch_name_re, + flags=re.VERBOSE | re.UNICODE, + ) def _get_ref(self, name): if name: @@ -694,9 +698,43 @@ class Scanner(object): "Return true if the file exists at the given commit." return bool(self.get_file_at_commit(filename, sha)) - @staticmethod - def _find_scan_stop_point(earliest_version, versions_by_date, - collapse_pre_releases): + def _get_earlier_branch(self, branch): + "Return the name of the branch created before the given branch." + # FIXME(dhellmann): Assumes branches come in order based on + # name. That may not be true for projects that branch based on + # version numbers instead of names. + if branch.startswith('origin/'): + branch = branch[7:] + LOG.debug('looking for the branch before %s', branch) + refs = self._repo.get_refs() + LOG.debug('refs %s', list(refs.keys())) + branch_names = set() + for r in refs.keys(): + name = None + r = r.decode('utf-8') + if r.startswith('refs/remotes/origin/'): + name = r[20:] + elif r.startswith('refs/heads/'): + name = r[11:] + if name and self.branch_name_re.search(name): + branch_names.add(name) + branch_names = list(sorted(branch_names)) + if branch not in branch_names: + LOG.debug('Could not find branch %r among %s', + branch, branch_names) + return None + LOG.debug('found branches %s', branch_names) + current = branch_names.index(branch) + if current == 0: + # This is the first branch. + LOG.debug('%s appears to be the first branch', branch) + return None + previous = branch_names[current - 1] + LOG.debug('found earlier branch %s', previous) + return previous + + def _find_scan_stop_point(self, earliest_version, versions_by_date, + collapse_pre_releases, branch): """Return the version to use to stop the scan. Use the list of versions_by_date to get the tag with a @@ -711,6 +749,7 @@ class Scanner(object): :param collapse_pre_releases: Boolean indicating whether we are collapsing pre-releases or not. If false, the next tag is used, regardless of its version. + :param branch: The name of the branch we are scanning. """ if not earliest_version: @@ -722,7 +761,17 @@ class Scanner(object): # The version we were given is not present, use a full # scan. return None - if not collapse_pre_releases: + # We need to look for the previous branch's root. + if branch and branch != 'master': + previous_branch = self._get_earlier_branch(branch) + if not previous_branch: + # This was the first branch, so scan the whole + # history. + return None + previous_base = self._get_branch_base(previous_branch) + return previous_base + is_pre_release = bool(self.pre_release_tag_re.search(earliest_version)) + if is_pre_release and not collapse_pre_releases: # We just take the next tag. return versions_by_date[idx] # We need to look for a different version. @@ -770,7 +819,8 @@ class Scanner(object): # If the user has told us where to stop, use that as the # default. scan_stop_tag = self._find_scan_stop_point( - earliest_version, versions_by_date, collapse_pre_releases) + earliest_version, versions_by_date, + collapse_pre_releases, branch) # If the user has not told us where to stop, try to work it # out for ourselves. If branch is set and is not "master", @@ -779,10 +829,15 @@ class Scanner(object): if (stop_at_branch_base and (not earliest_version) and branch and (branch != 'master')): LOG.debug('determining earliest_version from branch') - earliest_version = self._get_branch_base(branch) + branch_base = self._get_branch_base(branch) scan_stop_tag = self._find_scan_stop_point( - earliest_version, versions_by_date, - collapse_pre_releases) + branch_base, versions_by_date, + collapse_pre_releases, branch) + if not scan_stop_tag: + earliest_version = branch_base + else: + idx = versions_by_date.index(scan_stop_tag) + earliest_version = versions_by_date[idx - 1] if earliest_version and collapse_pre_releases: if self.pre_release_tag_re.search(earliest_version): # The earliest version won't actually be the pre-release @@ -932,6 +987,7 @@ class Scanner(object): # Combine pre-releases into the final release, if we are told to # and the final release exists. if collapse_pre_releases: + LOG.debug('collapsing pre-release versions into final releases') collapsing = files_and_tags files_and_tags = collections.OrderedDict() for ov in versions_by_date: @@ -958,12 +1014,16 @@ class Scanner(object): files_and_tags[canonical_ver] = [] files_and_tags[canonical_ver].extend(collapsing[ov]) + LOG.debug('files_and_tags %s', + {k: len(v) for k, v in files_and_tags.items()}) # Only return the parts of files_and_tags that actually have # filenames associated with the versions. + LOG.debug('trimming') trimmed = collections.OrderedDict() for ov in versions_by_date: if not files_and_tags.get(ov): continue + LOG.debug('keeping %s', ov) # 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 @@ -977,6 +1037,7 @@ class Scanner(object): # If we have been told to stop at a version, we can do that # now. if earliest_version and ov == earliest_version: + LOG.debug('stopping trimming at %s', earliest_version) break LOG.debug( diff --git a/reno/tests/test_scanner.py b/reno/tests/test_scanner.py index a419104..5fc8d71 100644 --- a/reno/tests/test_scanner.py +++ b/reno/tests/test_scanner.py @@ -1367,80 +1367,149 @@ class BranchTest(Base): self.assertEqual(head1, head2) -class ScanStopPointTest(Base): +class ScanStopPointPrereleaseVersionsTest(Base): def setUp(self): - super(ScanStopPointTest, self).setUp() + super(ScanStopPointPrereleaseVersionsTest, self).setUp() self.scanner = scanner.Scanner(self.c) + self._make_python_package() + self._add_notes_file('slug1') + self.repo.git('tag', '-s', '-m', 'first series', '1.0.0.0rc1') + self.repo.git('checkout', '-b', 'stable/a') + self._add_notes_file('slug2') + self._add_notes_file('slug3') + self.repo.git('tag', '-s', '-m', 'second tag', '1.0.0') + self.repo.git('checkout', 'master') + self._add_notes_file('slug4') + self._add_notes_file('slug5') + self.repo.git('tag', '-s', '-m', 'second series', '2.0.0.0b3') + self._add_notes_file('slug6') + self._add_notes_file('slug7') + self.repo.git('tag', '-s', '-m', 'second tag', '2.0.0.0rc1') + self.repo.git('checkout', '-b', 'stable/b') + self._add_notes_file('slug8') + self._add_notes_file('slug9') + self.repo.git('tag', '-s', '-m', 'third tag', '2.0.0') + self.repo.git('checkout', 'master') + + def test_beta_collapse(self): + self.assertEqual( + '1.0.0.0rc1', + self.scanner._find_scan_stop_point( + '2.0.0.0b3', ['2.0.0.0b3', '1.0.0.0rc1'], + True, 'master'), + ) + + def test_rc_collapse_master(self): + self.assertEqual( + '1.0.0.0rc1', + self.scanner._find_scan_stop_point( + '2.0.0.0rc1', ['2.0.0.0rc1', '2.0.0.0b3', '1.0.0.0rc1'], + True, 'master'), + ) + + def test_rc_collapse_branch(self): + self.assertEqual( + '1.0.0.0rc1', + self.scanner._find_scan_stop_point( + '2.0.0.0rc1', ['2.0.0.0rc1', '2.0.0.0b3', '1.0.0.0rc1'], + True, 'stable/b'), + ) + + def test_rc_no_collapse(self): + self.assertEqual( + '2.0.0.0b3', + self.scanner._find_scan_stop_point( + '2.0.0.0rc1', ['2.0.0.0rc1', '2.0.0.0b3', '1.0.0.0rc1'], + False, 'master'), + ) + + def test_stable_branch_with_collapse(self): + self.assertEqual( + '1.0.0.0rc1', + self.scanner._find_scan_stop_point( + '2.0.0', ['2.0.0', '2.0.0.0rc1', '2.0.0.0b3', '1.0.0.0rc1'], + True, 'stable/b'), + ) + + # def test_nova_newton(self): + # self.assertEqual( + # '13.0.0.0rc3', + # self.scanner._find_scan_stop_point( + # '14.0.0', + # [u'14.0.3', u'14.0.2', u'14.0.1', u'14.0.0.0rc2', + # u'14.0.0', u'14.0.0.0rc1', u'14.0.0.0b3', u'14.0.0.0b2', + # u'14.0.0.0b1', u'13.0.0.0rc3', u'13.0.0', u'13.0.0.0rc2', + # u'13.0.0.0rc1', u'13.0.0.0b3', u'13.0.0.0b2', u'13.0.0.0b1', + # u'12.0.0.0rc3', u'12.0.0', u'12.0.0.0rc2', u'12.0.0.0rc1', + # u'12.0.0.0b3', u'12.0.0.0b2', u'12.0.0.0b1', u'12.0.0a0', + # u'2015.1.0rc3', u'2015.1.0', u'2015.1.0rc2', u'2015.1.0rc1', + # u'2015.1.0b3', u'2015.1.0b2', u'2015.1.0b1', u'2014.2.rc2', + # u'2014.2', u'2014.2.rc1', u'2014.2.b3', u'2014.2.b2', + # u'2014.2.b1', u'2014.1.rc1', u'2014.1.b3', u'2014.1.b2', + # u'2014.1.b1', u'2013.2.rc1', u'2013.2.b3', u'2013.1.rc1', + # u'folsom-2', u'folsom-1', u'essex-1', u'diablo-2', + # u'diablo-1', u'2011.2', u'2011.2rc1', u'2011.2gamma1', + # u'2011.1rc1', u'0.9.0'], + # True), + # ) + + +class ScanStopPointRegularVersionsTest(Base): + + def setUp(self): + super(ScanStopPointRegularVersionsTest, self).setUp() + self.scanner = scanner.Scanner(self.c) + self._make_python_package() + self._add_notes_file('slug1') + self.repo.git('tag', '-s', '-m', 'first series', '1.0.0') + self.repo.git('checkout', '-b', 'stable/a') + self._add_notes_file('slug2') + self._add_notes_file('slug3') + self.repo.git('tag', '-s', '-m', 'second tag', '1.0.1') + self.repo.git('checkout', 'master') + self._add_notes_file('slug4') + self._add_notes_file('slug5') + self.repo.git('tag', '-s', '-m', 'second series', '2.0.0') + self._add_notes_file('slug6') + self._add_notes_file('slug7') + self.repo.git('tag', '-s', '-m', 'second tag', '2.0.1') + self.repo.git('checkout', '-b', 'stable/b') + self._add_notes_file('slug8') + self._add_notes_file('slug9') + self.repo.git('tag', '-s', '-m', 'third tag', '2.0.2') + self.repo.git('checkout', 'master') def test_invalid_earliest_version(self): self.assertIsNone( self.scanner._find_scan_stop_point( - 'not.a.numeric.version', [], True), + 'not.a.numeric.version', [], True, 'stable/b'), ) def test_none(self): self.assertIsNone( self.scanner._find_scan_stop_point( - None, [], True), + None, [], True, 'stable/b'), ) def test_unknown_version(self): self.assertIsNone( self.scanner._find_scan_stop_point( - '1.0.0', [], True), + '2.0.2', [], True, 'stable/b'), ) def test_only_version(self): self.assertIsNone( self.scanner._find_scan_stop_point( - '1.0.0', ['1.0.0'], True), + '2.0.2', ['1.0.0'], True, 'stable/b'), ) - def test_beta_collapse(self): + def test_find_prior_branch(self): self.assertEqual( '1.0.0', self.scanner._find_scan_stop_point( - '2.0.0.0b1', ['2.0.0', '2.0.0.0rc1', '2.0.0.0b1', '1.0.0'], - True), - ) - - def test_rc_collapse(self): - self.assertEqual( - '1.0.0', - self.scanner._find_scan_stop_point( - '2.0.0.0rc1', ['2.0.0', '2.0.0.0rc1', '2.0.0.0b1', '1.0.0'], - True), - ) - - def test_rc_no_collapse(self): - self.assertEqual( - '2.0.0.0b1', - self.scanner._find_scan_stop_point( - '2.0.0.0rc1', ['2.0.0', '2.0.0.0rc1', '2.0.0.0b1', '1.0.0'], - False), - ) - - def test_nova_newton(self): - self.assertEqual( - '13.0.0.0rc3', - self.scanner._find_scan_stop_point( - '14.0.0', - [u'14.0.3', u'14.0.2', u'14.0.1', u'14.0.0.0rc2', - u'14.0.0', u'14.0.0.0rc1', u'14.0.0.0b3', u'14.0.0.0b2', - u'14.0.0.0b1', u'13.0.0.0rc3', u'13.0.0', u'13.0.0.0rc2', - u'13.0.0.0rc1', u'13.0.0.0b3', u'13.0.0.0b2', u'13.0.0.0b1', - u'12.0.0.0rc3', u'12.0.0', u'12.0.0.0rc2', u'12.0.0.0rc1', - u'12.0.0.0b3', u'12.0.0.0b2', u'12.0.0.0b1', u'12.0.0a0', - u'2015.1.0rc3', u'2015.1.0', u'2015.1.0rc2', u'2015.1.0rc1', - u'2015.1.0b3', u'2015.1.0b2', u'2015.1.0b1', u'2014.2.rc2', - u'2014.2', u'2014.2.rc1', u'2014.2.b3', u'2014.2.b2', - u'2014.2.b1', u'2014.1.rc1', u'2014.1.b3', u'2014.1.b2', - u'2014.1.b1', u'2013.2.rc1', u'2013.2.b3', u'2013.1.rc1', - u'folsom-2', u'folsom-1', u'essex-1', u'diablo-2', - u'diablo-1', u'2011.2', u'2011.2rc1', u'2011.2gamma1', - u'2011.1rc1', u'0.9.0'], - True), + '2.0.2', ['2.0.2', '2.0.1', '2.0.0', '1.0.0'], + True, 'stable/b'), )