[MERGE] Inventory-free checkout
aaron at aaronbentley.com
Thu Jan 3 21:28:12 GMT 2008
-----BEGIN PGP SIGNED MESSAGE-----
John Arbash Meinel wrote:
> John Arbash Meinel has voted tweak.
> Status is now: Conditionally approved
> + def _apply_removals(self, removals):
> + for path in sorted(removals, reverse=True):
> + dirname, basename = osutils.split(path)
> + block_i, entry_i, d_present, f_present = \
> + self._get_block_entry_index(dirname, basename, 0)
> + entry = self._dirblocks[block_i][entry_i]
> + entry = ('a', '', 0, False, '')
> ^- I think apply_removals should be using "DirState._make_absent". As it
> handles when the removal is "permanent". (For example, renaming a file 2
> times, deleting a file which was just added, etc.)
Thanks for the pointer. The DirState code is still very unfamiliar.
> - active_tree_ids = set((f for f in self._tree.inventory if
> + active_tree_ids = set((f for f in
> self._tree.iter_all_file_ids() if
> f not in removed_tree_ids))
> ^- This seems better written as:
> active_tree_ids = set(self._tree.iter_all_file_ids())
> or even
> active_tree_ids = set(self._tree.iter_all_file_ids()) - removed_tree_ids
Okay, will update.
> I do wonder if "iter_all_file_ids()" is the best interface, given our
> recent concerns about 'iter' performance.
Oh, I wasn't aware there were big concerns. I would be happy with an
API that returned a set or list, too. But we do need a way to get the
> - if len(wt.inventory) > 1: # more than just a root
> + if len(list(wt)) > 1: # more than just a root
> ^- It seems like there should be a better way to find out that we have
>>=2 entries than building up a list of all 55k entries.
There are more efficient ways, but if there are >=2 entries, we'll error
out, so I thought efficiency wasn't very important, and I'd go for clarity.
> The biggest thing is using _make_absent to make sure all references are
> properly cleaned up, rather than just setting the field to
Will do. Do you also want me to change _iter_all_file_ids to something
else? Set? List? And if you want me to change the len(list(wt)) > 2,
I'll do that too.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
-----END PGP SIGNATURE-----
More information about the bazaar