Bazaar

Merge lp:~lifeless/bzr/CHKRepo._iter_inventory_xmls into lp:~bzr/bzr/trunk-old

  1. CHKRepo._iter_inventory_xmls
  2. Merge into trunk-old
Proposed by Robert Collins
Status: Merged
Merged at revision: not available
Proposed branch: lp:~lifeless/bzr/CHKRepo._iter_inventory_xmls
Merge into: lp:~bzr/bzr/trunk-old
Diff against target: 42 lines (has conflicts) 
Text conflict in bzrlib/repofmt/groupcompress_repo.py
To merge this branch: bzr merge lp:~lifeless/bzr/CHKRepo._iter_inventory_xmls
Reviewer Review Type Date Requested Status
John A Meinel Approve
Review via email: mp+10132@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

So, we wanted to avoid doing this, but I think we've learnt about
most/all of the places that were silently calling this code path now -
and fixed them (or they are deprecated, like wt3).

So I think we should do this.

-Rob

Revision history for this message
Martin Pool (mbp) wrote :

It seems reasonable; weak approve though you might want a review from John.

Revision history for this message
John A Meinel (jameinel) wrote :

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

Robert Collins wrote:
> Robert Collins has proposed merging lp:~lifeless/bzr/CHKRepo._iter_inventory_xmls into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>
> So, we wanted to avoid doing this, but I think we've learnt about
> most/all of the places that were silently calling this code path now -
> and fixed them (or they are deprecated, like wt3).
>
> So I think we should do this.
>
> -Rob
>
>

So I don't see why this is necessary:
- - inv = self.branch.repository.deserialise_inventory(
- - new_revision, xml)
+ inv =
self.branch.repository._serializer.read_inventory_from_string(
+ xml, new_revision)

Looking at "deserialise_inventory" it:
 1) does exactly this, only allows some small amount of caching of
    inventory entries and
 2) Checks that the revision_id of the overall inventory is set
    properly (older xml formats didn't have a revision-id field
    populated.)

Other than that, I understand your comments, and we can probably merge this.

To we have a per_repository test that get_inventory_xml does something
useful?

John
=:->

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

iEYEARECAAYFAkqFbdEACgkQJdeBCYSNAAMYpwCeMkejwe9/D3+FhmUv1VZgp72r
QbsAoK7tViOHBU/nyh/h7bYSQtknShFP
=l/Xi
-----END PGP SIGNATURE-----

Revision history for this message
Robert Collins (lifeless) wrote :

On Fri, 2009-08-14 at 14:00 +0000, John A Meinel wrote:
>
> So I don't see why this is necessary:
> - inv = self.branch.repository.deserialise_inventory(
> - new_revision, xml)
> + inv =
> self.branch.repository._serializer.read_inventory_from_string(
> + xml, new_revision)
>
> Looking at "deserialise_inventory" it:
> 1) does exactly this, only allows some small amount of caching of
> inventory entries and
> 2) Checks that the revision_id of the overall inventory is set
> properly (older xml formats didn't have a revision-id field
> populated.)

repo.deserialize_inventory on a chk repo looks for a chk inventory meta
record.
repo._serializer.read_inventory_from_string parses a format-10 xml
inventory.

:(

-Rob

Revision history for this message
Robert Collins (lifeless) wrote :

ping

Revision history for this message
John A Meinel (jameinel) wrote :

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

Robert Collins wrote:
> ping

Sure. When I was looking through the source code, I didn't see the function:
793 def deserialise_inventory(self, revision_id, bytes):
794 -> return
inventory.CHKInventory.deserialise(self.chk_bytes, bytes,
795 (revision_id,))

Anyway, good enough.

  review: approve

John
=:->

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

iEYEARECAAYFAkqKBt8ACgkQJdeBCYSNAAMyLgCgqHOIhugvadlP5eqhER9Z84GO
sswAnjz0aqhD7vM/CBQH7EG12euWi2wF
=yOu5
-----END PGP SIGNATURE-----

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/repofmt/groupcompress_repo.py'
2--- bzrlib/repofmt/groupcompress_repo.py 2009-08-14 09:37:57 +0000
3+++ bzrlib/repofmt/groupcompress_repo.py 2009-08-18 02:36:21 +0000
4@@ -809,10 +809,23 @@
5 for key in keys:
6 yield inventory.CHKInventory.deserialise(self.chk_bytes, texts[key], key)
7
8+<<<<<<< TREE
9 def _iter_inventory_xmls(self, revision_ids, ordering):
10 # Without a native 'xml' inventory, this method doesn't make sense, so
11 # make it raise to trap naughty direct users.
12 raise NotImplementedError(self._iter_inventory_xmls)
13+=======
14+ def _iter_inventory_xmls(self, revision_ids):
15+ # Without a native 'xml' inventory, this method doesn't make sense.
16+ # However older working trees, and older bundles want it - so we supply
17+ # it allowing get_inventory_xml to work. Bundles currently use the
18+ # serializer directly; this also isn't ideal, but there isn't an xml
19+ # iteration interface offered at all for repositories. We could make
20+ # _iter_inventory_xmls be part of the contract, even if kept private.
21+ inv_to_str = self._serializer.write_inventory_to_string
22+ for inv in self.iter_inventories(revision_ids):
23+ yield inv_to_str(inv), inv.revision_id
24+>>>>>>> MERGE-SOURCE
25
26 def _find_present_inventory_keys(self, revision_keys):
27 parent_map = self.inventories.get_parent_map(revision_keys)
28
29=== modified file 'bzrlib/workingtree.py'
30--- bzrlib/workingtree.py 2009-08-04 04:36:34 +0000
31+++ bzrlib/workingtree.py 2009-08-18 02:36:21 +0000
32@@ -1893,8 +1893,8 @@
33 firstline = xml.split('\n', 1)[0]
34 if (not 'revision_id="' in firstline or
35 'format="7"' not in firstline):
36- inv = self.branch.repository.deserialise_inventory(
37- new_revision, xml)
38+ inv = self.branch.repository._serializer.read_inventory_from_string(
39+ xml, new_revision)
40 xml = self._create_basis_xml_from_inventory(new_revision, inv)
41 self._write_basis_inventory(xml)
42 except (errors.NoSuchRevision, errors.RevisionNotPresent):