bzr 2.0.1 breaks fastimport into non-chk repositories

[Ian Clatworthy]

Revision 4669 in the 2.0.x branch breaks selftest fastimport and it's a serious problem: fastimport into non-chk repositories has stopped working.

[Ian Clatworthy (ian-clatworthy)]

Fixed in rev 257.

[John A Meinel (jameinel)]

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

Ian Clatworthy wrote:
> Fixed in rev 257.
>
> ** Changed in: bzr-fastimport
> Status: In Progress => Fix Released
>

A big concern here is that the "stable" branch breaks the import. Can
you tell us what we broke so that we can evaluate if we really released
stable bugfixes in 2.0.1?

John
=:->

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

iEYEARECAAYFAkrlAo0ACgkQJdeBCYSNAANtTQCeMXTQdPwY1rJpwJWKdFl+0rkT
oi4AnRyZ/YnZ7g+pig26oj/6wWaK9Y2R
=A8Vi
-----END PGP SIGNATURE-----

[Ian Clatworthy (ian-clatworthy)]

John A Meinel wrote:

> A big concern here is that the "stable" branch breaks the import. Can
> you tell us what we broke so that we can evaluate if we really released
> stable bugfixes in 2.0.1?

Here was the stack trace from one of the selftest failures ...

> Traceback (most recent call last):
> File "/home/ian/.bazaar/plugins/fastimport/tests/test_generic_processor.py", line 1466, in test_copy_file_in_root
> handler.process(self.file_command_iter(src_path, dest_path))
> File "/home/ian/.bazaar/plugins/fastimport/processor.py", line 95, in process
> self._process(command_iter)
> File "/home/ian/.bazaar/plugins/fastimport/processors/generic_processor.py", line 280, in _process
> processor.ImportProcessor._process(self, command_iter)
> File "/home/ian/.bazaar/plugins/fastimport/processor.py", line 117, in _process
> handler(self, cmd)
> File "/home/ian/.bazaar/plugins/fastimport/processors/generic_processor.py", line 487, in commit_handler
> handler.process()
> File "/home/ian/.bazaar/plugins/fastimport/processor.py", line 208, in process
> self.post_process_files()
> File "/home/ian/.bazaar/plugins/fastimport/bzr_commit_handler.py", line 600, in post_process_files
> lambda revision_ids: self._get_inventories(revision_ids))
> File "/home/ian/.bazaar/plugins/fastimport/revision_store.py", line 236, in load_using_delta
> rev_id, basis_inv, inv_delta, present_parents, parent_invs)
> File "/home/ian/.bazaar/plugins/fastimport/revision_store.py", line 321, in _add_inventory_by_delta
> new_inv = basis_inv.create_by_apply_delta(inv_delta, revision_id)
> File "/home/ian/Projects/bzr/bzr/2.0/bzrlib/inventory.py", line 1204, in create_by_apply_delta
> new_inv.apply_delta(inventory_delta)
> File "/home/ian/Projects/bzr/bzr/2.0/bzrlib/inventory.py", line 1187, in apply_delta
> "New id is already present in target.")
> InconsistentDelta: An inconsistent delta was supplied involving '', 'TREE_ROOT'
> reason: New id is already present in target.
>

fastimport was providing a delta with TREE_ROOT already in it which is
correct for CHK repositories but not earlier ones. It had some
conditional processing deciding what to do based on whether
"create_by_apply_delta" existed. You added a method called that to
Inventory as part of the work to speed up "bzr log DIR", breaking my
logic. The fix was to change fastimport to test using "isinstance"
instead of "hasattr".

Ian C.

[John A Meinel (jameinel)]

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

...

>> File "/home/ian/Projects/bzr/bzr/2.0/bzrlib/inventory.py", line 1187, in apply_delta
>> "New id is already present in target.")
>> InconsistentDelta: An inconsistent delta was supplied involving '', 'TREE_ROOT'
>> reason: New id is already present in target.
>>
>
> fastimport was providing a delta with TREE_ROOT already in it which is
> correct for CHK repositories but not earlier ones. It had some
> conditional processing deciding what to do based on whether
> "create_by_apply_delta" existed. You added a method called that to
> Inventory as part of the work to speed up "bzr log DIR", breaking my
> logic. The fix was to change fastimport to test using "isinstance"
> instead of "hasattr".
>
> Ian C.
>

This hints to me that we have something broken internally. We should
either always or never allow a given delta. Can you open a bug
specifically about this? Hopefully with at least some detail. I wouldn't
be surprised to find the tree root special cased in several locations
(such as 'WT.add()' automatically adds the tree root if it doesn't
exist, but I believe that isn't true with MemoryTree...)

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

iEYEARECAAYFAkrlJNIACgkQJdeBCYSNAAMw7gCgxVCo2+l758tbB4NOKgnZu1FG
IacAoK43z+RbV32Zl0URbH2/6ZqDvv0T
=EXVI
-----END PGP SIGNATURE-----