[MERGE] Inventory.iter_just_entries()

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ian Clatworthy wrote:
> John Arbash Meinel wrote:
> 
>> My main concern is that this is a very arbitrary order, which isn't
>> particularly stable. Both 'iter_entries()' and 'iter_entries_by_dir()'
>> have a very defined order based on the path.
> 
> Right. The docstring makes it clear that the order is whatever the
> implementation chooses as being most efficient. One can always either
> sort or fall back to using iter_entries() or iter_entries_by_dir().
> There's also an entries() method but it seems less useful given it
> explicitly leaves out the root.
> 
>> Do you have a specific use case for this that we can evaluate? I know
>> you mentioned it earlier. You can just give me the name of that thread
>> if it is easier.
> 
> In brisbane-core, examples include:
> 
> * pack_repo.find_text_key_references()
> * pack_repo.fileids_altered_by_revision_ids()

These are essentially the same function, and should not be walking
inventories directly. They need to be updated to walk from multiple
roots at the same time.

fileids_altered_by... especially could be done with
chk_map.iter_interesting_nodes().

It is a bit harder to do that for find_text_key_references() because not
only are we looking for (file_id, revision_id), we also want to know if
that exact value is present in the inventory of revision_id.

So maybe we could use iter_just_entries as a stop-gap for
find_text_key_references (especially since that is really only done
during reconcile).

> 
> In bzr.dev, Repository._install_revisions() is another example.
> 

I don't see an "_install_revisions()" function in my copy of bzr.dev. It
certainly was an old code path, but I'm pretty sure it has been replaced
 entirely by InterDifferingSerializer.

> I strongly agree that iter_just_entries() shouldn't be used without
> careful consideration in each case. That's part of the reason why
> I didn't make changes throughout the codebase to begin using it
> in this patch. I do feel however that generating data that isn't
> needed is generally wasteful so we need some way of avoiding that in
> the inventory API.
> 
> Ian C.

I'm fine with it as a stop-gap, but I think the code that is using it
should actually be written differently. As there are limits to time and
energy, we can start with this until we get to the point of changing things.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkm/pAcACgkQJdeBCYSNAANgcQCgg1BjFDuofhBhwHORhPSLu4+i
waMAoK5fHCJQ0oEGYHN/5pJdM3iR1HD3
=DNBe
-----END PGP SIGNATURE-----