[MERGE] Native dirstate update-basis-via-delta
John Arbash Meinel
john at arbash-meinel.com
Wed Oct 24 02:16:58 BST 2007
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
...
>> + self._update_basis_apply_deletes(deletes)
>> + deletes = []
>>
>> ^- In the inner loop you seem to apply all deletes whenever you see a renamed
>> file. This seems a little odd to me, though I won't say it is incorrect.
>
> The comment right above it explains why in it's first sentence; if you
> could see why it didn't gel for you that would be great.
Because you wrote a small book. And the language is a bit convoluted. Maybe just:
# Expunge deletes that we've seen so that deleted children of a renamed
# directory are properly handled.
>> Explicitly using:
>>
>> absent = ('r', 'a')
>> ...
>> if kind not in absent:
>> ...
>>
>> Is probably slower than either
>>
...
>
> Any specific area? We have a number of tests of this already -
> wt_implementations.get_parents tests just about every permutation I
> could think of.
Ok, there does seem to be sufficient testing. Just that you were adding a patch
but not really adding a lot of tests.
It seems like the tests were actually added for the inventory-based form a
while ago, and now there is a custom WT4 implementation.
The one I didn't see tested was swapping a parent with a child. eg:
bzr mv foo/bar bar
bzr mv foo bar/foo
You did
bzr mv foo tmp
bzr mv bar foo
bzr mv tmp bar
Which is close, but not quite the same. I believe the parent to child swap is
the one that forces a lot of the deletes before adds work.
Otherwise, I think you covered this, just not directly in this patch.
>
>> One problem we have with dirstate, is that if the basis inventory gets skewed
>> from reality, it can do nasty things, and we have very limited ability to
>> detect it.
>>
>> Probably 'bzr check' should assert that "tree.basis_tree()" gives the same
>> result at "tree.branch.repository.revision_tree(tree.last_revision())". That
>> would at least go a way towards giving security in something like this which is
>> mutating over time. (There is no point where we forcibly "sync" and verify that
>> everything is 100% correct.)
>
> I think thats a good idea. Also bzr check should call tree._validate().
>
> Do you want these done as part of this patch?
I would like the comparison to be done. It at least gives us a tool in case we
suspect something is broken.
>
> Also, do you think the _id_index should be adjusted/reset by this? I
> think resetting it is ok and sufficient.
>
> -Rob
It is necessary and sufficient to reset it. Adjusting it is far too much work
(because of all the edge cases).
John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFHHp0JJdeBCYSNAAMRAvQ5AKCogWt3BqHnusxALxVaOEajaWDkAQCbBO/T
MZipIiwxMNywmixrIaqz3eg=
=qKt4
-----END PGP SIGNATURE-----
More information about the bazaar
mailing list