diff --git a/releasenotes/notes/avoid-clashing-uids-e84ffe8132ce996d.yaml b/releasenotes/notes/avoid-clashing-uids-e84ffe8132ce996d.yaml new file mode 100644 index 0000000..4ea18c6 --- /dev/null +++ b/releasenotes/notes/avoid-clashing-uids-e84ffe8132ce996d.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fix a problem caused by failing to process multiple files with the + same UID portion of the filename. Ignore existing cases as long as + there is a corrective patch to remove them. Prevent new cases from + being introduced. See https://bugs.launchpad.net/reno/+bug/1688042 + for details. \ No newline at end of file diff --git a/reno/scanner.py b/reno/scanner.py index 95ceff6..e64fb97 100644 --- a/reno/scanner.py +++ b/reno/scanner.py @@ -150,6 +150,11 @@ class _ChangeAggregator(object): _delete_op = set([diff_tree.CHANGE_DELETE]) _add_op = set([diff_tree.CHANGE_ADD]) + def __init__(self): + # Track UIDs that had a duplication issue but have been + # deleted so we know not to throw an error for them. + self._deleted_bad_uids = set() + def aggregate_changes(self, walk_entry, changes): sha = walk_entry.commit.id by_uid = collections.defaultdict(list) @@ -216,15 +221,19 @@ class _ChangeAggregator(object): (uid, diff_tree.CHANGE_DELETE, c[1], sha) for c in changes ) + self._deleted_bad_uids.add(uid) elif types == self._add_op: # There were multiple files in one commit using the # same UID but different slugs. Warn the user about # this case and then ignore the files. We allow delete # (see above) to ensure they can be cleaned up. msg = ('%s: found several files in one commit (%s)' - ' with the same UID, ignoring them: %s' % + ' with the same UID: %s' % (uid, sha, [c[1] for c in changes])) - LOG.warning(msg) + if uid not in self._deleted_bad_uids: + raise ValueError(msg) + else: + LOG.warning(msg) else: raise ValueError('Unrecognized changes: {!r}'.format( changes)) diff --git a/reno/tests/test_scanner.py b/reno/tests/test_scanner.py index 7e1a750..1e6b9bc 100644 --- a/reno/tests/test_scanner.py +++ b/reno/tests/test_scanner.py @@ -1798,14 +1798,16 @@ class AggregateChangesTest(Base): results, ) - def test_add_multiple(self): + def test_add_multiple_after_delete(self): # Adding multiple files in one commit using the same UID but - # different slug causes the files to be ignored. + # different slug after we have seen a delete for the same UID + # causes the files to be ignored. entry = mock.Mock() n = self.get_note_num() + uid = '%016x' % n changes = [] for i in range(2): - name = 'prefix/add%d-%016x.yaml' % (i, n) + name = 'prefix/add%d-%s.yaml' % (i, uid) entry.commit.id = 'commit-id' changes.append( diff_tree.TreeChange( @@ -1818,9 +1820,49 @@ class AggregateChangesTest(Base): ) ) ) + # Set up the aggregator as though it had already seen a delete + # operation. Since the scan happens in reverse chronological + # order, the delete would have happened after the add, and we + # can ignore the files because the error has been corrected in + # a later patch. + self.aggregator._deleted_bad_uids.add(uid) results = list(self.aggregator.aggregate_changes(entry, changes)) self.assertEqual([], results) + def test_add_multiple_without_delete(self): + # Adding multiple files in one commit using the same UID but + # different slug without a delete operation causes an + # exception. + entry = mock.Mock() + n = self.get_note_num() + uid = '%016x' % n + changes = [] + for i in range(2): + name = 'prefix/add%d-%s.yaml' % (i, uid) + entry.commit.id = 'commit-id' + changes.append( + diff_tree.TreeChange( + type=diff_tree.CHANGE_ADD, + old=objects.TreeEntry(path=None, mode=None, sha=None), + new=objects.TreeEntry( + path=name.encode('utf-8'), + mode='0222', + sha='not-a-hash', + ) + ) + ) + + # aggregate_changes() is a generator, so we have to wrap it in + # list() to process the data, so we need a little temporary + # function to do that and pass to assertRaises(). + def get_results(): + return list(self.aggregator.aggregate_changes(entry, changes)) + + self.assertRaises( + ValueError, + get_results, + ) + def test_delete(self): entry = mock.Mock() n = self.get_note_num()