deal with non-unique UIDs

When we find multiple files added with the same UID in one patch, ignore
them and emit a warning. That's bad input data, and we don't want to use
them.

Allow multiple files with the same UID to be deleted in a patch to
support cleaning up existing situations where the add case was not
caught properly.

Related-Bug: #1688042
Change-Id: I37fee0660ff541677d26770818764f7de2a2d863
Signed-off-by: Doug Hellmann <doug@doughellmann.com>

reno/scanner.py

@@ -201,6 +201,23 @@ def _aggregate_changes(walk_entry, changes, notesdir):
                 # different commits.
                 for c in changes:
                     results.append((uid, diff_tree.CHANGE_MODIFY, c[1], sha))
+            elif types == set([diff_tree.CHANGE_DELETE]):
+                # There were multiple files in one commit using the
+                # same UID but different slugs. Treat them as
+                # different files and allow them to be deleted.
+                results.extend(
+                    (uid, diff_tree.CHANGE_DELETE, c[1], sha)
+                    for c in changes
+                )
+            elif types == set([diff_tree.CHANGE_ADD]):
+                # 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.
+                LOG.warning(
+                    ('%s: found several files in one commit (%s)'
+                     ' with the same UID, ignoring them: %s'),
+                    uid, sha, [c[1] for c in changes])
             else:
                 raise ValueError('Unrecognized changes: {!r}'.format(changes))
     return results

reno/tests/test_scanner.py

@@ -1794,6 +1794,29 @@ class AggregateChangesTest(Base):
             results,
         )
 
+    def test_add_multiple(self):
+        # Adding multiple files in one commit using the same UID but
+        # different slug causes the files to be ignored.
+        entry = mock.Mock()
+        n = self.get_note_num()
+        changes = []
+        for i in range(2):
+            name = 'prefix/add%d-%016x.yaml' % (i, n)
+            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',
+                    )
+                )
+            )
+        results = list(scanner._aggregate_changes(entry, changes, 'prefix'))
+        self.assertEqual([], results)
+
     def test_delete(self):
         entry = mock.Mock()
         n = self.get_note_num()
@@ -1816,6 +1839,31 @@ class AggregateChangesTest(Base):
             results,
         )
 
+    def test_delete_multiple(self):
+        # Delete multiple files in one commit using the same UID but
+        # different slug.
+        entry = mock.Mock()
+        n = self.get_note_num()
+        changes = []
+        expected = []
+        for i in range(2):
+            name = 'prefix/delete%d-%016x.yaml' % (i, n)
+            entry.commit.id = 'commit-id'
+            changes.append(
+                diff_tree.TreeChange(
+                    type=diff_tree.CHANGE_DELETE,
+                    old=objects.TreeEntry(
+                        path=name.encode('utf-8'),
+                        mode='0222',
+                        sha='not-a-hash',
+                    ),
+                    new=objects.TreeEntry(path=None, mode=None, sha=None),
+                )
+            )
+            expected.append(('%016x' % n, 'delete', name, 'commit-id'))
+        results = list(scanner._aggregate_changes(entry, changes, 'prefix'))
+        self.assertEqual(expected, results)
+
     def test_change(self):
         entry = mock.Mock()
         n = self.get_note_num()