At http://people.ubuntu.com/~robertc/baz2.0/pending/apply-inventory-delta
------------------------------------------------------------ revno: 4510 revision-id: [email protected] parent: [email protected] committer: Robert Collins <[email protected]> branch nick: apply-inventory-delta timestamp: Thu 2009-07-09 15:32:25 +1000 message: Parents used in a delta must be directories. === modified file 'bzrlib/dirstate.py' --- a/bzrlib/dirstate.py 2009-07-08 02:25:39 +0000 +++ b/bzrlib/dirstate.py 2009-07-09 05:32:25 +0000 @@ -1285,7 +1285,8 @@ removals = {} for old_path, new_path, file_id, inv_entry in sorted(delta, reverse=True): if (file_id in insertions) or (file_id in removals): - raise AssertionError("repeated file id in delta %r" % (file_id,)) + raise errors.InconsistentDelta(old_path or new_path, file_id, + "repeated file_id") if old_path is not None: old_path = old_path.encode('utf-8') removals[file_id] = old_path @@ -1399,6 +1400,9 @@ # At the same time, to reduce interface friction we convert the input # inventory entries to dirstate. root_only = ('', '') + # Accumulate parent references (path and id), to check for parentless + # items or items placed under files/links/tree-references. + parents = set() for old_path, new_path, file_id, inv_entry in delta: if inv_entry is not None and file_id != inv_entry.file_id: raise errors.InconsistentDelta(new_path, file_id, @@ -1406,6 +1410,9 @@ if old_path is None: adds.append((None, encode(new_path), file_id, inv_to_entry(inv_entry), True)) + # note the parent for validation + dirname, basename = osutils.split(new_path) + parents.add((dirname, inv_entry.parent_id)) elif new_path is None: deletes.append((encode(old_path), None, file_id, None, True)) elif (old_path, new_path) != root_only: @@ -1450,6 +1457,9 @@ (source_path, target_path, entry[0][2], None, False)) deletes.append( (encode(old_path), new_path, file_id, None, False)) + # note the parent for validation + dirname, basename = osutils.split(new_path) + parents.add((dirname, inv_entry.parent_id)) else: # changes to just the root should not require remove/insertion # of everything. @@ -1463,12 +1473,17 @@ self._update_basis_apply_adds(adds) # Apply in-situ changes. self._update_basis_apply_changes(changes) - except errors.BzrError: + # Validate parents + self._update_basis_check_parents(parents) + except errors.BzrError, e: + if 'integrity error' not in str(e): + raise # _get_entry raises BzrError when a request is inconsistent; we # want such errors to be shown as InconsistentDelta - and that # fits the behaviour we trigger. Partof this is driven by dirstate # only supporting deltas that turn the basis into a closer fit to # the active tree. + self._changes_aborted = True raise errors.InconsistentDeltaDelta(delta, "error from _get_entry.") self._dirblock_state = DirState.IN_MEMORY_MODIFIED @@ -1584,6 +1599,18 @@ # it is being resurrected here, so blank it out temporarily. self._dirblocks[block_index][1][entry_index][1][1] = null + def _update_basis_check_parents(self, parents): + """Check that parents required by the delta are all intact.""" + for dirname, file_id in parents: + # Get the entry - the ensures that file_id, dirname exists and has + # the right file id. + entry = self._get_entry(1, file_id, dirname) + # Parents of things must be directories + if entry[1][1][0] != 'd': + self._changes_aborted = True + raise errors.InconsistentDelta(dirname, file_id, + "This parent is not a directory.") + def _observed_sha1(self, entry, sha1, stat_value, _stat_to_minikind=_stat_to_minikind, _pack_stat=pack_stat): """Note the sha1 of a file. === modified file 'bzrlib/inventory.py' --- a/bzrlib/inventory.py 2009-07-08 02:25:39 +0000 +++ b/bzrlib/inventory.py 2009-07-09 05:32:25 +0000 @@ -1166,7 +1166,11 @@ replacement.revision = new_entry.revision replacement.children = children.pop(replacement.file_id, {}) new_entry = replacement - self.add(new_entry) + try: + self.add(new_entry) + except AttributeError: + raise errors.InconsistentDelta(new_path, new_entry.file_id, + "Parent is not a directory.") if len(children): # Get the parent id that was deleted parent_id, children = children.popitem() @@ -1629,6 +1633,8 @@ inventory_delta = _check_delta_unique_new_paths(inventory_delta) # Check for entries that don't match the fileid inventory_delta = _check_delta_ids_match_entry(inventory_delta) + # All changed entries need to have their parents be directories. + parents = set() for old_path, new_path, file_id, entry in inventory_delta: # file id changes if new_path == '': @@ -1649,6 +1655,7 @@ # Update caches. It's worth doing this whether # we're propagating the old caches or not. result._path_to_fileid_cache[new_path] = file_id + parents.add(entry.parent_id) if old_path is None: old_key = None else: @@ -1674,6 +1681,10 @@ result.id_to_entry.apply_delta(id_to_entry_delta) if parent_id_basename_delta: result.parent_id_basename_to_file_id.apply_delta(parent_id_basename_delta) + for parent in parents: + if result[parent].kind != 'directory': + raise errors.InconsistentDelta(result.id2path(parent), parent, + 'Not a directory, but given children') return result @classmethod === modified file 'bzrlib/tests/test_inv.py' --- a/bzrlib/tests/test_inv.py 2009-07-08 02:25:39 +0000 +++ b/bzrlib/tests/test_inv.py 2009-07-09 05:32:25 +0000 @@ -110,10 +110,11 @@ raise else: repo.commit_write_group() + # Set the basis state as the trees current state + tree._write_inventory(basis) # This reads basis from the repo and puts it into the tree's local # cache, if it has one. tree.set_parent_ids(['basis']) - inv = tree.inventory paths = {} parents = set() for old, new, id, entry in delta: @@ -123,9 +124,10 @@ parents.add(osutils.dirname(new)) parents = osutils.minimum_path_selection(parents) parents.discard('') - if parents: - # Put place holders in the tree to permit adding the other entries. - import pdb;pdb.set_trace() + # Put place holders in the tree to permit adding the other entries. + for parent in parents: + if not tree.path2id(parent): + import pdb;pdb.set_trace() if paths: # Many deltas may cause this mini-apply to fail, but we want to see what # the delta application code says, not the prep that we do to deal with @@ -292,6 +294,21 @@ self.assertRaises(errors.InconsistentDelta, self.apply_delta, self, inv, delta) + def test_parent_is_not_directory(self): + inv = self.get_empty_inventory() + file1 = inventory.InventoryFile('id1', 'path', inv.root.file_id) + file1.revision = 'result' + file1.text_size = 0 + file1.text_sha1 = "" + file2 = inventory.InventoryFile('id2', 'path2', 'id1') + file2.revision = 'result' + file2.text_size = 0 + file2.text_sha1 = "" + inv.add(file1) + delta = [(None, 'path/path2', 'id2', file2)] + self.assertRaises(errors.InconsistentDelta, self.apply_delta, self, + inv, delta) + class TestInventoryEntry(TestCase): -- bazaar-commits mailing list [email protected] https://lists.ubuntu.com/mailman/listinfo/bazaar-commits
