[MERGE] Update to bzr.dev.

[John Arbash Meinel]

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

Martin Pool wrote:
| I haven't finished everything by any means, but here are some partial comments.
|
| Great ratio of addition to deletion, and the new code is pretty clear and clean.
|
| I generally indexing it by tuples is pretty good, but I am a bit disturbed by
| the new prevalence of code like this::
|
| +            # Adapt to tuple interface:
| +            if len(key) == 2:
| +                prefix = key[:1]
| +            else:
| +                prefix = ()
|
| All this slicing of tuples is just a bit unclear and can be wrong; particularly
| when the same code deals with tuples of different lengths, and because things
| can be so confusing if a string is passed where there should be a tuple.
|
| I see Andrew also mentioned this so maybe we should do something about it. :-)
|
| I'm not sure what to do.  Some of it is endemic to Python.  Perhaps we could try
| to get everything to at least use the same tuple length for keys, so that at
| least we can unpack it through "a, b, c = key" and avoid conditionals.
|

...

| +    @needs_read_lock
|      def has_revisions(self, revision_ids):
|          """Probe to find out the presence of multiple revisions.
|
|          :param revision_ids: An iterable of revision_ids.
|          :return: A set of the revision_ids that were present.
|          """
| -        raise NotImplementedError(self.has_revisions)
| -
| -        return self._revision_store.has_revision_id(revision_id,
| -                                                    self.get_transaction())
| +        parent_map = self.revisions.get_parent_map(
| +            [(rev_id,) for rev_id in revision_ids])
| +        result = set()
| +        if _mod_revision.NULL_REVISION in revision_ids:
| +            result.add(_mod_revision.NULL_REVISION)
| +        result.update([key[0] for key in parent_map])
| +        return result
|
| This is a pretty low-level method; I'd tend to make the caller do the locking.
| The concrete implementations did not have auto locking before.
|

My best guess is that the old form used a PassThrough transaction, and thus
didn't actually require a lock of any kind. get_parent_map() will now raise
ObjectNotLocked, so it changes has_revisions() to require callers to have a lock.

So said a different way.. .has_revisions() didn't need callers to have a read
lock before, because it would work without a lock. Adding @needs_read_lock is
probably trying to preserve *that* behavior.

And considering all things, I'd like to take the path of least resistance right
now, and leave @needs_read_lock in.

...

| +    def _iter_inventory_xmls(self, revision_ids):
| +        keys = [(revision_id,) for revision_id in revision_ids]
| +        stream = self.inventories.get_record_stream(keys, 'unordered', True)
| +        texts = {}
| +        for record in stream:
| +            if record.storage_kind != 'absent':
| +                texts[record.key] = record.get_bytes_as('fulltext')
| +            else:
| +                raise errors.NoSuchRevision(self, record.key)
| +        for key in keys:
| +            yield texts[key], key[-1]
| +
|
| Can we rename this to _iter_inventory_xmls; making apis that assume they're xml
| is not so good when we may want to go away from that.

I think you are wanting a different name, since it *is* called
iter_inventory_xmls. Maybe "iter_inventory_texts" or "iter_serialized_inventories".


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

iEYEARECAAYFAkhYHJ0ACgkQJdeBCYSNAAMbGACfa5DJcTewfGd4PMnw7H9kAMJs
VJ0AnAi609PrZDmnUvg5p1vl9cFwgXC9
=cf9k
-----END PGP SIGNATURE-----