RFC: difference in dirstate iter_changes and inventory iter_changes

John Arbash Meinel john at arbash-meinel.com
Tue Aug 12 00:17:45 BST 2008


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

Robert Collins wrote:
> On Fri, 2008-08-08 at 08:00 -0500, John Arbash Meinel wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> Robert Collins wrote:
>>> There is a slight difference between the two iter_changes
>>> implementations.
>>>
>>> Specifically, the inventory based one includes missing files, whereas
>>> the dirstate one is skipping them. We have a test about this, but I
>>> think its wrong :)
>>>
>>> tree.add('foo')
>>> os.unlink('foo')
>>> len(tree.iter_changes(tree.basis_tree()))
>>> -> 1 for inventory based iter_changes
>>> -> 0 for dirstate
>>>
>>> any objection to my making these consistently 1 ? 
>>>
>>> -Rob
>>>
>> I think it would be good to be consistent.
>>
>> I think part of the problem with Inventory based iter_changes is that it
>> does not accurately represent what is on disk, which is bad.
> 
> I think you're confused :)
> 
> iter_changes will return a kind that is accurate with whats on disk for
> the inventory based version; it returns None for the kind of a missing
> file.
> 
> -Rob

Yeah, it seems there was a new "Tree._comparison_data()" api added,
which goes directly to disk to determine the kind, executable and stat
value, rather than trusting the InventoryEntry directly. I missed that
patch.

Certainly it seems a bit... odd... to have so many paths to get
different bits of information.

We have "Tree._comparison_data", "Tree.path_content_summary()". I'm not
100% sure why it is okay for the generic InterTree algorithm to use a
private method on Tree. Especially since the base Tree class has
"NotImplementedError" which means that any Tree implementation doesn't
just naturally get one.
If it is part of the official tree api, doesn't it need to be public and
tested?

We've certainly run into this in the past. With _make_parent_provider
being a required Repository interface (because one repo.get_graph()
assumes that the other repo has it, regardless of type.)

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

iEYEARECAAYFAkigyJkACgkQJdeBCYSNAAPrBwCguGxEjUew2i+8mQRIHye8L5cO
iTEAoJP8BKD71Fzdua9IRIOraL4L2f0f
=DjAz
-----END PGP SIGNATURE-----



More information about the bazaar mailing list